Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #12057 -- Corrected regression of caching performance when a mo…

…del 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
  • Loading branch information...
commit 96658ef2d24bd05c75aa98c5318d49f7e26549e7 1 parent c5c7791
Russell Keith-Magee authored
2  AUTHORS
@@ -422,7 +422,7 @@ answer newbie questions, and generally made Django that much better:
422 422
     Travis Terry <tdterry7@gmail.com>
423 423
     thebjorn <bp@datakortet.no>
424 424
     Zach Thompson <zthompson47@gmail.com>
425  
-    Michael Thornhill
  425
+    Michael Thornhill <michael.thornhill@gmail.com>
426 426
     Deepak Thukral <deep.thukral@gmail.com>
427 427
     tibimicu@gmx.net
428 428
     tobias@neuyork.de
12  django/db/models/base.py
@@ -293,7 +293,14 @@ def __init__(self, *args, **kwargs):
293 293
                         if rel_obj is None and field.null:
294 294
                             val = None
295 295
                 else:
296  
-                    val = kwargs.pop(field.attname, field.get_default())
  296
+                    try:
  297
+                        val = kwargs.pop(field.attname)
  298
+                    except KeyError:
  299
+                        # This is done with an exception rather than the
  300
+                        # default argument on pop because we don't want
  301
+                        # get_default() to be evaluated, and then not used.
  302
+                        # Refs #12057.
  303
+                        val = field.get_default()
297 304
             else:
298 305
                 val = field.get_default()
299 306
             if is_related_object:
@@ -346,7 +353,7 @@ def __reduce__(self):
346 353
         """
347 354
         data = self.__dict__
348 355
         if not self._deferred:
349  
-            return (self.__class__, (), data)
  356
+            return super(Model, self).__reduce__()
350 357
         defers = []
351 358
         pk_val = None
352 359
         for field in self._meta.fields:
@@ -359,6 +366,7 @@ def __reduce__(self):
359 366
                     # once.
360 367
                     obj = self.__class__.__dict__[field.attname]
361 368
                     model = obj.model_ref()
  369
+
362 370
         return (model_unpickle, (model, defers), data)
363 371
 
364 372
     def _get_pk_val(self, meta=None):
11  tests/regressiontests/cache/models.py
... ...
@@ -0,0 +1,11 @@
  1
+from django.db import models
  2
+from datetime import datetime
  3
+
  4
+def expensive_calculation():
  5
+    expensive_calculation.num_runs += 1
  6
+    return datetime.now()
  7
+
  8
+class Poll(models.Model):
  9
+    question = models.CharField(max_length=200)
  10
+    answer = models.CharField(max_length=200)
  11
+    pub_date = models.DateTimeField('date published', default=expensive_calculation)
42  tests/regressiontests/cache/tests.py
@@ -16,6 +16,7 @@
16 16
 from django.http import HttpResponse, HttpRequest
17 17
 from django.utils.cache import patch_vary_headers, get_cache_key, learn_cache_key
18 18
 from django.utils.hashcompat import md5_constructor
  19
+from regressiontests.cache.models import Poll, expensive_calculation
19 20
 
20 21
 # functions/classes for complex data type tests
21 22
 def f():
@@ -211,6 +212,47 @@ def test_data_types(self):
211 212
         self.cache.set("stuff", stuff)
212 213
         self.assertEqual(self.cache.get("stuff"), stuff)
213 214
 
  215
+    def test_cache_read_for_model_instance(self):
  216
+        # Don't want fields with callable as default to be called on cache read
  217
+        expensive_calculation.num_runs = 0
  218
+        Poll.objects.all().delete()
  219
+        my_poll = Poll.objects.create(question="Well?")
  220
+        self.assertEqual(Poll.objects.count(), 1)
  221
+        pub_date = my_poll.pub_date
  222
+        self.cache.set('question', my_poll)
  223
+        cached_poll = self.cache.get('question')
  224
+        self.assertEqual(cached_poll.pub_date, pub_date)
  225
+        # We only want the default expensive calculation run once
  226
+        self.assertEqual(expensive_calculation.num_runs, 1)
  227
+
  228
+    def test_cache_write_for_model_instance_with_deferred(self):
  229
+        # Don't want fields with callable as default to be called on cache write
  230
+        expensive_calculation.num_runs = 0
  231
+        Poll.objects.all().delete()
  232
+        my_poll = Poll.objects.create(question="What?")
  233
+        self.assertEqual(expensive_calculation.num_runs, 1)
  234
+        defer_qs = Poll.objects.all().defer('question')
  235
+        self.assertEqual(defer_qs.count(), 1)
  236
+        self.assertEqual(expensive_calculation.num_runs, 1)
  237
+        self.cache.set('deferred_queryset', defer_qs)
  238
+        # cache set should not re-evaluate default functions
  239
+        self.assertEqual(expensive_calculation.num_runs, 1)
  240
+
  241
+    def test_cache_read_for_model_instance_with_deferred(self):
  242
+        # Don't want fields with callable as default to be called on cache read
  243
+        expensive_calculation.num_runs = 0
  244
+        Poll.objects.all().delete()
  245
+        my_poll = Poll.objects.create(question="What?")
  246
+        self.assertEqual(expensive_calculation.num_runs, 1)
  247
+        defer_qs = Poll.objects.all().defer('question')
  248
+        self.assertEqual(defer_qs.count(), 1)
  249
+        self.cache.set('deferred_queryset', defer_qs)
  250
+        self.assertEqual(expensive_calculation.num_runs, 1)
  251
+        runs_before_cache_read = expensive_calculation.num_runs
  252
+        cached_polls = self.cache.get('deferred_queryset')
  253
+        # We only want the default expensive calculation run on creation and set
  254
+        self.assertEqual(expensive_calculation.num_runs, runs_before_cache_read)
  255
+
214 256
     def test_expiration(self):
215 257
         # Cache values can be set to expire
216 258
         self.cache.set('expire1', 'very quickly', 1)

0 notes on commit 96658ef

Please sign in to comment.
Something went wrong with that request. Please try again.