From b56891f5bf335bb9c0fe03a8904d5029404eee65 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 6 Dec 2023 12:46:01 +0300 Subject: [PATCH 1/2] test filter by null values --- tests/conftest.py | 1 + tests/fixtures/entities.py | 13 +++++++++ tests/test_api/test_api_sqla_with_includes.py | 28 +++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 45ecccfa..5c18aa3a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -43,6 +43,7 @@ user_2_comment_for_one_u1_post, user_2_posts, user_3, + user_4, workplace_1, workplace_2, ) diff --git a/tests/fixtures/entities.py b/tests/fixtures/entities.py index 23eb3547..c70e2bf2 100644 --- a/tests/fixtures/entities.py +++ b/tests/fixtures/entities.py @@ -70,6 +70,19 @@ async def user_3(async_session: AsyncSession): await async_session.commit() +@async_fixture() +async def user_4(async_session: AsyncSession): + user = build_user( + email=None + ) + async_session.add(user) + await async_session.commit() + await async_session.refresh(user) + yield user + await async_session.delete(user) + await async_session.commit() + + async def build_user_bio(async_session: AsyncSession, user: User, **fields): bio = UserBio(user=user, **fields) async_session.add(bio) diff --git a/tests/test_api/test_api_sqla_with_includes.py b/tests/test_api/test_api_sqla_with_includes.py index 215a8e04..3a72a35e 100644 --- a/tests/test_api/test_api_sqla_with_includes.py +++ b/tests/test_api/test_api_sqla_with_includes.py @@ -2025,6 +2025,34 @@ async def test_field_filters_with_values_from_different_models( "meta": {"count": 0, "totalPages": 1}, } + @mark.parametrize("filter_dict, expected_email_is_null", [ + param([{"name": "email", "op": "is_", "val": None}], True), + param([{"name": "email", "op": "isnot", "val": None}], False) + ]) + async def test_filter_by_null( + self, + app: FastAPI, + client: AsyncClient, + user_1: User, + user_4: User, + filter_dict, + expected_email_is_null + ): + assert user_1.email is not None + assert user_4.email is None + + url = app.url_path_for("get_user_list") + params = {"filter": dumps(filter_dict)} + + response = await client.get(url, params=params) + assert response.status_code == 200, response.text + + data = response.json() + + assert len(data['data']) == 1 + assert (data['data'][0]['attributes']['email'] is None) == expected_email_is_null + + async def test_composite_filter_by_one_field( self, app: FastAPI, From 042433b741afbedb70794b94bb0b2d1a3e321cc4 Mon Sep 17 00:00:00 2001 From: German Bernadskiy Date: Fri, 8 Dec 2023 16:07:38 +1000 Subject: [PATCH 2/2] fixed filter by null condition updated null filtering logic added example with filter by not null added example with filter by null --- docs/filtering.rst | 16 ++++ .../data_layers/filtering/sqlalchemy.py | 18 +++- tests/conftest.py | 1 - tests/fixtures/entities.py | 13 --- tests/test_api/test_api_sqla_with_includes.py | 89 +++++++++++++++---- 5 files changed, 105 insertions(+), 32 deletions(-) diff --git a/docs/filtering.rst b/docs/filtering.rst index da37edb7..000d63dd 100644 --- a/docs/filtering.rst +++ b/docs/filtering.rst @@ -119,6 +119,22 @@ You can also use boolean combination of operations: GET /user?filter=[{"name":"group.name","op":"ilike","val":"%admin%"},{"or":[{"not":{"name":"first_name","op":"eq","val":"John"}},{"and":[{"name":"first_name","op":"like","val":"%Jim%"},{"name":"date_create","op":"gt","val":"1990-01-01"}]}]}] HTTP/1.1 Accept: application/vnd.api+json + +Filtering records by a field that is null + +.. sourcecode:: http + + GET /user?filter=[{"name":"name","op":"is_","val":null}] HTTP/1.1 + Accept: application/vnd.api+json + +Filtering records by a field that is not null + +.. sourcecode:: http + + GET /user?filter=[{"name":"name","op":"isnot","val":null}] HTTP/1.1 + Accept: application/vnd.api+json + + Common available operators: * any: used to filter on "to many" relationships diff --git a/fastapi_jsonapi/data_layers/filtering/sqlalchemy.py b/fastapi_jsonapi/data_layers/filtering/sqlalchemy.py index e676c021..93c7a61a 100644 --- a/fastapi_jsonapi/data_layers/filtering/sqlalchemy.py +++ b/fastapi_jsonapi/data_layers/filtering/sqlalchemy.py @@ -67,6 +67,12 @@ def __init__(self, model: Type[TypeModel], filter_: dict, schema: Type[TypeSchem self.filter_ = filter_ self.schema = schema + def _check_can_be_none(self, fields: list[ModelField]) -> bool: + """ + Return True if None is possible value for target field + """ + return any(field_item.allow_none for field_item in fields) + def _cast_value_with_scheme(self, field_types: List[ModelField], value: Any) -> Tuple[Any, List[str]]: errors: List[str] = [] casted_value = cast_failed @@ -109,6 +115,15 @@ def create_filter(self, schema_field: ModelField, model_column, operator, value) fields = list(schema_field.sub_fields) else: fields = [schema_field] + + can_be_none = self._check_can_be_none(fields) + + if value is None: + if can_be_none: + return getattr(model_column, self.operator)(value) + + raise InvalidFilters(detail=f"The field `{schema_field.name}` can't be null") + types = [i.type_ for i in fields] clear_value = None errors: List[str] = [] @@ -133,8 +148,9 @@ def create_filter(self, schema_field: ModelField, model_column, operator, value) ) # Если None, при этом поле обязательное (среди типов в аннотации нет None, то кидаем ошибку) - if clear_value is None and not any(not i_f.required for i_f in fields): + if clear_value is None and not can_be_none: raise InvalidType(detail=", ".join(errors)) + return getattr(model_column, self.operator)(clear_value) def _separate_types(self, types: List[Type]) -> Tuple[List[Type], List[Type]]: diff --git a/tests/conftest.py b/tests/conftest.py index 5c18aa3a..45ecccfa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -43,7 +43,6 @@ user_2_comment_for_one_u1_post, user_2_posts, user_3, - user_4, workplace_1, workplace_2, ) diff --git a/tests/fixtures/entities.py b/tests/fixtures/entities.py index c70e2bf2..23eb3547 100644 --- a/tests/fixtures/entities.py +++ b/tests/fixtures/entities.py @@ -70,19 +70,6 @@ async def user_3(async_session: AsyncSession): await async_session.commit() -@async_fixture() -async def user_4(async_session: AsyncSession): - user = build_user( - email=None - ) - async_session.add(user) - await async_session.commit() - await async_session.refresh(user) - yield user - await async_session.delete(user) - await async_session.commit() - - async def build_user_bio(async_session: AsyncSession, user: User, **fields): bio = UserBio(user=user, **fields) async_session.add(bio) diff --git a/tests/test_api/test_api_sqla_with_includes.py b/tests/test_api/test_api_sqla_with_includes.py index 3a72a35e..28bd2967 100644 --- a/tests/test_api/test_api_sqla_with_includes.py +++ b/tests/test_api/test_api_sqla_with_includes.py @@ -2025,33 +2025,88 @@ async def test_field_filters_with_values_from_different_models( "meta": {"count": 0, "totalPages": 1}, } - @mark.parametrize("filter_dict, expected_email_is_null", [ - param([{"name": "email", "op": "is_", "val": None}], True), - param([{"name": "email", "op": "isnot", "val": None}], False) - ]) + @mark.parametrize( + ("filter_dict", "expected_email_is_null"), + [ + param([{"name": "email", "op": "is_", "val": None}], True), + param([{"name": "email", "op": "isnot", "val": None}], False), + ], + ) async def test_filter_by_null( - self, - app: FastAPI, - client: AsyncClient, - user_1: User, - user_4: User, - filter_dict, - expected_email_is_null + self, + app: FastAPI, + async_session: AsyncSession, + client: AsyncClient, + user_1: User, + user_2: User, + filter_dict: dict, + expected_email_is_null: bool, ): - assert user_1.email is not None - assert user_4.email is None + user_2.email = None + await async_session.commit() + + target_user = user_2 if expected_email_is_null else user_1 url = app.url_path_for("get_user_list") params = {"filter": dumps(filter_dict)} response = await client.get(url, params=params) - assert response.status_code == 200, response.text + assert response.status_code == status.HTTP_200_OK, response.text + + response_json = response.json() + + assert len(data := response_json["data"]) == 1 + assert data[0]["id"] == str(target_user.id) + assert data[0]["attributes"]["email"] == target_user.email + + async def test_filter_by_null_error_when_null_is_not_possible_value( + self, + async_session: AsyncSession, + user_1: User, + ): + resource_type = "user_with_nullable_email" - data = response.json() + class UserWithNotNullableEmailSchema(UserSchema): + email: str - assert len(data['data']) == 1 - assert (data['data'][0]['attributes']['email'] is None) == expected_email_is_null + app = build_app_custom( + model=User, + schema=UserWithNotNullableEmailSchema, + schema_in_post=UserWithNotNullableEmailSchema, + schema_in_patch=UserWithNotNullableEmailSchema, + resource_type=resource_type, + ) + user_1.email = None + await async_session.commit() + url = app.url_path_for(f"get_{resource_type}_list") + params = { + "filter": dumps( + [ + { + "name": "email", + "op": "is_", + "val": None, + }, + ], + ), + } + + async with AsyncClient(app=app, base_url="http://test") as client: + response = await client.get(url, params=params) + assert response.status_code == status.HTTP_400_BAD_REQUEST, response.text + assert response.json() == { + "detail": { + "errors": [ + { + "detail": "The field `email` can't be null", + "source": {"parameter": "filters"}, + "status_code": status.HTTP_400_BAD_REQUEST, + "title": "Invalid filters querystring parameter.", + }, + ], + }, + } async def test_composite_filter_by_one_field( self,