]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
more changes to merge(dont_load); since we now have a guarantee that
authorMike Bayer <mike_mp@zzzcomputing.com>
Fri, 23 Nov 2007 19:01:40 +0000 (19:01 +0000)
committerMike Bayer <mike_mp@zzzcomputing.com>
Fri, 23 Nov 2007 19:01:40 +0000 (19:01 +0000)
all merged instances aren't dirty, we can copy the attribues without using
any append/replace events, and therefore don't have any concern of lazy loaders
firing off.  added tests to ensure that '_is_orphan()' doesn't get screwed up,
and also that the 'dirty' list on the new session stays empty, which is an
extra bonus of this approach.

CHANGES
lib/sqlalchemy/orm/properties.py
test/orm/merge.py

diff --git a/CHANGES b/CHANGES
index 0165bc006d9fba5c68106f27eef9df4e39fdf9fe..d453bda38d95027f7c553f7c7925b6b0f65c332d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -19,9 +19,11 @@ CHANGES
      changes on it, in the case that dont_load=True is used....this will
      now raise an error.  This is due to complexities in merging the 
      "committed state" of the given instance to correctly correspond to the
-     newly copied instance.  Since the use case for dont_load=True is 
-     caching, the given instances shouldn't have any uncommitted changes on them
-     anyway.
+     newly copied instance, as well as other modified state.  
+     Since the use case for dont_load=True is caching, the given instances 
+     shouldn't have any uncommitted changes on them anyway.
+     We also copy the instances over without using any events now, so that
+     the 'dirty' list on the new session remains unaffected.
 
 0.4.1
 -----
index cbc21a2c3fbdecdf4360f5971662e1d04080cb29..46224aac658f5167c3c286e9c7ded824acd64742 100644 (file)
@@ -292,13 +292,19 @@ class PropertyLoader(StrategizedProperty):
             for current in list(childlist):
                 obj = session.merge(current, entity_name=self.mapper.entity_name, dont_load=dont_load, _recursive=_recursive)
                 if obj is not None:
-                    dest_list.append_with_event(obj)
+                    if dont_load:
+                        dest_list.append_without_event(obj)
+                    else:
+                        dest_list.append_with_event(obj)
         else:
             current = list(childlist)[0]
             if current is not None:
                 obj = session.merge(current, entity_name=self.mapper.entity_name, dont_load=dont_load, _recursive=_recursive)
                 if obj is not None:
-                    setattr(dest, self.key, obj)
+                    if dont_load:
+                        dest.__dict__[self.key] = obj
+                    else:
+                        setattr(dest, self.key, obj)
 
     def cascade_iterator(self, type, object, recursive, halt_on=None):
         if not type in self.cascade:
index f6df8e9add067d1599b3d8de4c86fcd3dff883cd..a58665136fb24eb31c5e4fb883d3d97dc4948263 100644 (file)
@@ -261,6 +261,7 @@ class MergeTest(AssertMixin):
         
         sess3 = create_session()
         u3 = sess3.merge(u2, dont_load=True)
+        assert not sess3.dirty
         def go():
             sess3.flush()
         self.assert_sql_count(testbase.db, go, 0)
@@ -287,6 +288,7 @@ class MergeTest(AssertMixin):
         
         sess2 = create_session()
         u2 = sess2.merge(u, dont_load=True)
+        assert not sess2.dirty
         # assert merged instance has a mapper and lazy load proceeds
         assert hasattr(u2, '_entity_name')
         assert mapperlib.has_mapper(u2)
@@ -294,7 +296,90 @@ class MergeTest(AssertMixin):
             assert u2.addresses != []
             assert len(u2.addresses) == 1
         self.assert_sql_count(testbase.db, go, 1)
+
+    def test_noload_sets_backrefs(self):
+        mapper(User, users, properties={
+            'addresses':relation(mapper(Address, addresses),backref='user')
+        })
+        sess = create_session()
+        u = User()
+        u.user_id = 7
+        u.user_name = "fred"
+        a1 = Address()
+        a1.email_address='foo@bar.com'
+        u.addresses.append(a1)
+
+        sess.save(u)
+        sess.flush()
+
+        assert u.addresses[0].user is u
+
+        sess2 = create_session()
+        u2 = sess2.merge(u, dont_load=True)
+        assert not sess2.dirty
+        def go():
+            assert u2.addresses[0].user is u2
+        self.assert_sql_count(testbase.db, go, 0)
+    
+    def test_noload_preserves_parents(self):
+        """test that merge with noload does not trigger a 'delete-orphan' operation.
+        
+        merge with noload sets attributes without using events.  this means the
+        'hasparent' flag is not propagated to the newly merged instance.  in fact this
+        works out OK, because the '_state.parents' collection on the newly 
+        merged instance is empty; since the mapper doesn't see an active 'False' setting
+        in this collection when _is_orphan() is called, it does not count as an orphan 
+        (i.e. this is the 'optimistic' logic in mapper._is_orphan().)
+        """
+        
+        mapper(User, users, properties={
+            'addresses':relation(mapper(Address, addresses),backref='user', cascade="all, delete-orphan")
+        })
+        sess = create_session()
+        u = User()
+        u.user_id = 7
+        u.user_name = "fred"
+        a1 = Address()
+        a1.email_address='foo@bar.com'
+        u.addresses.append(a1)
+        sess.save(u)
+        sess.flush()
+
+        assert u.addresses[0].user is u
+
+        sess2 = create_session()
+        u2 = sess2.merge(u, dont_load=True)
+        assert not sess2.dirty
+        a2 = u2.addresses[0]
+        a2.email_address='somenewaddress'
+        assert not object_mapper(a2)._is_orphan(a2)
+        sess2.flush()
+        sess2.clear()
+        assert sess2.query(User).get(u2.user_id).addresses[0].email_address == 'somenewaddress'
         
+        # this use case is not supported; this is with a pending Address on the pre-merged
+        # object, and we currently dont support 'dirty' objects being merged with dont_load=True.  
+        # in this case, the empty '_state.parents' collection would be an issue,
+        # since the optimistic flag is False in _is_orphan() for pending instances.
+        # so if we start supporting 'dirty' with dont_load=True, this test will need to pass
+        sess = create_session()
+        u = sess.query(User).get(7)
+        u.addresses.append(Address())
+        sess2 = create_session()
+        try:
+            u2 = sess2.merge(u, dont_load=True)
+            assert False
 
+            # if dont_load is changed to support dirty objects, this code needs to pass
+            a2 = u2.addresses[0]
+            a2.email_address='somenewaddress'
+            assert not object_mapper(a2)._is_orphan(a2)
+            sess2.flush()
+            sess2.clear()
+            assert sess2.query(User).get(u2.user_id).addresses[0].email_address == 'somenewaddress'
+        except exceptions.InvalidRequestError, e:
+            assert "dont_load=True option does not support" in str(e)
+         
+        
 if __name__ == "__main__":    
     testbase.main()