From d559f9fc94a9635984470634a7b6714e168361a0 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 8 Jul 2010 10:16:13 -0400 Subject: [PATCH] - 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] --- CHANGES | 8 +++++- lib/sqlalchemy/orm/attributes.py | 9 +++++- lib/sqlalchemy/orm/mapper.py | 8 ++++-- lib/sqlalchemy/orm/state.py | 2 +- lib/sqlalchemy/orm/util.py | 9 ++---- test/orm/test_mapper.py | 48 ++++++++++++++++++++++++-------- 6 files changed, 61 insertions(+), 23 deletions(-) diff --git a/CHANGES b/CHANGES index 6f188b10e0..fb9603e524 100644 --- 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 4390538a02..e987402871 100644 --- 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 b122b49510..89de7b91bb 100644 --- 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 9ee31bff80..82e7e91301 100644 --- 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 651b0256b1..a2e3c54331 100644 --- 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 e609e5ba0f..2de77fd2c5 100644 --- 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: -- 2.47.2