Fix adding property to mapper before mapping is complete

Fixed issue where calling :meth:`.Mapper.add_property` within mapper event
hooks such as :meth:`.MapperEvents.instrument_class`,
:meth:`.MapperEvents.after_mapper_constructed`, or
:meth:`.MapperEvents.before_mapper_configured` would raise an
``AttributeError`` because the mapper's internal property collections were
not yet initialized. The :meth:`.Mapper.add_property` method now handles
early-stage property additions correctly, allowing properties including
column properties, deferred columns, and relationships to be added during
mapper initialization events.  Pull request courtesy G Allajmi.

Fixes: #12858
Closes: #13023
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/13023
Pull-request-sha: fa9a0ae4bb07819307e6c9d6ec5fd4c9706bf67e

Change-Id: Ibb794c401102a8d23d157b40353f272d5735b49a

diff --git a/doc/build/changelog/unreleased_20/12858.rst b/doc/build/changelog/unreleased_20/12858.rst
new file mode 100644 (file)
index 0000000..fd0cd30
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/12858.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 12858
+
+    Fixed issue where calling :meth:`.Mapper.add_property` within mapper event
+    hooks such as :meth:`.MapperEvents.instrument_class`,
+    :meth:`.MapperEvents.after_mapper_constructed`, or
+    :meth:`.MapperEvents.before_mapper_configured` would raise an
+    ``AttributeError`` because the mapper's internal property collections were
+    not yet initialized. The :meth:`.Mapper.add_property` method now handles
+    early-stage property additions correctly, allowing properties including
+    column properties, deferred columns, and relationships to be added during
+    mapper initialization events.  Pull request courtesy G Allajmi.

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index fb40c6f10a8fa53292e49aaa19c5a610a97454af..370f1a7cc6b5367a57204f999fb7a689ed9cac9c 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2037,13 +2037,21 @@ class Mapper(
             "_configure_property(%s, %s)", key, prop_arg.__class__.__name__
         )
 
+        # early setup mode - don't assign any props, only
+        # ensure a Column is turned into a ColumnProperty.
+        # see #12858
+        early_setup = not hasattr(self, "_props")
+
         if not isinstance(prop_arg, MapperProperty):
             prop: MapperProperty[Any] = self._property_from_column(
-                key, prop_arg
+                key, prop_arg, early_setup
             )
         else:
             prop = prop_arg
 
+        if early_setup:
+            return prop
+
         if isinstance(prop, properties.ColumnProperty):
             col = self.persist_selectable.corresponding_column(prop.columns[0])
 
@@ -2290,16 +2298,17 @@ class Mapper(
 
     @util.preload_module("sqlalchemy.orm.descriptor_props")
     def _property_from_column(
-        self,
-        key: str,
-        column: KeyedColumnElement[Any],
+        self, key: str, column: KeyedColumnElement[Any], early_setup: bool
     ) -> ColumnProperty[Any]:
         """generate/update a :class:`.ColumnProperty` given a
         :class:`_schema.Column` or other SQL expression object."""
 
         descriptor_props = util.preloaded.orm_descriptor_props
 
-        prop = self._props.get(key)
+        if early_setup:
+            prop = None
+        else:
+            prop = self._props.get(key)
 
         if isinstance(prop, properties.ColumnProperty):
             return self._reconcile_prop_with_incoming_columns(

diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py
index 8bb8bb32c2ae02f96b22846444fed2a9b9d0d303..a46ee57e2f91d0976db23b0d84989703e93d9886 100644 (file)
--- a/test/orm/test_mapper.py
+++ b/test/orm/test_mapper.py
@@ -4,6 +4,7 @@ import re
 
 import sqlalchemy as sa
 from sqlalchemy import column
+from sqlalchemy import event
 from sqlalchemy import ForeignKey
 from sqlalchemy import func
 from sqlalchemy import inspect
@@ -1031,6 +1032,129 @@ class MapperTest(_fixtures.FixtureTest, AssertsCompiledSQL):
         assert hasattr(User, "addresses")
         assert "addresses" in [p.key for p in m1._polymorphic_properties]
 
+    @testing.variation(
+        "property_type", ["Column", "ColumnProperty", "Relationship"]
+    )
+    @testing.variation(
+        "event_name",
+        [
+            "instrument_class",
+            "after_mapper_constructed",
+            "mapper_configured",
+            "before_mapper_configured",
+        ],
+    )
+    def test_add_property_in_mapper_event(
+        self, property_type, connection, metadata, event_name
+    ):
+        """test #12858, ability to add properties of all types within
+        event hooks where the mapper is not necessarily set up with property
+        collections.
+
+        """
+
+        Address = self.classes.Address
+
+        users = Table(
+            "deferred_users",
+            metadata,
+            Column("id", Integer, primary_key=True),
+            Column("name", String),
+        )
+
+        # use an ad-hoc User class.   Assigning an event holder to a
+        # class right now means that class permanently has that mapper
+        # event associated with it, whenever a new mapper is created.
+        # if you call event.remove(User), it removes the event from the mapper,
+        # but not the "event hold" that associates it again on a new mapper.
+        # this is probably a bug but not the one we came here today to fix
+        class User:
+            pass
+
+        def setup_props(mapper, class_):
+            if property_type.ColumnProperty:
+                col = Column("new_column", String)
+                mapper.local_table.append_column(col)
+                mapper.add_property(
+                    col.key, deferred(col, group="deferred_group")
+                )
+            elif property_type.Column:
+                col = Column("new_column", String)
+                mapper.local_table.append_column(col)
+                mapper.add_property(col.key, col)
+            elif property_type.Relationship:
+                mapper.add_property("addresses", relationship(Address))
+            else:
+                property_type.fail()
+
+        event.listen(User, event_name.name, setup_props)
+
+        if property_type.ColumnProperty:
+            self.mapper(
+                User,
+                users,
+                properties={
+                    "name": deferred(users.c.name, group="deferred_group")
+                },
+            )
+        else:
+            self.mapper(User, users)
+
+        if property_type.Relationship:
+            addresses_table = Table(
+                "deferred_addresses",
+                metadata,
+                Column("id", Integer, primary_key=True),
+                Column("user_id", ForeignKey("deferred_users.id")),
+            )
+            self.mapper(Address, addresses_table)
+        configure_mappers()
+
+        if property_type.Relationship:
+            assert hasattr(User, "addresses")
+        else:
+            assert hasattr(User, "new_column")
+
+        metadata.create_all(connection)
+        sess = fixture_session(bind=connection)
+
+        if property_type.Relationship:
+            u = User(
+                id=1,
+                name="testuser",
+                addresses=[Address()],
+            )
+        else:
+            u = User(id=1, name="testuser", new_column="test_value")
+        sess.add(u)
+        sess.commit()
+
+        sess = Session(connection)
+        user = sess.query(User).filter_by(id=1).first()
+        if property_type.ColumnProperty:
+            assert "id" in user.__dict__
+
+            # check that the column is deferred
+            assert "new_column" not in user.__dict__
+            assert "name" not in user.__dict__
+
+            eq_(user.new_column, "test_value")
+
+            assert "new_column" in user.__dict__
+            assert "name" in user.__dict__
+            eq_(user.name, "testuser")
+
+        elif property_type.Column:
+            assert "id" in user.__dict__
+            assert "new_column" in user.__dict__
+            assert "name" in user.__dict__
+            eq_(user.new_column, "test_value")
+            eq_(user.name, "testuser")
+        elif property_type.Relationship:
+            eq_(user.addresses[0].user_id, 1)
+        else:
+            property_type.fail()
+
     def test_replace_col_prop_w_syn(self):
         users, User = self.tables.users, self.classes.User