Fix 500 when trying to filter on an unknown subfield

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

src/tests/test_dynamic_api/test_filters.py

@@ -250,6 +250,24 @@ 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=test",
+        ],
+    )
+    def test_filter_loose_relation_invalid(
+        api_client, statistieken_model, statistieken_data, query
+    ):
+        """Prove that field-resolving on invalid fields is properly handled."""
+        engine = create_filter_engine(query)
+        with pytest.raises(ValidationError):
+            engine.filter_queryset(statistieken_model.objects.all())
+
     @staticmethod
     @pytest.mark.parametrize(
         "query,expect",
@@ -366,6 +384,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 +611,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):