From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 10 Mar 2015 21:51:35 +0000 (-0400)
Subject: - Fixed bug where the session attachment error "object is already
X-Git-Tag: rel_1_0_0b1~16
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=66fa5b50a53ebe234f19e23b7dfa6ff310969996;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- Fixed bug where the session attachment error "object is already
attached to session X" would fail to prevent the object from
also being attached to the new session, in the case that execution
continued after the error raise occurred.
fixes #3301
---

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index b54a43aaea..474cec0939 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -23,6 +23,15 @@
     series as well.  For changes that are specific to 1.0 with an emphasis
     on compatibility concerns, see :doc:`/changelog/migration_10`.
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3301
+
+        Fixed bug where the session attachment error "object is already
+        attached to session X" would fail to prevent the object from
+        also being attached to the new session, in the case that execution
+        continued after the error raise occurred.
+
     .. change::
         :tags: bug, ext
         :tickets: 3219, 3240
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index f02d8b54ef..bc9444040e 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1830,7 +1830,7 @@ class Session(_SessionClassMethods):
                 "function to send this object back to the transient state." %
                 state_str(state)
             )
-        self._before_attach(state)
+        self._before_attach(state, check_identity_map=False)
         self._deleted.pop(state, None)
         if discard_existing:
             self.identity_map.replace(state)
@@ -1910,13 +1910,12 @@ class Session(_SessionClassMethods):
         self._attach(state, include_before=True)
         state._load_pending = True
 
-    def _before_attach(self, state):
+    def _before_attach(self, state, check_identity_map=True):
         if state.session_id != self.hash_key and \
                 self.dispatch.before_attach:
             self.dispatch.before_attach(self, state.obj())
 
-    def _attach(self, state, include_before=False):
-        if state.key and \
+        if check_identity_map and state.key and \
             state.key in self.identity_map and \
                 not self.identity_map.contains_state(state):
             raise sa_exc.InvalidRequestError(
@@ -1932,10 +1931,11 @@ class Session(_SessionClassMethods):
                 "(this is '%s')" % (state_str(state),
                                     state.session_id, self.hash_key))
 
+    def _attach(self, state, include_before=False):
+
         if state.session_id != self.hash_key:
-            if include_before and \
-                    self.dispatch.before_attach:
-                self.dispatch.before_attach(self, state.obj())
+            if include_before:
+                self._before_attach(state)
             state.session_id = self.hash_key
             if state.modified and state._strong_obj is None:
                 state._strong_obj = state.obj()
diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index 2aa0cd3eb9..58551d763e 100644
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -505,6 +505,25 @@ class SessionStateTest(_fixtures.FixtureTest):
         assert user not in s
         assert s.query(User).count() == 0
 
+    def test_already_attached(self):
+        User = self.classes.User
+        users = self.tables.users
+        mapper(User, users)
+
+        s1 = Session()
+        s2 = Session()
+
+        u1 = User(id=1, name='u1')
+        make_transient_to_detached(u1)  # shorthand for actually persisting it
+        s1.add(u1)
+
+        assert_raises_message(
+            sa.exc.InvalidRequestError,
+            "Object '<User.*?>' is already attached to session",
+            s2.add, u1
+        )
+        assert u1 not in s2
+        assert not s2.identity_map.keys()
 
     @testing.uses_deprecated()
     def test_identity_conflict(self):
@@ -562,7 +581,7 @@ class SessionStateTest(_fixtures.FixtureTest):
         assert u2 is not None and u2 is not u1
         assert u2 in sess
 
-        assert_raises(Exception, lambda: sess.add(u1))
+        assert_raises(AssertionError, lambda: sess.add(u1))
 
         sess.expunge(u2)
         assert u2 not in sess