Raise error for non-model fields in Meta.fields. (carltongibson#1061)

django_filters/filterset.py

@@ -344,12 +344,14 @@ def get_filters(cls):
                 if field is not None:
                     filters[filter_name] = cls.filter_for_field(field, field_name, lookup_expr)
 
-        # filter out declared filters
-        undefined = [f for f in undefined if f not in cls.declared_filters]
+        # Allow Meta.fields to contain declared filters *only* when a list/tuple
+        if isinstance(cls._meta.fields, (list, tuple)):
+            undefined = [f for f in undefined if f not in cls.declared_filters]
+
         if undefined:
             raise TypeError(
-                "'Meta.fields' contains fields that are not defined on this FilterSet: "
-                "%s" % ', '.join(undefined)
+                "'Meta.fields' must not contain non-model field names: %s"
+                % ', '.join(undefined)
             )
 
         # Add in declared filters. This is necessary since we don't enforce adding

docs/ref/filterset.txt

@@ -71,6 +71,25 @@ include both transforms and lookups, as detailed in the `lookup reference`__.
 
 __ https://docs.djangoproject.com/en/stable/ref/models/lookups/#module-django.db.models.lookups
 
+Note that it is **not** necessary to included declared filters in a ``fields``
+list - doing so will have no effect - and including declarative aliases in a
+``fields`` dict will raise an error.
+
+.. code-block:: python
+
+    class UserFilter(django_filters.FilterSet):
+        username = filters.CharFilter()
+        login_timestamp = filters.IsoDateTimeFilter(field_name='last_login')
+
+        class Meta:
+            model = User
+            fields = {
+                'username': ['exact', 'contains'],
+                'login_timestamp': ['exact'],
+            }
+
+    TypeError("'Meta.fields' contains fields that are not defined on this FilterSet: login_timestamp")
+
 
 .. _exclude:
 

tests/rest_framework/test_backends.py

@@ -121,7 +121,7 @@ def test_filterset_fields_malformed(self):
         view.filterset_fields = ['non_existent']
         queryset = FilterableItem.objects.all()
 
-        msg = "'Meta.fields' contains fields that are not defined on this FilterSet: non_existent"
+        msg = "'Meta.fields' must not contain non-model field names: non_existent"
         with self.assertRaisesMessage(TypeError, msg):
             backend.get_filterset_class(view, queryset)
 
@@ -173,7 +173,7 @@ class View(FilterFieldsRootView):
 
         backend = DjangoFilterBackend()
 
-        msg = "'Meta.fields' contains fields that are not defined on this FilterSet: non_existent"
+        msg = "'Meta.fields' must not contain non-model field names: non_existent"
         with self.assertRaisesMessage(TypeError, msg):
             backend.get_schema_fields(View())
 

tests/test_filterset.py

@@ -429,56 +429,71 @@ class F(FilterSet):
             username = CharFilter()
 
             class Meta:
-                model = Book
-                fields = {'id': ['exact'],
-                          'username': ['exact'],
-                          }
+                model = User
+                fields = {
+                    'id': ['exact'],
+                    'username': ['exact'],
+                }
 
         self.assertEqual(len(F.declared_filters), 1)
         self.assertEqual(len(F.base_filters), 2)
 
         expected_list = ['id', 'username']
         self.assertTrue(checkItemsEqual(list(F.base_filters), expected_list))
 
-    def test_meta_fields_containing_unknown(self):
-        with self.assertRaises(TypeError) as excinfo:
+    def test_meta_fields_list_containing_unknown_fields(self):
+        msg = ("'Meta.fields' must not contain non-model field names: "
+               "other, another")
+
+        with self.assertRaisesMessage(TypeError, msg):
             class F(FilterSet):
                 username = CharFilter()
 
                 class Meta:
                     model = Book
                     fields = ('username', 'price', 'other', 'another')
 
-        self.assertEqual(
-            str(excinfo.exception),
-            "'Meta.fields' contains fields that are not defined on this FilterSet: "
-            "other, another"
-        )
+    def test_meta_fields_dict_containing_unknown_fields(self):
+        msg = "'Meta.fields' must not contain non-model field names: other"
+
+        with self.assertRaisesMessage(TypeError, msg):
+            class F(FilterSet):
+
+                class Meta:
+                    model = Book
+                    fields = {
+                        'id': ['exact'],
+                        'title': ['exact'],
+                        'other': ['exact'],
+                    }
 
-    def test_meta_fields_dictionary_containing_unknown(self):
-        with self.assertRaises(TypeError):
+    def test_meta_fields_dict_containing_declarative_alias(self):
+        # Meta.fields dict cannot generate lookups for an *aliased* field
+        msg = "'Meta.fields' must not contain non-model field names: other"
+
+        with self.assertRaisesMessage(TypeError, msg):
             class F(FilterSet):
+                other = CharFilter()
 
                 class Meta:
                     model = Book
-                    fields = {'id': ['exact'],
-                              'title': ['exact'],
-                              'other': ['exact'],
-                              }
+                    fields = {
+                        'id': ['exact'],
+                        'title': ['exact'],
+                        'other': ['exact'],
+                    }
 
     def test_meta_fields_invalid_lookup(self):
         # We want to ensure that non existent lookups (or just simple misspellings)
         # throw a useful exception containg the field and lookup expr.
-        with self.assertRaises(FieldLookupError) as context:
+        msg = "Unsupported lookup 'flub' for field 'tests.User.username'."
+
+        with self.assertRaisesMessage(FieldLookupError, msg):
             class F(FilterSet):
                 class Meta:
                     model = User
                     fields = {'username': ['flub']}
 
-        exc = str(context.exception)
-        self.assertIn('tests.User.username', exc)
-        self.assertIn('flub', exc)
-
     def test_meta_exlude_with_declared_and_declared_wins(self):
         class F(FilterSet):
             username = CharFilter()