From 2ab2208c7d78accb41317ae52ab0abdd5fb731ea Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Wed, 26 Aug 2020 11:44:34 -0400
Subject: [PATCH] Document caveat about backrefs and
 attribute_mapped_collection

Fixes: #5538
Change-Id: I2bda6bed40d35560a71bf0ed09d141047ce59e82
(cherry picked from commit fe772672b4fc00df0b66aca92e2092779a844a2d)
---
 doc/build/orm/collections.rst     | 82 +++++++++++++++++++++++++++++++
 lib/sqlalchemy/orm/collections.py | 11 +++--
 2 files changed, 89 insertions(+), 4 deletions(-)

diff --git a/doc/build/orm/collections.rst b/doc/build/orm/collections.rst
index 5063aff4af..dbcd8d5654 100644
--- a/doc/build/orm/collections.rst
+++ b/doc/build/orm/collections.rst
@@ -302,6 +302,88 @@ Dictionary mappings are often combined with the "Association Proxy" extension to
 streamlined dictionary views.  See :ref:`proxying_dictionaries` and :ref:`composite_association_proxy`
 for examples.
 
+.. _key_collections_mutations:
+
+Dealing with Key Mutations and back-populating for Dictionary collections
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When using :func:`.attribute_mapped_collection`, the "key" for the dictionary
+is taken from an attribute on the target object.   **Changes to this key
+are not tracked**.  This means that the key must be assigned towards when
+it is first used, and if the key changes, the collection will not be mutated.
+A typical example where this might be an issue is when relying upon backrefs
+to populate an attribute mapped collection.  Given the following::
+
+    class A(Base):
+        __tablename__ = "a"
+
+        id = Column(Integer, primary_key=True)
+        bs = relationship(
+            "B",
+            collection_class=attribute_mapped_collection("data"),
+            back_populates="a",
+        )
+
+
+    class B(Base):
+        __tablename__ = "b"
+        id = Column(Integer, primary_key=True)
+        a_id = Column(ForeignKey("a.id"))
+        data = Column(String)
+
+        a = relationship("A", back_populates="bs")
+
+Above, if we create a ``B()`` that refers to a specific ``A()``, the back
+populates will then add the ``B()`` to the ``A.bs`` collection, however
+if the value of ``B.data`` is not set yet, the key will be ``None``::
+
+    >>> a1 = A()
+    >>> b1 = B(a=a1)
+    >>> a1.bs
+    {None: <test3.B object at 0x7f7b1023ef70>}
+
+
+Setting ``b1.data`` after the fact does not update the collection::
+
+    >>> b1.data = 'the key'
+    >>> a1.bs
+    {None: <test3.B object at 0x7f7b1023ef70>}
+
+
+This can also be seen if one attempts to set up ``B()`` in the constructor.
+The order of arguments changes the result::
+
+    >>> B(a=a1, data='the key')
+    <test3.B object at 0x7f7b10114280>
+    >>> a1.bs
+    {None: <test3.B object at 0x7f7b10114280>}
+
+vs::
+
+    >>> B(data='the key', a=a1)
+    <test3.B object at 0x7f7b10114340>
+    >>> a1.bs
+    {'the key': <test3.B object at 0x7f7b10114340>}
+
+If backrefs are being used in this way, ensure that attributes are populated
+in the correct order using an ``__init__`` method.
+
+An event handler such as the following may also be used to track changes in the
+collection as well::
+
+    from sqlalchemy import event
+
+    from sqlalchemy.orm import attributes
+
+    @event.listens_for(B.data, "set")
+    def set_item(obj, value, previous, initiator):
+        if obj.a is not None:
+            previous = None if previous == attributes.NO_VALUE else previous
+            obj.a.bs[value] = obj
+            obj.a.bs.pop(previous)
+
+
+
 .. autofunction:: attribute_mapped_collection
 
 .. autofunction:: column_mapped_collection
diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py
index 846b360f78..6a55e1ab5f 100644
--- a/lib/sqlalchemy/orm/collections.py
+++ b/lib/sqlalchemy/orm/collections.py
@@ -269,10 +269,13 @@ def attribute_mapped_collection(attr_name):
     'attr_name' attribute of entities in the collection, where ``attr_name``
     is the string name of the attribute.
 
-    The key value must be immutable for the lifetime of the object.  You
-    can not, for example, map on foreign key values if those key values will
-    change during the session, i.e. from None to a database-assigned integer
-    after a session flush.
+    .. warning:: the key value must be assigned to its final value
+       **before** it is accessed by the attribute mapped collection.
+       Additionally, changes to the key attribute are **not tracked**
+       automatically, which means the key in the dictionary is not
+       automatically synchronized with the key value on the target object
+       itself.  See the section :ref:`key_collections_mutations`
+       for an example.
 
     """
     getter = _SerializableAttrGetter(attr_name)
-- 
2.47.2