From 45e6875752fcaf7d3a60907959ed9d154cca0d5d Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 28 Jan 2012 14:33:29 -0500 Subject: [PATCH] - [bug] Fixed bug where "merge" cascade could mis-interpret an unloaded attribute, if the load_on_pending flag were used with relationship(). Thanks to Kent Bower for tests. [ticket:2374] --- CHANGES | 6 ++++ lib/sqlalchemy/orm/unitofwork.py | 5 ++- test/orm/test_merge.py | 52 ++++++++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 85a705ea73..9c86e50a81 100644 --- a/CHANGES +++ b/CHANGES @@ -73,6 +73,12 @@ CHANGES needed ORM attribute access within __eq__() or similar. [ticket:2362] + - [bug] Fixed bug where "merge" cascade could + mis-interpret an unloaded attribute, if the + load_on_pending flag were used with + relationship(). Thanks to Kent Bower + for tests. [ticket:2374] + - sql - [feature] Added "false()" and "true()" expression constructs to sqlalchemy.sql namespace, though diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 757deffef0..3cd0f15eb1 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -66,7 +66,10 @@ def track_cascade_events(descriptor, prop): not sess._contains_state(newvalue_state): sess._save_or_update_state(newvalue_state) - if oldvalue is not None and prop.cascade.delete_orphan: + if oldvalue is not None and \ + oldvalue is not attributes.PASSIVE_NO_RESULT and \ + prop.cascade.delete_orphan: + # possible to reach here with attributes.NEVER_SET ? oldvalue_state = attributes.instance_state(oldvalue) if oldvalue_state in sess._new and \ diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py index 511ab19eae..f7d9a4d313 100644 --- a/test/orm/test_merge.py +++ b/test/orm/test_merge.py @@ -1254,8 +1254,6 @@ class MutableMergeTest(fixtures.MappedTest): d3 = sess.merge(d2) eq_(d3.data, ["this", "is", "another", "list"]) - - class CompositeNullPksTest(fixtures.MappedTest): @classmethod def define_tables(cls, metadata): @@ -1293,4 +1291,54 @@ class CompositeNullPksTest(fixtures.MappedTest): return sess.merge(d1) self.assert_sql_count(testing.db, go, 0) +class LoadOnPendingTest(fixtures.MappedTest): + """Test interaction of merge() with load_on_pending relationships""" + @classmethod + def define_tables(cls, metadata): + rocks_table = Table("rocks", metadata, + Column("id", Integer, primary_key=True), + Column("description", String(10)), + ) + bugs_table = Table("bugs", metadata, + Column("id", Integer, primary_key=True), + Column("rockid", Integer, ForeignKey('rocks.id')), + ) + + @classmethod + def setup_classes(cls): + class Rock(cls.Basic, fixtures.ComparableEntity): + pass + class Bug(cls.Basic, fixtures.ComparableEntity): + pass + def _setup_delete_orphan_o2o(self): + mapper(self.classes.Rock, self.tables.rocks, + properties={'bug': relationship(self.classes.Bug, + cascade='all,delete-orphan', + load_on_pending=True, + uselist=False) + }) + mapper(self.classes.Bug, self.tables.bugs) + self.sess = sessionmaker()() + + def _merge_delete_orphan_o2o_with(self, bug): + # create a transient rock with passed bug + r = self.classes.Rock(id=0, description='moldy') + r.bug = bug + m = self.sess.merge(r) + # we've already passed ticket #2374 problem since merge() returned, + # but for good measure: + assert m is not r + eq_(m,r) + + def test_merge_delete_orphan_o2o_none(self): + """one to one delete_orphan relationships marked load_on_pending should + be able to merge() with attribute None""" + self._setup_delete_orphan_o2o() + self._merge_delete_orphan_o2o_with(None) + + def test_merge_delete_orphan_o2o(self): + """one to one delete_orphan relationships marked load_on_pending should + be able to merge()""" + self._setup_delete_orphan_o2o() + self._merge_delete_orphan_o2o_with(self.classes.Bug(id=1)) -- 2.47.2