From 75a3f84e5f9c3be645be2ec8906e63b27e9847e5 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 3 Mar 2013 13:51:54 -0500 Subject: - Improved checking for an existing backref name conflict during mapper configuration; will now test for name conflicts on superclasses and subclasses, in addition to the current mapper, as these conflicts break things just as much. This is new for 0.8, but see below for a warning that will also be triggered in 0.7.11. - Improved the error message emitted when a "backref loop" is detected, that is when an attribute event triggers a bidirectional assignment between two other attributes with no end. This condition can occur not just when an object of the wrong type is assigned, but also when an attribute is mis-configured to backref into an existing backref pair. Also in 0.7.11. - A warning is emitted when a MapperProperty is assigned to a mapper that replaces an existing property, if the properties in question aren't plain column-based properties. Replacement of relationship properties is rarely (ever?) what is intended and usually refers to a mapper mis-configuration. Also in 0.7.11. [ticket:2674] --- lib/sqlalchemy/orm/attributes.py | 24 ++++++++++++++++++------ lib/sqlalchemy/orm/mapper.py | 12 +++++++++++- lib/sqlalchemy/orm/properties.py | 19 +++++++++++++------ 3 files changed, 42 insertions(+), 13 deletions(-) (limited to 'lib/sqlalchemy') diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index c9385daaa..b281fabec 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -433,6 +433,9 @@ class AttributeImpl(object): self.expire_missing = expire_missing + def __str__(self): + return "%s.%s" % (self.class_.__name__, self.key) + def _get_active_history(self): """Backwards compat for impl.active_history""" @@ -1043,11 +1046,18 @@ def backref_listeners(attribute, key, uselist): parent_token = attribute.impl.parent_token - def _acceptable_key_err(child_state, initiator): + def _acceptable_key_err(child_state, initiator, child_impl): raise ValueError( - "Object %s not associated with attribute of " - "type %s" % (orm_util.state_str(child_state), - manager_of_class(initiator.class_)[initiator.key])) + "Bidirectional attribute conflict detected: " + 'Passing object %s to attribute "%s" ' + 'triggers a modify event on attribute "%s" ' + 'via the backref "%s".' % ( + orm_util.state_str(child_state), + initiator.parent_token, + child_impl.parent_token, + attribute.impl.parent_token + ) + ) def emit_backref_from_scalar_set_event(state, child, oldchild, initiator): if oldchild is child: @@ -1069,7 +1079,7 @@ def backref_listeners(attribute, key, uselist): child_impl = child_state.manager[key].impl if initiator.parent_token is not parent_token and \ initiator.parent_token is not child_impl.parent_token: - _acceptable_key_err(state, initiator) + _acceptable_key_err(state, initiator, child_impl) child_impl.append( child_state, child_dict, @@ -1085,9 +1095,11 @@ def backref_listeners(attribute, key, uselist): child_state, child_dict = instance_state(child), \ instance_dict(child) child_impl = child_state.manager[key].impl + + print initiator.parent_token, parent_token, child_impl.parent_token if initiator.parent_token is not parent_token and \ initiator.parent_token is not child_impl.parent_token: - _acceptable_key_err(state, initiator) + _acceptable_key_err(state, initiator, child_impl) child_impl.append( child_state, child_dict, diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 447a5fce1..4e7b4d272 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1094,7 +1094,7 @@ class Mapper(_InspectionAttr): # initialized; check for 'readonly' if hasattr(self, '_readonly_props') and \ (not hasattr(col, 'table') or - col.table not in self._cols_by_table): + col.table not in self._cols_by_table): self._readonly_props.add(prop) else: @@ -1132,6 +1132,16 @@ class Mapper(_InspectionAttr): "%r for column %r" % (syn, key, key, syn) ) + if key in self._props and \ + not isinstance(prop, properties.ColumnProperty) and \ + not isinstance(self._props[key], properties.ColumnProperty): + util.warn("Property %s on %s being replaced with new " + "property %s; the old property will be discarded" % ( + self._props[key], + self, + prop, + )) + self._props[key] = prop if not self.non_primary: diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index a0d8c92d1..79a1b81d3 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -816,7 +816,10 @@ class RelationshipProperty(StrategizedProperty): adapt_source=adapt_source) def __str__(self): - return str(self.parent.class_.__name__) + "." + self.key + if self.parent: + return str(self.parent.class_.__name__) + "." + self.key + else: + return "." + self.key def merge(self, session, @@ -1201,11 +1204,15 @@ class RelationshipProperty(StrategizedProperty): else: backref_key, kwargs = self.backref mapper = self.mapper.primary_mapper() - if mapper.has_property(backref_key): - raise sa_exc.ArgumentError("Error creating backref " - "'%s' on relationship '%s': property of that " - "name exists on mapper '%s'" % (backref_key, - self, mapper)) + + check = set(mapper.iterate_to_root()).\ + union(mapper.self_and_descendants) + for m in check: + if m.has_property(backref_key): + raise sa_exc.ArgumentError("Error creating backref " + "'%s' on relationship '%s': property of that " + "name exists on mapper '%s'" % (backref_key, + self, m)) # determine primaryjoin/secondaryjoin for the # backref. Use the one we had, so that -- cgit v1.2.1