From 346e2bc0a64f5d451d951d04a7ee36b1dd7ea8fa Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 24 Mar 2021 17:43:06 -0400 Subject: [PATCH] Use class-local metadata for declarative base Fixed regression where the ``.metadata`` attribute on a per class level would not be honored, breaking the use case of per-class-hierarchy :class:`.schema.MetaData` for abstract declarative classes and mixins. Fixes: #6128 Change-Id: I5c15436b5c5171105dc1a0192fa744daf79a344d --- doc/build/changelog/unreleased_14/6128.rst | 12 +++ doc/build/orm/declarative_config.rst | 68 +++++++++++++++ lib/sqlalchemy/orm/decl_base.py | 8 +- test/aaa_profiling/test_memusage.py | 2 +- test/orm/declarative/test_mixin.py | 98 ++++++++++++++++++++++ test/requirements.py | 7 ++ 6 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6128.rst diff --git a/doc/build/changelog/unreleased_14/6128.rst b/doc/build/changelog/unreleased_14/6128.rst new file mode 100644 index 0000000000..5eb7f4a6ff --- /dev/null +++ b/doc/build/changelog/unreleased_14/6128.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, regression, orm, declarative + :tickets: 6128 + + Fixed regression where the ``.metadata`` attribute on a per class level + would not be honored, breaking the use case of per-class-hierarchy + :class:`.schema.MetaData` for abstract declarative classes and mixins. + + + .. seealso:: + + :ref:`declarative_metadata` \ No newline at end of file diff --git a/doc/build/orm/declarative_config.rst b/doc/build/orm/declarative_config.rst index 48c270b4ed..8e3bc58824 100644 --- a/doc/build/orm/declarative_config.rst +++ b/doc/build/orm/declarative_config.rst @@ -240,6 +240,74 @@ configuration via the :meth:`.MapperEvents.before_configured` event:: .. versionadded:: 0.9.3 + +.. _declarative_metadata: + +``metadata`` +~~~~~~~~~~~~ + +The :class:`_schema.MetaData` collection normally used to assign a new +:class:`_schema.Table` is the :attr:`_orm.registry.metadata` attribute +associated with the :class:`_orm.registry` object in use. When using a +declarative base class such as that generated by :func:`_orm.declarative_base` +as well as :meth:`_orm.registry.generate_base`, this :class:`_schema.MetaData` +is also normally present also as an attribute named ``.metadata`` that's +directly on the base class, and thus also on the mapped class via +inheritance. Declarative uses this attribute, when present, in order to +determine the target :class:`_schema.MetaData` collection, or if not +present, uses the :class:`_schema.MetaData` associated directly with the +:class:`_orm.registry`. + +This attribute may also be assigned towards in order to affect the +:class:`_schema.MetaData` collection to be used on a per-mapped-hierarchy basis +for a single base and/or :class:`_orm.registry`. This takes effect whether a +declarative base class is used or if the :meth:`_orm.registry.mapped` decorator +is used directly, thus allowing patterns such as the metadata-per-abstract base +example in the next section, :ref:`declarative_abstract`. A similar pattern can +be illustrated using :meth:`_orm.registry.mapped` as follows:: + + reg = registry() + + class BaseOne: + metadata = MetaData() + + class BaseTwo: + metadata = MetaData() + + @reg.mapped + class ClassOne: + __tablename__ = 't1' # will use reg.metadata + + id = Column(Integer, primary_key=True) + + @reg.mapped + class ClassTwo(BaseOne): + __tablename__ = 't1' # will use BaseOne.metadata + + id = Column(Integer, primary_key=True) + + @reg.mapped + class ClassThree(BaseOne): + __tablename__ = 't1' # will use BaseTwo.metadata + + id = Column(Integer, primary_key=True) + + +.. versionchanged:: 1.4.3 The :meth:`_orm.registry.mapped` decorator will + honor an attribute named ``.metadata`` on the class as an alternate + :class:`_schema.MetaData` collection to be used in place of the + :class:`_schema.MetaData` that's on the :class:`_orm.registry` itself. + This matches the behavior of the base class returned by the + :meth:`_orm.registry.generate_base` and :meth:`_orm.declarative_base` + method/function. Note this feature was broken due to a regression in + 1.4.0, 1.4.1 and 1.4.2, even when using :func:`_orm.declarative_base`; + 1.4.3 is needed to restore the behavior. + + +.. seealso:: + + :ref:`declarative_abstract` + .. _declarative_abstract: ``__abstract__`` diff --git a/lib/sqlalchemy/orm/decl_base.py b/lib/sqlalchemy/orm/decl_base.py index e55056fdff..5a5d98a958 100644 --- a/lib/sqlalchemy/orm/decl_base.py +++ b/lib/sqlalchemy/orm/decl_base.py @@ -783,7 +783,7 @@ class _ClassScanMapperConfig(_MapperConfig): "__table__", table_cls( tablename, - manager.registry.metadata, + self._metadata_for_cls(manager), *(tuple(declared_columns) + tuple(args)), **table_kw ), @@ -800,6 +800,12 @@ class _ClassScanMapperConfig(_MapperConfig): ) self.local_table = table + def _metadata_for_cls(self, manager): + if hasattr(self.cls, "metadata"): + return self.cls.metadata + else: + return manager.registry.metadata + def _setup_inheritance(self, mapper_kw): table = self.local_table cls = self.cls diff --git a/test/aaa_profiling/test_memusage.py b/test/aaa_profiling/test_memusage.py index b1dd29a7ee..d8896a3d61 100644 --- a/test/aaa_profiling/test_memusage.py +++ b/test/aaa_profiling/test_memusage.py @@ -341,7 +341,7 @@ class MemUsageTest(EnsureZeroed): class MemUsageWBackendTest(EnsureZeroed): __tags__ = ("memory_intensive",) - __requires__ = "cpython", "memory_process_intensive" + __requires__ = "cpython", "memory_process_intensive", "no_asyncio" __sparse_backend__ = True # ensure a pure growing test trips the assertion diff --git a/test/orm/declarative/test_mixin.py b/test/orm/declarative/test_mixin.py index ad4832c357..05628641a9 100644 --- a/test/orm/declarative/test_mixin.py +++ b/test/orm/declarative/test_mixin.py @@ -243,6 +243,104 @@ class DeclarativeMixinTest(DeclarativeTestBase): eq_(Manager.__table__.name, "manager") + def test_same_base_multiple_metadata(self): + m1 = MetaData() + m2 = MetaData() + + class B1(Base): + __abstract__ = True + metadata = m1 + + class B2(Base): + __abstract__ = True + metadata = m2 + + def fullname(self): + return self.name + " " + self.surname + + class User(B1): + __tablename__ = "user" + + id = Column(Integer, primary_key=True) + name = Column(String) + surname = Column(String) + + class AD(B1): + __tablename__ = "address" + + id = Column(Integer, primary_key=True) + + class OtherUser(B2): + __tablename__ = "user" + + id = Column(Integer, primary_key=True) + username = Column(String) + + class BUser(Base): + __tablename__ = "user" + + id = Column(Integer, primary_key=True) + login = Column(String) + + eq_(set(m1.tables), {"user", "address"}) + eq_(set(m2.tables), {"user"}) + eq_(set(Base.registry.metadata.tables), {"user"}) + + eq_(Base.registry.metadata.tables["user"].c.keys(), ["id", "login"]) + eq_(m1.tables["user"].c.keys(), ["id", "name", "surname"]) + eq_(m2.tables["user"].c.keys(), ["id", "username"]) + + def test_same_registry_multiple_metadata(self): + m1 = MetaData() + m2 = MetaData() + + reg = registry() + + class B1(object): + metadata = m1 + + class B2(object): + metadata = m2 + + def fullname(self): + return self.name + " " + self.surname + + @reg.mapped + class User(B1): + __tablename__ = "user" + + id = Column(Integer, primary_key=True) + name = Column(String) + surname = Column(String) + + @reg.mapped + class AD(B1): + __tablename__ = "address" + + id = Column(Integer, primary_key=True) + + @reg.mapped + class OtherUser(B2): + __tablename__ = "user" + + id = Column(Integer, primary_key=True) + username = Column(String) + + @reg.mapped + class BUser(object): + __tablename__ = "user" + + id = Column(Integer, primary_key=True) + login = Column(String) + + eq_(set(m1.tables), {"user", "address"}) + eq_(set(m2.tables), {"user"}) + eq_(set(reg.metadata.tables), {"user"}) + + eq_(reg.metadata.tables["user"].c.keys(), ["id", "login"]) + eq_(m1.tables["user"].c.keys(), ["id", "name", "surname"]) + eq_(m2.tables["user"].c.keys(), ["id", "username"]) + def test_not_allowed(self): class MyMixin: foo = Column(Integer, ForeignKey("bar.id")) diff --git a/test/requirements.py b/test/requirements.py index df4a3b6011..27bf17c0ab 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -1457,6 +1457,13 @@ class DefaultRequirements(SuiteRequirements): + skip_if(self._sqlite_file_db) ) + @property + def no_asyncio(self): + def go(config): + return config.db.dialect.is_async + + return skip_if(go) + @property def no_mssql_freetds(self): return self.mssql_freetds.not_() -- 2.47.2