Fixed #29478 -- Added support for mangled names to cached_property.

Co-Authored-By: Sergey Fedoseev <fedoseev.sergey@gmail.com>

django/utils/functional.py

@@ -3,6 +3,8 @@
 import operator
 from functools import total_ordering, wraps
 
+from django.utils.version import PY36, get_docs_version
+
 
 # You can't trivially replace this with `functools.partial` because this binds
 # to classes and returns bound instances, whereas functools.partial (on
@@ -18,13 +20,54 @@ class cached_property:
     Decorator that converts a method with a single self argument into a
     property cached on the instance.
 
-    Optional ``name`` argument allows you to make cached properties of other
-    methods. (e.g.  url = cached_property(get_absolute_url, name='url') )
+    A cached property can be made out of an existing method:
+    (e.g. ``url = cached_property(get_absolute_url)``).
+    On Python < 3.6, the optional ``name`` argument must be provided, e.g.
+    ``url = cached_property(get_absolute_url, name='url')``.
     """
+    name = None
+
+    @staticmethod
+    def func(instance):
+        raise TypeError(
+            'Cannot use cached_property instance without calling '
+            '__set_name__() on it.'
+        )
+
+    @staticmethod
+    def _is_mangled(name):
+        return name.startswith('__') and not name.endswith('__')
+
     def __init__(self, func, name=None):
-        self.func = func
+        if PY36:
+            self.real_func = func
+        else:
+            func_name = func.__name__
+            name = name or func_name
+            if not (isinstance(name, str) and name.isidentifier()):
+                raise ValueError(
+                    "%r can't be used as the name of a cached_property." % name,
+                )
+            if self._is_mangled(name):
+                raise ValueError(
+                    'cached_property does not work with mangled methods on '
+                    'Python < 3.6 without the appropriate `name` argument. See '
+                    'https://docs.djangoproject.com/en/%s/ref/utils/'
+                    '#cached-property-mangled-name' % get_docs_version(),
+                )
+            self.name = name
+            self.func = func
         self.__doc__ = getattr(func, '__doc__')
-        self.name = name or func.__name__
+
+    def __set_name__(self, owner, name):
+        if self.name is None:
+            self.name = name
+            self.func = self.real_func
+        elif name != self.name:
+            raise TypeError(
+                "Cannot assign the same cached_property to two different names "
+                "(%r and %r)." % (self.name, name)
+            )
 
     def __get__(self, instance, cls=None):
         """

docs/ref/utils.txt

@@ -492,13 +492,19 @@ https://web.archive.org/web/20110718035220/http://diveintomark.org/archives/2004
     database by some other process in the brief interval between subsequent
     invocations of a method on the same instance.
 
-    You can use the ``name`` argument to make cached properties of other
-    methods. For example, if you had an expensive ``get_friends()`` method and
-    wanted to allow calling it without retrieving the cached value, you could
-    write::
+    You can make cached properties of methods. For example, if you had an
+    expensive ``get_friends()`` method and wanted to allow calling it without
+    retrieving the cached value, you could write::
 
         friends = cached_property(get_friends, name='friends')
 
+    You only need the ``name`` argument for Python < 3.6 support.
+
+    .. versionchanged:: 2.2
+
+        Older versions of Django require the ``name`` argument for all versions
+        of Python.
+
     While ``person.get_friends()`` will recompute the friends on each call, the
     value of the cached property will persist until you delete it as described
     above::
@@ -510,8 +516,11 @@ https://web.archive.org/web/20110718035220/http://diveintomark.org/archives/2004
 
     .. warning::
 
-        ``cached_property`` doesn't work properly with a mangled__ name unless
-        it's passed a ``name`` of the form ``_Class__attribute``::
+        .. _cached-property-mangled-name:
+
+        On Python < 3.6, ``cached_property`` doesn't work properly with a
+        mangled__ name unless it's passed a ``name`` of the form
+        ``_Class__attribute``::
 
             __friends = cached_property(get_friends, name='_Person__friends')
 

docs/releases/2.2.txt

@@ -351,6 +351,35 @@ To simplify a few parts of Django's database handling, `sqlparse
 <https://pypi.org/project/sqlparse/>`_ is now a required dependency. It's
 automatically installed along with Django.
 
+``cached_property`` aliases
+---------------------------
+
+In usage like::
+
+    from django.utils.functional import cached_property
+
+    class A:
+
+        @cached_property
+        def base(self):
+            return ...
+
+        alias = base
+
+``alias`` is not cached. Such usage now raises ``TypeError: Cannot assign the
+same cached_property to two different names ('base' and 'alias').`` on Python
+3.6 and later.
+
+Use this instead::
+
+    import operator
+
+    class A:
+
+        ...
+
+        alias = property(operator.attrgetter('base'))
+
 Miscellaneous
 -------------
 

tests/utils_tests/test_functional.py

@@ -1,9 +1,11 @@
 import unittest
 
+from django.test import SimpleTestCase
 from django.utils.functional import cached_property, lazy
+from django.utils.version import PY36
 
 
-class FunctionalTestCase(unittest.TestCase):
+class FunctionalTests(SimpleTestCase):
     def test_lazy(self):
         t = lazy(lambda: tuple(range(3)), list, tuple)
         for a, b in zip(t(), range(3)):
@@ -47,43 +49,168 @@ def __bytes__(self):
         self.assertEqual(str(t), "Î am ā Ǩlâzz.")
         self.assertEqual(bytes(t), b"\xc3\x8e am \xc4\x81 binary \xc7\xa8l\xc3\xa2zz.")
 
-    def test_cached_property(self):
-        """
-        cached_property caches its value and that it behaves like a property
-        """
-        class A:
+    def assertCachedPropertyWorks(self, attr, Class):
+        with self.subTest(attr=attr):
+            def get(source):
+                return getattr(source, attr)
+
+            obj = Class()
 
+            class SubClass(Class):
+                pass
+
+            subobj = SubClass()
+            # Docstring is preserved.
+            self.assertEqual(get(Class).__doc__, 'Here is the docstring...')
+            self.assertEqual(get(SubClass).__doc__, 'Here is the docstring...')
+            # It's cached.
+            self.assertEqual(get(obj), get(obj))
+            self.assertEqual(get(subobj), get(subobj))
+            # The correct value is returned.
+            self.assertEqual(get(obj)[0], 1)
+            self.assertEqual(get(subobj)[0], 1)
+            # State isn't shared between instances.
+            obj2 = Class()
+            subobj2 = SubClass()
+            self.assertNotEqual(get(obj), get(obj2))
+            self.assertNotEqual(get(subobj), get(subobj2))
+            # It behaves like a property when there's no instance.
+            self.assertIsInstance(get(Class), cached_property)
+            self.assertIsInstance(get(SubClass), cached_property)
+            # 'other_value' doesn't become a property.
+            self.assertTrue(callable(obj.other_value))
+            self.assertTrue(callable(subobj.other_value))
+
+    def test_cached_property(self):
+        """cached_property caches its value and behaves like a property."""
+        class Class:
             @cached_property
             def value(self):
                 """Here is the docstring..."""
                 return 1, object()
 
+            @cached_property
+            def __foo__(self):
+                """Here is the docstring..."""
+                return 1, object()
+
             def other_value(self):
-                return 1
+                """Here is the docstring..."""
+                return 1, object()
 
             other = cached_property(other_value, name='other')
 
-        # docstring should be preserved
-        self.assertEqual(A.value.__doc__, "Here is the docstring...")
+        attrs = ['value', 'other', '__foo__']
+        for attr in attrs:
+            self.assertCachedPropertyWorks(attr, Class)
 
-        a = A()
+    @unittest.skipUnless(PY36, '__set_name__ is new in Python 3.6')
+    def test_cached_property_auto_name(self):
+        """
+        cached_property caches its value and behaves like a property
+        on mangled methods or when the name kwarg isn't set.
+        """
+        class Class:
+            @cached_property
+            def __value(self):
+                """Here is the docstring..."""
+                return 1, object()
+
+            def other_value(self):
+                """Here is the docstring..."""
+                return 1, object()
+
+            other = cached_property(other_value)
+            other2 = cached_property(other_value, name='different_name')
+
+        attrs = ['_Class__value', 'other']
+        for attr in attrs:
+            self.assertCachedPropertyWorks(attr, Class)
+
+        # An explicit name is ignored.
+        obj = Class()
+        obj.other2
+        self.assertFalse(hasattr(obj, 'different_name'))
+
+    @unittest.skipUnless(PY36, '__set_name__ is new in Python 3.6')
+    def test_cached_property_reuse_different_names(self):
+        """Disallow this case because the decorated function wouldn't be cached."""
+        with self.assertRaises(RuntimeError) as ctx:
+            class ReusedCachedProperty:
+                @cached_property
+                def a(self):
+                    pass
+
+                b = a
+
+        self.assertEqual(
+            str(ctx.exception.__context__),
+            str(TypeError(
+                "Cannot assign the same cached_property to two different "
+                "names ('a' and 'b')."
+            ))
+        )
+
+    @unittest.skipUnless(PY36, '__set_name__ is new in Python 3.6')
+    def test_cached_property_reuse_same_name(self):
+        """
+        Reusing a cached_property on different classes under the same name is
+        allowed.
+        """
+        counter = 0
 
-        # check that it is cached
-        self.assertEqual(a.value, a.value)
+        @cached_property
+        def _cp(_self):
+            nonlocal counter
+            counter += 1
+            return counter
 
-        # check that it returns the right thing
-        self.assertEqual(a.value[0], 1)
+        class A:
+            cp = _cp
 
-        # check that state isn't shared between instances
-        a2 = A()
-        self.assertNotEqual(a.value, a2.value)
+        class B:
+            cp = _cp
 
-        # check that it behaves like a property when there's no instance
-        self.assertIsInstance(A.value, cached_property)
+        a = A()
+        b = B()
+        self.assertEqual(a.cp, 1)
+        self.assertEqual(b.cp, 2)
+        self.assertEqual(a.cp, 1)
 
-        # check that overriding name works
-        self.assertEqual(a.other, 1)
-        self.assertTrue(callable(a.other_value))
+    @unittest.skipUnless(PY36, '__set_name__ is new in Python 3.6')
+    def test_cached_property_set_name_not_called(self):
+        cp = cached_property(lambda s: None)
+
+        class Foo:
+            pass
+
+        Foo.cp = cp
+        msg = 'Cannot use cached_property instance without calling __set_name__() on it.'
+        with self.assertRaisesMessage(TypeError, msg):
+            Foo().cp
+
+    @unittest.skipIf(PY36, '__set_name__ is new in Python 3.6')
+    def test_cached_property_mangled_error(self):
+        msg = (
+            'cached_property does not work with mangled methods on '
+            'Python < 3.6 without the appropriate `name` argument.'
+        )
+        with self.assertRaisesMessage(ValueError, msg):
+            @cached_property
+            def __value(self):
+                pass
+        with self.assertRaisesMessage(ValueError, msg):
+            def func(self):
+                pass
+            cached_property(func, name='__value')
+
+    @unittest.skipIf(PY36, '__set_name__ is new in Python 3.6')
+    def test_cached_property_name_validation(self):
+        msg = "%s can't be used as the name of a cached_property."
+        with self.assertRaisesMessage(ValueError, msg % "'<lambda>'"):
+            cached_property(lambda x: None)
+        with self.assertRaisesMessage(ValueError, msg % 42):
+            cached_property(str, name=42)
 
     def test_lazy_equality(self):
         """