From 36275b0c2d8d3501bc0023e80d8433c0d54c0d9a Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 27 Apr 2017 11:24:41 -0400 Subject: [PATCH] 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 --- doc/build/changelog/changelog_12.rst | 11 +++ lib/sqlalchemy/ext/associationproxy.py | 3 +- test/ext/test_associationproxy.py | 100 ++++++++++++++++++++++++- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index 2b67414948..b53ed4530f 100644 --- 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 6f570a1fa9..1c735ca4d8 100644 --- 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 018c2bc2ad..c3891408ac 100644 --- 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 -- 2.39.5