From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 1 Sep 2009 22:56:50 +0000 (+0000)
Subject: - merged r6316 from trunk for [ticket:1519]
X-Git-Tag: rel_0_5_6~6
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=7fed56b36c3597e3e17e4d2cf8668de20c781387;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- merged r6316 from trunk for [ticket:1519]
---

diff --git a/CHANGES b/CHANGES
index d7478f9e86..fd04e91d21 100644
--- a/CHANGES
+++ b/CHANGES
@@ -14,6 +14,10 @@ CHANGES
       bidirectional reference to declare itself as "viewonly"
       [ticket:1507]
 
+    - Fixed bug which prevented two entities from mutually
+      replacing each other's primary key values within a single
+      flush() for some orderings of operations.  [ticket:1519]
+      
     - Fixed an obscure issue whereby a joined-table subclass
       with a self-referential eager load on the base class
       would populate the related object's "subclass" table with
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index c010a217bd..914da874d5 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1017,8 +1017,10 @@ class Session(object):
             if state.key is None:
                 state.key = instance_key
             elif state.key != instance_key:
-                # primary key switch
-                self.identity_map.remove(state)
+                # primary key switch.
+                # use discard() in case another state has already replaced this
+                # one in the identity map (see test/orm/test_naturalpks.py ReversePKsTest)
+                self.identity_map.discard(state)
                 state.key = instance_key
             
             self.identity_map.replace(state)
diff --git a/test/orm/test_naturalpks.py b/test/orm/test_naturalpks.py
index 1376c402e7..f5d6d97434 100644
--- a/test/orm/test_naturalpks.py
+++ b/test/orm/test_naturalpks.py
@@ -355,7 +355,63 @@ class NaturalPKTest(_base.MappedTest):
         r = sess.query(Item).with_parent(u2).all()
         eq_(Item(itemname='item2'), r[0])
 
+class ReversePKsTest(_base.MappedTest):
+    """reverse the primary keys of two entities and ensure bookkeeping succeeds."""
+    
+    
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'user', metadata,
+            Column('code', Integer, primary_key=True),
+            Column('status', Integer, primary_key=True),
+            Column('username', String, nullable=False),
+            )
+    
+    @classmethod
+    def setup_classes(cls):
+        class User(_base.ComparableEntity):
+            def __init__(self, code, status, username):
+                self.code = code
+                self.status = status
+                self.username = username
+
+    @testing.resolve_artifact_names
+    def test_reverse(self):
+        PUBLISHED, EDITABLE, ARCHIVED = 1, 2, 3
+        
+        mapper(User, user)
+
+        session = sa.orm.sessionmaker()()
+        
+        a_published = User(0, PUBLISHED, u'a')
+        session.add(a_published)
+        session.commit()
+
+        a_editable = User(0, EDITABLE, u'a')
+
+        session.add(a_editable)
+        session.commit()
+
+        # do the switch in both directions - 
+        # one or the other should raise the error
+        # based on platform dictionary ordering
+        a_published.status = ARCHIVED
+        a_editable.status = PUBLISHED
+
+        session.commit()
+        assert session.query(User).get([0, PUBLISHED]) is a_editable
+        assert session.query(User).get([0, ARCHIVED]) is a_published
+
+        a_published.status = PUBLISHED
+        a_editable.status = EDITABLE
+
+        session.commit()
+
+        assert session.query(User).get([0, PUBLISHED]) is a_published
+        assert session.query(User).get([0, EDITABLE]) is a_editable
 
+    
 class SelfRefTest(_base.MappedTest):
     __unsupported_on__ = 'mssql' # mssql doesn't allow ON UPDATE on self-referential keys