From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 31 Aug 2009 21:53:49 +0000 (+0000)
Subject: - Fixed incorrect exception raise in
X-Git-Tag: rel_0_6beta1~309
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2cb92cbbd0dda5646c98ddb29592064221740753;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- Fixed incorrect exception raise in
Weak/StrongIdentityMap.add()
[ticket:1506]
---

diff --git a/CHANGES b/CHANGES
index 4018496c53..dfefd8c12b 100644
--- 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 889b65ba7f..d0d0a7962e 100644
--- 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 3689ca0b7f..9c5c87f3f2 100644
--- 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 8562366c59..6ccacdc98f 100644
--- 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()()