From 96658ef2d24bd05c75aa98c5318d49f7e26549e7 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 29 Oct 2009 14:32:01 +0000 Subject: [PATCH] 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 --- AUTHORS | 2 +- django/db/models/base.py | 12 ++++++-- tests/regressiontests/cache/models.py | 11 +++++++ tests/regressiontests/cache/tests.py | 42 +++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index ac22cc6f40..72f6b35f2a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -422,7 +422,7 @@ answer newbie questions, and generally made Django that much better: Travis Terry thebjorn Zach Thompson - Michael Thornhill + Michael Thornhill Deepak Thukral tibimicu@gmx.net tobias@neuyork.de diff --git a/django/db/models/base.py b/django/db/models/base.py index 1e081ae92e..2c05009823 100644 --- a/django/db/models/base.py +++ b/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): diff --git a/tests/regressiontests/cache/models.py b/tests/regressiontests/cache/models.py index e69de29bb2..643fd22629 100644 --- a/tests/regressiontests/cache/models.py +++ b/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) diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py index 1b54ab935f..593b7262aa 100644 --- a/tests/regressiontests/cache/tests.py +++ b/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)