Merge pull request #35 from SweetProcess/tests-for-caching-property

Adding cached_property to the fields method

Author: dbrgn

drf_dynamic_fields/__init__.py

@@ -4,6 +4,7 @@
 import warnings
 
 from django.conf import settings
+from django.utils.functional import cached_property
 
 
 class DynamicFieldsMixin(object):
@@ -12,7 +13,7 @@ class DynamicFieldsMixin(object):
     which fields should be displayed.
     """
 
-    @property
+    @cached_property
     def fields(self):
         """
         Filters the fields according to the `fields` query parameter.

tests/models.py

@@ -5,9 +5,12 @@
 
 
 class Teacher(models.Model):
-    """No fields, no fun."""
+    name = models.CharField(max_length=30)
+    age = models.IntegerField()
 
 
 class School(models.Model):
     """Schools just have teachers, no students."""
+
+    name = models.CharField(max_length=30)
     teachers = models.ManyToManyField(Teacher)

tests/serializers.py

@@ -18,17 +18,15 @@ class TeacherSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
 
     class Meta:
         model = Teacher
-        fields = ('id', 'request_info')
+        fields = ("id", "request_info", "age", "name")
 
     def get_request_info(self, teacher):
         """
         a meaningless method that attempts
         to access the request object.
         """
-        request = self.context['request']
-        return request.build_absolute_uri(
-            '/api/v1/teacher/{}'.format(teacher.pk)
-        )
+        request = self.context["request"]
+        return request.build_absolute_uri("/api/v1/teacher/{}".format(teacher.pk))
 
 
 class SchoolSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
@@ -41,4 +39,4 @@ class SchoolSerializer(DynamicFieldsMixin, serializers.ModelSerializer):
 
     class Meta:
         model = School
-        fields = ('id', 'teachers')
+        fields = ("id", "teachers", "name")

tests/test_mixins.py

@@ -3,7 +3,7 @@
 
 """
 test_drf-dynamic-fields
-------------
+-----------
 
 Tests for `drf-dynamic-fields` mixins
 """
@@ -25,147 +25,153 @@ def test_removes_fields(self):
         Does it actually remove fields?
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?fields=id')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?fields=id")
+        serializer = TeacherSerializer(context={"request": request})
 
-        self.assertEqual(
-            set(serializer.fields.keys()),
-            set(('id',))
-        )
+        self.assertEqual(set(serializer.fields.keys()), set(("id",)))
 
     def test_fields_left_alone(self):
         """
         What if no fields param is passed? It should not touch the fields.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/")
+        serializer = TeacherSerializer(context={"request": request})
 
         self.assertEqual(
-            set(serializer.fields.keys()),
-            set(('id', 'request_info'))
+            set(serializer.fields.keys()), set(("id", "request_info", "age", "name"))
         )
 
     def test_fields_all_gone(self):
         """
         If we pass a blank fields list, then no fields should return.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?fields')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?fields")
+        serializer = TeacherSerializer(context={"request": request})
 
-        self.assertEqual(
-            set(serializer.fields.keys()),
-            set()
-        )
+        self.assertEqual(set(serializer.fields.keys()), set())
 
     def test_ordinary_serializer(self):
         """
         Check the full JSON output of the serializer.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?fields=id')
-        teacher = Teacher.objects.create()
+        request = rf.get("/api/v1/schools/1/?fields=id,age")
+        teacher = Teacher.objects.create(name="Susan", age=34)
 
-        serializer = TeacherSerializer(teacher, context={'request': request})
+        serializer = TeacherSerializer(teacher, context={"request": request})
 
-        self.assertEqual(
-            serializer.data, {
-                'id': teacher.id
-            }
-        )
+        self.assertEqual(serializer.data, {"id": teacher.id, "age": teacher.age})
 
     def test_omit(self):
         """
         Check a basic usage of omit.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?omit=request_info')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?omit=request_info")
+        serializer = TeacherSerializer(context={"request": request})
 
-        self.assertEqual(
-            set(serializer.fields.keys()),
-            set(('id',))
-        )
+        self.assertEqual(set(serializer.fields.keys()), set(("id", "name", "age")))
 
     def test_omit_and_fields_used(self):
         """
         Can they be used together.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?fields=id,request_info&omit=request_info')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?fields=id,request_info&omit=request_info")
+        serializer = TeacherSerializer(context={"request": request})
 
-        self.assertEqual(
-            set(serializer.fields.keys()),
-            set(('id',))
-        )
+        self.assertEqual(set(serializer.fields.keys()), set(("id",)))
 
     def test_omit_everything(self):
         """
         Can remove it all tediously.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?omit=id,request_info')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?omit=id,request_info,age,name")
+        serializer = TeacherSerializer(context={"request": request})
 
-        self.assertEqual(
-            set(serializer.fields.keys()),
-            set()
-        )
+        self.assertEqual(set(serializer.fields.keys()), set())
 
     def test_omit_nothing(self):
         """
         Blank omit doesn't affect anything.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?omit')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?omit")
+        serializer = TeacherSerializer(context={"request": request})
 
         self.assertEqual(
-            set(serializer.fields.keys()),
-            set(('id', 'request_info'))
+            set(serializer.fields.keys()), set(("id", "request_info", "name", "age"))
         )
 
     def test_omit_non_existant_field(self):
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?omit=pretend')
-        serializer = TeacherSerializer(context={'request': request})
+        request = rf.get("/api/v1/schools/1/?omit=pretend")
+        serializer = TeacherSerializer(context={"request": request})
 
         self.assertEqual(
-            set(serializer.fields.keys()),
-            set(('id', 'request_info'))
+            set(serializer.fields.keys()), set(("id", "request_info", "name", "age"))
         )
 
     def test_as_nested_serializer(self):
         """
         Nested serializers are not filtered.
         """
         rf = RequestFactory()
-        request = rf.get('/api/v1/schools/1/?fields=teachers')
+        request = rf.get("/api/v1/schools/1/?fields=teachers")
 
-        school = School.objects.create()
+        school = School.objects.create(name="Python Heights High")
         teachers = [
-            Teacher.objects.create(),
-            Teacher.objects.create()
+            Teacher.objects.create(name="Shane", age=45),
+            Teacher.objects.create(name="Kaz", age=29),
         ]
         school.teachers.add(*teachers)
 
-        serializer = SchoolSerializer(school, context={'request': request})
+        serializer = SchoolSerializer(school, context={"request": request})
 
-        request_info = 'http://testserver/api/v1/teacher/{}'
+        request_info = "http://testserver/api/v1/teacher/{}"
 
         self.assertEqual(
-            serializer.data, {
-                'teachers': [
-                    OrderedDict([
-                        ('id', teachers[0].id),
-                        ('request_info', request_info.format(teachers[0].id))
-                    ]),
-                    OrderedDict([
-                        ('id', teachers[1].id),
-                        ('request_info', request_info.format(teachers[1].id))
-                    ])
+            serializer.data,
+            {
+                "teachers": [
+                    OrderedDict(
+                        [
+                            ("id", teachers[0].id),
+                            ("request_info", request_info.format(teachers[0].id)),
+                            ("age", teachers[0].age),
+                            ("name", teachers[0].name),
+                        ]
+                    ),
+                    OrderedDict(
+                        [
+                            ("id", teachers[1].id),
+                            ("request_info", request_info.format(teachers[1].id)),
+                            ("age", teachers[1].age),
+                            ("name", teachers[1].name),
+                        ]
+                    ),
                 ],
-            }
+            },
         )
+
+    def test_serializer_reuse_with_changing_request(self):
+        """
+        `fields` is a cached property. Changing the request on an already
+        instantiated serializer will not result in a changed fields attribute.
+
+        This was a deliberate choice we have made in favor of speeding up
+        access to the slow `fields` attribute.
+        """
+
+        rf = RequestFactory()
+        request = rf.get("/api/v1/schools/1/?fields=id")
+        serializer = TeacherSerializer(context={"request": request})
+        self.assertEqual(set(serializer.fields.keys()), {"id"})
+
+        # now change the request on this instantiated serializer.
+        request2 = rf.get("/api/v1/schools/1/?fields=id,name")
+        serializer.context["request"] = request2
+        self.assertEqual(set(serializer.fields.keys()), {"id"})

tests/test_requests.py

@@ -0,0 +1,64 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+test_drf-dynamic-fields
+------------
+
+Test for the full request cycle using dynamic fields mixns
+"""
+from collections import OrderedDict
+
+from django.test import TestCase, RequestFactory
+
+from rest_framework.reverse import reverse
+
+from .serializers import SchoolSerializer, TeacherSerializer
+from .models import Teacher, School
+
+
+class TestDynamicFieldsViews(TestCase):
+    """
+    Testing using dynamic fields in request framework views.
+    """
+
+    def setUp(self):
+        """
+        Create some teachers and schools.
+        """
+        teachers = [("Craig", 34), ("Kaz", 29), ("Sun", 62)]
+
+        schools = ["Python Heights High", "Ruby Consolidated", "Java Coffee School"]
+
+        t = [Teacher.objects.create(name=name, age=age) for name, age in teachers]
+
+        for name in schools:
+            s = School.objects.create(name=name)
+            s.teachers.add(*t)
+
+    def test_teacher_basic(self):
+
+        response = self.client.get(reverse("teacher-list"))
+        for teacher in response.data:
+            self.assertEqual(teacher.keys(), {"id", "request_info", "age", "name"})
+
+    def test_teacher_fields(self):
+
+        response = self.client.get(reverse("teacher-list"), {"fields": "id,age"})
+        for teacher in response.data:
+            self.assertEqual(teacher.keys(), {"id", "age"})
+
+    def test_teacher_omit(self):
+
+        response = self.client.get(reverse("teacher-list"), {"omit": "id,age"})
+        for teacher in response.data:
+            self.assertEqual(teacher.keys(), {"request_info", "name"})
+
+    def test_nested_teacher_fields(self):
+
+        response = self.client.get(reverse("school-list"), {"fields": "name,teachers"})
+        for school in response.data:
+            self.assertEqual(school.keys(), {"teachers", "name"})
+            self.assertEqual(
+                school["teachers"][0].keys(), {"id", "request_info", "age", "name"}
+            )

tests/urls.py

@@ -1,5 +1,10 @@
 # -*- coding: utf-8
-"""
-Empty urls for test.
-"""
-urlpatterns = []  # noqa.
+from rest_framework import routers
+
+from .views import SchoolViewSet, TeacherViewSet
+
+router = routers.SimpleRouter()
+router.register("teachers", TeacherViewSet)
+router.register("schools", SchoolViewSet)
+
+urlpatterns = router.urls

tests/views.py

@@ -0,0 +1,14 @@
+from rest_framework.viewsets import ModelViewSet
+
+from .models import School, Teacher
+from .serializers import SchoolSerializer, TeacherSerializer
+
+
+class TeacherViewSet(ModelViewSet):
+    queryset = Teacher.objects.all()
+    serializer_class = TeacherSerializer
+
+
+class SchoolViewSet(ModelViewSet):
+    queryset = School.objects.all()
+    serializer_class = SchoolSerializer