From: Mike Bayer Date: Wed, 30 Jan 2019 05:53:10 +0000 (-0600) Subject: Add informative failure modes to _DeferredMapperConfig X-Git-Tag: rel_1_3_0b3~11^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=147f6d382c3b6f06c8efa5a24c07c9849473e7af;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Add informative failure modes to _DeferredMapperConfig Added some helper exceptions that invoke when a mapping based on :class:`.AbstractConcreteBase`, :class:`.DeferredReflection`, or :class:`.AutoMap` is used before the mapping is ready to be used, which contain descriptive information on the class, rather than falling through into other failure modes that are less informative. Fixes: #4470 Change-Id: I9bc51697f63cedaa7809a0adb17b2398c209e289 --- diff --git a/doc/build/changelog/unreleased_13/4470.rst b/doc/build/changelog/unreleased_13/4470.rst new file mode 100644 index 0000000000..f859dca76e --- /dev/null +++ b/doc/build/changelog/unreleased_13/4470.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm, declarative + :tickets: 4470 + + Added some helper exceptions that invoke when a mapping based on + :class:`.AbstractConcreteBase`, :class:`.DeferredReflection`, or + :class:`.AutoMap` is used before the mapping is ready to be used, which + contain descriptive information on the class, rather than falling through + into other failure modes that are less informative. + diff --git a/lib/sqlalchemy/ext/automap.py b/lib/sqlalchemy/ext/automap.py index 55ab325a32..5fa4e88220 100644 --- a/lib/sqlalchemy/ext/automap.py +++ b/lib/sqlalchemy/ext/automap.py @@ -517,6 +517,7 @@ from .declarative import declarative_base as _declarative_base from .declarative.base import _DeferredMapperConfig from .. import util from ..orm import backref +from ..orm import exc as orm_exc from ..orm import interfaces from ..orm import relationship from ..orm.mapper import _CONFIGURE_MUTEX @@ -841,6 +842,16 @@ class AutomapBase(object): """ + @classmethod + def _sa_raise_deferred_config(cls): + raise orm_exc.UnmappedClassError( + cls, + msg="Class %s is a subclass of AutomapBase. " + "Mappings are not produced until the .prepare() " + "method is called on the class hierarchy." + % orm_exc._safe_cls_name(cls), + ) + def automap_base(declarative_base=None, **kw): r"""Produce a declarative automap base. diff --git a/lib/sqlalchemy/ext/declarative/api.py b/lib/sqlalchemy/ext/declarative/api.py index 139481ba59..d6a96b7282 100644 --- a/lib/sqlalchemy/ext/declarative/api.py +++ b/lib/sqlalchemy/ext/declarative/api.py @@ -17,12 +17,15 @@ from .base import _DeferredMapperConfig from .base import _del_attribute from .clsregistry import _class_resolver from ... import exc +from ... import inspection from ... import util from ...orm import attributes from ...orm import comparable_property +from ...orm import exc as orm_exc from ...orm import interfaces from ...orm import properties from ...orm import synonym as _orm_synonym +from ...orm.base import _inspect_mapped_class from ...orm.base import _mapper_or_none from ...orm.util import polymorphic_union from ...schema import MetaData @@ -507,6 +510,32 @@ class AbstractConcreteBase(ConcreteBase): and is only used for selecting. Compare to :class:`.ConcreteBase`, which does create a persisted table for the base class. + .. note:: + + The :class:`.AbstractConcreteBase` class does not intend to set up the + mapping for the base class until all the subclasses have been defined, + as it needs to create a mapping against a selectable that will include + all subclass tables. In order to achieve this, it waits for the + **mapper configuration event** to occur, at which point it scans + through all the configured subclasses and sets up a mapping that will + query against all subclasses at once. + + While this event is normally invoked automatically, in the case of + :class:`.AbstractConcreteBase`, it may be necessary to invoke it + explicitly after **all** subclass mappings are defined, if the first + operation is to be a query against this base class. To do so, invoke + :func:`.configure_mappers` once all the desired classes have been + configured:: + + from sqlalchemy.orm import configure_mappers + + configure_mappers() + + .. seealso:: + + :func:`.orm.configure_mappers` + + Example:: from sqlalchemy.ext.declarative import AbstractConcreteBase @@ -524,6 +553,8 @@ class AbstractConcreteBase(ConcreteBase): 'polymorphic_identity':'manager', 'concrete':True} + configure_mappers() + The abstract base class is handled by declarative in a special way; at class configuration time, it behaves like a declarative mixin or an ``__abstract__`` base class. Once classes are configured @@ -560,6 +591,8 @@ class AbstractConcreteBase(ConcreteBase): 'polymorphic_identity':'manager', 'concrete':True} + configure_mappers() + When we make use of our mappings however, both ``Manager`` and ``Employee`` will have an independently usable ``.company`` attribute:: @@ -635,6 +668,18 @@ class AbstractConcreteBase(ConcreteBase): if sm and sm.concrete and cls in scls.__bases__: sm._set_concrete_base(m) + @classmethod + def _sa_raise_deferred_config(cls): + raise orm_exc.UnmappedClassError( + cls, + msg="Class %s is a subclass of AbstractConcreteBase and " + "has a mapping pending until all subclasses are defined. " + "Call the sqlalchemy.orm.configure_mappers() function after " + "all subclasses have been defined to " + "complete the mapping of this class." + % orm_exc._safe_cls_name(cls), + ) + class DeferredReflection(object): """A helper class for construction of mappings based on @@ -744,6 +789,16 @@ class DeferredReflection(object): if local_table is not None: cls._reflect_table(local_table, engine) + @classmethod + def _sa_raise_deferred_config(cls): + raise orm_exc.UnmappedClassError( + cls, + msg="Class %s is a subclass of DeferredReflection. " + "Mappings are not produced until the .prepare() " + "method is called on the class hierarchy." + % orm_exc._safe_cls_name(cls), + ) + @classmethod def _reflect_table(cls, table, engine): Table( @@ -755,3 +810,17 @@ class DeferredReflection(object): autoload_with=engine, schema=table.schema, ) + + +@inspection._inspects(DeclarativeMeta) +def _inspect_decl_meta(cls): + mp = _inspect_mapped_class(cls) + if mp is None: + if _DeferredMapperConfig.has_cls(cls): + _DeferredMapperConfig.raise_unmapped_for_cls(cls) + raise orm_exc.UnmappedClassError( + cls, + msg="Class %s has a deferred mapping on it. It is not yet " + "usable as a mapped class." % orm_exc._safe_cls_name(cls), + ) + return mp diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py index 80cd23bc8d..62db282d1f 100644 --- a/lib/sqlalchemy/ext/declarative/base.py +++ b/lib/sqlalchemy/ext/declarative/base.py @@ -15,6 +15,7 @@ from ... import event from ... import exc from ... import util from ...orm import class_mapper +from ...orm import exc as orm_exc from ...orm import mapper from ...orm import synonym from ...orm.attributes import QueryableAttribute @@ -716,6 +717,17 @@ class _DeferredMapperConfig(_MapperConfig): # 2.6 fails on weakref if class_ is an old style class return isinstance(class_, type) and weakref.ref(class_) in cls._configs + @classmethod + def raise_unmapped_for_cls(cls, class_): + if hasattr(class_, "_sa_raise_deferred_config"): + class_._sa_raise_deferred_config() + + raise orm_exc.UnmappedClassError( + class_, + msg="Class %s has a deferred mapping on it. It is not yet " + "usable as a mapped class." % orm_exc._safe_cls_name(class_), + ) + @classmethod def config_for_cls(cls, class_): return cls._configs[weakref.ref(class_)] diff --git a/test/ext/declarative/test_basic.py b/test/ext/declarative/test_basic.py index 8b60a11763..3fe2f1bfe9 100644 --- a/test/ext/declarative/test_basic.py +++ b/test/ext/declarative/test_basic.py @@ -15,6 +15,7 @@ from sqlalchemy import util from sqlalchemy.ext import declarative as decl from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.ext.declarative import synonym_for +from sqlalchemy.ext.declarative.base import _DeferredMapperConfig from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import backref from sqlalchemy.orm import class_mapper @@ -25,6 +26,7 @@ from sqlalchemy.orm import composite from sqlalchemy.orm import configure_mappers from sqlalchemy.orm import create_session from sqlalchemy.orm import deferred +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import joinedload from sqlalchemy.orm import mapper from sqlalchemy.orm import properties @@ -116,6 +118,37 @@ class DeclarativeTest(DeclarativeTestBase): eq_(a1, Address(email="two")) eq_(a1.user, User(name="u1")) + def test_deferred_reflection_default_error(self): + class MyExt(object): + @classmethod + def prepare(cls): + "sample prepare method" + to_map = _DeferredMapperConfig.classes_for_base(cls) + for thingy in to_map: + thingy.map() + + @classmethod + def _sa_decl_prepare(cls): + pass + + class User(MyExt, Base): + __tablename__ = "user" + id = Column(Integer, primary_key=True) + + assert_raises_message( + orm_exc.UnmappedClassError, + "Class test.ext.declarative.test_basic.User has a deferred " + "mapping on it. It is not yet usable as a mapped class.", + Session().query, + User, + ) + + User.prepare() + + self.assert_compile( + Session().query(User), 'SELECT "user".id AS user_id FROM "user"' + ) + def test_unicode_string_resolve(self): class User(Base, fixtures.ComparableEntity): __tablename__ = "users" diff --git a/test/ext/declarative/test_inheritance.py b/test/ext/declarative/test_inheritance.py index 17b915da07..083fdb0dbb 100644 --- a/test/ext/declarative/test_inheritance.py +++ b/test/ext/declarative/test_inheritance.py @@ -14,6 +14,7 @@ from sqlalchemy.orm import close_all_sessions from sqlalchemy.orm import configure_mappers from sqlalchemy.orm import create_session from sqlalchemy.orm import deferred +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import mapper from sqlalchemy.orm import polymorphic_union from sqlalchemy.orm import relationship @@ -30,7 +31,6 @@ from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table from test.orm.test_events import _RemoveListeners - Base = None @@ -1317,7 +1317,9 @@ class OverlapColPrecedenceTest(DeclarativeTestBase): self._run_test(Engineer, "eid", "pid") -class ConcreteInhTest(_RemoveListeners, DeclarativeTestBase): +class ConcreteInhTest( + _RemoveListeners, DeclarativeTestBase, testing.AssertsCompiledSQL +): def _roundtrip( self, Employee, @@ -1489,6 +1491,63 @@ class ConcreteInhTest(_RemoveListeners, DeclarativeTestBase): self._roundtrip(Employee, Manager, Engineer, Boss, polymorphic=False) + def test_abstract_concrete_base_didnt_configure(self): + class Employee(AbstractConcreteBase, Base, fixtures.ComparableEntity): + pass + + assert_raises_message( + orm_exc.UnmappedClassError, + "Class test.ext.declarative.test_inheritance.Employee is a " + "subclass of AbstractConcreteBase and has a mapping pending " + "until all subclasses are defined. Call the " + r"sqlalchemy.orm.configure_mappers\(\) function after all " + "subclasses have been defined to complete the " + "mapping of this class.", + Session().query, + Employee, + ) + + configure_mappers() + + # no subclasses yet. + assert_raises_message( + orm_exc.UnmappedClassError, + ".*and has a mapping pending", + Session().query, + Employee, + ) + + class Manager(Employee): + __tablename__ = "manager" + employee_id = Column( + Integer, primary_key=True, test_needs_autoincrement=True + ) + name = Column(String(50)) + golf_swing = Column(String(40)) + __mapper_args__ = { + "polymorphic_identity": "manager", + "concrete": True, + } + + # didnt call configure_mappers() again + assert_raises_message( + orm_exc.UnmappedClassError, + ".*and has a mapping pending", + Session().query, + Employee, + ) + + configure_mappers() + + self.assert_compile( + Session().query(Employee), + "SELECT pjoin.employee_id AS pjoin_employee_id, " + "pjoin.name AS pjoin_name, pjoin.golf_swing AS pjoin_golf_swing, " + "pjoin.type AS pjoin_type FROM (SELECT manager.employee_id " + "AS employee_id, manager.name AS name, manager.golf_swing AS " + "golf_swing, 'manager' AS type FROM manager) AS pjoin", + ) + def test_abstract_concrete_extension(self): class Employee(AbstractConcreteBase, Base, fixtures.ComparableEntity): pass diff --git a/test/ext/declarative/test_reflection.py b/test/ext/declarative/test_reflection.py index df1f0580c4..8517acf0ba 100644 --- a/test/ext/declarative/test_reflection.py +++ b/test/ext/declarative/test_reflection.py @@ -6,9 +6,11 @@ from sqlalchemy.ext import declarative as decl from sqlalchemy.ext.declarative.base import _DeferredMapperConfig from sqlalchemy.orm import clear_mappers from sqlalchemy.orm import create_session +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import relationship from sqlalchemy.orm import Session from sqlalchemy.testing import assert_raises +from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import eq_ from sqlalchemy.testing import fixtures from sqlalchemy.testing.schema import Column @@ -254,6 +256,26 @@ class DeferredReflectionTest(DeferredReflectBase): eq_(a1, Address(email="two")) eq_(a1.user, User(name="u1")) + def test_exception_prepare_not_called(self): + class User(decl.DeferredReflection, fixtures.ComparableEntity, Base): + __tablename__ = "users" + addresses = relationship("Address", backref="user") + + class Address( + decl.DeferredReflection, fixtures.ComparableEntity, Base + ): + __tablename__ = "addresses" + + assert_raises_message( + orm_exc.UnmappedClassError, + "Class test.ext.declarative.test_reflection.User is a " + "subclass of DeferredReflection. Mappings are not produced " + r"until the .prepare\(\) method is called on the class " + "hierarchy.", + Session().query, + User, + ) + def test_basic_deferred(self): class User(decl.DeferredReflection, fixtures.ComparableEntity, Base): __tablename__ = "users" diff --git a/test/ext/test_automap.py b/test/ext/test_automap.py index e2dd5b11e6..25c43f1730 100644 --- a/test/ext/test_automap.py +++ b/test/ext/test_automap.py @@ -11,8 +11,11 @@ from sqlalchemy import testing from sqlalchemy.ext.automap import automap_base from sqlalchemy.ext.automap import generate_relationship from sqlalchemy.orm import configure_mappers +from sqlalchemy.orm import exc as orm_exc from sqlalchemy.orm import interfaces from sqlalchemy.orm import relationship +from sqlalchemy.orm import Session +from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import fixtures from sqlalchemy.testing.mock import Mock from sqlalchemy.testing.mock import patch @@ -54,6 +57,23 @@ class AutomapTest(fixtures.MappedTest): u1 = User(name="u1", addresses_collection=set([a1])) assert a1.user is u1 + def test_exception_prepare_not_called(self): + Base = automap_base(metadata=self.metadata) + + class User(Base): + __tablename__ = "users" + + s = Session() + + assert_raises_message( + orm_exc.UnmappedClassError, + "Class test.ext.test_automap.User is a subclass of AutomapBase. " + r"Mappings are not produced until the .prepare\(\) method is " + "called on the class hierarchy.", + s.query, + User, + ) + def test_relationship_explicit_override_m2o(self): Base = automap_base(metadata=self.metadata)