From b9f8635f58ad743995cad2081b3dc395e55761e5 Mon Sep 17 00:00:00 2001 From: Michal Petrucha Date: Sun, 1 May 2016 18:32:40 +0200 Subject: [PATCH] Refs #16508 -- Added invalidation of stale cached instances of GenericForeignKey targets. --- django/contrib/contenttypes/fields.py | 39 ++++++++++++++++----------- tests/generic_relations/tests.py | 17 ++++++++++++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/django/contrib/contenttypes/fields.py b/django/contrib/contenttypes/fields.py index e4bc292642..99633a5c67 100644 --- a/django/contrib/contenttypes/fields.py +++ b/django/contrib/contenttypes/fields.py @@ -208,7 +208,7 @@ class GenericForeignKey(object): lambda obj: (obj._get_pk_val(), obj.__class__), gfk_key, True, - self.cache_attr) + self.name) def is_cached(self, instance): return hasattr(instance, self.cache_attr) @@ -217,26 +217,35 @@ class GenericForeignKey(object): if instance is None: return self + # Don't use getattr(instance, self.ct_field) here because that might + # reload the same ContentType over and over (#5570). Instead, get the + # content type ID here, and later when the actual instance is needed, + # use ContentType.objects.get_for_id(), which has a global cache. + f = self.model._meta.get_field(self.ct_field) + ct_id = getattr(instance, f.get_attname(), None) + pk_val = getattr(instance, self.fk_field) + try: - return getattr(instance, self.cache_attr) + rel_obj = getattr(instance, self.cache_attr) except AttributeError: rel_obj = None + else: + if rel_obj and (ct_id != self.get_content_type(obj=rel_obj, using=instance._state.db).id or + rel_obj._meta.pk.to_python(pk_val) != rel_obj._get_pk_val()): + rel_obj = None - # Make sure to use ContentType.objects.get_for_id() to ensure that - # lookups are cached (see ticket #5570). This takes more code than - # the naive ``getattr(instance, self.ct_field)``, but has better - # performance when dealing with GFKs in loops and such. - f = self.model._meta.get_field(self.ct_field) - ct_id = getattr(instance, f.get_attname(), None) - if ct_id is not None: - ct = self.get_content_type(id=ct_id, using=instance._state.db) - try: - rel_obj = ct.get_object_for_this_type(pk=getattr(instance, self.fk_field)) - except ObjectDoesNotExist: - pass - setattr(instance, self.cache_attr, rel_obj) + if rel_obj is not None: return rel_obj + if ct_id is not None: + ct = self.get_content_type(id=ct_id, using=instance._state.db) + try: + rel_obj = ct.get_object_for_this_type(pk=pk_val) + except ObjectDoesNotExist: + pass + setattr(instance, self.cache_attr, rel_obj) + return rel_obj + def __set__(self, instance, value): ct = None fk = None diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py index 91b1640b57..c22a6ef7e0 100644 --- a/tests/generic_relations/tests.py +++ b/tests/generic_relations/tests.py @@ -555,6 +555,23 @@ id="id_generic_relations-taggeditem-content_type-object_id-1-id" />

""" % tag with self.assertRaises(IntegrityError): TaggedItem.objects.create(tag="shiny", content_object=quartz) + def test_cache_invalidation_for_content_type_id(self): + # Create a Vegetable and Mineral with the same id. + new_id = max(Vegetable.objects.order_by('-id')[0].id, + Mineral.objects.order_by('-id')[0].id) + 1 + broccoli = Vegetable.objects.create(id=new_id, name="Broccoli") + diamond = Mineral.objects.create(id=new_id, name="Diamond", hardness=7) + tag = TaggedItem.objects.create(content_object=broccoli, tag="yummy") + tag.content_type = ContentType.objects.get_for_model(diamond) + self.assertEqual(tag.content_object, diamond) + + def test_cache_invalidation_for_object_id(self): + broccoli = Vegetable.objects.create(name="Broccoli") + cauliflower = Vegetable.objects.create(name="Cauliflower") + tag = TaggedItem.objects.create(content_object=broccoli, tag="yummy") + tag.object_id = cauliflower.id + self.assertEqual(tag.content_object, cauliflower) + class CustomWidget(forms.TextInput): pass