diff --git a/src/dso_api/dynamic_api/filters/parser.py b/src/dso_api/dynamic_api/filters/parser.py index 2c0f0d83e..e349caa4e 100644 --- a/src/dso_api/dynamic_api/filters/parser.py +++ b/src/dso_api/dynamic_api/filters/parser.py @@ -527,16 +527,20 @@ def _get_field_by_id( # noqa: C901 # Found regular field, or forward relation return FilterPathPart(field=field, is_many=field.is_array) - try: - additional_relation = parent.get_additional_relation_by_id(name) - except SchemaObjectNotFound: - return None - else: - # The additional relation name is used as ORM path to navigate over the relation. - # Yet when directly filtering, the value/lookup should work directly - # against the primary key of the reverse table (hence field is also resolved here) - return FilterPathPart( - field=additional_relation.related_table.identifier_fields[0], - reverse_field=additional_relation, - is_many=True, - ) + if isinstance(parent, DatasetTableSchema): + # For tables, see if a reverse relation can be traversed. This is not possible for fields. + try: + additional_relation = parent.get_additional_relation_by_id(name) + except SchemaObjectNotFound: + return None + else: + # The additional relation name is used as ORM path to navigate over the relation. + # Yet when directly filtering, the value/lookup should work directly + # against the primary key of the reverse table (hence field is also resolved here) + return FilterPathPart( + field=additional_relation.related_table.identifier_fields[0], + reverse_field=additional_relation, + is_many=True, + ) + + return None diff --git a/src/tests/test_dynamic_api/test_filters.py b/src/tests/test_dynamic_api/test_filters.py index f75b969d8..160c5fef7 100644 --- a/src/tests/test_dynamic_api/test_filters.py +++ b/src/tests/test_dynamic_api/test_filters.py @@ -250,6 +250,26 @@ def test_filter_loose_relation( result = engine.filter_queryset(statistieken_model.objects.all()) assert result.count() == expect, result + @staticmethod + @pytest.mark.parametrize( + "query", + [ + "nonexistant", + "buurt.nonexistant=test", + "buurt.identificatie.nonexistant=test", + "buurt.naam.nonexistant", + ], + ) + def test_filter_loose_relation_invalid( + api_client, statistieken_model, statistieken_data, query + ): + """Test that filtering on loose relations works. + This also checks whether the temporal records only return a single item. + """ + engine = create_filter_engine(query) + with pytest.raises(ValidationError): + engine.filter_queryset(statistieken_model.objects.all()) + @staticmethod @pytest.mark.parametrize( "query,expect", @@ -366,6 +386,24 @@ def test_array_field(parkeervak, query, expect): assert response.status_code == 200, data assert len(data["_embedded"]["parkeervakken"]) == expect + @staticmethod + @pytest.mark.parametrize( + ["query", "expect_code"], + [ + ("regimes.dagen=ma,di,wo,do,vr", 200), + ("regimes.dagen.foo=test", 400), + ("regimes.foo=test", 400), # subfield has different codepath if not found. + ("foobar=test", 400), + ], + ) + def test_parsing_invalid( + parkeervakken_parkeervak_model, parkeervakken_regime_model, query, expect_code + ): + """Prove that looking for a relation on a nested field won't crash.""" + response = APIClient().get(f"/v1/parkeervakken/parkeervakken/?{query}") + data = read_response_json(response) + assert response.status_code == expect_code, data + @staticmethod def test_numerical_filters(parkeervakken_parkeervak_model, filled_router): """Test comparisons on numerical fields.""" @@ -575,6 +613,26 @@ def test_filter_reverse_m2m(movies_data_with_actors, filled_router): names = [a["name"] for a in data["_embedded"]["actor"]] assert names == ["John Doe", "Jane Doe"] + @staticmethod + @pytest.mark.parametrize( + "url", + [ + "/v1/movies/movie/?nonexistent=foo123", # invalid normal field + "/v1/movies/movie/?name.nonexistent=foo123", # not a relation + "/v1/movies/category/?movies.nonexistent=foo123", # using reverse FK + "/v1/movies/category/?movies.name.nonexistent=foo123", # using reverse FK + "/v1/movies/actor/?movies.nonexistent=foo123", # using reverse M2M + "/v1/movies/actor/?movies.name.nonexistent=foo123", # using reverse M2M + ], + ) + def test_filter_invalid_field(movies_model, filled_router, url): + """Prove that walking over M2M models works and doesn't crash the parser. + Note this uses the "movies" dataset, not the hardcoded movie models/serializers. + """ + response = APIClient().get(url) + data = read_response_json(response) + assert response.status_code == 400, data + @staticmethod @pytest.mark.parametrize("params", [{"e_type": "whatever"}, {"_sort": "e_type"}]) def test_snake_case_400(params, parkeervakken_parkeervak_model):