From 4c556b4836c09afc3e2822a1973940c917cdce8b Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Tue, 17 Dec 2024 12:02:20 +0100 Subject: [PATCH 1/7] feat: add lazy_join decorator --- .../decorators/decorators_stack.rb | 4 +- .../lazy_join_collection_decorator.rb | 91 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb diff --git a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/decorators_stack.rb b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/decorators_stack.rb index 1f6b4c213..8f3749a93 100644 --- a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/decorators_stack.rb +++ b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/decorators_stack.rb @@ -5,7 +5,7 @@ class DecoratorsStack attr_reader :datasource, :schema, :search, :early_computed, :late_computed, :action, :relation, :late_op_emulate, :early_op_emulate, :validation, :sort, :rename_field, :publication, :write, :chart, :hook, :segment, - :binary, :override + :binary, :override, :lazy_join def initialize(datasource) @customizations = [] @@ -19,6 +19,8 @@ def initialize(datasource) last = @early_op_emulate = DatasourceDecorator.new(last, OperatorsEmulate::OperatorsEmulateCollectionDecorator) last = DatasourceDecorator.new(last, OperatorsEquivalence::OperatorsEquivalenceCollectionDecorator) last = @relation = DatasourceDecorator.new(last, Relation::RelationCollectionDecorator) + # lazy join is just before relation, to avoid relations to do useless stuff + last = @lazy_join = DatasourceDecorator.new(last, LazyJoin::LazyJoinCollectionDecorator) last = @late_computed = DatasourceDecorator.new(last, Computed::ComputeCollectionDecorator) last = @late_op_emulate = DatasourceDecorator.new(last, OperatorsEmulate::OperatorsEmulateCollectionDecorator) last = DatasourceDecorator.new(last, OperatorsEquivalence::OperatorsEquivalenceCollectionDecorator) diff --git a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb new file mode 100644 index 000000000..bd40aa920 --- /dev/null +++ b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb @@ -0,0 +1,91 @@ +module ForestAdminDatasourceCustomizer + module Decorators + module LazyJoin + class LazyJoinCollectionDecorator < ForestAdminDatasourceToolkit::Decorators::CollectionDecorator + include ForestAdminDatasourceToolkit::Decorators + include ForestAdminDatasourceToolkit::Components::Query + include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + + def list(caller, filter, projection) + simplified_projection = get_projection_without_useless_joins(projection) + refined_filter = refine_filter(caller, filter) + records = child_collection.list(caller, refined_filter, simplified_projection) + + apply_joins_on_records(projection, simplified_projection, records) + end + + def refine_filter(_caller, filter = nil) + super + filter&.override( + condition_tree: filter.condition_tree&.replace_leafs do |leaf| + if useless_join?(leaf.field.split(':')[0], filter.condition_tree.projection) + leaf.override(field: get_foreign_key_for_projection(leaf)) + else + leaf + end + end + ) + end + + private + + def get_foreign_key_for_projection(field_name) + relation_name = field_name.split(':')[0] + relation_schema = schema[:fields][relation_name] + + relation_schema.foreign_key + end + + def useless_join?(relation_name, projection) + relation_schema = schema[:fields][relation_name] + sub_projection = projection.relation[relation_name] + + relation_schema.type == 'ManyToOne' && + sub_projection.size == 1 && + sub_projection[0] == relation_schema.foreign_key_target + end + + def get_projection_without_useless_joins(projection) + new_projection = Projection.new(projection) + + projection.relations.each do |relation_name, relation_projection| + next unless useless_join?(relation_name, projection) + + # remove foreign key target from projection + new_projection.delete("#{relation_name}:#{relation_projection[0]}") + + # add foreign keys to projection + fk_field = get_foreign_key_for_projection("#{relation_name}:#{relation_projection[0]}") + new_projection << fk_field + end + end + + def apply_joins_on_records(initial_projection, requested_projection, records) + return records if initial_projection == requested_projection + + projections_to_add = Projection.new(initial_projection.reject do |field| + requested_projection.include?(field) + end) + projections_to_rm = Projection.new(requested_projection.reject { |field| initial_projection.include?(field) }) + + records.each do |record| + # add to records relation:id + projections_to_add.each do |relation_name, relation_projection| + relation_schema = schema[:fields][relation_name] + + if relation_schema.type == 'ManyToOne' + fk_value = record[get_foreign_key_for_projection("#{relation_name}:#{relation_projection[0]}")] + record[relation_name] = fk_value.nil? ? nil : { relation_projection[0] => fk_value } + end + + # remove foreign keys + projections_to_rm.each { |field| record.delete(field) } + end + end + + records + end + end + end + end +end From dbe618da502d1ef233d07b8021e63e9983ec7503 Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Tue, 17 Dec 2024 15:51:26 +0100 Subject: [PATCH 2/7] test: add tests on lazy join decorator --- .../lazy_join_collection_decorator.rb | 10 +- .../lazy_join_collection_decorator_spec.rb | 201 ++++++++++++++++++ 2 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb diff --git a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb index bd40aa920..138b58d4d 100644 --- a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb +++ b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb @@ -19,7 +19,7 @@ def refine_filter(_caller, filter = nil) filter&.override( condition_tree: filter.condition_tree&.replace_leafs do |leaf| if useless_join?(leaf.field.split(':')[0], filter.condition_tree.projection) - leaf.override(field: get_foreign_key_for_projection(leaf)) + leaf.override(field: get_foreign_key_for_projection(leaf.field)) else leaf end @@ -38,7 +38,7 @@ def get_foreign_key_for_projection(field_name) def useless_join?(relation_name, projection) relation_schema = schema[:fields][relation_name] - sub_projection = projection.relation[relation_name] + sub_projection = projection.relations[relation_name] relation_schema.type == 'ManyToOne' && sub_projection.size == 1 && @@ -58,6 +58,8 @@ def get_projection_without_useless_joins(projection) fk_field = get_foreign_key_for_projection("#{relation_name}:#{relation_projection[0]}") new_projection << fk_field end + + new_projection end def apply_joins_on_records(initial_projection, requested_projection, records) @@ -70,10 +72,10 @@ def apply_joins_on_records(initial_projection, requested_projection, records) records.each do |record| # add to records relation:id - projections_to_add.each do |relation_name, relation_projection| + projections_to_add.relations.each do |relation_name, relation_projection| relation_schema = schema[:fields][relation_name] - if relation_schema.type == 'ManyToOne' + if relation_schema && relation_schema.type == 'ManyToOne' fk_value = record[get_foreign_key_for_projection("#{relation_name}:#{relation_projection[0]}")] record[relation_name] = fk_value.nil? ? nil : { relation_projection[0] => fk_value } end diff --git a/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb new file mode 100644 index 000000000..5ec46e018 --- /dev/null +++ b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb @@ -0,0 +1,201 @@ +require 'spec_helper' + +module ForestAdminDatasourceCustomizer + module Decorators + module LazyJoin + include ForestAdminDatasourceToolkit + include ForestAdminDatasourceToolkit::Components::Query + include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + include ForestAdminDatasourceToolkit::Decorators + include ForestAdminDatasourceToolkit::Schema + + describe LazyJoinCollectionDecorator do + subject(:lazy_join_collection_decorator) { described_class } + + let(:caller) { instance_double(ForestAdminDatasourceToolkit::Components::Caller) } + let(:aggregation) { instance_double(ForestAdminDatasourceToolkit::Components::Query::Aggregation) } + + before do + datasource = Datasource.new + @collection_book = collection_build( + name: 'book', + schema: { + fields: { + 'id' => numeric_primary_key_build, + 'author_id' => column_build(column_type: 'Number'), + 'author' => many_to_one_build(foreign_collection: 'person', foreign_key: 'author_id'), + 'title' => column_build + } + } + ) + + @collection_person = collection_build( + name: 'person', + schema: { + fields: { + 'id' => numeric_primary_key_build, + 'books' => one_to_many_build(foreign_collection: 'person', origin_key: 'author_id'), + 'first_name' => column_build, + 'last_name' => column_build + } + } + ) + datasource.add_collection(@collection_book) + datasource.add_collection(@collection_person) + + @datasource_decorator = DatasourceDecorator.new(datasource, lazy_join_collection_decorator) + end + + context 'when call list' do + it 'not join when projection ask for target field only' do + allow(@collection_book).to receive(:list).and_return( + [{ 'id' => 1, 'author_id' => 2 }, { 'id' => 2, 'author_id' => 5 }] + ) + + result = @datasource_decorator.get_collection('book') + .list(caller, Filter.new, Projection.new(%w[id author:id])) + + expect(@collection_book).to have_received(:list) do |_caller, _filter, projection| + expect(projection).to eq(%w[id author_id]) + end + expect(result).to eq([{ 'id' => 1, 'author' => { 'id' => 2 } }, { 'id' => 2, 'author' => { 'id' => 5 } }]) + end + + it 'join when projection ask for multiple fields in foreign collection' do + allow(@collection_book).to receive(:list).and_return( + [ + { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + ] + ) + + result = @datasource_decorator.get_collection('book') + .list(caller, Filter.new, Projection.new(%w[id author:id author:first_name])) + + expect(@collection_book).to have_received(:list) do |_caller, _filter, projection| + expect(projection).to eq(%w[id author:id author:first_name]) + end + expect(result).to eq( + [ + { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + ] + ) + end + + it 'not join when when condition tree is on foreign key target' do + allow(@collection_book).to receive(:list).and_return( + [{ 'id' => 1, 'author_id' => 2 }, { 'id' => 2, 'author_id' => 5 }, { 'id' => 3, 'author_id' => 5 }] + ) + + @datasource_decorator.get_collection('book').list( + caller, + Filter.new(condition_tree: Nodes::ConditionTreeLeaf.new('author:id', Operators::IN, [2, 5])), + Projection.new(%w[id author:id]) + ) + + expect(@collection_book).to have_received(:list) do |_caller, filter, projection| + condition_tree = Nodes::ConditionTreeLeaf.new('author_id', Operators::IN, [2, 5]).to_h + expect(filter.condition_tree.to_h).to eq(condition_tree) + expect(projection).to eq(%w[id author_id]) + end + end + + it 'join when when condition tree is on foreign key collection_field' do + allow(@collection_book).to receive(:list).and_return( + [ + { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } }, + { 'id' => 3, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + ] + ) + + condition_tree = Nodes::ConditionTreeLeaf.new('author:first_name', Operators::EQUAL, 'J.K') + @datasource_decorator.get_collection('book').list( + caller, + Filter.new(condition_tree: condition_tree), + Projection.new(%w[id author:id]) + ) + + expect(@collection_book).to have_received(:list) do |_caller, filter, projection| + expect(filter.condition_tree).to eq(condition_tree) + expect(projection).to eq(%w[id author_id]) + end + end + + it 'disable join on condition tree but not in projection' do + allow(@collection_book).to receive(:list).and_return( + [ + { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } }, + { 'id' => 3, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + ] + ) + + condition_tree = Nodes::ConditionTreeLeaf.new('author:id', Operators::IN, [2, 5]) + result = @datasource_decorator.get_collection('book').list( + caller, + Filter.new(condition_tree: condition_tree), + Projection.new(%w[id author:first_name]) + ) + + expect(@collection_book).to have_received(:list) do |_caller, filter, projection| + condition_tree = Nodes::ConditionTreeLeaf.new('author_id', Operators::IN, [2, 5]) + expect(filter.condition_tree.to_h).to eq(condition_tree.to_h) + expect(projection).to eq(%w[id author:first_name]) + end + expect(result).to eq( + [ + { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } }, + { 'id' => 3, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + ] + ) + end + + it 'disable join on projection but not on condition tree' do + allow(@collection_book).to receive(:list).and_return( + [ + { 'id' => 1, 'author_id' => 2 }, + { 'id' => 2, 'author_id' => 5 }, + { 'id' => 3, 'author_id' => 5 } + ] + ) + + condition_tree = Nodes::ConditionTreeLeaf.new('author:first_name', Operators::IN, %w[Isaac J.K]) + result = @datasource_decorator.get_collection('book').list( + caller, + Filter.new(condition_tree: condition_tree), + Projection.new(%w[id author:id]) + ) + + expect(@collection_book).to have_received(:list) do |_caller, filter, projection| + expect(filter.condition_tree.to_h).to eq(condition_tree.to_h) + expect(projection).to eq(%w[id author_id]) + end + expect(result).to eq( + [ + { 'id' => 1, 'author' => { 'id' => 2 } }, + { 'id' => 2, 'author' => { 'id' => 5 } }, + { 'id' => 3, 'author' => { 'id' => 5 } } + ] + ) + end + + it 'correctly handle null relations' do + allow(@collection_book).to receive(:list).and_return( + [{ 'id' => 1, 'author_id' => 2 }, { 'id' => 2, 'author_id' => nil }] + ) + + result = @datasource_decorator.get_collection('book').list(caller, Filter.new, Projection.new(%w[id author:id])) + + expect(@collection_book).to have_received(:list) do |_caller, _filter, projection| + expect(projection).to eq(%w[id author_id]) + end + expect(result).to eq([{ 'id' => 1, 'author' => { 'id' => 2 } }, { 'id' => 2, 'author' => nil }]) + end + end + end + end + end +end From 4791081855dd35365a28f919e276a7213aa8942a Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Thu, 19 Dec 2024 17:51:03 +0100 Subject: [PATCH 3/7] feat: apply lazy join on aggregate --- .../lazy_join_collection_decorator.rb | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb index 138b58d4d..aa74352ad 100644 --- a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb +++ b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb @@ -14,8 +14,26 @@ def list(caller, filter, projection) apply_joins_on_records(projection, simplified_projection, records) end + def aggregate(caller, filter, aggregation, limit = nil) + refined_filter = refine_filter(caller, filter) + replaced = {} + refined_aggregation = aggregation.replace_fields do |field_name| + if useless_join?(field_name.split(':')[0], aggregation.projection) + new_field_name = get_foreign_key_for_projection(field_name) + replaced[new_field_name] = field_name + + new_field_name + else + field_name + end + end + + results = child_collection.aggregate(caller, refined_filter, refined_aggregation, limit) + + apply_joins_on_aggregate_result(aggregation, refined_aggregation, results, replaced) + end + def refine_filter(_caller, filter = nil) - super filter&.override( condition_tree: filter.condition_tree&.replace_leafs do |leaf| if useless_join?(leaf.field.split(':')[0], filter.condition_tree.projection) @@ -71,7 +89,7 @@ def apply_joins_on_records(initial_projection, requested_projection, records) projections_to_rm = Projection.new(requested_projection.reject { |field| initial_projection.include?(field) }) records.each do |record| - # add to records relation:id + # add to record relation:id projections_to_add.relations.each do |relation_name, relation_projection| relation_schema = schema[:fields][relation_name] @@ -87,6 +105,24 @@ def apply_joins_on_records(initial_projection, requested_projection, records) records end + + def apply_joins_on_aggregate_result(initial_aggregation, requested_aggregation, results, fields_to_replace) + return result if initial_aggregation == requested_aggregation + + results.each do |result| + group = {} + result['group'].each do |field, value| + if fields_to_replace.include?(field) + group[fields_to_replace[field]] = value + else + group[field] = value + end + end + result['group'] = group + end + + results + end end end end From 935491d57af6845ca8b8059455b5a1d1702688ff Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Thu, 19 Dec 2024 17:51:22 +0100 Subject: [PATCH 4/7] test: add tests on lazy join decorator --- .../lazy_join_collection_decorator_spec.rb | 149 +++++++++++++++++- 1 file changed, 142 insertions(+), 7 deletions(-) diff --git a/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb index 5ec46e018..d8914ec50 100644 --- a/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb +++ b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb @@ -24,7 +24,8 @@ module LazyJoin 'id' => numeric_primary_key_build, 'author_id' => column_build(column_type: 'Number'), 'author' => many_to_one_build(foreign_collection: 'person', foreign_key: 'author_id'), - 'title' => column_build + 'title' => column_build, + 'price' => column_build(column_type: 'Number') } } ) @@ -126,9 +127,9 @@ module LazyJoin it 'disable join on condition tree but not in projection' do allow(@collection_book).to receive(:list).and_return( [ - { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, - { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } }, - { 'id' => 3, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + { 'id' => 1, 'author' => { 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'first_name' => 'J.K' } }, + { 'id' => 3, 'author' => { 'first_name' => 'J.K' } } ] ) @@ -146,9 +147,9 @@ module LazyJoin end expect(result).to eq( [ - { 'id' => 1, 'author' => { 'id' => 2, 'first_name' => 'Isaac' } }, - { 'id' => 2, 'author' => { 'id' => 5, 'first_name' => 'J.K' } }, - { 'id' => 3, 'author' => { 'id' => 5, 'first_name' => 'J.K' } } + { 'id' => 1, 'author' => { 'first_name' => 'Isaac' } }, + { 'id' => 2, 'author' => { 'first_name' => 'J.K' } }, + { 'id' => 3, 'author' => { 'first_name' => 'J.K' } } ] ) end @@ -195,6 +196,140 @@ module LazyJoin expect(result).to eq([{ 'id' => 1, 'author' => { 'id' => 2 } }, { 'id' => 2, 'author' => nil }]) end end + + context 'when call aggregate' do + it 'not join on aggregate when group by foreign key' do + allow(@collection_book).to receive(:aggregate).and_return( + [ + { 'value' => 1824.11, 'group' => { 'author_id' => 2 } }, + { 'value' => 824.11, 'group' => { 'author_id' => 3 } } + ] + ) + + result = @datasource_decorator.get_collection('book').aggregate( + caller, + Filter.new, + Aggregation.new(operation: 'Sum', field: 'price', groups: [{ field: 'author:id' }]) + ) + + expect(@collection_book).to have_received(:aggregate) do |_caller, _filter, aggregation| + expect(aggregation.to_h).to eq( + Aggregation.new(operation: 'Sum', field: 'price', groups: [{ field: 'author_id', operation: nil }]).to_h + ) + end + expect(result).to eq( + [ + { 'value' => 1824.11, 'group' => { 'author:id' => 2 } }, + { 'value' => 824.11, 'group' => { 'author:id' => 3 } } + ] + ) + end + + it 'join on aggregate when group by foreign field' do + allow(@collection_book).to receive(:aggregate).and_return( + [ + { 'value' => 1824.11, 'group' => { 'author:first_name' => 'Isaac' } }, + { 'value' => 824.11, 'group' => { 'author:first_name' => 'JK' } } + ] + ) + + result = @datasource_decorator.get_collection('book').aggregate( + caller, + Filter.new, + Aggregation.new(operation: 'Sum', field: 'price', groups: [{ field: 'author:first_name' }]) + ) + + expect(@collection_book).to have_received(:aggregate) do |_caller, _filter, aggregation| + expect(aggregation.to_h).to eq( + Aggregation.new( + operation: 'Sum', + field: 'price', + groups: [{ field: 'author:first_name', operation: nil }] + ).to_h + ) + end + expect(result).to eq( + [ + { 'value' => 1824.11, 'group' => { 'author:first_name' => 'Isaac' } }, + { 'value' => 824.11, 'group' => { 'author:first_name' => 'JK' } } + ] + ) + end + + it 'not join on aggregate when group by foreign pk and filter on foreign pk' do + allow(@collection_book).to receive(:aggregate).and_return( + [ + { 'value' => 1824.11, 'group' => { 'author_id' => 2 } }, + { 'value' => 824.11, 'group' => { 'author_id' => 3 } } + ] + ) + + result = @datasource_decorator.get_collection('book').aggregate( + caller, + Filter.new(condition_tree: Nodes::ConditionTreeLeaf.new('author:id', Operators::NOT_EQUAL, 50)), + Aggregation.new(operation: 'Sum', field: 'price', groups: [{ field: 'author:id' }]) + ) + + expect(@collection_book).to have_received(:aggregate) do |_caller, filter, aggregation| + expect(filter.condition_tree.to_h).to eq( + Nodes::ConditionTreeLeaf.new('author_id', Operators::NOT_EQUAL, 50).to_h + ) + expect(aggregation.to_h).to eq( + Aggregation.new( + operation: 'Sum', + field: 'price', + groups: [{ field: 'author_id', operation: nil }] + ).to_h + ) + end + expect(result).to eq( + [ + { 'value' => 1824.11, 'group' => { 'author:id' => 2 } }, + { 'value' => 824.11, 'group' => { 'author:id' => 3 } } + ] + ) + end + + it 'join on aggregate when group by foreign pk and filter on foreign field' do + allow(@collection_book).to receive(:aggregate).and_return( + [ + { 'value' => 1824.11, 'group' => { 'author_id' => 2 } }, + { 'value' => 824.11, 'group' => { 'author_id' => 3 } } + ] + ) + + result = @datasource_decorator.get_collection('book').aggregate( + caller, + Filter.new( + condition_tree: Nodes::ConditionTreeLeaf.new( + 'author:first_name', + Operators::NOT_EQUAL, + 'wrong_name' + ) + ), + Aggregation.new(operation: 'Sum', field: 'price', groups: [{ field: 'author:id' }]) + ) + + expect(@collection_book).to have_received(:aggregate) do |_caller, filter, aggregation| + expect(filter.condition_tree.to_h).to eq( + Nodes::ConditionTreeLeaf.new('author:first_name', Operators::NOT_EQUAL, 'wrong_name').to_h + ) + expect(aggregation.to_h).to eq( + Aggregation.new( + operation: 'Sum', + field: 'price', + groups: [{ field: 'author_id', operation: nil }] + ).to_h + ) + end + expect(result).to eq( + [ + { 'value' => 1824.11, 'group' => { 'author:id' => 2 } }, + { 'value' => 824.11, 'group' => { 'author:id' => 3 } } + ] + ) + end + end end end end From 41c58dd247c9442407931db3bea81fd2f38845a2 Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Fri, 20 Dec 2024 11:22:26 +0100 Subject: [PATCH 5/7] fix: query aggregate and charts route --- .../lib/forest_admin_agent/routes/charts/charts.rb | 10 +++++----- .../utils/query.rb | 7 ++----- .../utils/query_aggregate.rb | 6 ++++++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb index 6d330a62d..a37a09877 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/charts/charts.rb @@ -99,7 +99,7 @@ def make_pie result = @collection.aggregate(@caller, @filter, aggregation) - PieChart.new(result.map { |row| { key: row[:group][group_field], value: row[:value] } }).serialize + PieChart.new(result.map { |row| { key: row['group'][group_field], value: row['value'] } }).serialize end def make_line @@ -124,7 +124,7 @@ def make_line ) values = {} - rows.each { |row| values[row[:group][group_by_field_name]] = row[:value] } + rows.each { |row| values[row['group'][group_by_field_name]] = row['value'] } dates = values.keys.sort current = dates[0] last = dates.last @@ -189,8 +189,8 @@ def make_leaderboard result = rows.map do |row| { - key: row[:group][aggregation.groups[0][:field]], - value: row[:value] + key: row['group'][aggregation.groups[0][:field]], + value: row['value'] } end @@ -206,7 +206,7 @@ def compute_value(filter) field: @args[:params][:aggregateFieldName]) result = @collection.aggregate(@caller, filter, aggregation) - result[0][:value] || 0 + result[0]['value'] || 0 end end end diff --git a/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb b/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb index 631e4b949..5cbf90487 100644 --- a/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb +++ b/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb @@ -109,11 +109,8 @@ def build_select end def apply_select - unless @projection.nil? - @query = @query.select(@select.join(', ')) - - @query = @query.includes(format_relation_projection(@projection)) - end + @query = @query.select(@select.join(', ')) if @select + @query = @query.includes(format_relation_projection(@projection)) unless @projection.nil? @query end diff --git a/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query_aggregate.rb b/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query_aggregate.rb index 5a2b8c82d..ff5e1001b 100644 --- a/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query_aggregate.rb +++ b/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query_aggregate.rb @@ -44,6 +44,12 @@ def compute_result_aggregate(rows) } end end + + def add_join_relation(relation_name) + @query = @query.left_joins(relation_name.to_sym) + + @query + end end end end From 3868bfbbf026b753e964b594153bcb28ac09e209 Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Fri, 20 Dec 2024 11:28:44 +0100 Subject: [PATCH 6/7] fix: tests on charts route --- .../routes/charts/charts_spec.rb | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb index 0d4498c1c..7e78f9692 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/charts/charts_spec.rb @@ -148,7 +148,7 @@ module Charts type: 'Value', timezone: 'Europe/Paris' }) - allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ value: 10, group: [] }]) + allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ 'value' => 10, 'group' => [] }]) result = chart.handle_request(args) expect(result).to match( @@ -175,8 +175,8 @@ module Charts }) @datasource.get_collection('book') allow(@datasource.get_collection('book')).to receive(:aggregate).and_return( - [{ value: 10, group: [] }], # first call - [{ value: 5, group: [] }] # second call + [{ 'value' => 10, 'group' => [] }], # first call + [{ 'value' => 5, 'group' => [] }] # second call ) result = chart.handle_request(args) @@ -203,7 +203,7 @@ module Charts type: 'Objective', timezone: 'Europe/Paris' }) - allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ value: 10, group: [] }]) + allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ 'value' => 10, 'group' => [] }]) result = chart.handle_request(args) expect(result).to match( @@ -231,8 +231,8 @@ module Charts }) allow(@datasource.get_collection('book')).to receive(:aggregate).and_return( [ - { value: 100, group: { 'year' => 2021 } }, - { value: 150, group: { 'year' => 2022 } } + { 'value' => 100, 'group' => { 'year' => 2021 } }, + { 'value' => 150, 'group' => { 'year' => 2022 } } ] ) result = chart.handle_request(args) @@ -263,8 +263,8 @@ module Charts }) allow(@datasource.get_collection('book')).to receive(:aggregate).and_return( [ - { value: 10, group: { 'date' => Time.parse('2022-01-03 00:00:00') } }, - { value: 15, group: { 'date' => Time.parse('2022-01-07 00:00:00') } } + { 'value' => 10, 'group' => { 'date' => Time.parse('2022-01-03 00:00:00') } }, + { 'value' => 15, 'group' => { 'date' => Time.parse('2022-01-07 00:00:00') } } ] ) result = chart.handle_request(args) @@ -299,8 +299,8 @@ module Charts }) allow(@datasource.get_collection('book')).to receive(:aggregate).and_return( [ - { value: 10, group: { 'date' => Time.parse('2022-01-03 00:00:00') } }, - { value: 15, group: { 'date' => Time.parse('2022-01-10 00:00:00') } } + { 'value' => 10, 'group' => { 'date' => Time.parse('2022-01-03 00:00:00') } }, + { 'value' => 15, 'group' => { 'date' => Time.parse('2022-01-10 00:00:00') } } ] ) result = chart.handle_request(args) @@ -332,8 +332,8 @@ module Charts }) allow(@datasource.get_collection('book')).to receive(:aggregate).and_return( [ - { value: 10, group: { 'date' => Time.parse('2022-01-01 00:00:00') } }, - { value: 15, group: { 'date' => Time.parse('2022-02-01 00:00:00') } } + { 'value' => 10, 'group' => { 'date' => Time.parse('2022-01-01 00:00:00') } }, + { 'value' => 15, 'group' => { 'date' => Time.parse('2022-02-01 00:00:00') } } ] ) result = chart.handle_request(args) @@ -365,8 +365,8 @@ module Charts }) allow(@datasource.get_collection('book')).to receive(:aggregate).and_return( [ - { value: 10, group: { 'date' => Time.parse('2022-01-01 00:00:00') } }, - { value: 15, group: { 'date' => Time.parse('2023-01-01 00:00:00') } } + { 'value' => 10, 'group' => { 'date' => Time.parse('2022-01-01 00:00:00') } }, + { 'value' => 15, 'group' => { 'date' => Time.parse('2023-01-01 00:00:00') } } ] ) result = chart.handle_request(args) @@ -402,8 +402,8 @@ module Charts allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource) allow(@datasource.get_collection('review')).to receive(:aggregate).and_return( [ - { value: 10, group: { 'author' => 'Isaac Asimov' } }, - { value: 15, group: { 'author' => 'Jules Verne' } } + { 'value' => 10, 'group' => { 'author' => 'Isaac Asimov' } }, + { 'value' => 15, 'group' => { 'author' => 'Jules Verne' } } ] ) result = chart.handle_request(args) @@ -434,8 +434,8 @@ module Charts allow(@datasource.get_collection('book')).to receive(:datasource).and_return(@datasource) allow(@datasource.get_collection('book_review')).to receive(:aggregate).and_return( [ - { value: 10, group: { 'book:year' => 2022 } }, - { value: 15, group: { 'book:year' => 2023 } } + { 'value' => 10, 'group' => { 'book:year' => 2022 } }, + { 'value' => 15, 'group' => { 'book:year' => 2023 } } ] ) result = chart.handle_request(args) @@ -489,7 +489,7 @@ module Charts timezone: 'Europe/Paris' } ) - allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ value: 10, group: [] }]) + allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ 'value' => 10, 'group' => [] }]) chart.handle_request(args) expect(chart.filter).to have_attributes( @@ -510,7 +510,7 @@ module Charts aggregator: 'Count', timezone: 'Europe/Paris' }) - allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ value: 10, group: [] }]) + allow(@datasource.get_collection('book')).to receive(:aggregate).and_return([{ 'value' => 10, 'group' => [] }]) chart.handle_request(args) expect(chart.filter).to have_attributes( From 4fbb147152aabe1fea676bafa115f134d0053ad8 Mon Sep 17 00:00:00 2001 From: Nicolas Alexandre Date: Mon, 6 Jan 2025 17:40:34 +0100 Subject: [PATCH 7/7] fix: apply_joins_on_records and add test test --- .../lazy_join_collection_decorator.rb | 6 ++--- .../lazy_join_collection_decorator_spec.rb | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb index aa74352ad..321f52ea6 100644 --- a/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb +++ b/packages/forest_admin_datasource_customizer/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator.rb @@ -97,10 +97,10 @@ def apply_joins_on_records(initial_projection, requested_projection, records) fk_value = record[get_foreign_key_for_projection("#{relation_name}:#{relation_projection[0]}")] record[relation_name] = fk_value.nil? ? nil : { relation_projection[0] => fk_value } end - - # remove foreign keys - projections_to_rm.each { |field| record.delete(field) } end + + # remove foreign keys + projections_to_rm.each { |field| record.delete(field) } end records diff --git a/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb index d8914ec50..5620f7b5c 100644 --- a/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb +++ b/packages/forest_admin_datasource_customizer/spec/lib/forest_admin_datasource_customizer/decorators/lazy_join/lazy_join_collection_decorator_spec.rb @@ -23,7 +23,9 @@ module LazyJoin fields: { 'id' => numeric_primary_key_build, 'author_id' => column_build(column_type: 'Number'), + 'editor_id' => column_build(column_type: 'Number'), 'author' => many_to_one_build(foreign_collection: 'person', foreign_key: 'author_id'), + 'editor' => many_to_one_build(foreign_collection: 'person', foreign_key: 'editor_id'), 'title' => column_build, 'price' => column_build(column_type: 'Number') } @@ -62,6 +64,28 @@ module LazyJoin expect(result).to eq([{ 'id' => 1, 'author' => { 'id' => 2 } }, { 'id' => 2, 'author' => { 'id' => 5 } }]) end + it 'not join when projection ask for target field only with multiple relations' do + allow(@collection_book).to receive(:list).and_return( + [ + { 'id' => 1, 'author_id' => 2, 'editor_id' => 3 }, + { 'id' => 2, 'author_id' => 5, 'editor_id' => 4 } + ] + ) + + result = @datasource_decorator.get_collection('book') + .list(caller, Filter.new, Projection.new(%w[id author:id editor:id])) + + expect(@collection_book).to have_received(:list) do |_caller, _filter, projection| + expect(projection).to eq(%w[id author_id editor_id]) + end + expect(result).to eq( + [ + { 'id' => 1, 'author' => { 'id' => 2 }, 'editor' => { 'id' => 3 } }, + { 'id' => 2, 'author' => { 'id' => 5 }, 'editor' => { 'id' => 4 } } + ] + ) + end + it 'join when projection ask for multiple fields in foreign collection' do allow(@collection_book).to receive(:list).and_return( [