[Sqlalchemy-tickets] Issue #3301: session._update_impl adds the item to identity map even if error thrown (zzzeek/sqlalchemy)

[Mike B. <iss...@bi...>]

New issue 3301: session._update_impl adds the item to identity map even if error thrown
https://bitbucket.org/zzzeek/sqlalchemy/issue/3301/session_update_impl-adds-the-item-to

Mike Bayer:


```
#!python

from sqlalchemy import Column, String, Integer, ForeignKey
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import Session, session

Base = declarative_base()

class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String)

s1 = Session()
s2 = Session()
u1 = User(id=1, name='u1')
session.make_transient_to_detached(u1)  # shorthand for actually persisting it
s1.add(u1)

try:
    s2.add(u1)
except:
    assert not s2.identity_map.keys()
```

patch:


```
#!diff

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 0e272dc..2b26831 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1795,7 +1795,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)
@@ -1875,13 +1875,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(
@@ -1897,10 +1896,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()

```


Responsible: zzzeek