From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 24 Mar 2009 01:19:45 +0000 (+0000)
Subject:     - Fixed bug in dynamic_loader() where append/remove events
X-Git-Tag: rel_0_5_3~1
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=533a0ab955c7044dcd8e56a2973599b5b74ab9e8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- 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 786b9c15f6..bef324894c 100644
--- 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 0de5b98ff5..319364910c 100644
--- 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 5e01443a68..d8af4e74f5 100644
--- 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 e72acac9aa..825530b6ea 100644
--- 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={