- Improved the check for an "unmapped class",

including the case where the superclass is mapped
but the subclass is not.  Any attempts to access
cls._sa_class_manager.mapper now raise
UnmappedClassError().  [ticket:1142]

diff --git a/CHANGES b/CHANGES
index 6f188b10e00d8674b8ea828963209247bf0a4e1b..fb9603e524b91f41744e7b42fe99887e873118e0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -13,7 +13,13 @@ CHANGES
     key has changed, or if passive_deletes is False 
     and a delete of the parent has occurred.
     [ticket:1845]
-  
+
+  - Improved the check for an "unmapped class", 
+    including the case where the superclass is mapped
+    but the subclass is not.  Any attempts to access
+    cls._sa_class_manager.mapper now raise 
+    UnmappedClassError().  [ticket:1142]
+    
 0.6.2
 =====
 - orm

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 4390538a0224dd732a78a659bbe851d9e440276e..e9874028717efe93c27a518519e5e2554dd2751d 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -918,7 +918,6 @@ class ClassManager(dict):
         self.class_ = class_
         self.factory = None  # where we came from, for inheritance bookkeeping
         self.info = {}
-        self.mapper = None
         self.new_init = None
         self.mutable_attributes = set()
         self.local_attrs = {}
@@ -933,6 +932,14 @@ class ClassManager(dict):
         self.manage()
         self._instrument_init()
     
+    @property
+    def is_mapped(self):
+        return 'mapper' in self.__dict__
+        
+    @util.memoized_property
+    def mapper(self):
+        raise exc.UnmappedClassError(self.class_)
+        
     def _configure_create_arguments(self, 
                             _source=None, 
                             deferred_scalar_loader=None):

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index b122b495101c033a1e6d3919dd4cf628beef49ec..89de7b91bb9bfac6059cc567a7affe6bdb52bbe8 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -361,7 +361,7 @@ class Mapper(object):
         manager = attributes.manager_of_class(self.class_)
         
         if self.non_primary:
-            if not manager or manager.mapper is None:
+            if not manager or not manager.is_mapped:
                 raise sa_exc.InvalidRequestError(
                     "Class %s has no primary mapper configured.  Configure "
                     "a primary mapper first before setting up a non primary "
@@ -372,7 +372,7 @@ class Mapper(object):
 
         if manager is not None:
             assert manager.class_ is self.class_
-            if manager.mapper:
+            if manager.is_mapped:
                 raise sa_exc.ArgumentError(
                     "Class '%s' already has a primary mapper defined. "
                     "Use non_primary=True to "
@@ -429,7 +429,9 @@ class Mapper(object):
         if hasattr(self, '_compile_failed'):
             del self._compile_failed
             
-        if not self.non_primary and self.class_manager.mapper is self:
+        if not self.non_primary and \
+            self.class_manager.is_mapped and \
+            self.class_manager.mapper is self:
             attributes.unregister_class(self.class_)
 
     def _configure_pks(self):

diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index 9ee31bff80a74be954739469eb43c15e7b76755f..82e7e91301d1a92366e0555b422148515514a49f 100644 (file)
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -163,7 +163,7 @@ class InstanceState(object):
                         "Cannot deserialize object of type %r - no mapper() has"
                         " been configured for this class within the current Python process!" %
                         self.class_)
-        elif manager.mapper and not manager.mapper.compiled:
+        elif manager.is_mapped and not manager.mapper.compiled:
             manager.mapper.compile()
             
         self.committed_state = state.get('committed_state', {})

diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 651b0256b1ef781493efd5ec454dbdd38ccff0a4..a2e3c54331bf4bced68ea898e84caa9782522304 100644 (file)
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -570,9 +570,9 @@ def object_mapper(instance):
     """
     try:
         state = attributes.instance_state(instance)
-        if not state.manager.mapper:
-            raise exc.UnmappedInstanceError(instance)
         return state.manager.mapper
+    except exc.UnmappedClassError:
+        raise exc.UnmappedInstanceError(instance)
     except exc.NO_STATE:
         raise exc.UnmappedInstanceError(instance)
 
@@ -582,14 +582,11 @@ def class_mapper(class_, compile=True):
     Raises UnmappedClassError if no mapping is configured.
 
     """
+    
     try:
         class_manager = attributes.manager_of_class(class_)
         mapper = class_manager.mapper
 
-        # HACK until [ticket:1142] is complete
-        if mapper is None:
-            raise AttributeError
-
     except exc.NO_STATE:
         raise exc.UnmappedClassError(class_)
 

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index e609e5ba0f95c08c8226142f589750998e12cda6..2de77fd2c5fe7d1d7eb246e15653b0d11de9860f 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -437,26 +437,39 @@ class MapperTest(_fixtures.FixtureTest):
     def test_illegal_non_primary(self):
         mapper(User, users)
         mapper(Address, addresses)
-        try:
+        assert_raises_message(
+            sa.exc.ArgumentError,
+            "Attempting to assign a new relationship 'addresses' "
+            "to a non-primary mapper on class 'User'",
             mapper(User, users, non_primary=True, properties={
                 'addresses':relationship(Address)
-            }).compile()
-            assert False
-        except sa.exc.ArgumentError, e:
-            assert "Attempting to assign a new relationship 'addresses' to a non-primary mapper on class 'User'" in str(e)
+            }).compile
+        )
 
     @testing.resolve_artifact_names
     def test_illegal_non_primary_2(self):
-        try:
-            mapper(User, users, non_primary=True)
-            assert False
-        except sa.exc.InvalidRequestError, e:
-            assert "Configure a primary mapper first" in str(e)
+        assert_raises_message(
+            sa.exc.InvalidRequestError,
+            "Configure a primary mapper first",
+            mapper, User, users, non_primary=True)
+
+    @testing.resolve_artifact_names
+    def test_illegal_non_primary_3(self):
+        class Base(object):
+            pass
+        class Sub(Base):
+            pass
+        mapper(Base, users)
+        assert_raises_message(sa.exc.InvalidRequestError, 
+                "Configure a primary mapper first",
+                mapper, Sub, addresses, non_primary=True
+            )
 
     @testing.resolve_artifact_names
     def test_prop_filters(self):
         t = Table('person', MetaData(),
-                  Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+                  Column('id', Integer, primary_key=True,
+                                        test_needs_autoincrement=True),
                   Column('type', String(128)),
                   Column('name', String(128)),
                   Column('employee_number', Integer),
@@ -1062,6 +1075,19 @@ class MapperTest(_fixtures.FixtureTest):
 
         assert_raises(sa.orm.exc.UnmappedClassError, sa.orm.compile_mappers)
 
+    @testing.resolve_artifact_names
+    def test_unmapped_subclass_error(self):
+        class Base(object):
+            pass
+        class Sub(Base):
+            pass
+        
+        mapper(Base, users)
+        sa.orm.compile_mappers()
+        assert_raises(sa.orm.exc.UnmappedClassError,
+                        create_session().add, Sub())
+        
+        
     @testing.resolve_artifact_names
     def test_oldstyle_mixin(self):
         class OldStyle: