From 0ecd97885610f3e72b621f847d22bcb361db657f Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 10:30:49 +0100 Subject: [PATCH 01/15] chore: first draft of this decorator --- .../decorators/decorator_stack.py | 2 + .../decorators/perf_optimizer/__init__.py | 0 .../decorators/perf_optimizer/collection.py | 48 +++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/__init__.py create mode 100644 src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py index 78af2edbb..a97efde47 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py @@ -12,6 +12,7 @@ OperatorEquivalenceCollectionDecorator, ) from forestadmin.datasource_toolkit.decorators.override.collection import OverrideCollectionDecorator +from forestadmin.datasource_toolkit.decorators.perf_optimizer.collection import PerfOptimizerCollectionDecorator from forestadmin.datasource_toolkit.decorators.publication.datasource import PublicationDataSourceDecorator from forestadmin.datasource_toolkit.decorators.relation.collections import RelationCollectionDecorator from forestadmin.datasource_toolkit.decorators.rename_field.collections import RenameFieldCollectionDecorator @@ -32,6 +33,7 @@ def __init__(self, datasource: Datasource) -> None: # Step 0: Do not query datasource when we know the result with yield an empty set. last = self.override = DatasourceDecorator(last, OverrideCollectionDecorator) # type: ignore last = self.empty = DatasourceDecorator(last, EmptyCollectionDecorator) # type: ignore + last = self.perf_optimizer = DatasourceDecorator(last, PerfOptimizerCollectionDecorator) # type: ignore # Step 1: Computed-Relation-Computed sandwich (needed because some emulated relations depend # on computed fields, and some computed fields depend on relation...) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/__init__.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py new file mode 100644 index 000000000..2cd392bd7 --- /dev/null +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py @@ -0,0 +1,48 @@ +from typing import List + +from forestadmin.agent_toolkit.utils.context import User +from forestadmin.datasource_toolkit.decorators.collection_decorator import CollectionDecorator +from forestadmin.datasource_toolkit.interfaces.fields import is_many_to_one +from forestadmin.datasource_toolkit.interfaces.query.filter.paginated import PaginatedFilter +from forestadmin.datasource_toolkit.interfaces.query.projections import Projection +from forestadmin.datasource_toolkit.interfaces.records import RecordsDataAlias + + +class PerfOptimizerCollectionDecorator(CollectionDecorator): + async def list(self, caller: User, filter_: PaginatedFilter, projection: Projection) -> List[RecordsDataAlias]: + simplified_projection = self._get_simplified_projection(projection) + + ret = await super().list(caller, filter_, simplified_projection) + + return self._apply_simplification_to_records(projection, simplified_projection, ret) + + def _get_simplified_projection(self, projection: Projection) -> Projection: + returned_projection = Projection(*projection) + for relation, relation_projections in projection.relations.items(): + relation_schema = self.schema["fields"][relation] + + if is_many_to_one(relation_schema): + if len(relation_projections) == 1 and relation_schema["foreign_key_target"] == relation_projections[0]: + returned_projection.remove(f"{relation}:{relation_projections[0]}") + + return returned_projection + + def _apply_simplification_to_records( + self, desired_projection: Projection, record_projection: Projection, records: List[RecordsDataAlias] + ) -> List[RecordsDataAlias]: + if record_projection == desired_projection: + return records + + projection_differences = Projection(*[p for p in desired_projection if p not in record_projection]) + + for relation, relation_projections in projection_differences.relations.items(): + relation_schema = self.schema["fields"][relation] + + if is_many_to_one(relation_schema): + if len(relation_projections) == 1 and relation_schema["foreign_key_target"] == relation_projections[0]: + for record in records: + # TODO: verify about this assertion and remove it, or deal it another way + assert relation not in record + record[relation] = {relation_projections[0]: record[relation_schema["foreign_key_target"]]} + + return records From bd331fe1d3be2fb49df219619efc1d7cd9eb7b61 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 11:10:00 +0100 Subject: [PATCH 02/15] chore: handle foreign key fields too --- .../decorators/perf_optimizer/collection.py | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py index 2cd392bd7..ebc15a85f 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py @@ -23,26 +23,37 @@ def _get_simplified_projection(self, projection: Projection) -> Projection: if is_many_to_one(relation_schema): if len(relation_projections) == 1 and relation_schema["foreign_key_target"] == relation_projections[0]: + # remove foreign key target from projection returned_projection.remove(f"{relation}:{relation_projections[0]}") + # add foreign keys to projection + if relation_schema["foreign_key"] not in returned_projection: + returned_projection.append(relation_schema["foreign_key"]) return returned_projection def _apply_simplification_to_records( - self, desired_projection: Projection, record_projection: Projection, records: List[RecordsDataAlias] + self, initial_projection: Projection, requested_projection: Projection, records: List[RecordsDataAlias] ) -> List[RecordsDataAlias]: - if record_projection == desired_projection: + if requested_projection == initial_projection: return records - projection_differences = Projection(*[p for p in desired_projection if p not in record_projection]) + projections_to_add = Projection(*[p for p in initial_projection if p not in requested_projection]) + projections_to_rm = Projection(*[p for p in requested_projection if p not in initial_projection]) - for relation, relation_projections in projection_differences.relations.items(): - relation_schema = self.schema["fields"][relation] + for record in records: + # add to records relation:id + for relation, relation_projections in projections_to_add.relations.items(): + relation_schema = self.schema["fields"][relation] - if is_many_to_one(relation_schema): - if len(relation_projections) == 1 and relation_schema["foreign_key_target"] == relation_projections[0]: - for record in records: - # TODO: verify about this assertion and remove it, or deal it another way - assert relation not in record + if is_many_to_one(relation_schema): + if ( + len(relation_projections) == 1 + and relation_schema["foreign_key_target"] == relation_projections[0] + ): record[relation] = {relation_projections[0]: record[relation_schema["foreign_key_target"]]} + # remove foreign keys + for projection in projections_to_rm: + del record[projection] + return records From c591ddbe2975107f1c4bef582cc55efb8269bb8c Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 11:12:47 +0100 Subject: [PATCH 03/15] chore: remove useless if --- .../decorators/perf_optimizer/collection.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py index ebc15a85f..145e3432d 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py @@ -46,11 +46,7 @@ def _apply_simplification_to_records( relation_schema = self.schema["fields"][relation] if is_many_to_one(relation_schema): - if ( - len(relation_projections) == 1 - and relation_schema["foreign_key_target"] == relation_projections[0] - ): - record[relation] = {relation_projections[0]: record[relation_schema["foreign_key_target"]]} + record[relation] = {relation_projections[0]: record[relation_schema["foreign_key_target"]]} # remove foreign keys for projection in projections_to_rm: From 8e0c5bbb6e94e40ee6b6b998c28b104e3cd927b9 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 11:23:37 +0100 Subject: [PATCH 04/15] refactor: modify names --- .../datasource_toolkit/decorators/decorator_stack.py | 4 ++-- .../{perf_optimizer => lazy_join}/__init__.py | 0 .../{perf_optimizer => lazy_join}/collection.py | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) rename src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/{perf_optimizer => lazy_join}/__init__.py (100%) rename src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/{perf_optimizer => lazy_join}/collection.py (87%) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py index a97efde47..1b78ee06b 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py @@ -7,12 +7,12 @@ from forestadmin.datasource_toolkit.decorators.datasource_decorator import DatasourceDecorator from forestadmin.datasource_toolkit.decorators.empty.collection import EmptyCollectionDecorator from forestadmin.datasource_toolkit.decorators.hook.collections import CollectionHookDecorator +from forestadmin.datasource_toolkit.decorators.lazy_join.collection import LazyJoinCollectionDecorator from forestadmin.datasource_toolkit.decorators.operators_emulate.collections import OperatorsEmulateCollectionDecorator from forestadmin.datasource_toolkit.decorators.operators_equivalence.collections import ( OperatorEquivalenceCollectionDecorator, ) from forestadmin.datasource_toolkit.decorators.override.collection import OverrideCollectionDecorator -from forestadmin.datasource_toolkit.decorators.perf_optimizer.collection import PerfOptimizerCollectionDecorator from forestadmin.datasource_toolkit.decorators.publication.datasource import PublicationDataSourceDecorator from forestadmin.datasource_toolkit.decorators.relation.collections import RelationCollectionDecorator from forestadmin.datasource_toolkit.decorators.rename_field.collections import RenameFieldCollectionDecorator @@ -33,7 +33,7 @@ def __init__(self, datasource: Datasource) -> None: # Step 0: Do not query datasource when we know the result with yield an empty set. last = self.override = DatasourceDecorator(last, OverrideCollectionDecorator) # type: ignore last = self.empty = DatasourceDecorator(last, EmptyCollectionDecorator) # type: ignore - last = self.perf_optimizer = DatasourceDecorator(last, PerfOptimizerCollectionDecorator) # type: ignore + last = self.lazy_joins = DatasourceDecorator(last, LazyJoinCollectionDecorator) # type: ignore # Step 1: Computed-Relation-Computed sandwich (needed because some emulated relations depend # on computed fields, and some computed fields depend on relation...) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/__init__.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/__init__.py similarity index 100% rename from src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/__init__.py rename to src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/__init__.py diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py similarity index 87% rename from src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py rename to src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py index 145e3432d..78738821a 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/perf_optimizer/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py @@ -8,15 +8,15 @@ from forestadmin.datasource_toolkit.interfaces.records import RecordsDataAlias -class PerfOptimizerCollectionDecorator(CollectionDecorator): +class LazyJoinCollectionDecorator(CollectionDecorator): async def list(self, caller: User, filter_: PaginatedFilter, projection: Projection) -> List[RecordsDataAlias]: - simplified_projection = self._get_simplified_projection(projection) + simplified_projection = self._get_projection_without_joins(projection) ret = await super().list(caller, filter_, simplified_projection) - return self._apply_simplification_to_records(projection, simplified_projection, ret) + return self._apply_joins_on_records(projection, simplified_projection, ret) - def _get_simplified_projection(self, projection: Projection) -> Projection: + def _get_projection_without_joins(self, projection: Projection) -> Projection: returned_projection = Projection(*projection) for relation, relation_projections in projection.relations.items(): relation_schema = self.schema["fields"][relation] @@ -31,7 +31,7 @@ def _get_simplified_projection(self, projection: Projection) -> Projection: return returned_projection - def _apply_simplification_to_records( + def _apply_joins_on_records( self, initial_projection: Projection, requested_projection: Projection, records: List[RecordsDataAlias] ) -> List[RecordsDataAlias]: if requested_projection == initial_projection: From d5fba76401e5dc8540b247af028d317339577df7 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 11:30:13 +0100 Subject: [PATCH 05/15] chore: fix existing tests --- src/datasource_toolkit/tests/decorators/test_decorator_stack.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/datasource_toolkit/tests/decorators/test_decorator_stack.py b/src/datasource_toolkit/tests/decorators/test_decorator_stack.py index 07fd3f2be..21f2033a4 100644 --- a/src/datasource_toolkit/tests/decorators/test_decorator_stack.py +++ b/src/datasource_toolkit/tests/decorators/test_decorator_stack.py @@ -9,6 +9,7 @@ from forestadmin.datasource_toolkit.decorators.decorator_stack import DecoratorStack from forestadmin.datasource_toolkit.decorators.empty.collection import EmptyCollectionDecorator from forestadmin.datasource_toolkit.decorators.hook.collections import CollectionHookDecorator +from forestadmin.datasource_toolkit.decorators.lazy_join.collection import LazyJoinCollectionDecorator from forestadmin.datasource_toolkit.decorators.operators_emulate.collections import OperatorsEmulateCollectionDecorator from forestadmin.datasource_toolkit.decorators.operators_equivalence.collections import ( OperatorEquivalenceCollectionDecorator, @@ -69,6 +70,7 @@ def test_creation_instantiate_all_decorator_with_datasource_decorator(self): call_list = [ call(self.datasource, EmptyCollectionDecorator), + call(self.datasource, LazyJoinCollectionDecorator), call(self.datasource, ComputedCollectionDecorator), call(self.datasource, OperatorsEmulateCollectionDecorator), call(self.datasource, OperatorEquivalenceCollectionDecorator), From 1e9a9a301dbad205c9ea5090e1981ef938731843 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 14:44:35 +0100 Subject: [PATCH 06/15] chore: add tests --- .../decorators/decorator_stack.py | 2 +- .../tests/decorators/lazy_join/__init__.py | 0 .../lazy_join/test_lazy_join_decorator.py | 108 ++++++++++++++++++ .../tests/decorators/test_decorator_stack.py | 4 +- 4 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 src/datasource_toolkit/tests/decorators/lazy_join/__init__.py create mode 100644 src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py index 1b78ee06b..5783b72fc 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py @@ -32,8 +32,8 @@ def __init__(self, datasource: Datasource) -> None: # Step 0: Do not query datasource when we know the result with yield an empty set. last = self.override = DatasourceDecorator(last, OverrideCollectionDecorator) # type: ignore - last = self.empty = DatasourceDecorator(last, EmptyCollectionDecorator) # type: ignore last = self.lazy_joins = DatasourceDecorator(last, LazyJoinCollectionDecorator) # type: ignore + last = self.empty = DatasourceDecorator(last, EmptyCollectionDecorator) # type: ignore # Step 1: Computed-Relation-Computed sandwich (needed because some emulated relations depend # on computed fields, and some computed fields depend on relation...) diff --git a/src/datasource_toolkit/tests/decorators/lazy_join/__init__.py b/src/datasource_toolkit/tests/decorators/lazy_join/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py b/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py new file mode 100644 index 000000000..40adcd9bd --- /dev/null +++ b/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py @@ -0,0 +1,108 @@ +import asyncio +import sys +from unittest import TestCase +from unittest.mock import AsyncMock, patch + +from forestadmin.datasource_toolkit.decorators.lazy_join.collection import LazyJoinCollectionDecorator + +if sys.version_info >= (3, 9): + import zoneinfo +else: + from backports import zoneinfo + +from forestadmin.agent_toolkit.utils.context import User +from forestadmin.datasource_toolkit.collections import Collection +from forestadmin.datasource_toolkit.datasources import Datasource +from forestadmin.datasource_toolkit.decorators.datasource_decorator import DatasourceDecorator +from forestadmin.datasource_toolkit.interfaces.fields import Column, FieldType, ManyToOne, OneToOne, PrimitiveType +from forestadmin.datasource_toolkit.interfaces.query.filter.unpaginated import Filter +from forestadmin.datasource_toolkit.interfaces.query.projections import Projection + + +class TestEmptyCollectionDecorator(TestCase): + + @classmethod + def setUpClass(cls) -> None: + cls.loop = asyncio.new_event_loop() + Collection.__abstractmethods__ = set() # to instantiate abstract class + cls.datasource: Datasource = Datasource() + cls.datasource.get_collection = lambda x: cls.datasource._collections[x] + cls.mocked_caller = User( + rendering_id=1, + user_id=1, + tags={}, + email="dummy@user.fr", + first_name="dummy", + last_name="user", + team="operational", + timezone=zoneinfo.ZoneInfo("Europe/Paris"), + request={"ip": "127.0.0.1"}, + ) + + cls.collection_book = Collection("Book", cls.datasource) + cls.collection_book.add_fields( + { + "id": Column(column_type=PrimitiveType.NUMBER, is_primary_key=True, type=FieldType.COLUMN), + "author_id": Column(column_type=PrimitiveType.NUMBER, type=FieldType.COLUMN), + "author": ManyToOne( + foreign_collection="Person", + foreign_key="author_id", + foreign_key_target="id", + type=FieldType.MANY_TO_ONE, + ), + "title": Column( + column_type=PrimitiveType.STRING, + type=FieldType.COLUMN, + ), + } + ) + cls.collection_person = Collection("Person", cls.datasource) + cls.collection_person.add_fields( + { + "id": Column(column_type=PrimitiveType.NUMBER, is_primary_key=True, type=FieldType.COLUMN), + "first_name": Column(column_type=PrimitiveType.STRING, type=FieldType.COLUMN), + "last_name": Column(column_type=PrimitiveType.STRING, type=FieldType.COLUMN), + "book": OneToOne(origin_key="author_id", origin_key_target="id", foreign_collection="Book"), + } + ) + + cls.datasource.add_collection(cls.collection_book) + cls.datasource.add_collection(cls.collection_person) + cls.datasource_decorator = DatasourceDecorator(cls.datasource, LazyJoinCollectionDecorator) + cls.decorated_book_collection = cls.datasource_decorator.get_collection("Book") + + def test_should_not_join_when_projection_ask_for_target_field_only(self): + with patch.object( + self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 1}] + ) as mock_list: + result = self.loop.run_until_complete( + self.decorated_book_collection.list( + self.mocked_caller, + Filter({}), + Projection("id", "author:id"), + ) + ) + mock_list.assert_awaited_once_with(self.mocked_caller, Filter({}), Projection("id", "author_id")) + + # should contain author object, without author_id FK + self.assertEqual([{"id": 1, "author": {"id": 1}}], result) + + def test_should_join_when_projection_ask_for_multiple_fields_in_relation(self): + with patch.object( + self.collection_book, + "list", + new_callable=AsyncMock, + return_value=[{"id": 1, "author": {"id": 1, "first_name": "Isaac"}}], + ) as mock_list: + result = self.loop.run_until_complete( + self.decorated_book_collection.list( + self.mocked_caller, + Filter({}), + Projection("id", "author:id", "author:first_name"), + ) + ) + mock_list.assert_awaited_once_with( + self.mocked_caller, Filter({}), Projection("id", "author:id", "author:first_name") + ) + + self.assertEqual([{"id": 1, "author": {"id": 1, "first_name": "Isaac"}}], result) diff --git a/src/datasource_toolkit/tests/decorators/test_decorator_stack.py b/src/datasource_toolkit/tests/decorators/test_decorator_stack.py index 21f2033a4..608b8ba73 100644 --- a/src/datasource_toolkit/tests/decorators/test_decorator_stack.py +++ b/src/datasource_toolkit/tests/decorators/test_decorator_stack.py @@ -14,6 +14,7 @@ from forestadmin.datasource_toolkit.decorators.operators_equivalence.collections import ( OperatorEquivalenceCollectionDecorator, ) +from forestadmin.datasource_toolkit.decorators.override.collection import OverrideCollectionDecorator from forestadmin.datasource_toolkit.decorators.relation.collections import RelationCollectionDecorator from forestadmin.datasource_toolkit.decorators.rename_field.collections import RenameFieldCollectionDecorator from forestadmin.datasource_toolkit.decorators.schema.collection import SchemaCollectionDecorator @@ -69,8 +70,9 @@ def test_creation_instantiate_all_decorator_with_datasource_decorator(self): DecoratorStack(self.datasource) call_list = [ - call(self.datasource, EmptyCollectionDecorator), + call(self.datasource, OverrideCollectionDecorator), call(self.datasource, LazyJoinCollectionDecorator), + call(self.datasource, EmptyCollectionDecorator), call(self.datasource, ComputedCollectionDecorator), call(self.datasource, OperatorsEmulateCollectionDecorator), call(self.datasource, OperatorEquivalenceCollectionDecorator), From 124add485f53c3d732ba81e47d94fe737faffac8 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 14:54:36 +0100 Subject: [PATCH 07/15] chore: modify function naming --- .../datasource_toolkit/decorators/lazy_join/collection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py index 78738821a..524e28ef4 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py @@ -10,13 +10,13 @@ class LazyJoinCollectionDecorator(CollectionDecorator): async def list(self, caller: User, filter_: PaginatedFilter, projection: Projection) -> List[RecordsDataAlias]: - simplified_projection = self._get_projection_without_joins(projection) + simplified_projection = self._get_projection_without_useless_joins(projection) ret = await super().list(caller, filter_, simplified_projection) return self._apply_joins_on_records(projection, simplified_projection, ret) - def _get_projection_without_joins(self, projection: Projection) -> Projection: + def _get_projection_without_useless_joins(self, projection: Projection) -> Projection: returned_projection = Projection(*projection) for relation, relation_projections in projection.relations.items(): relation_schema = self.schema["fields"][relation] From cb8f1075e3234d2c4b0e3d8ec9baebf8be88210f Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 16:53:36 +0100 Subject: [PATCH 08/15] chore: handle refine filter --- .../decorators/lazy_join/collection.py | 57 +++++++++++++++---- .../query/condition_tree/nodes/base.py | 9 ++- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py index 524e28ef4..2bb9a5e81 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py @@ -1,9 +1,10 @@ -from typing import List +from typing import List, Union from forestadmin.agent_toolkit.utils.context import User from forestadmin.datasource_toolkit.decorators.collection_decorator import CollectionDecorator from forestadmin.datasource_toolkit.interfaces.fields import is_many_to_one from forestadmin.datasource_toolkit.interfaces.query.filter.paginated import PaginatedFilter +from forestadmin.datasource_toolkit.interfaces.query.filter.unpaginated import Filter from forestadmin.datasource_toolkit.interfaces.query.projections import Projection from forestadmin.datasource_toolkit.interfaces.records import RecordsDataAlias @@ -16,18 +17,54 @@ async def list(self, caller: User, filter_: PaginatedFilter, projection: Project return self._apply_joins_on_records(projection, simplified_projection, ret) + async def _refine_filter( + self, caller: User, _filter: Union[Filter, PaginatedFilter, None] + ) -> Union[Filter, PaginatedFilter, None]: + if _filter is None or _filter.condition_tree is None: + return _filter + + _filter.condition_tree = _filter.condition_tree.replace( + lambda leaf: ( + { + "field": self._get_fk_field_for_projection(leaf.field), + "operator": leaf.operator, + "value": leaf.value, + } + if self._is_useless_join(leaf.field.split(":")[0], _filter.condition_tree.projection) + else leaf + ) # type:ignore + ) + + return _filter + + def _is_useless_join(self, relation: str, projection: Projection) -> bool: + relation_schema = self.schema["fields"][relation] + sub_projections = projection.relations[relation] + + return ( + is_many_to_one(relation_schema) + and len(sub_projections) == 1 + and sub_projections[0] == relation_schema["foreign_key_target"] + ) + + def _get_fk_field_for_projection(self, projection: str) -> str: + relation_name = projection.split(":")[0] + relation_schema = self.schema["fields"][relation_name] + + # assert is_many_to_one(relation_schema) + return relation_schema["foreign_key"] # type:ignore + def _get_projection_without_useless_joins(self, projection: Projection) -> Projection: returned_projection = Projection(*projection) for relation, relation_projections in projection.relations.items(): - relation_schema = self.schema["fields"][relation] - - if is_many_to_one(relation_schema): - if len(relation_projections) == 1 and relation_schema["foreign_key_target"] == relation_projections[0]: - # remove foreign key target from projection - returned_projection.remove(f"{relation}:{relation_projections[0]}") - # add foreign keys to projection - if relation_schema["foreign_key"] not in returned_projection: - returned_projection.append(relation_schema["foreign_key"]) + if self._is_useless_join(relation, projection): + # remove foreign key target from projection + returned_projection.remove(f"{relation}:{relation_projections[0]}") + + # add foreign keys to projection + fk_field = self._get_fk_field_for_projection(relation) + if fk_field not in returned_projection: + returned_projection.append(fk_field) return returned_projection diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/interfaces/query/condition_tree/nodes/base.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/interfaces/query/condition_tree/nodes/base.py index 9688aa67c..889df3060 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/interfaces/query/condition_tree/nodes/base.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/interfaces/query/condition_tree/nodes/base.py @@ -1,6 +1,8 @@ +from __future__ import annotations + import abc import sys -from typing import Any, Awaitable, Callable, Dict, List, TypeVar, Union +from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, List, TypeVar, Union if sys.version_info >= (3, 9): import zoneinfo @@ -13,6 +15,9 @@ from forestadmin.datasource_toolkit.interfaces.records import RecordsDataAlias from typing_extensions import TypedDict +if TYPE_CHECKING: + from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.leaf import ConditionTreeLeaf + class ConditionTreeException(DatasourceToolkitException): pass @@ -75,7 +80,7 @@ class ConditionTreeComponent(TypedDict): HandlerResult = TypeVar("HandlerResult") -HandlerAlias = Callable[[ConditionTree], HandlerResult] +HandlerAlias = Callable[["ConditionTreeLeaf"], HandlerResult] ReplacerAlias = HandlerAlias[Union[ConditionTree, ConditionTreeComponent]] AsyncReplacerAlias = HandlerAlias[Awaitable[Union[ConditionTree, ConditionTreeComponent]]] CallbackAlias = HandlerAlias[None] From ad7e3c1b707c8ac7d7b0aa343c73414cc98ce85c Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 18:05:45 +0100 Subject: [PATCH 09/15] fix: correctly refined filter and call child ds --- .../datasource_toolkit/decorators/lazy_join/collection.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py index 2bb9a5e81..efbeb242a 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py @@ -1,4 +1,4 @@ -from typing import List, Union +from typing import List, Union, cast from forestadmin.agent_toolkit.utils.context import User from forestadmin.datasource_toolkit.decorators.collection_decorator import CollectionDecorator @@ -11,9 +11,10 @@ class LazyJoinCollectionDecorator(CollectionDecorator): async def list(self, caller: User, filter_: PaginatedFilter, projection: Projection) -> List[RecordsDataAlias]: + refined_filter = cast(PaginatedFilter, await self._refine_filter(caller, filter_)) simplified_projection = self._get_projection_without_useless_joins(projection) - ret = await super().list(caller, filter_, simplified_projection) + ret = await self.child_collection.list(caller, refined_filter, simplified_projection) return self._apply_joins_on_records(projection, simplified_projection, ret) From e4bb20daf3061fba7095861d772629e8f2b8d856 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 18:06:02 +0100 Subject: [PATCH 10/15] fix: bad remapping of results --- .../datasource_toolkit/decorators/lazy_join/collection.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py index efbeb242a..b2e51dab1 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py @@ -84,7 +84,11 @@ def _apply_joins_on_records( relation_schema = self.schema["fields"][relation] if is_many_to_one(relation_schema): - record[relation] = {relation_projections[0]: record[relation_schema["foreign_key_target"]]} + record[relation] = { + relation_projections[0]: record[ + self._get_fk_field_for_projection(f"{relation}:{relation_projections[0]}") + ] + } # remove foreign keys for projection in projections_to_rm: From e23b8cd6c2d0fb44ea6586f4f8e4a79d3b770926 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Mon, 9 Dec 2024 18:06:50 +0100 Subject: [PATCH 11/15] chore: add few tests --- .../lazy_join/test_lazy_join_decorator.py | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py b/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py index 40adcd9bd..b1257b61c 100644 --- a/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py +++ b/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py @@ -3,8 +3,6 @@ from unittest import TestCase from unittest.mock import AsyncMock, patch -from forestadmin.datasource_toolkit.decorators.lazy_join.collection import LazyJoinCollectionDecorator - if sys.version_info >= (3, 9): import zoneinfo else: @@ -14,13 +12,14 @@ from forestadmin.datasource_toolkit.collections import Collection from forestadmin.datasource_toolkit.datasources import Datasource from forestadmin.datasource_toolkit.decorators.datasource_decorator import DatasourceDecorator +from forestadmin.datasource_toolkit.decorators.lazy_join.collection import LazyJoinCollectionDecorator from forestadmin.datasource_toolkit.interfaces.fields import Column, FieldType, ManyToOne, OneToOne, PrimitiveType +from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.leaf import ConditionTreeLeaf from forestadmin.datasource_toolkit.interfaces.query.filter.unpaginated import Filter from forestadmin.datasource_toolkit.interfaces.query.projections import Projection class TestEmptyCollectionDecorator(TestCase): - @classmethod def setUpClass(cls) -> None: cls.loop = asyncio.new_event_loop() @@ -73,7 +72,7 @@ def setUpClass(cls) -> None: def test_should_not_join_when_projection_ask_for_target_field_only(self): with patch.object( - self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 1}] + self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 2}] ) as mock_list: result = self.loop.run_until_complete( self.decorated_book_collection.list( @@ -85,14 +84,14 @@ def test_should_not_join_when_projection_ask_for_target_field_only(self): mock_list.assert_awaited_once_with(self.mocked_caller, Filter({}), Projection("id", "author_id")) # should contain author object, without author_id FK - self.assertEqual([{"id": 1, "author": {"id": 1}}], result) + self.assertEqual([{"id": 1, "author": {"id": 2}}], result) def test_should_join_when_projection_ask_for_multiple_fields_in_relation(self): with patch.object( self.collection_book, "list", new_callable=AsyncMock, - return_value=[{"id": 1, "author": {"id": 1, "first_name": "Isaac"}}], + return_value=[{"id": 1, "author": {"id": 2, "first_name": "Isaac"}}], ) as mock_list: result = self.loop.run_until_complete( self.decorated_book_collection.list( @@ -105,4 +104,38 @@ def test_should_join_when_projection_ask_for_multiple_fields_in_relation(self): self.mocked_caller, Filter({}), Projection("id", "author:id", "author:first_name") ) - self.assertEqual([{"id": 1, "author": {"id": 1, "first_name": "Isaac"}}], result) + self.assertEqual([{"id": 1, "author": {"id": 2, "first_name": "Isaac"}}], result) + + def test_should_replace_in_condition_tree(self): + with patch.object( + self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 2}] + ) as mock_list: + self.loop.run_until_complete( + self.decorated_book_collection.list( + self.mocked_caller, + Filter({"condition_tree": ConditionTreeLeaf("author:id", "equal", 2)}), + Projection("id", "author:id"), + ) + ) + mock_list.assert_awaited_once_with( + self.mocked_caller, + Filter({"condition_tree": ConditionTreeLeaf("author_id", "equal", 2)}), + Projection("id", "author_id"), + ) + + def test_should_not_replace_in_condition_tree(self): + with patch.object( + self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 2}] + ) as mock_list: + self.loop.run_until_complete( + self.decorated_book_collection.list( + self.mocked_caller, + Filter({"condition_tree": ConditionTreeLeaf("author:first_name", "equal", "Isaac")}), + Projection("id", "author:id"), + ) + ) + mock_list.assert_awaited_once_with( + self.mocked_caller, + Filter({"condition_tree": ConditionTreeLeaf("author:first_name", "equal", "Isaac")}), + Projection("id", "author_id"), + ) From 7640f0a97c1d06e5e94a701c38aee925d73a50df Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 10 Dec 2024 09:44:59 +0100 Subject: [PATCH 12/15] chore: change the position in decorator stack --- .../datasource_toolkit/decorators/decorator_stack.py | 2 +- src/datasource_toolkit/tests/decorators/test_decorator_stack.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py index 5783b72fc..1dd5c5d0f 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py @@ -32,7 +32,6 @@ def __init__(self, datasource: Datasource) -> None: # Step 0: Do not query datasource when we know the result with yield an empty set. last = self.override = DatasourceDecorator(last, OverrideCollectionDecorator) # type: ignore - last = self.lazy_joins = DatasourceDecorator(last, LazyJoinCollectionDecorator) # type: ignore last = self.empty = DatasourceDecorator(last, EmptyCollectionDecorator) # type: ignore # Step 1: Computed-Relation-Computed sandwich (needed because some emulated relations depend @@ -42,6 +41,7 @@ def __init__(self, datasource: Datasource) -> None: last = self.early_op_emulate = DatasourceDecorator(last, OperatorsEmulateCollectionDecorator) last = self.early_op_equivalence = DatasourceDecorator(last, OperatorEquivalenceCollectionDecorator) last = self.relation = DatasourceDecorator(last, RelationCollectionDecorator) + last = self.lazy_joins = DatasourceDecorator(last, LazyJoinCollectionDecorator) # type: ignore last = self.late_computed = DatasourceDecorator(last, ComputedCollectionDecorator) last = self.late_op_emulate = DatasourceDecorator(last, OperatorsEmulateCollectionDecorator) last = self.late_op_equivalence = DatasourceDecorator(last, OperatorEquivalenceCollectionDecorator) diff --git a/src/datasource_toolkit/tests/decorators/test_decorator_stack.py b/src/datasource_toolkit/tests/decorators/test_decorator_stack.py index 608b8ba73..7e9a84341 100644 --- a/src/datasource_toolkit/tests/decorators/test_decorator_stack.py +++ b/src/datasource_toolkit/tests/decorators/test_decorator_stack.py @@ -71,12 +71,12 @@ def test_creation_instantiate_all_decorator_with_datasource_decorator(self): call_list = [ call(self.datasource, OverrideCollectionDecorator), - call(self.datasource, LazyJoinCollectionDecorator), call(self.datasource, EmptyCollectionDecorator), call(self.datasource, ComputedCollectionDecorator), call(self.datasource, OperatorsEmulateCollectionDecorator), call(self.datasource, OperatorEquivalenceCollectionDecorator), call(self.datasource, RelationCollectionDecorator), + call(self.datasource, LazyJoinCollectionDecorator), call(self.datasource, ComputedCollectionDecorator), call(self.datasource, OperatorsEmulateCollectionDecorator), call(self.datasource, OperatorEquivalenceCollectionDecorator), From d4f84cc2765672107193d1fbd25414cfb38401f8 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 10 Dec 2024 10:22:19 +0100 Subject: [PATCH 13/15] chore: add tests --- .../lazy_join/test_lazy_join_decorator.py | 126 +++++++++++++++--- 1 file changed, 106 insertions(+), 20 deletions(-) diff --git a/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py b/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py index b1257b61c..64f8c789e 100644 --- a/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py +++ b/src/datasource_toolkit/tests/decorators/lazy_join/test_lazy_join_decorator.py @@ -13,9 +13,9 @@ from forestadmin.datasource_toolkit.datasources import Datasource from forestadmin.datasource_toolkit.decorators.datasource_decorator import DatasourceDecorator from forestadmin.datasource_toolkit.decorators.lazy_join.collection import LazyJoinCollectionDecorator -from forestadmin.datasource_toolkit.interfaces.fields import Column, FieldType, ManyToOne, OneToOne, PrimitiveType +from forestadmin.datasource_toolkit.interfaces.fields import Column, FieldType, ManyToOne, OneToMany, PrimitiveType from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.leaf import ConditionTreeLeaf -from forestadmin.datasource_toolkit.interfaces.query.filter.unpaginated import Filter +from forestadmin.datasource_toolkit.interfaces.query.filter.paginated import PaginatedFilter from forestadmin.datasource_toolkit.interfaces.query.projections import Projection @@ -61,7 +61,7 @@ def setUpClass(cls) -> None: "id": Column(column_type=PrimitiveType.NUMBER, is_primary_key=True, type=FieldType.COLUMN), "first_name": Column(column_type=PrimitiveType.STRING, type=FieldType.COLUMN), "last_name": Column(column_type=PrimitiveType.STRING, type=FieldType.COLUMN), - "book": OneToOne(origin_key="author_id", origin_key_target="id", foreign_collection="Book"), + "books": OneToMany(origin_key="author_id", origin_key_target="id", foreign_collection="Book"), } ) @@ -72,70 +72,156 @@ def setUpClass(cls) -> None: def test_should_not_join_when_projection_ask_for_target_field_only(self): with patch.object( - self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 2}] + self.collection_book, + "list", + new_callable=AsyncMock, + return_value=[{"id": 1, "author_id": 2}, {"id": 2, "author_id": 5}], ) as mock_list: result = self.loop.run_until_complete( self.decorated_book_collection.list( self.mocked_caller, - Filter({}), + PaginatedFilter({}), Projection("id", "author:id"), ) ) - mock_list.assert_awaited_once_with(self.mocked_caller, Filter({}), Projection("id", "author_id")) + mock_list.assert_awaited_once_with(self.mocked_caller, PaginatedFilter({}), Projection("id", "author_id")) # should contain author object, without author_id FK - self.assertEqual([{"id": 1, "author": {"id": 2}}], result) + self.assertEqual([{"id": 1, "author": {"id": 2}}, {"id": 2, "author": {"id": 5}}], result) - def test_should_join_when_projection_ask_for_multiple_fields_in_relation(self): + def test_should_join_when_projection_ask_for_multiple_fields_in_foreign_collection(self): with patch.object( self.collection_book, "list", new_callable=AsyncMock, - return_value=[{"id": 1, "author": {"id": 2, "first_name": "Isaac"}}], + return_value=[ + {"id": 1, "author": {"id": 2, "first_name": "Isaac"}}, + {"id": 2, "author": {"id": 5, "first_name": "J.K."}}, + ], ) as mock_list: result = self.loop.run_until_complete( self.decorated_book_collection.list( self.mocked_caller, - Filter({}), + PaginatedFilter({}), Projection("id", "author:id", "author:first_name"), ) ) mock_list.assert_awaited_once_with( - self.mocked_caller, Filter({}), Projection("id", "author:id", "author:first_name") + self.mocked_caller, PaginatedFilter({}), Projection("id", "author:id", "author:first_name") ) - self.assertEqual([{"id": 1, "author": {"id": 2, "first_name": "Isaac"}}], result) + self.assertEqual( + [ + {"id": 1, "author": {"id": 2, "first_name": "Isaac"}}, + {"id": 2, "author": {"id": 5, "first_name": "J.K."}}, + ], + result, + ) - def test_should_replace_in_condition_tree(self): + def test_should_not_join_when_condition_tree_is_on_foreign_key_target(self): with patch.object( - self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 2}] + self.collection_book, + "list", + new_callable=AsyncMock, + return_value=[{"id": 1, "author_id": 2}, {"id": 2, "author_id": 5}, {"id": 3, "author_id": 5}], ) as mock_list: self.loop.run_until_complete( self.decorated_book_collection.list( self.mocked_caller, - Filter({"condition_tree": ConditionTreeLeaf("author:id", "equal", 2)}), + PaginatedFilter({"condition_tree": ConditionTreeLeaf("author:id", "in", [2, 5])}), Projection("id", "author:id"), ) ) mock_list.assert_awaited_once_with( self.mocked_caller, - Filter({"condition_tree": ConditionTreeLeaf("author_id", "equal", 2)}), + PaginatedFilter({"condition_tree": ConditionTreeLeaf("author_id", "in", [2, 5])}), Projection("id", "author_id"), ) - def test_should_not_replace_in_condition_tree(self): + def test_should_join_when_condition_tree_is_on_foreign_collection_fields(self): with patch.object( - self.collection_book, "list", new_callable=AsyncMock, return_value=[{"id": 1, "author_id": 2}] + self.collection_book, + "list", + new_callable=AsyncMock, + return_value=[{"id": 1, "author_id": 2}, {"id": 2, "author_id": 5}, {"id": 3, "author_id": 5}], ) as mock_list: self.loop.run_until_complete( self.decorated_book_collection.list( self.mocked_caller, - Filter({"condition_tree": ConditionTreeLeaf("author:first_name", "equal", "Isaac")}), + PaginatedFilter( + {"condition_tree": ConditionTreeLeaf("author:first_name", "in", ["Isaac", "J.K."])} + ), + Projection("id", "author:id"), + ) + ) + mock_list.assert_awaited_once_with( + self.mocked_caller, + PaginatedFilter({"condition_tree": ConditionTreeLeaf("author:first_name", "in", ["Isaac", "J.K."])}), + Projection("id", "author_id"), + ) + + def test_should_disable_join_on_condition_tree_but_not_in_projection(self): + with patch.object( + self.collection_book, + "list", + new_callable=AsyncMock, + return_value=[ + {"id": 1, "author": {"first_name": "Isaac"}}, + {"id": 2, "author": {"first_name": "J.K."}}, + {"id": 3, "author": {"first_name": "J.K."}}, + ], + ) as mock_list: + response = self.loop.run_until_complete( + self.decorated_book_collection.list( + self.mocked_caller, + PaginatedFilter({"condition_tree": ConditionTreeLeaf("author:id", "in", [2, 5])}), + Projection("id", "author:first_name"), + ) + ) + mock_list.assert_awaited_once_with( + self.mocked_caller, + PaginatedFilter({"condition_tree": ConditionTreeLeaf("author_id", "in", [2, 5])}), + Projection("id", "author:first_name"), + ) + self.assertEqual( + [ + {"id": 1, "author": {"first_name": "Isaac"}}, + {"id": 2, "author": {"first_name": "J.K."}}, + {"id": 3, "author": {"first_name": "J.K."}}, + ], + response, + ) + + def test_should_disable_join_on_projection_but_not_in_condition_tree(self): + with patch.object( + self.collection_book, + "list", + new_callable=AsyncMock, + return_value=[ + {"id": 1, "author_id": 2}, + {"id": 2, "author_id": 5}, + {"id": 3, "author_id": 5}, + ], + ) as mock_list: + response = self.loop.run_until_complete( + self.decorated_book_collection.list( + self.mocked_caller, + PaginatedFilter( + {"condition_tree": ConditionTreeLeaf("author:first_name", "in", ["Isaac", "J.K."])} + ), Projection("id", "author:id"), ) ) mock_list.assert_awaited_once_with( self.mocked_caller, - Filter({"condition_tree": ConditionTreeLeaf("author:first_name", "equal", "Isaac")}), + PaginatedFilter({"condition_tree": ConditionTreeLeaf("author:first_name", "in", ["Isaac", "J.K."])}), Projection("id", "author_id"), ) + self.assertEqual( + [ + {"id": 1, "author": {"id": 2}}, + {"id": 2, "author": {"id": 5}}, + {"id": 3, "author": {"id": 5}}, + ], + response, + ) From e76b149015850fbe066f588172dbdedd37256073 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 10 Dec 2024 10:59:46 +0100 Subject: [PATCH 14/15] chore: code style --- .../decorators/lazy_join/collection.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py index b2e51dab1..e89783717 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/lazy_join/collection.py @@ -2,7 +2,8 @@ from forestadmin.agent_toolkit.utils.context import User from forestadmin.datasource_toolkit.decorators.collection_decorator import CollectionDecorator -from forestadmin.datasource_toolkit.interfaces.fields import is_many_to_one +from forestadmin.datasource_toolkit.interfaces.fields import ManyToOne, is_many_to_one +from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.leaf import ConditionTreeLeaf from forestadmin.datasource_toolkit.interfaces.query.filter.paginated import PaginatedFilter from forestadmin.datasource_toolkit.interfaces.query.filter.unpaginated import Filter from forestadmin.datasource_toolkit.interfaces.query.projections import Projection @@ -11,9 +12,9 @@ class LazyJoinCollectionDecorator(CollectionDecorator): async def list(self, caller: User, filter_: PaginatedFilter, projection: Projection) -> List[RecordsDataAlias]: - refined_filter = cast(PaginatedFilter, await self._refine_filter(caller, filter_)) simplified_projection = self._get_projection_without_useless_joins(projection) + refined_filter = cast(PaginatedFilter, await self._refine_filter(caller, filter_)) ret = await self.child_collection.list(caller, refined_filter, simplified_projection) return self._apply_joins_on_records(projection, simplified_projection, ret) @@ -26,14 +27,14 @@ async def _refine_filter( _filter.condition_tree = _filter.condition_tree.replace( lambda leaf: ( - { - "field": self._get_fk_field_for_projection(leaf.field), - "operator": leaf.operator, - "value": leaf.value, - } + ConditionTreeLeaf( + self._get_fk_field_for_projection(leaf.field), + leaf.operator, + leaf.value, + ) if self._is_useless_join(leaf.field.split(":")[0], _filter.condition_tree.projection) else leaf - ) # type:ignore + ) ) return _filter @@ -50,10 +51,9 @@ def _is_useless_join(self, relation: str, projection: Projection) -> bool: def _get_fk_field_for_projection(self, projection: str) -> str: relation_name = projection.split(":")[0] - relation_schema = self.schema["fields"][relation_name] + relation_schema = cast(ManyToOne, self.schema["fields"][relation_name]) - # assert is_many_to_one(relation_schema) - return relation_schema["foreign_key"] # type:ignore + return relation_schema["foreign_key"] def _get_projection_without_useless_joins(self, projection: Projection) -> Projection: returned_projection = Projection(*projection) From aff9aed53fed96409aa5ef5f1e0c2efb3317a72f Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Tue, 10 Dec 2024 11:07:35 +0100 Subject: [PATCH 15/15] chore: add comment --- .../datasource_toolkit/decorators/decorator_stack.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py index 1dd5c5d0f..5a1d297fd 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/decorators/decorator_stack.py @@ -41,7 +41,8 @@ def __init__(self, datasource: Datasource) -> None: last = self.early_op_emulate = DatasourceDecorator(last, OperatorsEmulateCollectionDecorator) last = self.early_op_equivalence = DatasourceDecorator(last, OperatorEquivalenceCollectionDecorator) last = self.relation = DatasourceDecorator(last, RelationCollectionDecorator) - last = self.lazy_joins = DatasourceDecorator(last, LazyJoinCollectionDecorator) # type: ignore + # lazy join is just before relation, to avoid relations to do useless stuff + last = self.lazy_joins = DatasourceDecorator(last, LazyJoinCollectionDecorator) last = self.late_computed = DatasourceDecorator(last, ComputedCollectionDecorator) last = self.late_op_emulate = DatasourceDecorator(last, OperatorsEmulateCollectionDecorator) last = self.late_op_equivalence = DatasourceDecorator(last, OperatorEquivalenceCollectionDecorator)