Fixed #32980 -- Made models cache related managers.

django/contrib/contenttypes/fields.py

@@ -505,6 +505,14 @@ class ReverseGenericManyToOneDescriptor(ReverseManyToOneDescriptor):
             self.rel,
         )
 
+    @cached_property
+    def related_manager_cache_key(self):
+        # By default, GenericRel instances will be marked as hidden unless
+        # related_query_name is given (their accessor name being "+" when
+        # hidden), which would cause multiple GenericRelations declared on a
+        # single model to collide, so always use the remote field's name.
+        return self.field.get_cache_name()
+
 
 def create_generic_related_manager(superclass, rel):
     """

django/db/models/base.py

@@ -382,11 +382,18 @@ class ModelBase(type):
         return cls._meta.default_manager
 
 
-class ModelStateFieldsCacheDescriptor:
+class ModelStateCacheDescriptor:
+    """
+    Upon first access, replace itself with an empty dictionary on the instance.
+    """
+
+    def __set_name__(self, owner, name):
+        self.attribute_name = name
+
     def __get__(self, instance, cls=None):
         if instance is None:
             return self
-        res = instance.fields_cache = {}
+        res = instance.__dict__[self.attribute_name] = {}
         return res
 
 
@@ -398,7 +405,20 @@ class ModelState:
     # explicit (non-auto) PKs. This impacts validation only; it has no effect
     # on the actual save.
     adding = True
-    fields_cache = ModelStateFieldsCacheDescriptor()
+    fields_cache = ModelStateCacheDescriptor()
+    related_managers_cache = ModelStateCacheDescriptor()
+
+    def __getstate__(self):
+        state = self.__dict__.copy()
+        if 'fields_cache' in state:
+            state['fields_cache'] = self.fields_cache.copy()
+        # Manager instances stored in related_managers_cache won't necessarily
+        # be deserializable if they were dynamically created via an inner
+        # scope, e.g. create_forward_many_to_many_manager() and
+        # create_generic_related_manager().
+        if 'related_managers_cache' in state:
+            state['related_managers_cache'] = {}
+        return state
 
 
 class Model(metaclass=ModelBase):
@@ -552,7 +572,6 @@ class Model(metaclass=ModelBase):
         """Hook to allow choosing the attributes to pickle."""
         state = self.__dict__.copy()
         state['_state'] = copy.copy(state['_state'])
-        state['_state'].fields_cache = state['_state'].fields_cache.copy()
         return state
 
     def __setstate__(self, state):

django/db/models/fields/related_descriptors.py

@@ -511,6 +511,14 @@ class ReverseManyToOneDescriptor:
         self.rel = rel
         self.field = rel.field
 
+    @cached_property
+    def related_manager_cache_key(self):
+        # Being able to access the manager instance precludes it from being
+        # hidden. The rel's accessor name is used to allow multiple managers
+        # to the same model to coexist. e.g. post.attached_comment_set and
+        # post.attached_link_set are separately cached.
+        return self.rel.get_cache_name()
+
     @cached_property
     def related_manager_cls(self):
         related_model = self.rel.related_model
@@ -532,8 +540,11 @@ class ReverseManyToOneDescriptor:
         """
         if instance is None:
             return self
-
+        key = self.related_manager_cache_key
-        return self.related_manager_cls(instance)
+        instance_cache = instance._state.related_managers_cache
+        if key not in instance_cache:
+            instance_cache[key] = self.related_manager_cls(instance)
+        return instance_cache[key]
 
     def _get_set_deprecation_msg_params(self):
         return (
@@ -797,6 +808,17 @@ class ManyToManyDescriptor(ReverseManyToOneDescriptor):
             reverse=self.reverse,
         )
 
+    @cached_property
+    def related_manager_cache_key(self):
+        if self.reverse:
+            # Symmetrical M2Ms won't have an accessor name, but should never
+            # end up in the reverse branch anyway, as the related_name ends up
+            # being hidden, and no public manager is created.
+            return self.rel.get_cache_name()
+        else:
+            # For forward managers, defer to the field name.
+            return self.field.get_cache_name()
+
     def _get_set_deprecation_msg_params(self):
         return (
             '%s side of a many-to-many set' % ('reverse' if self.reverse else 'forward'),

docs/releases/4.1.txt

@@ -255,7 +255,10 @@ Upstream support for MariaDB 10.2 ends in May 2022. Django 4.1 supports MariaDB
 Miscellaneous
 -------------
 
-* ...
+* Related managers for :class:`~django.db.models.ForeignKey`,
+  :class:`~django.db.models.ManyToManyField`, and
+  :class:`~django.contrib.contenttypes.fields.GenericRelation` are now cached
+  on the :class:`~django.db.models.Model` instance to which they belong.
 
 .. _deprecated-features-4.1:
 

tests/custom_managers/models.py

@@ -155,6 +155,22 @@ class Book(models.Model):
         base_manager_name = 'annotated_objects'
 
 
+class ConfusedBook(models.Model):
+    title = models.CharField(max_length=50)
+    author = models.CharField(max_length=30)
+    favorite_things = GenericRelation(
+        Person,
+        content_type_field='favorite_thing_type',
+        object_id_field='favorite_thing_id',
+    )
+    less_favorite_things = GenericRelation(
+        FunPerson,
+        content_type_field='favorite_thing_type',
+        object_id_field='favorite_thing_id',
+        related_query_name='favorite_things',
+    )
+
+
 class FastCarManager(models.Manager):
     def get_queryset(self):
         return super().get_queryset().filter(top_speed__gt=150)

tests/custom_managers/tests.py

@@ -2,10 +2,10 @@ from django.db import models
 from django.test import TestCase
 
 from .models import (
-    Book, Car, CustomManager, CustomQuerySet, DeconstructibleCustomManager,
+    Book, Car, ConfusedBook, CustomManager, CustomQuerySet,
-    FastCarAsBase, FastCarAsDefault, FunPerson, OneToOneRestrictedModel,
+    DeconstructibleCustomManager, FastCarAsBase, FastCarAsDefault, FunPerson,
-    Person, PersonFromAbstract, PersonManager, PublishedBookManager,
+    OneToOneRestrictedModel, Person, PersonFromAbstract, PersonManager,
-    RelatedModel, RestrictedModel,
+    PublishedBookManager, RelatedModel, RestrictedModel,
 )
 
 
@@ -169,6 +169,10 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
+    def test_fk_related_manager_reused(self):
+        self.assertIs(self.b1.favorite_books, self.b1.favorite_books)
+        self.assertIn('favorite_books', self.b1._state.related_managers_cache)
+
     def test_gfk_related_manager(self):
         Person.objects.create(first_name="Bugs", last_name="Bunny", fun=True, favorite_thing=self.b1)
         Person.objects.create(first_name="Droopy", last_name="Dog", fun=False, favorite_thing=self.b1)
@@ -205,6 +209,60 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
+    def test_gfk_related_manager_reused(self):
+        self.assertIs(
+            self.b1.fun_people_favorite_things,
+            self.b1.fun_people_favorite_things,
+        )
+        self.assertIn(
+            'fun_people_favorite_things',
+            self.b1._state.related_managers_cache,
+        )
+
+    def test_gfk_related_manager_not_reused_when_alternate(self):
+        self.assertIsNot(
+            self.b1.favorite_things(manager='fun_people'),
+            self.b1.favorite_things(manager='fun_people'),
+        )
+
+    def test_gfk_related_manager_no_overlap_when_not_hidden(self):
+        """
+        If a GenericRelation defines a related_query_name (and thus the
+        related_name) which shadows another GenericRelation, it should not
+        cause those separate managers to clash.
+        """
+        book = ConfusedBook.objects.create(
+            title='How to program', author='Rodney Dangerfield',
+        )
+        person = Person.objects.create(
+            first_name='Bugs', last_name='Bunny', fun=True, favorite_thing=book,
+        )
+        fun_person = FunPerson.objects.create(
+            first_name='Droopy', last_name='Dog', fun=False, favorite_thing=book,
+        )
+        # The managers don't collide in the internal cache.
+        self.assertIsNot(book.favorite_things, book.less_favorite_things)
+        self.assertIs(book.favorite_things, book.favorite_things)
+        self.assertIs(book.less_favorite_things, book.less_favorite_things)
+        # Both managers are cached separately despite the collision in names.
+        self.assertIn('favorite_things', book._state.related_managers_cache)
+        self.assertIn('less_favorite_things', book._state.related_managers_cache)
+        # "less_favorite_things" isn't available as a reverse related manager,
+        # so never ends up in the cache.
+        self.assertQuerysetEqual(fun_person.favorite_things.all(), [book])
+        with self.assertRaises(AttributeError):
+            fun_person.less_favorite_things
+        self.assertIn('favorite_things', fun_person._state.related_managers_cache)
+        self.assertNotIn(
+            'less_favorite_things',
+            fun_person._state.related_managers_cache,
+        )
+        # The GenericRelation doesn't exist for Person, only FunPerson, so the
+        # exception prevents the cache from being polluted.
+        with self.assertRaises(AttributeError):
+            person.favorite_things
+        self.assertNotIn('favorite_things', person._state.related_managers_cache)
+
     def test_m2m_related_manager(self):
         bugs = Person.objects.create(first_name="Bugs", last_name="Bunny", fun=True)
         self.b1.authors.add(bugs)
@@ -245,6 +303,16 @@ class CustomManagerTests(TestCase):
             ordered=False,
         )
 
+    def test_m2m_related_forward_manager_reused(self):
+        self.assertIs(self.b1.authors, self.b1.authors)
+        self.assertIn('authors', self.b1._state.related_managers_cache)
+
+    def test_m2m_related_revers_manager_reused(self):
+        bugs = Person.objects.create(first_name='Bugs', last_name='Bunny')
+        self.b1.authors.add(bugs)
+        self.assertIs(bugs.books, bugs.books)
+        self.assertIn('books', bugs._state.related_managers_cache)
+
     def test_removal_through_default_fk_related_manager(self, bulk=True):
         bugs = FunPerson.objects.create(first_name="Bugs", last_name="Bunny", fun=True, favorite_book=self.b1)
         droopy = FunPerson.objects.create(first_name="Droopy", last_name="Dog", fun=False, favorite_book=self.b1)

tests/m2m_regress/tests.py

@@ -31,6 +31,14 @@ class M2MRegressionTests(TestCase):
         self.assertSequenceEqual(e1.topics.all(), [t1])
         self.assertSequenceEqual(e1.related.all(), [t2])
 
+    def test_m2m_managers_reused(self):
+        s1 = SelfRefer.objects.create(name='s1')
+        e1 = Entry.objects.create(name='e1')
+        self.assertIs(s1.references, s1.references)
+        self.assertIs(s1.related, s1.related)
+        self.assertIs(e1.topics, e1.topics)
+        self.assertIs(e1.related, e1.related)
+
     def test_internal_related_name_not_in_error_msg(self):
         # The secret internal related names for self-referential many-to-many
         # fields shouldn't appear in the list when an error is made.

tests/model_inheritance_regress/tests.py

@@ -354,6 +354,11 @@ class ModelInheritanceTest(TestCase):
         parties = list(p4.bachelorparty_set.all())
         self.assertEqual(parties, [bachelor, messy_parent])
 
+    def test_abstract_base_class_m2m_relation_inheritance_manager_reused(self):
+        p1 = Person.objects.create(name='Alice')
+        self.assertIs(p1.birthdayparty_set, p1.birthdayparty_set)
+        self.assertIs(p1.bachelorparty_set, p1.bachelorparty_set)
+
     def test_abstract_verbose_name_plural_inheritance(self):
         """
         verbose_name_plural correctly inherited from ABC if inheritance chain

tests/model_regress/test_state.py

@@ -1,8 +1,11 @@
-from django.db.models.base import ModelState, ModelStateFieldsCacheDescriptor
+from django.db.models.base import ModelState, ModelStateCacheDescriptor
 from django.test import SimpleTestCase
 
 
 class ModelStateTests(SimpleTestCase):
 
     def test_fields_cache_descriptor(self):
-        self.assertIsInstance(ModelState.fields_cache, ModelStateFieldsCacheDescriptor)
+        self.assertIsInstance(ModelState.fields_cache, ModelStateCacheDescriptor)
+
+    def test_related_managers_descriptor(self):
+        self.assertIsInstance(ModelState.related_managers_cache, ModelStateCacheDescriptor)

tests/prefetch_related/tests.py

@@ -1147,6 +1147,14 @@ class ForeignKeyToFieldTest(TestCase):
                 ]
             )
 
+    def test_m2m_manager_reused(self):
+        author = Author.objects.prefetch_related(
+            'favorite_authors',
+            'favors_me',
+        ).first()
+        self.assertIs(author.favorite_authors, author.favorite_authors)
+        self.assertIs(author.favors_me, author.favors_me)
+
 
 class LookupOrderingTest(TestCase):
     """