Browse Source

Fixed #12057 -- Corrected regression of caching performance when a model contained a callable default. Thanks to Michael Thornhill for the excellent assistance tracking this problem.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@11681 bcc190cf-cafb-0310-a4f2-bffc1f526a37
Russell Keith-Magee 15 years ago
parent
commit
96658ef2d2
4 changed files with 64 additions and 3 deletions
  1. 1 1
      AUTHORS
  2. 10 2
      django/db/models/base.py
  3. 11 0
      tests/regressiontests/cache/models.py
  4. 42 0
      tests/regressiontests/cache/tests.py

+ 1 - 1
AUTHORS

@@ -422,7 +422,7 @@ answer newbie questions, and generally made Django that much better:
     Travis Terry <tdterry7@gmail.com>
     thebjorn <bp@datakortet.no>
     Zach Thompson <zthompson47@gmail.com>
-    Michael Thornhill
+    Michael Thornhill <michael.thornhill@gmail.com>
     Deepak Thukral <deep.thukral@gmail.com>
     tibimicu@gmx.net
     tobias@neuyork.de

+ 10 - 2
django/db/models/base.py

@@ -293,7 +293,14 @@ class Model(object):
                         if rel_obj is None and field.null:
                             val = None
                 else:
-                    val = kwargs.pop(field.attname, field.get_default())
+                    try:
+                        val = kwargs.pop(field.attname)
+                    except KeyError:
+                        # This is done with an exception rather than the
+                        # default argument on pop because we don't want
+                        # get_default() to be evaluated, and then not used.
+                        # Refs #12057.
+                        val = field.get_default()
             else:
                 val = field.get_default()
             if is_related_object:
@@ -346,7 +353,7 @@ class Model(object):
         """
         data = self.__dict__
         if not self._deferred:
-            return (self.__class__, (), data)
+            return super(Model, self).__reduce__()
         defers = []
         pk_val = None
         for field in self._meta.fields:
@@ -359,6 +366,7 @@ class Model(object):
                     # once.
                     obj = self.__class__.__dict__[field.attname]
                     model = obj.model_ref()
+
         return (model_unpickle, (model, defers), data)
 
     def _get_pk_val(self, meta=None):

+ 11 - 0
tests/regressiontests/cache/models.py

@@ -0,0 +1,11 @@
+from django.db import models
+from datetime import datetime
+
+def expensive_calculation():
+    expensive_calculation.num_runs += 1
+    return datetime.now()
+
+class Poll(models.Model):
+    question = models.CharField(max_length=200)
+    answer = models.CharField(max_length=200)
+    pub_date = models.DateTimeField('date published', default=expensive_calculation)

+ 42 - 0
tests/regressiontests/cache/tests.py

@@ -16,6 +16,7 @@ from django.core.cache.backends.base import InvalidCacheBackendError
 from django.http import HttpResponse, HttpRequest
 from django.utils.cache import patch_vary_headers, get_cache_key, learn_cache_key
 from django.utils.hashcompat import md5_constructor
+from regressiontests.cache.models import Poll, expensive_calculation
 
 # functions/classes for complex data type tests
 def f():
@@ -211,6 +212,47 @@ class BaseCacheTests(object):
         self.cache.set("stuff", stuff)
         self.assertEqual(self.cache.get("stuff"), stuff)
 
+    def test_cache_read_for_model_instance(self):
+        # Don't want fields with callable as default to be called on cache read
+        expensive_calculation.num_runs = 0
+        Poll.objects.all().delete()
+        my_poll = Poll.objects.create(question="Well?")
+        self.assertEqual(Poll.objects.count(), 1)
+        pub_date = my_poll.pub_date
+        self.cache.set('question', my_poll)
+        cached_poll = self.cache.get('question')
+        self.assertEqual(cached_poll.pub_date, pub_date)
+        # We only want the default expensive calculation run once
+        self.assertEqual(expensive_calculation.num_runs, 1)
+
+    def test_cache_write_for_model_instance_with_deferred(self):
+        # Don't want fields with callable as default to be called on cache write
+        expensive_calculation.num_runs = 0
+        Poll.objects.all().delete()
+        my_poll = Poll.objects.create(question="What?")
+        self.assertEqual(expensive_calculation.num_runs, 1)
+        defer_qs = Poll.objects.all().defer('question')
+        self.assertEqual(defer_qs.count(), 1)
+        self.assertEqual(expensive_calculation.num_runs, 1)
+        self.cache.set('deferred_queryset', defer_qs)
+        # cache set should not re-evaluate default functions
+        self.assertEqual(expensive_calculation.num_runs, 1)
+
+    def test_cache_read_for_model_instance_with_deferred(self):
+        # Don't want fields with callable as default to be called on cache read
+        expensive_calculation.num_runs = 0
+        Poll.objects.all().delete()
+        my_poll = Poll.objects.create(question="What?")
+        self.assertEqual(expensive_calculation.num_runs, 1)
+        defer_qs = Poll.objects.all().defer('question')
+        self.assertEqual(defer_qs.count(), 1)
+        self.cache.set('deferred_queryset', defer_qs)
+        self.assertEqual(expensive_calculation.num_runs, 1)
+        runs_before_cache_read = expensive_calculation.num_runs
+        cached_polls = self.cache.get('deferred_queryset')
+        # We only want the default expensive calculation run on creation and set
+        self.assertEqual(expensive_calculation.num_runs, runs_before_cache_read)
+
     def test_expiration(self):
         # Cache values can be set to expire
         self.cache.set('expire1', 'very quickly', 1)