Call proxied collection before invoking creator in associationlist.append()

Improved the association proxy list collection so that premature
autoflush against a newly created association object can be prevented
in the case where ``list.append()`` is being used, and a lazy load
would be invoked when the association proxy accesses the endpoint
collection.  The endpoint collection is now accessed first before
the creator is invoked to produce the association object.

Change-Id: I008a6dbdfe5b1c0dfd02189c3d954d83a65f3fc5
Fixes: #3941

diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst
index 2b67414948941bced537e51efa9c438a2c4bbdb8..b53ed4530fcb5411c6779f02a8c8b0d9c81e30ed 100644 (file)
--- a/doc/build/changelog/changelog_12.rst
+++ b/doc/build/changelog/changelog_12.rst
@@ -24,6 +24,17 @@
         impact how the expression behaves in larger contexts as well as
         in result-row-handling.
 
+    .. change:: 3941
+        :tags: bug, ext
+        :tickets: 3941
+
+        Improved the association proxy list collection so that premature
+        autoflush against a newly created association object can be prevented
+        in the case where ``list.append()`` is being used, and a lazy load
+        would be invoked when the association proxy accesses the endpoint
+        collection.  The endpoint collection is now accessed first before
+        the creator is invoked to produce the association object.
+
     .. change:: 3969
         :tags: bug, sql
         :tickets: 3969

diff --git a/lib/sqlalchemy/ext/associationproxy.py b/lib/sqlalchemy/ext/associationproxy.py
index 6f570a1fa955154a069bf69cb3a4eb168de6890f..1c735ca4d8b97cec743313f286d41bb840cfa388 100644 (file)
--- a/lib/sqlalchemy/ext/associationproxy.py
+++ b/lib/sqlalchemy/ext/associationproxy.py
@@ -606,8 +606,9 @@ class _AssociationList(_AssociationCollection):
         return
 
     def append(self, value):
+        col = self.col
         item = self._create(value)
-        self.col.append(item)
+        col.append(item)
 
     def count(self, value):
         return sum([1 for _ in

diff --git a/test/ext/test_associationproxy.py b/test/ext/test_associationproxy.py
index 018c2bc2ad789ef198b822267f5e9a4ed670a5f5..c3891408aca251256638a0d90e9dd30da951365c 100644 (file)
--- a/test/ext/test_associationproxy.py
+++ b/test/ext/test_associationproxy.py
@@ -50,6 +50,103 @@ class ObjectCollection(object):
         return iter(self.values)
 
 
+class AutoFlushTest(fixtures.TablesTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'parent', metadata,
+            Column('id', Integer, primary_key=True,
+                   test_needs_autoincrement=True))
+        Table(
+            'association', metadata,
+            Column('parent_id', ForeignKey('parent.id'), primary_key=True),
+            Column('child_id', ForeignKey('child.id'), primary_key=True),
+            Column('name', String(50))
+        )
+        Table(
+            'child', metadata,
+            Column('id', Integer, primary_key=True,
+                   test_needs_autoincrement=True),
+            Column('name', String(50))
+        )
+
+    def _fixture(self, collection_class, is_dict=False):
+        class Parent(object):
+            collection = association_proxy("_collection", "child")
+
+        class Child(object):
+            def __init__(self, name):
+                self.name = name
+
+        class Association(object):
+            if is_dict:
+                def __init__(self, key, child):
+                    self.child = child
+            else:
+                def __init__(self, child):
+                    self.child = child
+
+        mapper(Parent, self.tables.parent, properties={
+            "_collection": relationship(Association,
+                                        collection_class=collection_class,
+                                        backref="parent")
+        })
+        mapper(Association, self.tables.association, properties={
+            "child": relationship(Child, backref="association")
+        })
+        mapper(Child, self.tables.child)
+
+        return Parent, Child, Association
+
+    def _test_premature_flush(self, collection_class, fn, is_dict=False):
+        Parent, Child, Association = self._fixture(
+            collection_class, is_dict=is_dict)
+
+        session = Session(testing.db, autoflush=True, expire_on_commit=True)
+
+        p1 = Parent()
+        c1 = Child('c1')
+        c2 = Child('c2')
+        session.add(p1)
+        session.add(c1)
+        session.add(c2)
+
+        fn(p1.collection, c1)
+        session.commit()
+
+        fn(p1.collection, c2)
+        session.commit()
+
+        is_(c1.association[0].parent, p1)
+        is_(c2.association[0].parent, p1)
+
+        session.close()
+
+    def test_list_append(self):
+        self._test_premature_flush(
+            list, lambda collection, obj: collection.append(obj))
+
+    def test_list_extend(self):
+        self._test_premature_flush(
+            list, lambda collection, obj: collection.extend([obj]))
+
+    def test_set_add(self):
+        self._test_premature_flush(
+            set, lambda collection, obj: collection.add(obj))
+
+    def test_set_extend(self):
+        self._test_premature_flush(
+            set, lambda collection, obj: collection.update([obj]))
+
+    def test_dict_set(self):
+        def set_(collection, obj):
+            collection[obj.name] = obj
+
+        self._test_premature_flush(
+            collections.attribute_mapped_collection('name'),
+            set_, is_dict=True)
+
+
 class _CollectionOperations(fixtures.TestBase):
     def setup(self):
         collection_class = self.collection_class
@@ -84,7 +181,7 @@ class _CollectionOperations(fixtures.TestBase):
                     self.name = name
 
         mapper(Parent, parents_table, properties={
-            '_children': relationship(Child, lazy='joined',
+            '_children': relationship(Child, lazy='joined', backref='parent',
                                       collection_class=collection_class)})
         mapper(Child, children_table)
 
@@ -260,6 +357,7 @@ class _CollectionOperations(fixtures.TestBase):
             assert True
 
 
+
 class DefaultTest(_CollectionOperations):
     collection_class = None