Move caching logic into 'get_subset'

Author: Ryan P Kilby

rest_framework_filters/filterset.py

@@ -58,6 +58,7 @@ class FilterSet(six.with_metaclass(FilterSetMetaclass, filterset.FilterSet)):
             'filter_class': filters.IsoDateTimeFilter,
         },
     }
+    _subset_cache = {}
 
     def __init__(self, *args, **kwargs):
         self._related_filterset_cache = kwargs.pop('cache', {})
@@ -116,21 +117,7 @@ def get_filters(self):
             if not isinstance(related_filter, filters.RelatedFilter):
                 continue
 
-            # get known filter names
-            filterset_class = related_filter.filterset
-            filter_names = [filterset_class.get_filter_name(param) for param in rel_data.keys()]
-
-            # filter out empty values - indicates an unknown field (author__foobar__isnull)
-            filter_names = [f for f in filter_names if f is not None]
-
-            # attempt to retrieve related filterset subset from the cache
-            key = self.cache_key(filterset_class, filter_names)
-            subset_class = self.cache_get(key)
-
-            # otherwise build and insert it into the cache
-            if subset_class is None:
-                subset_class = related_filter.filterset.get_subset(filter_names)
-                self.cache_set(key, subset_class)
+            subset_class = related_filter.filterset.get_subset(rel_data)
 
             # initialize and copy filters
             filterset = subset_class(data=rel_data)
@@ -163,33 +150,56 @@ def get_filter_name(cls, param):
             return related_param
 
     @classmethod
-    def get_subset(cls, filter_names):
+    def get_subset(cls, params):
         """
-        Returns a FilterSet subclass that contains the subset of filters
-        specified in `filter_names`. This is useful for creating FilterSets
-        used across relationships, as it minimizes the deepcopy overhead
-        incurred when instantiating the FilterSet.
+        Returns a FilterSubset class that contains the subset of filters
+        specified in the requested `params`. This is useful for creating
+        FilterSets that traverse relationships, as it helps to minimize
+        the deepcopy overhead incurred when instantiating the FilterSet.
         """
-        class FilterSetSubset(cls):
+        # Determine names of filters from query params and remove empty values.
+        # param names that traverse relations are translated to just the local
+        # filter names. eg, `author__username` => `author`. Empty values are
+        # removed, as they indicate an unknown field eg, author__foobar__isnull
+        filter_names = [cls.get_filter_name(param) for param in params]
+        filter_names = [f for f in filter_names if f is not None]
+
+        # attempt to retrieve related filterset subset from the cache
+        key = cls.cache_key(filter_names)
+        subset_class = cls.cache_get(key)
+
+        # if no cached subset, then derive base_filters and create new subset
+        if subset_class is not None:
+            return subset_class
+
+        class FilterSubsetMetaclass(FilterSetMetaclass):
+            def __new__(cls, name, bases, attrs):
+                new_class = super(FilterSubsetMetaclass, cls).__new__(cls, name, bases, attrs)
+                new_class.base_filters = OrderedDict([
+                    (name, f)
+                    for name, f in six.iteritems(new_class.base_filters)
+                    if name in filter_names
+                ])
+                return new_class
+
+        class FilterSubset(six.with_metaclass(FilterSubsetMetaclass, cls)):
             pass
 
-        FilterSetSubset.__name__ = str('%sSubset' % (cls.__name__))
-        FilterSetSubset.base_filters = OrderedDict([
-            (name, f)
-            for name, f in six.iteritems(cls.base_filters)
-            if name in filter_names
-        ])
-
-        return FilterSetSubset
+        FilterSubset.__name__ = str('%sSubset' % (cls.__name__, ))
+        cls.cache_set(key, FilterSubset)
+        return FilterSubset
 
-    def cache_key(self, filterset, filter_names):
-        return '%sSubset-%s' % (filterset.__name__, '-'.join(sorted(filter_names)), )
+    @classmethod
+    def cache_key(cls, filter_names):
+        return '%sSubset-%s' % (cls.__name__, '-'.join(sorted(filter_names)), )
 
-    def cache_get(self, key):
-        return self._related_filterset_cache.get(key)
+    @classmethod
+    def cache_get(cls, key):
+        return cls._subset_cache.get(key)
 
-    def cache_set(self, key, value):
-        self._related_filterset_cache[key] = value
+    @classmethod
+    def cache_set(cls, key, value):
+        cls._subset_cache[key] = value
 
     @property
     def qs(self):

tests/test_filterset.py

@@ -304,17 +304,6 @@ def test_m2m_relation(self):
         titles = set([p.title for p in f])
         self.assertEqual(titles, set(["First post", "Second post"]))
 
-    def test_get_subset(self):
-        related_filter = NoteFilterWithRelated.base_filters['author']
-        filterset_class = related_filter.filterset.get_subset(['email'])
-
-        # ensure that the class name is useful when debugging
-        self.assertEqual(filterset_class.__name__, 'UserFilterSubset')
-
-        # ensure that the FilterSet subset only contains the requested fields
-        self.assertIn('email', filterset_class.base_filters)
-        self.assertEqual(len(filterset_class.base_filters), 1)
-
     def test_nonexistent_related_field(self):
         """
         Invalid filter keys (including those on related filters) are invalid
@@ -335,6 +324,32 @@ def test_nonexistent_related_field(self):
         self.assertEqual(len(list(f)), 4)
 
 
+class FilterSubsetTests(TestCase):
+
+    def test_get_subset(self):
+        filterset_class = UserFilter.get_subset(['email'])
+
+        # ensure that the class name is useful when debugging
+        self.assertEqual(filterset_class.__name__, 'UserFilterSubset')
+
+        # ensure that the FilterSet subset only contains the requested fields
+        self.assertIn('email', filterset_class.base_filters)
+        self.assertEqual(len(filterset_class.base_filters), 1)
+
+    def test_related_subset(self):
+        # related filters should only return the local RelatedFilter
+        filterset_class = NoteFilterWithRelated.get_subset(['title', 'author__email'])
+
+        self.assertIn('title', filterset_class.base_filters)
+        self.assertIn('author', filterset_class.base_filters)
+        self.assertEqual(len(filterset_class.base_filters), 2)
+
+    def test_non_filter_subset(self):
+        # non-filter params should be ignored
+        filterset_class = NoteFilterWithRelated.get_subset(['foobar'])
+        self.assertEqual(len(filterset_class.base_filters), 0)
+
+
 class MethodFilterTests(TestCase):
 
     @classmethod