From 4ebb75ccfe380de6bff5a70d3a8497177fb521ed Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Fri, 21 Feb 2025 14:56:34 +0100 Subject: [PATCH 1/6] fix: in and notin containing null are converted to or --- .../datasource_django/utils/query_factory.py | 5 +++++ .../utils/query_factory.py | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/datasource_django/forestadmin/datasource_django/utils/query_factory.py b/src/datasource_django/forestadmin/datasource_django/utils/query_factory.py index 4bc12d090..9f246c17b 100644 --- a/src/datasource_django/forestadmin/datasource_django/utils/query_factory.py +++ b/src/datasource_django/forestadmin/datasource_django/utils/query_factory.py @@ -248,6 +248,11 @@ def _build_leaf_condition(cls, leaf: ConditionTreeLeaf) -> models.Q: value = leaf.value if key == "__isnull": value = True + + if key == "__in" and isinstance(value, list) and None in value: + q_obj = cls.build(ConditionTreeBranch("or", [ConditionTreeLeaf(leaf.field, "equal", v) for v in value])) + return ~q_obj if should_negate else q_obj + if should_negate: return ~models.Q(**{f"{field}{key}": value}) else: diff --git a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py index 3dfe5a558..e6a46b9aa 100644 --- a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py +++ b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py @@ -6,6 +6,7 @@ from forestadmin.datasource_sqlalchemy.utils.relationships import Relationships, merge_relationships from forestadmin.datasource_sqlalchemy.utils.type_converter import FilterOperator from forestadmin.datasource_toolkit.exceptions import DatasourceToolkitException +from forestadmin.datasource_toolkit.interfaces.fields import Operator from forestadmin.datasource_toolkit.interfaces.query.aggregation import Aggregation from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.base import ConditionTree from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.branch import Aggregator, ConditionTreeBranch @@ -17,7 +18,7 @@ from forestadmin.datasource_toolkit.interfaces.records import RecordsDataAlias from sqlalchemy import and_ from sqlalchemy import column as SqlAlchemyColumn -from sqlalchemy import delete, or_, select, update +from sqlalchemy import delete, not_, or_, select, update from sqlalchemy.engine import Dialect from sqlalchemy.sql.elements import BooleanClauseList, UnaryExpression @@ -31,6 +32,22 @@ class ConditionTreeFactory: @classmethod def _build_leaf_condition(cls, collection: BaseSqlAlchemyCollection, leaf: ConditionTreeLeaf) -> Tuple[Any, Any]: + if leaf.operator in [Operator.IN, Operator.NOT_IN] and isinstance(leaf.value, list) and None in leaf.value: + operator, relationships = cls._build_branch_condition( + collection, + ConditionTreeBranch( + "or", + [ + ConditionTreeLeaf(leaf.field, Operator.EQUAL, None), + ConditionTreeLeaf(leaf.field, Operator.EQUAL, ""), + ], + ), + ) + return ( + operator if leaf.operator == Operator.IN else not_(operator), + relationships, + ) + projection = leaf.projection columns, relationships = collection.get_columns(projection) operator = FilterOperator.get_operator(columns, leaf.operator) From f27253333266b8ebc95ba6bcad0bb7eb9fdefa8a Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Fri, 21 Feb 2025 14:57:57 +0100 Subject: [PATCH 2/6] fix: blank operator is now always emulate --- .../forestadmin/datasource_django/utils/type_converter.py | 1 - .../datasource_sqlalchemy/utils/type_converter.py | 8 -------- .../forestadmin/datasource_toolkit/utils/operators.py | 1 - 3 files changed, 10 deletions(-) diff --git a/src/datasource_django/forestadmin/datasource_django/utils/type_converter.py b/src/datasource_django/forestadmin/datasource_django/utils/type_converter.py index 2c7cb552e..cff3dd26f 100644 --- a/src/datasource_django/forestadmin/datasource_django/utils/type_converter.py +++ b/src/datasource_django/forestadmin/datasource_django/utils/type_converter.py @@ -107,7 +107,6 @@ class FilterOperator(BaseFilterOperator): # operator: (lookup_expr, negate needed) Operator.EQUAL: ("__exact", False), Operator.NOT_EQUAL: ("__exact", True), - Operator.BLANK: ("__isnull", False), Operator.CONTAINS: ("__icontains", False), Operator.NOT_CONTAINS: ("__icontains", True), Operator.STARTS_WITH: ("__istartswith", False), diff --git a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py index 1eb397f1d..090b76835 100644 --- a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py +++ b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py @@ -68,7 +68,6 @@ class FilterOperator(BaseFilterOperator): OPERATORS = { Operator.EQUAL: "_equal_operator", Operator.NOT_EQUAL: "_not_equal_operator", - Operator.BLANK: "_blank_operator", Operator.CONTAINS: "_contains_operator", Operator.NOT_CONTAINS: "_not_contains_operator", Operator.STARTS_WITH: "_starts_with_operator", @@ -93,13 +92,6 @@ def _equal_operator(column: SqlAlchemyColumn): def _not_equal_operator(column: SqlAlchemyColumn): return column.__ne__ - @staticmethod - def _blank_operator(column: SqlAlchemyColumn): - def wrapped(_: str): - return or_([column.is_(None), column.__eq__("")]) - - return wrapped - @staticmethod def _contains_operator(column: SqlAlchemyColumn): def wrapped(value: str): diff --git a/src/datasource_toolkit/forestadmin/datasource_toolkit/utils/operators.py b/src/datasource_toolkit/forestadmin/datasource_toolkit/utils/operators.py index 981295cb7..0eaa68c61 100644 --- a/src/datasource_toolkit/forestadmin/datasource_toolkit/utils/operators.py +++ b/src/datasource_toolkit/forestadmin/datasource_toolkit/utils/operators.py @@ -5,7 +5,6 @@ class BaseFilterOperator: COMMON_OPERATORS: Set[Operator] = { - Operator.BLANK, Operator.EQUAL, Operator.MISSING, Operator.NOT_EQUAL, From 66b85cd79327a5e446085d0900af44cb91732d70 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Fri, 21 Feb 2025 15:02:34 +0100 Subject: [PATCH 3/6] chore: fix linting --- .../datasource_sqlalchemy/utils/type_converter.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py index 090b76835..526a6ab72 100644 --- a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py +++ b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/type_converter.py @@ -3,9 +3,9 @@ from forestadmin.datasource_toolkit.exceptions import DatasourceToolkitException from forestadmin.datasource_toolkit.interfaces.fields import ColumnAlias, Operator, PrimitiveType from forestadmin.datasource_toolkit.utils.operators import BaseFilterOperator -from sqlalchemy import ARRAY # type: ignore -from sqlalchemy import column as SqlAlchemyColumn # type: ignore -from sqlalchemy import func, not_, or_ # type: ignore +from sqlalchemy import ARRAY +from sqlalchemy import column as SqlAlchemyColumn +from sqlalchemy import func, not_ from sqlalchemy import types as sqltypes from sqlalchemy.dialects.postgresql import UUID From f4d711e5565e215588be187a49ce28e73a26fd8e Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Fri, 21 Feb 2025 15:06:59 +0100 Subject: [PATCH 4/6] chore: fix existing tests --- .../tests/utils/test_django_model_introspection.py | 3 --- src/datasource_sqlalchemy/tests/utils/test_model_converter.py | 1 - 2 files changed, 4 deletions(-) diff --git a/src/datasource_django/tests/utils/test_django_model_introspection.py b/src/datasource_django/tests/utils/test_django_model_introspection.py index a8507b94c..09367e50d 100644 --- a/src/datasource_django/tests/utils/test_django_model_introspection.py +++ b/src/datasource_django/tests/utils/test_django_model_introspection.py @@ -121,7 +121,6 @@ def test_build_should_correctly_introspect_uuid(self): self.assertEqual( field_schema["filter_operators"], { - Operator.BLANK, Operator.EQUAL, Operator.MISSING, Operator.NOT_EQUAL, @@ -141,7 +140,6 @@ def test_introspected_field_should_respect_django_capabilities(self): self.assertEqual( field_schema["filter_operators"], { - Operator.BLANK, Operator.EQUAL, Operator.MISSING, Operator.NOT_EQUAL, @@ -323,7 +321,6 @@ def test_build_should_handle_polymorphic_many_to_one(self): Operator.NOT_EQUAL, Operator.ENDS_WITH, Operator.PRESENT, - Operator.BLANK, Operator.EQUAL, Operator.NOT_IN, Operator.IN, diff --git a/src/datasource_sqlalchemy/tests/utils/test_model_converter.py b/src/datasource_sqlalchemy/tests/utils/test_model_converter.py index 4aa779f0f..845f8b648 100644 --- a/src/datasource_sqlalchemy/tests/utils/test_model_converter.py +++ b/src/datasource_sqlalchemy/tests/utils/test_model_converter.py @@ -26,7 +26,6 @@ class FieldEnum(enum.Enum): "validations": [], "filter_operators": { Operator.NOT_IN, - Operator.BLANK, Operator.EQUAL, Operator.PRESENT, Operator.MISSING, From 877a1cd2af77da3948953258ed21bb97c4fca4e7 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Fri, 21 Feb 2025 15:08:26 +0100 Subject: [PATCH 5/6] chore: stop guessing type, read it from schema --- .../forestadmin/datasource_sqlalchemy/collections.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/collections.py b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/collections.py index cf74b8beb..8b6f90b87 100644 --- a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/collections.py +++ b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/collections.py @@ -23,7 +23,7 @@ projections_to_records, ) from forestadmin.datasource_sqlalchemy.utils.relationships import Relationships, merge_relationships -from forestadmin.datasource_toolkit.interfaces.fields import PrimitiveType, RelationAlias +from forestadmin.datasource_toolkit.interfaces.fields import Column, PrimitiveType, RelationAlias from forestadmin.datasource_toolkit.interfaces.query.aggregation import AggregateResult, Aggregation from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.base import ConditionTree from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.leaf import ConditionTreeLeaf @@ -31,7 +31,6 @@ 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 -from forestadmin.datasource_toolkit.validations.type_getter import TypeGetter from sqlalchemy import Table from sqlalchemy import column as SqlAlchemyColumn from sqlalchemy import text @@ -208,7 +207,7 @@ async def update(self, caller: User, filter_: Optional[Filter], patch: RecordsDa def _cast_condition_tree(self, tree: ConditionTree) -> ConditionTree: if isinstance(tree, ConditionTreeLeaf): - if TypeGetter.get(tree.value, None) == PrimitiveType.DATE: + if cast(Column, self.schema["fields"][tree.field])["column_type"] == PrimitiveType.DATE: iso_format = tree.value if isinstance(iso_format, str): if iso_format[-1] == "Z": From 9a1e931eb47786a71757bdd598e1df38e17c71b7 Mon Sep 17 00:00:00 2001 From: Julien Barreau Date: Fri, 21 Feb 2025 16:03:44 +0100 Subject: [PATCH 6/6] chore: add tests for in operator containing null --- .../tests/test_django_collection.py | 47 +++++++++++++++++++ .../utils/query_factory.py | 5 +- .../tests/test_sqlalchemy_collections.py | 30 ++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/src/datasource_django/tests/test_django_collection.py b/src/datasource_django/tests/test_django_collection.py index 73f28af15..26c979494 100644 --- a/src/datasource_django/tests/test_django_collection.py +++ b/src/datasource_django/tests/test_django_collection.py @@ -181,6 +181,53 @@ async def test_datetime_and_date_should_be_correctly_serialized(self): ], ) + async def test_in_and_not_in_should_works_contains_none(self): + ret = await self.rating_collection.list( + self.mocked_caller, + PaginatedFilter({"condition_tree": ConditionTreeLeaf("rating_pk", Operator.IN, [1, None])}), + Projection("rating_pk", "rated_at", "rating", "book:author:birth_date"), + ) + + self.assertEqual( + ret, + [ + { + "rating_pk": 1, + "rating": 1, + "rated_at": datetime.datetime(2022, 12, 25, 10, 10, 10, tzinfo=datetime.timezone.utc), + "book": {"author": {"birth_date": datetime.date(1920, 2, 1)}}, + }, + ], + ) + + ret = await self.rating_collection.list( + self.mocked_caller, + PaginatedFilter( + { + "condition_tree": ConditionTreeBranch( + "and", + [ + ConditionTreeLeaf("rating_pk", Operator.IN, [1, 2]), + ConditionTreeLeaf("rating_pk", Operator.NOT_IN, [2, None]), + ], + ) + } + ), + Projection("rating_pk", "rated_at", "rating", "book:author:birth_date"), + ) + + self.assertEqual( + ret, + [ + { + "rating_pk": 1, + "rating": 1, + "rated_at": datetime.datetime(2022, 12, 25, 10, 10, 10, tzinfo=datetime.timezone.utc), + "book": {"author": {"birth_date": datetime.date(1920, 2, 1)}}, + }, + ], + ) + class TestDjangoCollectionCRUDListPolymorphism(TestDjangoCollectionCRUDList): def setUp(self) -> None: diff --git a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py index e6a46b9aa..8a399a0be 100644 --- a/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py +++ b/src/datasource_sqlalchemy/forestadmin/datasource_sqlalchemy/utils/query_factory.py @@ -37,10 +37,7 @@ def _build_leaf_condition(cls, collection: BaseSqlAlchemyCollection, leaf: Condi collection, ConditionTreeBranch( "or", - [ - ConditionTreeLeaf(leaf.field, Operator.EQUAL, None), - ConditionTreeLeaf(leaf.field, Operator.EQUAL, ""), - ], + [ConditionTreeLeaf(leaf.field, Operator.EQUAL, v) for v in leaf.value], ), ) return ( diff --git a/src/datasource_sqlalchemy/tests/test_sqlalchemy_collections.py b/src/datasource_sqlalchemy/tests/test_sqlalchemy_collections.py index 366515f8d..3cb794b98 100644 --- a/src/datasource_sqlalchemy/tests/test_sqlalchemy_collections.py +++ b/src/datasource_sqlalchemy/tests/test_sqlalchemy_collections.py @@ -5,6 +5,8 @@ from unittest import TestCase from unittest.mock import Mock, patch +from forestadmin.datasource_toolkit.interfaces.query.condition_tree.nodes.branch import ConditionTreeBranch + if sys.version_info >= (3, 9): import zoneinfo else: @@ -213,6 +215,34 @@ def test_list_with_filter(self): ) assert len(results) == 2 + def test_list_filter_in_and_not_in_with_null_in_values_should_work(self): + collection = self.datasource.get_collection("order") + filter_ = PaginatedFilter({"condition_tree": ConditionTreeLeaf("id", Operator.IN, [1, None])}) + + results = self.loop.run_until_complete( + collection.list(self.mocked_caller, filter_, Projection("id", "created_at")) + ) + + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["id"], 1) + + collection = self.datasource.get_collection("order") + filter_ = PaginatedFilter( + { + "condition_tree": ConditionTreeBranch( + "and", + [ConditionTreeLeaf("id", Operator.IN, [1, 2]), ConditionTreeLeaf("id", Operator.NOT_IN, [2, None])], + ) + } + ) + + results = self.loop.run_until_complete( + collection.list(self.mocked_caller, filter_, Projection("id", "created_at")) + ) + + self.assertEqual(len(results), 1) + self.assertEqual(results[0]["id"], 1) + def test_create(self): order = { "id": 11,