- Fixed incorrect exception raise in

Weak/StrongIdentityMap.add()
[ticket:1506]

diff --git a/CHANGES b/CHANGES
index 4018496c53d7048ffc0d2f90549a7ac8a14004b2..dfefd8c12bca11c5673eaac5c30944ea0e7c9596 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -433,6 +433,10 @@ CHANGES
     - Fixed recursion issue which occured if a mapped object's
       `__len__()` or `__nonzero__()` method resulted in state
       changes.  [ticket:1501]
+
+    - Fixed incorrect exception raise in 
+      Weak/StrongIdentityMap.add() 
+      [ticket:1506]
       
     - Fixed a somewhat hypothetical issue which would result
       in the wrong primary key being calculated for a mapper

diff --git a/lib/sqlalchemy/orm/identity.py b/lib/sqlalchemy/orm/identity.py
index 889b65ba7fdb3e65eabd8d2b5a95689e0794edda..d0d0a7962eb95683f1ae42d6bb347e1efd54e4ae 100644 (file)
--- a/lib/sqlalchemy/orm/identity.py
+++ b/lib/sqlalchemy/orm/identity.py
@@ -121,7 +121,7 @@ class WeakInstanceDict(IdentityMap):
     def add(self, state):
         if state.key in self:
             if dict.__getitem__(self, state.key) is not state:
-                raise AssertionError("A conflicting state is already present in the identity map for key %r" % state.key)
+                raise AssertionError("A conflicting state is already present in the identity map for key %r" % (state.key, ))
         else:
             dict.__setitem__(self, state.key, state)
             self._manage_incoming_state(state)
@@ -208,11 +208,15 @@ class StrongInstanceDict(IdentityMap):
 
         dict.__setitem__(self, state.key, state.obj())
         self._manage_incoming_state(state)
-        
+
     def add(self, state):
-        dict.__setitem__(self, state.key, state.obj())
-        self._manage_incoming_state(state)
-    
+        if state.key in self:
+            if dict.__getitem__(self, state.key) is not state:
+                raise AssertionError("A conflicting state is already present in the identity map for key %r" % (state.key, ))
+        else:
+            dict.__setitem__(self, state.key, state.obj())
+            self._manage_incoming_state(state)
+        
     def remove(self, state):
         if attributes.instance_state(dict.pop(self, state.key)) is not state:
             raise AssertionError("State %s is not present in this identity map" % state)

diff --git a/lib/sqlalchemy/test/testing.py b/lib/sqlalchemy/test/testing.py
index 3689ca0b7fbb30deff62a6f9d9d69aed7646e075..9c5c87f3f2d4b5abb58427c7ae682170fcd3f6a6 100644 (file)
--- a/lib/sqlalchemy/test/testing.py
+++ b/lib/sqlalchemy/test/testing.py
@@ -497,9 +497,12 @@ def startswith_(a, fragment, msg=None):
 def assert_raises(except_cls, callable_, *args, **kw):
     try:
         callable_(*args, **kw)
-        assert False, "Callable did not raise an exception"
+        success = False
     except except_cls, e:
-        pass
+        success = True
+    
+    # assert outside the block so it works for AssertionError too !
+    assert success, "Callable did not raise an exception"
 
 def assert_raises_message(except_cls, msg, callable_, *args, **kwargs):
     try:

diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index 8562366c59da8bad790157a80b05a3cf47a03229..6ccacdc98fa99900cd20b6acf9fdea25239d25d9 100644 (file)
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -3,6 +3,7 @@ from sqlalchemy.test.util import gc_collect
 import inspect
 import pickle
 from sqlalchemy.orm import create_session, sessionmaker, attributes, make_transient
+from sqlalchemy.orm.attributes import instance_state
 import sqlalchemy as sa
 from sqlalchemy.test import engines, testing, config
 from sqlalchemy import Integer, String, Sequence
@@ -837,7 +838,26 @@ class SessionTest(_fixtures.FixtureTest):
         assert not s.dirty
         
         assert not s.identity_map
-    
+
+    @testing.resolve_artifact_names
+    def test_identity_conflict(self):
+        mapper(User, users)
+        for s in (
+            create_session(),
+            create_session(weak_identity_map=False),
+        ):
+            users.delete().execute()
+            u1 = User(name="ed")
+            s.add(u1)
+            s.flush()
+            s.expunge(u1)
+            u2 = s.query(User).first()
+            s.expunge(u2)
+            s.identity_map.add(sa.orm.attributes.instance_state(u1))
+
+            assert_raises(AssertionError, s.identity_map.add, sa.orm.attributes.instance_state(u2))
+        
+        
     @testing.resolve_artifact_names
     def test_weakref_with_cycles_o2m(self):
         s = sessionmaker()()