From: Mike Bayer Date: Sat, 28 Jan 2023 19:37:33 +0000 (-0500) Subject: add __init__ to DeclarativeBase directly X-Git-Tag: rel_2_0_1~10^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=960f6dcefe24c6894844366ef6fff60fbcf1ccd6;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git add __init__ to DeclarativeBase directly Fixed regression in :class:`.DeclarativeBase` class where the registry's default constructor would not be applied to the base itself, which is different from how the previous :func:`_orm.declarative_base` construct works. This would prevent a mapped class with its own ``__init__()`` method from calling ``super().__init__()`` in order to access the registry's default constructor and automatically populate attributes, instead hitting ``object.__init__()`` which would raise a ``TypeError`` on any arguments. This is a very simple change in code, however explaining it is very complicated. Fixes: #9171 Change-Id: I4baecdf671861a8198d835e286fe19a51ecda126 --- diff --git a/doc/build/changelog/unreleased_20/9171.rst b/doc/build/changelog/unreleased_20/9171.rst new file mode 100644 index 0000000000..2d8c914013 --- /dev/null +++ b/doc/build/changelog/unreleased_20/9171.rst @@ -0,0 +1,14 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 9171 + + Fixed regression in :class:`.DeclarativeBase` class where the registry's + default constructor would not be applied to the base itself, which is + different from how the previous :func:`_orm.declarative_base` construct + works. This would prevent a mapped class with its own ``__init__()`` method + from calling ``super().__init__()`` in order to access the registry's + default constructor and automatically populate attributes, instead hitting + ``object.__init__()`` which would raise a ``TypeError`` on any arguments. + + + diff --git a/lib/sqlalchemy/orm/decl_api.py b/lib/sqlalchemy/orm/decl_api.py index ecd81c7ed5..62e78c94d1 100644 --- a/lib/sqlalchemy/orm/decl_api.py +++ b/lib/sqlalchemy/orm/decl_api.py @@ -562,6 +562,9 @@ def _setup_declarative_base(cls: Type[Any]) -> None: if "metadata" not in cls.__dict__: cls.metadata = cls.registry.metadata # type: ignore + if "__init__" not in cls.__dict__: + cls.__init__ = cls.registry.constructor + class MappedAsDataclass(metaclass=DCTransformDeclarative): """Mixin class to indicate when mapping this class, also convert it to be @@ -678,6 +681,52 @@ class DeclarativeBase( where the base class returned cannot be recognized by type checkers without using plugins. + **__init__ behavior** + + In a plain Python class, the base-most ``__init__()`` method in the class + hierarchy is ``object.__init__()``, which accepts no arguments. However, + when the :class:`_orm.DeclarativeBase` subclass is first declared, the + class is given an ``__init__()`` method that links to the + :paramref:`_orm.registry.constructor` constructor function, if no + ``__init__()`` method is already present; this is the usual declarative + constructor that will assign keyword arguments as attributes on the + instance, assuming those attributes are established at the class level + (i.e. are mapped, or are linked to a descriptor). This constructor is + **never accessed by a mapped class without being called explicitly via + super()**, as mapped classes are themselves given an ``__init__()`` method + directly which calls :paramref:`_orm.registry.constructor`, so in the + default case works independently of what the base-most ``__init__()`` + method does. + + .. versionchanged:: 2.0.1 :class:`_orm.DeclarativeBase` has a default + constructor that links to :paramref:`_orm.registry.constructor` by + default, so that calls to ``super().__init__()`` can access this + constructor. Previously, due to an implementation mistake, this default + constructor was missing, and calling ``super().__init__()`` would invoke + ``object.__init__()``. + + The :class:`_orm.DeclarativeBase` subclass may also declare an explicit + ``__init__()`` method which will replace the use of the + :paramref:`_orm.registry.constructor` function at this level:: + + class Base(DeclarativeBase): + def __init__(self, id=None): + self.id = id + + Mapped classes still will not invoke this constructor implicitly; it + remains only accessible by calling ``super().__init__()``:: + + class MyClass(Base): + def __init__(self, id=None, name=None): + self.name = name + super().__init__(id=id) + + Note that this is a different behavior from what functions like the legacy + :func:`_orm.declarative_base` would do; the base created by those functions + would always install :paramref:`_orm.registry.constructor` for + ``__init__()``. + + """ if typing.TYPE_CHECKING: @@ -784,7 +833,7 @@ def _check_not_declarative(cls: Type[Any], base: Type[Any]) -> None: ) -class DeclarativeBaseNoMeta(inspection.Inspectable[Mapper[Any]]): +class DeclarativeBaseNoMeta(inspection.Inspectable[InstanceState[Any]]): """Same as :class:`_orm.DeclarativeBase`, but does not use a metaclass to intercept new attributes. diff --git a/test/orm/declarative/test_basic.py b/test/orm/declarative/test_basic.py index 28fdc97f23..e2108f8886 100644 --- a/test/orm/declarative/test_basic.py +++ b/test/orm/declarative/test_basic.py @@ -14,6 +14,7 @@ from sqlalchemy import String from sqlalchemy import testing from sqlalchemy import UniqueConstraint from sqlalchemy.ext.hybrid import hybrid_property +from sqlalchemy.orm import as_declarative from sqlalchemy.orm import backref from sqlalchemy.orm import class_mapper from sqlalchemy.orm import clear_mappers @@ -89,6 +90,104 @@ class DeclarativeBaseSetupsTest(fixtures.TestBase): with testing.expect_raises(exc.UnboundExecutionError): s.get_bind(User) + @testing.variation( + "base_type", + ["declbase", "declbasenometa", "declbasefn", "asdeclarative"], + ) + def test_reg_constructor_is_present(self, base_type): + """test #9171""" + + if base_type.declbase: + + class Base(DeclarativeBase): + pass + + elif base_type.declbasenometa: + + class Base(DeclarativeBaseNoMeta): + pass + + elif base_type.declbasefn: + Base = declarative_base() + elif base_type.asdeclarative: + + @as_declarative() + class Base: + pass + + else: + base_type.fail() + + # check for direct assignment + is_(Base.registry.constructor, Base.__init__) + is_(Base.__dict__["__init__"], Base.__init__) + + class fakeself: + foo = None + bar = None + + fs = fakeself() + Base.__init__(fs, foo="bar", bar="bat") + eq_(fs.foo, "bar") + eq_(fs.bar, "bat") + + @testing.variation( + "base_type", + ["declbase", "declbasenometa", "declbasefn", "asdeclarative"], + ) + def test_reg_constructor_custom_init(self, base_type): + """test for #9171 testing what an explicit __init__ does. + + Here we decide that newer DeclarativeBase superclasses should + honor the ``__init__`` that's given. + + """ + + m1 = mock.Mock() + + if base_type.declbase: + + class Base(DeclarativeBase): + def __init__(self, x=None): + m1.init(x) + + elif base_type.declbasenometa: + + class Base(DeclarativeBaseNoMeta): + def __init__(self, x=None): + m1.init(x) + + elif base_type.declbasefn: + + class _B: + def __init__(self, x=None): + m1.init(x) + + Base = declarative_base(cls=_B) + elif base_type.asdeclarative: + + @as_declarative() + class Base: + def __init__(self, x=None): + m1.init(x) + + else: + base_type.fail() + + class fakeself: + pass + + fs = fakeself() + + if base_type.declbase or base_type.declbasenometa: + Base.__init__(fs, x=5) + eq_(m1.mock_calls, [mock.call.init(5)]) + else: + with expect_raises_message( + TypeError, "'x' is an invalid keyword argument for fakeself" + ): + Base.__init__(fs, x=5) + def test_dispose_attrs(self): reg = registry()