--- /dev/null
+.. change::
+ :tags: bug, orm
+ :tickets: 9841
+
+ A deprecation warning is emitted whenever a property is added to a
+ :class:`_orm.Mapper` where an ORM mapped property were already configured,
+ or an attribute is already present on the class. Previously, there was a
+ non-deprecation warning for this case that did not emit consistently. The
+ logic for this warning has been improved so that it detects end-user
+ replacement of attribute while not having false positives for internal
+ Declarative and other cases where replacement of descriptors with new ones
+ is expected.
if _map_as_property_now:
self._configure_property(
- key, possible_col_prop, init=False
+ key,
+ possible_col_prop,
+ init=False,
)
# step 2: pull properties from the inherited mapper. reconcile
column_key = mapper._columntoproperty[column].key
self._configure_property(
- column_key, column, init=False, setparent=True
+ column_key,
+ column,
+ init=False,
+ setparent=True,
)
def _configure_polymorphic_setter(self, init=False):
"%r for column %r" % (syn, key, key, syn)
)
+ # replacement cases
+
+ # case one: prop is replacing a prop that we have mapped. this is
+ # independent of whatever might be in the actual class dictionary
if (
key in self._props
- and not isinstance(prop, properties.ColumnProperty)
and not isinstance(
- self._props[key],
- (
- properties.ColumnProperty,
- descriptor_props.ConcreteInheritedProperty,
- ),
+ self._props[key], descriptor_props.ConcreteInheritedProperty
)
+ and not isinstance(prop, descriptor_props.SynonymProperty)
):
- util.warn(
- "Property %s on %s being replaced with new "
- "property %s; the old property will be discarded"
- % (self._props[key], self, prop)
- )
+ if warn_for_existing:
+ util.warn_deprecated(
+ f"User-placed attribute {self.class_.__name__}.{key} on "
+ f"{self} is replacing an existing ORM-mapped attribute. "
+ "Behavior is not fully defined in this case. This "
+ "use is deprecated and will raise an error in a future "
+ "release",
+ "2.0",
+ )
oldprop = self._props[key]
self._path_registry.pop(oldprop, None)
- if (
+ # case two: prop is replacing an attribute on the class of some kind.
+ # we have to be more careful here since it's normal when using
+ # Declarative that all the "declared attributes" on the class
+ # get replaced.
+ elif (
warn_for_existing
and self.class_.__dict__.get(key, None) is not None
+ and not isinstance(prop, descriptor_props.SynonymProperty)
and not isinstance(
self._props.get(key, None),
- (descriptor_props.ConcreteInheritedProperty,),
+ descriptor_props.ConcreteInheritedProperty,
)
):
- util.warn(
- "User-placed attribute %r on %s being replaced with "
- 'new property "%s"; the old attribute will be discarded'
- % (self.class_.__dict__[key], self, prop)
+ util.warn_deprecated(
+ f"User-placed attribute {self.class_.__name__}.{key} on "
+ f"{self} is replacing an existing class-bound "
+ "attribute of the same name. "
+ "Behavior is not fully defined in this case. This "
+ "use is deprecated and will raise an error in a future "
+ "release",
+ "2.0",
)
self._props[key] = prop
the given MapperProperty is configured immediately.
"""
- prop = self._configure_property(key, prop, init=self.configured)
+ prop = self._configure_property(
+ key, prop, init=self.configured, warn_for_existing=True
+ )
assert isinstance(prop, MapperProperty)
self._init_properties[key] = prop
t1_mapper = self.mapper_registry.map_imperatively(T1, t1)
- @testing.emits_warning(
- r"This declarative base", r"Property .* being replaced"
- )
+ @testing.emits_warning(r"This declarative base")
+ @testing.expect_deprecated(r"User-placed attribute .* is replacing")
@profile_memory()
def go():
class T2:
self.sql_count_(1, go)
+ @testing.variation("prop_type", ["relationship", "col_prop"])
+ def test_prop_replacement_warns(self, prop_type: testing.Variation):
+ users, User = self.tables.users, self.classes.User
+ addresses, Address = self.tables.addresses, self.classes.Address
+
+ m = self.mapper(
+ User,
+ users,
+ properties={
+ "foo": column_property(users.c.name),
+ "addresses": relationship(Address),
+ },
+ )
+ self.mapper(Address, addresses)
+
+ if prop_type.relationship:
+ key = "addresses"
+ new_prop = relationship(Address)
+ elif prop_type.col_prop:
+ key = "foo"
+ new_prop = column_property(users.c.name)
+ else:
+ prop_type.fail()
+
+ with expect_deprecated(
+ f"Property User.{key} on Mapper|User|users being replaced "
+ f"with new property User.{key}; the old property will "
+ "be discarded",
+ ):
+ m.add_property(key, new_prop)
+
class DeprecatedOptionAllTest(OptionsPathTest, _fixtures.FixtureTest):
run_inserts = "once"
from sqlalchemy import column
from sqlalchemy import ForeignKey
from sqlalchemy import func
+from sqlalchemy import inspect
from sqlalchemy import Integer
from sqlalchemy import literal
from sqlalchemy import MetaData
from sqlalchemy.testing import assert_warns_message
from sqlalchemy.testing import AssertsCompiledSQL
from sqlalchemy.testing import eq_
+from sqlalchemy.testing import expect_deprecated
from sqlalchemy.testing import expect_raises_message
-from sqlalchemy.testing import expect_warnings
from sqlalchemy.testing import fixtures
from sqlalchemy.testing import is_
from sqlalchemy.testing import is_false
properties={"user": relationship(MyClass, backref="addresses")},
)
- attr_repr = re.sub(r"[\(\)]", ".", repr(MyClass.__dict__["addresses"]))
- with expect_warnings(
- rf"User-placed attribute {attr_repr} on "
- r"Mapper\[MyClass\(users\)\] being replaced with new property "
- '"MyClass.addresses"; the old attribute will be discarded'
+ with expect_deprecated(
+ re.escape(
+ f"User-placed attribute MyClass.addresses on "
+ f"{str(inspect(MyClass))} is replacing an existing "
+ "class-bound attribute of the same name. "
+ "Behavior is not fully defined in this case. This "
+ "use is deprecated and will raise an error in a future "
+ "release",
+ ),
):
configure_mappers()
):
configure_mappers()
elif attr_type.assocprox:
- with expect_warnings("User-placed attribute"):
+ with expect_deprecated(
+ "User-placed attribute .* replacing an "
+ "existing class-bound attribute"
+ ):
configure_mappers()
else:
attr_type.fail()
self.mapper(Address, addresses)
m.add_property("_name", deferred(users.c.name))
+
m.add_property("name", synonym("_name"))
m.add_property("addresses", relationship(Address))
u.name = "jacko"
assert m._columntoproperty[users.c.name] is m.get_property("_name")
- def test_replace_rel_prop_with_rel_warns(self):
- users, User = self.tables.users, self.classes.User
- addresses, Address = self.tables.addresses, self.classes.Address
-
- m = self.mapper(
- User, users, properties={"addresses": relationship(Address)}
- )
- self.mapper(Address, addresses)
-
- assert_warns_message(
- sa.exc.SAWarning,
- "Property User.addresses on Mapper|User|users being replaced "
- "with new property User.addresses; the old property will "
- "be discarded",
- m.add_property,
- "addresses",
- relationship(Address),
- )
-
@testing.combinations((True,), (False,))
def test_add_column_prop_adaption(self, autoalias):
"""test ultimately from #2316 revised for #8064"""
class SessionBindTest(QueryTest):
+ run_setup_mappers = "each"
+
@contextlib.contextmanager
def _assert_bind_args(self, session, expect_mapped_bind=True):
get_bind = mock.Mock(side_effect=session.get_bind)
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
