- Fixed bug in dynamic_loader() where append/remove events

      after construction time were not being propagated to the
      UOW to pick up on flush(). [ticket:1347]

diff --git a/CHANGES b/CHANGES
index 786b9c15f6dd87ec89d3c861746715eb1a8f025b..bef324894cc3f3c11401394ea3c0bb7c73d87ea9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -24,6 +24,10 @@ CHANGES
       might say SELECT A.*, B.* FROM A JOIN X, B JOIN Y.  
       Eager loading can also tack its joins onto those 
       multiple FROM clauses.  [ticket:1337]
+
+    - Fixed bug in dynamic_loader() where append/remove events
+      after construction time were not being propagated to the 
+      UOW to pick up on flush(). [ticket:1347]
       
     - Fixed bug where column_prefix wasn't being checked before
       not mapping an attribute that already had class-level 

diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index 0de5b98ff59f634f414785a715cbb0adc9631120..319364910c280d3f1033d6f605a370f9734f0172 100644 (file)
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -19,7 +19,7 @@ from sqlalchemy.orm import (
     )
 from sqlalchemy.orm.query import Query
 from sqlalchemy.orm.util import _state_has_identity, has_identity
-
+from sqlalchemy.orm import attributes
 
 class DynaLoader(strategies.AbstractRelationLoader):
     def init_class_attribute(self, mapper):
@@ -70,11 +70,12 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
         collection_history = self._modified_event(state)
         collection_history.added_items.append(value)
 
-        if self.trackparent and value is not None:
-            self.sethasparent(attributes.instance_state(value), True)
         for ext in self.extensions:
             ext.append(state, value, initiator or self)
 
+        if self.trackparent and value is not None:
+            self.sethasparent(attributes.instance_state(value), True)
+
     def fire_remove_event(self, state, value, initiator):
         collection_history = self._modified_event(state)
         collection_history.deleted_items.append(value)
@@ -86,10 +87,12 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
             ext.remove(state, value, initiator or self)
 
     def _modified_event(self, state):
-        state.modified = True
+        
         if self.key not in state.committed_state:
             state.committed_state[self.key] = CollectionHistory(self, state)
 
+        state.modified_event(self, False, attributes.NEVER_SET, passive=attributes.PASSIVE_NO_INITIALIZE)
+
         # this is a hack to allow the _base.ComparableEntity fixture
         # to work
         state.dict[self.key] = True

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 5e01443a6833ead2d03de6094cf16544b36ff75f..d8af4e74f5a548adc380f323ce12242bbb0bc7cc 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1357,6 +1357,7 @@ class Session(object):
             not self._deleted and not self._new):
             return
 
+        
         dirty = self._dirty_states
         if not dirty and not self._deleted and not self._new:
             self.identity_map.modified = False

diff --git a/test/orm/dynamic.py b/test/orm/dynamic.py
index e72acac9aa38878fc9606dbd220a6bde0b62e3a2..825530b6ea64a92f8a633d832a08975568497ae2 100644 (file)
--- a/test/orm/dynamic.py
+++ b/test/orm/dynamic.py
@@ -2,7 +2,7 @@ import testenv; testenv.configure_for_tests()
 import operator
 from sqlalchemy.orm import dynamic_loader, backref
 from testlib import testing
-from testlib.sa import Table, Column, Integer, String, ForeignKey, desc
+from testlib.sa import Table, Column, Integer, String, ForeignKey, desc, select, func
 from testlib.sa.orm import mapper, relation, create_session, Query
 from testlib.testing import eq_
 from testlib.compat import _function_named
@@ -155,6 +155,33 @@ class DynamicTest(_fixtures.FixtureTest):
 class FlushTest(_fixtures.FixtureTest):
     run_inserts = None
 
+    @testing.resolve_artifact_names
+    def test_events(self):
+        mapper(User, users, properties={
+            'addresses':dynamic_loader(mapper(Address, addresses))
+        })
+        sess = create_session()
+        u1 = User(name='jack')
+        a1 = Address(email_address='foo')
+        sess.add_all([u1, a1])
+        sess.flush()
+        
+        assert testing.db.scalar(select([func.count(1)]).where(addresses.c.user_id!=None)) == 0
+        u1 = sess.query(User).get(u1.id)
+        u1.addresses.append(a1)
+        sess.flush()
+
+        assert testing.db.execute(select([addresses]).where(addresses.c.user_id!=None)).fetchall() == [
+            (1, u1.id, 'foo')
+        ]
+        
+        u1.addresses.remove(a1)
+        sess.flush()
+        assert testing.db.scalar(select([func.count(1)]).where(addresses.c.user_id!=None)) == 0
+        
+        
+        
+        
     @testing.resolve_artifact_names
     def test_basic(self):
         mapper(User, users, properties={