Commits

Mike Bayer committed b923acc

- Fixed regression introduced in 0.6.0 unit of work refactor
that broke updates for bi-directional relationship()
with post_update=True. [ticket:1807]

Comments (0)

Files changed (3)

   - Fixed regression introduced in 0.6.0 involving improper
     history accounting on mutable attributes.  [ticket:1782]
   
+  - Fixed regression introduced in 0.6.0 unit of work refactor
+    that broke updates for bi-directional relationship()
+    with post_update=True. [ticket:1807]
+    
   - session.merge() will not expire attributes on the returned
     instance if that instance is "pending".  [ticket:1789]
 

lib/sqlalchemy/orm/dependency.py

         the aggreagte.
         
         """
-        if self.post_update and self._check_reverse(uow):
-            return
-        
         uow.register_preprocessor(self, True)
         
         
     def per_property_flush_actions(self, uow):
-        if self.post_update and self._check_reverse(uow):
-            return
-
         after_save = unitofwork.ProcessAll(uow, self, False, True)
         before_delete = unitofwork.ProcessAll(uow, self, True, True)
 
                                             clearkeys, uowcommit):
         raise NotImplementedError()
 
-    def _check_reverse(self, uow):
-        """return True if a comparable dependency processor has
-        already set up on the "reverse" side of a relationship.
-        
-        """
-        for p in self.prop._reverse_property:
-            if not p.viewonly and p._dependency_processor and \
-                uow.has_dep(p._dependency_processor):
-                return True
-        else:
-            return False
+    def _get_reversed_processed_set(self, uow):
+        if not self.prop._reverse_property:
+            return None
 
-    def _post_update(self, state, uowcommit, related):
+        process_key = tuple(sorted(
+                        [self.key] + 
+                        [p.key for p in self.prop._reverse_property]
+                    ))
+        return uow.memo(
+                            ('reverse_key', process_key), 
+                            set
+                        )
+
+    def _post_update(self, state, uowcommit, related, processed):
+        if processed is not None and state in processed:
+            return
         for x in related:
             if x is not None:
                 uowcommit.issue_post_update(
                         state, 
                         [r for l, r in self.prop.synchronize_pairs]
                 )
+                if processed is not None:
+                    processed.add(state)
+                
                 break
-
+        
     def _pks_changed(self, uowcommit, state):
         raise NotImplementedError()
 
         # key to the parent set to NULL this phase can be called 
         # safely for any cascade but is unnecessary if delete cascade
         # is on.
+        if self.post_update:
+            processed = self._get_reversed_processed_set(uowcommit)
+        
         if self.post_update or not self.passive_deletes == 'all':
             children_added = uowcommit.memo(('children_added', self), set)
 
                                     self._post_update(
                                             child, 
                                             uowcommit, 
-                                            [state])
+                                            [state], processed)
                     if self.post_update or not self.cascade.delete:
                         for child in set(history.unchanged).\
                                             difference(children_added):
                                     self._post_update(
                                             child, 
                                             uowcommit, 
-                                            [state])
+                                            [state], processed)
                         # technically, we can even remove each child from the
                         # collection here too.  but this would be a somewhat 
                         # inconsistent behavior since it wouldn't happen if the old
                         # parent wasn't deleted but child was moved.
                             
     def process_saves(self, uowcommit, states):
+        if self.post_update:
+            processed = self._get_reversed_processed_set(uowcommit)
+            
         for state in states:
             history = uowcommit.get_attribute_history(state, self.key, passive=True)
             if history:
                         self._post_update(
                                             child, 
                                             uowcommit, 
-                                            [state])
+                                            [state],
+                                            processed
+                                            )
 
                 for child in history.deleted:
                     if not self.cascade.delete_orphan and \
         if self.post_update and \
                 not self.cascade.delete_orphan and \
                 not self.passive_deletes == 'all':
+            
+            processed = self._get_reversed_processed_set(uowcommit)
+                
             # post_update means we have to update our 
             # row to not reference the child object
             # before we can DELETE the row
                         self._post_update(
                                             state, 
                                             uowcommit, 
-                                            history.sum())
+                                            history.sum(), processed)
 
     def process_saves(self, uowcommit, states):
+        if self.post_update:
+            processed = self._get_reversed_processed_set(uowcommit)
+            
         for state in states:
             history = uowcommit.get_attribute_history(state, self.key, passive=True)
             if history:
                 if self.post_update:
                     self._post_update(
                                         state, 
-                                        uowcommit, history.sum())
+                                        uowcommit, history.sum(), processed)
 
     def _synchronize(self, state, child, associationrow, clearkeys, uowcommit):
         if state is None or (not self.post_update and uowcommit.is_deleted(state)):
                             uowcommit.register_object(
                                 attributes.instance_state(c), isdelete=True)
     
-    def _get_reversed_processed_set(self, uow):
-        if not self.prop._reverse_property:
-            return None
-
-        process_key = tuple(sorted(
-                        [self.key] + 
-                        [p.key for p in self.prop._reverse_property]
-                    ))
-        return uow.memo(
-                            ('reverse_key', process_key), 
-                            set
-                        )
-
     def process_deletes(self, uowcommit, states):
         secondary_delete = []
         secondary_insert = []

test/orm/test_cycles.py

 from sqlalchemy.test import testing
 from sqlalchemy import Integer, String, ForeignKey
 from sqlalchemy.test.schema import Table, Column
-from sqlalchemy.orm import mapper, relationship, backref, create_session
+from sqlalchemy.orm import mapper, relationship, backref, \
+                            create_session, sessionmaker
 from sqlalchemy.test.testing import eq_
 from sqlalchemy.test.assertsql import RegexSQL, ExactSQL, CompiledSQL, AllOf
 from test.orm import _base
 
     @classmethod
     def setup_classes(cls):
-        class Person(_base.BasicEntity):
+        class Person(_base.ComparableEntity):
             pass
 
-        class Ball(_base.BasicEntity):
+        class Ball(_base.ComparableEntity):
             pass
 
     @testing.resolve_artifact_names
         )
 
     @testing.resolve_artifact_names
+    def test_post_update_backref(self):
+        """test bidirectional post_update."""
+        
+        mapper(Ball, ball)
+        mapper(Person, person, properties=dict(
+            balls=relationship(Ball,
+                           primaryjoin=ball.c.person_id == person.c.id,
+                           remote_side=ball.c.person_id, post_update=True,
+                           backref=backref('person', post_update=True)
+                           ),
+           favorite=relationship(Ball,
+                             primaryjoin=person.c.favorite_ball_id == ball.c.id,
+                             remote_side=person.c.favorite_ball_id)
+            
+            ))
+        
+        sess = sessionmaker()()
+        p1 = Person(data='p1')
+        p2 = Person(data='p2')
+        p3 = Person(data='p3')
+        
+        b1 = Ball(data='b1')
+        
+        b1.person = p1
+        sess.add_all([p1, p2, p3])
+        sess.commit()
+        
+        # switch here.  the post_update
+        # on ball.person can't get tripped up
+        # by the fact that there's a "reverse" prop.
+        b1.person = p2
+        sess.commit()
+        eq_(
+            p2, b1.person
+        )
+
+        # do it the other way 
+        p3.balls.append(b1)
+        sess.commit()
+        eq_(
+            p3, b1.person
+        )
+        
+
+
+    @testing.resolve_artifact_names
     def test_post_update_o2m(self):
         """A cycle between two rows, with a post_update on the one-to-many"""
 
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.