merge -r6305:6308 of trunk

diff --git a/CHANGES b/CHANGES
index 44b74f36c0edddf2b5d88fc03a8e3b3a72975c5d..9f817651496d942b336262e1d0a7c28a4c58ed1d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -65,6 +65,14 @@ CHANGES
       http://www.sqlalchemy.org/trac/wiki/UsageRecipes/PreFilteredQuery
       for an example.
 
+    - 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
       using the old polymorphic_union function - but this

diff --git a/lib/sqlalchemy/orm/identity.py b/lib/sqlalchemy/orm/identity.py
index 50301a13c3ba85a27fad3b70669b7ba43978d213..e1f1e3125792e8839df4234f5371a837e056be1d 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)
@@ -188,11 +188,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 attributes.instance_state(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/orm/state.py b/lib/sqlalchemy/orm/state.py
index 10a0f43eebaff6e56c65913697e39b6ca898d676..6e39c80a5426871a3e834f715bc6a33023807381 100644 (file)
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -276,7 +276,7 @@ class InstanceState(object):
                 instance_dict._modified.add(self)
 
         self.modified = True
-        if not self._strong_obj:
+        if self._strong_obj is None:
             self._strong_obj = self.obj()
 
     def commit(self, dict_, keys):

diff --git a/lib/sqlalchemy/test/testing.py b/lib/sqlalchemy/test/testing.py
index c7afcf0b0df02cb4de1cd688a292b311b38a4ffc..a4a9451a8f7c24157d803e21812aef041fe4ccba 100644 (file)
--- a/lib/sqlalchemy/test/testing.py
+++ b/lib/sqlalchemy/test/testing.py
@@ -454,9 +454,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_mapper.py b/test/orm/test_mapper.py
index 13913578a55b8c0d2d0de07628bbea0a5f830031..af8ea2aa4d7fbbaa92c745fc953c9cd8eedace27 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -2612,7 +2612,35 @@ class RequirementsTest(_base.MappedTest):
                                   sa.orm.eagerload_all('h3s.h1s')).all()
         eq_(len(h1s), 5)
 
+    @testing.resolve_artifact_names
+    def test_nonzero_len_recursion(self):
+        class H1(object):
+            def __len__(self):
+                return len(self.get_value())
+            
+            def get_value(self):
+                self.value = "foobar"
+                return self.value
+
+        class H2(object):
+            def __nonzero__(self):
+                return bool(self.get_value())
 
+            def get_value(self):
+                self.value = "foobar"
+                return self.value
+                
+        mapper(H1, ht1)
+        mapper(H2, ht1)
+        
+        h1 = H1()
+        h1.value = "Asdf"
+        h1.value = "asdf asdf" # ding
+
+        h2 = H2()
+        h2.value = "Asdf"
+        h2.value = "asdf asdf" # ding
+        
 class MagicNamesTest(_base.MappedTest):
 
     @classmethod

diff --git a/test/orm/test_session.py b/test/orm/test_session.py
index 328cbee8ee991dd685ddaeb9b91d1cc226a281ef..89923081a359ceedf12ff697e39cf2b309a2ba96 100644 (file)
--- a/test/orm/test_session.py
+++ b/test/orm/test_session.py
@@ -3,6 +3,7 @@ import gc
 import inspect
 import pickle
 from sqlalchemy.orm import create_session, sessionmaker, attributes
+from sqlalchemy.orm.attributes import instance_state
 import sqlalchemy as sa
 from sqlalchemy.test import engines, testing, config
 from sqlalchemy import Integer, String, Sequence
@@ -819,7 +820,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()()