- fixed reentrant mapper compile hang when

a declared attribute is used within ForeignKey,
ie. ForeignKey(MyOtherClass.someattribute)

diff --git a/CHANGES b/CHANGES
index 4efb2a97506e1270ea170f3f16e3c17b132427c2..c3fa9f1fc866240995a03c5be59b53b15d440d1e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -56,6 +56,10 @@ CHANGES
       table, so that other foreign keys to not-yet-declared 
       Table objects don't trigger an error.
       
+    - fixed reentrant mapper compile hang when 
+      a declared attribute is used within ForeignKey, 
+      ie. ForeignKey(MyOtherClass.someattribute)
+      
 - sql
     - Added COLLATE support via the .collate(<collation>)
       expression operator and collate(<expr>, <collation>) sql

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 15044ba34a2cb54698290c98a063e5a745dc4926..ba0644758f9aae6189137dba57599ae5df4b1a92 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -24,7 +24,8 @@ from sqlalchemy.orm.util import has_identity, _state_has_identity, _is_mapped_cl
 __all__ = ['Mapper', 'class_mapper', 'object_mapper', '_mapper_registry']
 
 _mapper_registry = weakref.WeakKeyDictionary()
-__new_mappers = False
+_new_mappers = False
+_already_compiling = False
 
 # a list of MapperExtensions that will be installed in all mappers by default
 global_extensions = []
@@ -35,7 +36,7 @@ global_extensions = []
 NO_ATTRIBUTE = util.symbol('NO_ATTRIBUTE')
 
 # lock used to synchronize the "mapper compile" step
-_COMPILE_MUTEX = util.threading.Lock()
+_COMPILE_MUTEX = util.threading.RLock()
 
 # initialize these lazily
 ColumnProperty = None
@@ -179,8 +180,8 @@ class Mapper(object):
         self.__compile_extensions()
         self.__compile_properties()
         self.__compile_pks()
-        global __new_mappers
-        __new_mappers = True
+        global _new_mappers
+        _new_mappers = True
         self.__log("constructed")
 
     def __log(self, msg):
@@ -327,14 +328,20 @@ class Mapper(object):
         repeatedly.
         """
         
-        global __new_mappers
-        if self.__props_init and not __new_mappers:
+        global _new_mappers
+        if self.__props_init and not _new_mappers:
             return self
+            
         _COMPILE_MUTEX.acquire()
+        global _already_compiling
+        if _already_compiling:
+            self.__initialize_properties()
+            return
+        _already_compiling = True
         try:
 
             # double-check inside mutex
-            if self.__props_init and not __new_mappers:
+            if self.__props_init and not _new_mappers:
                 return self
 
             # initialize properties on all mappers
@@ -342,9 +349,10 @@ class Mapper(object):
                 if not mapper.__props_init:
                     mapper.__initialize_properties()
 
-            __new_mappers = False
+            _new_mappers = False
             return self
         finally:
+            _already_compiling = False
             _COMPILE_MUTEX.release()
 
     def __initialize_properties(self):

diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index 66d1a17366df6a0b5982270e96596acfee702921..22df6b41a5f479d30ee3a3ed855cb4e63a456e1a 100644 (file)
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -802,6 +802,9 @@ class ForeignKey(SchemaItem):
                         "Could not create ForeignKey '%s' on table '%s': "
                         "table '%s' has no column named '%s'" % (
                         self._colspec, parenttable.name, table.name, str(e)))
+            
+            elif isinstance(self._colspec, expression.Operators):
+                self._column = self._colspec.clause_element()
             else:
                 self._column = self._colspec
 

diff --git a/test/ext/declarative.py b/test/ext/declarative.py
index a161d416f3e9dc9498909da8db0a8e25ca31e77e..ab07627ddaccbdcc24c8232772af551568307a84 100644 (file)
--- a/test/ext/declarative.py
+++ b/test/ext/declarative.py
@@ -77,10 +77,10 @@ class DeclarativeTest(TestBase, AssertsExecutionResults):
             user_id = Column('user_id', Integer, ForeignKey('users.id'))
             user = relation("User", primaryjoin=user_id==User.id, backref="addresses")
 
-        assert mapperlib._Mapper__new_mappers is True
+        assert mapperlib._new_mappers is True
         u = User()
         assert User.addresses
-        assert mapperlib._Mapper__new_mappers is False
+        assert mapperlib._new_mappers is False
 
     def test_nice_dependency_error(self):
         class User(Base):
@@ -449,8 +449,6 @@ class DeclarativeTest(TestBase, AssertsExecutionResults):
         self.assertEquals(sess.query(Company).filter(Company.employees.of_type(Engineer).any(Engineer.primary_language=='cobol')).first(), c2)
     
     def test_inheritance_with_undefined_relation(self):
-        Base = declarative_base()
-
         class Parent(Base):
            __tablename__ = 'parent'
            id = Column('id', Integer, primary_key=True)
@@ -473,6 +471,39 @@ class DeclarativeTest(TestBase, AssertsExecutionResults):
            __mapper_args__ = dict(polymorphic_identity = 'child2')
            
         compile_mappers()  # no exceptions here
+    
+    def test_reentrant_compile_via_foreignkey(self):
+        
+        class User(Base, Fixture):
+            __tablename__ = 'users'
+
+            id = Column('id', Integer, primary_key=True)
+            name = Column('name', String(50))
+            addresses = relation("Address", backref="user")
+
+        class Address(Base, Fixture):
+            __tablename__ = 'addresses'
+
+            id = Column('id', Integer, primary_key=True)
+            email = Column('email', String(50))
+            user_id = Column('user_id', Integer, ForeignKey(User.id))
+        
+        compile_mappers() # this forces a re-entrant compile() due to the User.id within the ForeignKey
+
+        Base.metadata.create_all()
+        u1 = User(name='u1', addresses=[
+            Address(email='one'),
+            Address(email='two'),
+        ])
+        sess = create_session()
+        sess.save(u1)
+        sess.flush()
+        sess.clear()
+
+        self.assertEquals(sess.query(User).all(), [User(name='u1', addresses=[
+            Address(email='one'),
+            Address(email='two'),
+        ])])
         
     def test_relation_reference(self):
         class Address(Base, Fixture):

diff --git a/test/orm/mapper.py b/test/orm/mapper.py
index 8d69085ae01dbaae56ee41298bd25ffe7e808b7f..7dce096145ca4e1a4ff0542abbf2fb966935bf48 100644 (file)
--- a/test/orm/mapper.py
+++ b/test/orm/mapper.py
@@ -59,23 +59,23 @@ class MapperTest(MapperSuperTest):
         self.assertRaises(exceptions.ArgumentError, mapper, User, s)
     
     def test_recompile_on_othermapper(self):
-        """test the global '__new_mappers' flag such that a compile 
+        """test the global '_new_mappers' flag such that a compile 
         trigger on an already-compiled mapper still triggers a check against all mappers."""
 
         from sqlalchemy.orm import mapperlib
         
         mapper(User, users)
         compile_mappers()
-        assert mapperlib._Mapper__new_mappers is False
+        assert mapperlib._new_mappers is False
         
         m = mapper(Address, addresses, properties={'user':relation(User, backref="addresses")})
         
         assert m._Mapper__props_init is False
-        assert mapperlib._Mapper__new_mappers is True
+        assert mapperlib._new_mappers is True
         u = User()
         assert User.addresses
-        assert mapperlib._Mapper__new_mappers is False
-        
+        assert mapperlib._new_mappers is False
+    
     def test_compileonsession(self):
         m = mapper(User, users)
         session = create_session()