- The mechanism by which attribute events pass along an

:class:`.AttributeImpl` as an "initiator" token has been changed;
the object is now an event-specific object called :class:`.attributes.Event`.
Additionally, the attribute system no longer halts events based
on a matching "initiator" token; this logic has been moved to be
specific to ORM backref event handlers, which are the typical source
of the re-propagation of an attribute event onto subsequent append/set/remove
operations.  End user code which emulates the behavior of backrefs
must now ensure that recursive event propagation schemes are halted,
if the scheme does not use the backref handlers.   Using this new system,
backref handlers can now peform a
"two-hop" operation when an object is appended to a collection,
associated with a new many-to-one, de-associated with the previous
many-to-one, and then removed from a previous collection.   Before this
change, the last step of removal from the previous collection would
not occur.
[ticket:2789]

3 files changed, 123 insertions, 52 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 13c2cf256..f4f0cc782 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -398,6 +398,59 @@ def create_proxied_attribute(descriptor):
                                       from_instance=descriptor)
     return Proxy
 
+OP_REMOVE = util.symbol("REMOVE")
+OP_APPEND = util.symbol("APPEND")
+OP_REPLACE = util.symbol("REPLACE")
+
+class Event(object):
+    """A token propagated throughout the course of a chain of attribute
+    events.
+
+    Serves as an indicator of the source of the event and also provides
+    a means of controlling propagation across a chain of attribute
+    operations.
+
+    The :class:`.Event` object is sent as the ``initiator`` argument
+    when dealing with the :meth:`.AttributeEvents.append`,
+    :meth:`.AttributeEvents.set`,
+    and :meth:`.AttributeEvents.remove` events.
+
+    The :class:`.Event` object is currently interpreted by the backref
+    event handlers, and is used to control the propagation of operations
+    across two mutually-dependent attributes.
+
+    .. versionadded:: 0.9.0
+
+    """
+
+    impl = None
+    """The :class:`.AttributeImpl` which is the current event initiator.
+    """
+
+    op = None
+    """The symbol :attr:`.OP_APPEND`, :attr:`.OP_REMOVE` or :attr:`.OP_REPLACE`,
+    indicating the source operation.
+
+    """
+
+    def __init__(self, attribute_impl, op):
+        self.impl = attribute_impl
+        self.op = op
+        self.parent_token = self.impl.parent_token
+
+    @classmethod
+    def _token_gen(self, op):
+        @util.memoized_property
+        def gen(self):
+            return Event(self, op)
+        return gen
+
+    @property
+    def key(self):
+        return self.impl.key
+
+    def hasparent(self, state):
+        return self.impl.hasparent(state)
 
 class AttributeImpl(object):
     """internal implementation for instrumented attributes."""
@@ -683,7 +736,7 @@ class ScalarAttributeImpl(AttributeImpl):
             old = dict_.get(self.key, NO_VALUE)
 
         if self.dispatch.remove:
-            self.fire_remove_event(state, dict_, old, None)
+            self.fire_remove_event(state, dict_, old, self._remove_token)
         state._modified_event(dict_, self, old)
         del dict_[self.key]
 
@@ -693,9 +746,6 @@ class ScalarAttributeImpl(AttributeImpl):
 
     def set(self, state, dict_, value, initiator,
                 passive=PASSIVE_OFF, check_old=None, pop=False):
-        if initiator and initiator.parent_token is self.parent_token:
-            return
-
         if self.dispatch._active_history:
             old = self.get(state, dict_, PASSIVE_RETURN_NEVER_SET)
         else:
@@ -707,14 +757,17 @@ class ScalarAttributeImpl(AttributeImpl):
         state._modified_event(dict_, self, old)
         dict_[self.key] = value
 
+    _replace_token = _append_token = Event._token_gen(OP_REPLACE)
+    _remove_token = Event._token_gen(OP_REMOVE)
+
     def fire_replace_event(self, state, dict_, value, previous, initiator):
         for fn in self.dispatch.set:
-            value = fn(state, value, previous, initiator or self)
+            value = fn(state, value, previous, initiator or self._replace_token)
         return value
 
     def fire_remove_event(self, state, dict_, value, initiator):
         for fn in self.dispatch.remove:
-            fn(state, value, initiator or self)
+            fn(state, value, initiator or self._remove_token)
 
     @property
     def type(self):
@@ -736,7 +789,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
 
     def delete(self, state, dict_):
         old = self.get(state, dict_)
-        self.fire_remove_event(state, dict_, old, self)
+        self.fire_remove_event(state, dict_, old, self._remove_token)
         del dict_[self.key]
 
     def get_history(self, state, dict_, passive=PASSIVE_OFF):
@@ -773,14 +826,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
                 passive=PASSIVE_OFF, check_old=None, pop=False):
         """Set a value on the given InstanceState.
 
-        `initiator` is the ``InstrumentedAttribute`` that initiated the
-        ``set()`` operation and is used to control the depth of a circular
-        setter operation.
-
         """
-        if initiator and initiator.parent_token is self.parent_token:
-            return
-
         if self.dispatch._active_history:
             old = self.get(state, dict_, passive=PASSIVE_ONLY_PERSISTENT)
         else:
@@ -801,12 +847,13 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
         value = self.fire_replace_event(state, dict_, value, old, initiator)
         dict_[self.key] = value
 
+
     def fire_remove_event(self, state, dict_, value, initiator):
         if self.trackparent and value is not None:
             self.sethasparent(instance_state(value), state, False)
 
         for fn in self.dispatch.remove:
-            fn(state, value, initiator or self)
+            fn(state, value, initiator or self._remove_token)
 
         state._modified_event(dict_, self, value)
 
@@ -818,7 +865,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
                 self.sethasparent(instance_state(previous), state, False)
 
         for fn in self.dispatch.set:
-            value = fn(state, value, previous, initiator or self)
+            value = fn(state, value, previous, initiator or self._replace_token)
 
         state._modified_event(dict_, self, previous)
 
@@ -902,9 +949,12 @@ class CollectionAttributeImpl(AttributeImpl):
 
         return [(instance_state(o), o) for o in current]
 
+    _append_token = Event._token_gen(OP_APPEND)
+    _remove_token = Event._token_gen(OP_REMOVE)
+
     def fire_append_event(self, state, dict_, value, initiator):
         for fn in self.dispatch.append:
-            value = fn(state, value, initiator or self)
+            value = fn(state, value, initiator or self._append_token)
 
         state._modified_event(dict_, self, NEVER_SET, True)
 
@@ -921,7 +971,7 @@ class CollectionAttributeImpl(AttributeImpl):
             self.sethasparent(instance_state(value), state, False)
 
         for fn in self.dispatch.remove:
-            fn(state, value, initiator or self)
+            fn(state, value, initiator or self._remove_token)
 
         state._modified_event(dict_, self, NEVER_SET, True)
 
@@ -948,8 +998,6 @@ class CollectionAttributeImpl(AttributeImpl):
             self.key, state, self.collection_factory)
 
     def append(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
-        if initiator and initiator.parent_token is self.parent_token:
-            return
         collection = self.get_collection(state, dict_, passive=passive)
         if collection is PASSIVE_NO_RESULT:
             value = self.fire_append_event(state, dict_, value, initiator)
@@ -960,9 +1008,6 @@ class CollectionAttributeImpl(AttributeImpl):
             collection.append_with_event(value, initiator)
 
     def remove(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
-        if initiator and initiator.parent_token is self.parent_token:
-            return
-
         collection = self.get_collection(state, state.dict, passive=passive)
         if collection is PASSIVE_NO_RESULT:
             self.fire_remove_event(state, dict_, value, initiator)
@@ -985,14 +1030,8 @@ class CollectionAttributeImpl(AttributeImpl):
                     passive=PASSIVE_OFF, pop=False):
         """Set a value on the given object.
 
-        `initiator` is the ``InstrumentedAttribute`` that initiated the
-        ``set()`` operation and is used to control the depth of a circular
-        setter operation.
         """
 
-        if initiator and initiator.parent_token is self.parent_token:
-            return
-
         self._set_iterable(
             state, dict_, value,
             lambda adapter, i: adapter.adapt_like_to_iterable(i))
@@ -1085,6 +1124,7 @@ def backref_listeners(attribute, key, uselist):
     # use easily recognizable names for stack traces
 
     parent_token = attribute.impl.parent_token
+    parent_impl = attribute.impl
 
     def _acceptable_key_err(child_state, initiator, child_impl):
         raise ValueError(
@@ -1108,10 +1148,14 @@ def backref_listeners(attribute, key, uselist):
             old_state, old_dict = instance_state(oldchild),\
                                     instance_dict(oldchild)
             impl = old_state.manager[key].impl
-            impl.pop(old_state,
-                        old_dict,
-                        state.obj(),
-                        initiator, passive=PASSIVE_NO_FETCH)
+
+            if initiator.impl is not impl or \
+                    initiator.op not in (OP_REPLACE, OP_REMOVE):
+                impl.pop(old_state,
+                            old_dict,
+                            state.obj(),
+                            parent_impl._append_token,
+                            passive=PASSIVE_NO_FETCH)
 
         if child is not None:
             child_state, child_dict = instance_state(child),\
@@ -1120,12 +1164,14 @@ def backref_listeners(attribute, key, uselist):
             if initiator.parent_token is not parent_token and \
                     initiator.parent_token is not child_impl.parent_token:
                 _acceptable_key_err(state, initiator, child_impl)
-            child_impl.append(
-                                child_state,
-                                child_dict,
-                                state.obj(),
-                                initiator,
-                                passive=PASSIVE_NO_FETCH)
+            elif initiator.impl is not child_impl or \
+                    initiator.op not in (OP_APPEND, OP_REPLACE):
+                child_impl.append(
+                                    child_state,
+                                    child_dict,
+                                    state.obj(),
+                                    initiator,
+                                    passive=PASSIVE_NO_FETCH)
         return child
 
     def emit_backref_from_collection_append_event(state, child, initiator):
@@ -1139,7 +1185,9 @@ def backref_listeners(attribute, key, uselist):
         if initiator.parent_token is not parent_token and \
                 initiator.parent_token is not child_impl.parent_token:
             _acceptable_key_err(state, initiator, child_impl)
-        child_impl.append(
+        elif initiator.impl is not child_impl or \
+                initiator.op not in (OP_APPEND, OP_REPLACE):
+            child_impl.append(
                                 child_state,
                                 child_dict,
                                 state.obj(),
@@ -1152,10 +1200,9 @@ def backref_listeners(attribute, key, uselist):
             child_state, child_dict = instance_state(child),\
                                         instance_dict(child)
             child_impl = child_state.manager[key].impl
-            # can't think of a path that would produce an initiator
-            # mismatch here, as it would require an existing collection
-            # mismatch.
-            child_impl.pop(
+            if initiator.impl is not child_impl or \
+                    initiator.op not in (OP_REMOVE, OP_REPLACE):
+                child_impl.pop(
                                 child_state,
                                 child_dict,
                                 state.obj(),
diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index 5814b47ca..fb46713d0 100644
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -78,6 +78,9 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
             history = self._get_collection_history(state, passive)
             return history.added_plus_unchanged
 
+    _append_token = attributes.Event._token_gen(attributes.OP_APPEND)
+    _remove_token = attributes.Event._token_gen(attributes.OP_REMOVE)
+
     def fire_append_event(self, state, dict_, value, initiator,
                                                     collection_history=None):
         if collection_history is None:
@@ -86,7 +89,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
         collection_history.add_added(value)
 
         for fn in self.dispatch.append:
-            value = fn(state, value, initiator or self)
+            value = fn(state, value, initiator or self._append_token)
 
         if self.trackparent and value is not None:
             self.sethasparent(attributes.instance_state(value), state, True)
@@ -102,7 +105,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl):
             self.sethasparent(attributes.instance_state(value), state, False)
 
         for fn in self.dispatch.remove:
-            fn(state, value, initiator or self)
+            fn(state, value, initiator or self._remove_token)
 
     def _modified_event(self, state, dict_):
 
diff --git a/lib/sqlalchemy/orm/events.py b/lib/sqlalchemy/orm/events.py
index 14b3a770e..9a7190746 100644
--- a/lib/sqlalchemy/orm/events.py
+++ b/lib/sqlalchemy/orm/events.py
@@ -1561,8 +1561,15 @@ class AttributeEvents(event.Events):
           is registered with ``retval=True``, the listener
           function must return this value, or a new value which
           replaces it.
-        :param initiator: the attribute implementation object
-          which initiated this event.
+        :param initiator: An instance of :class:`.attributes.Event`
+          representing the initiation of the event.  May be modified
+          from it's original value by backref handlers in order to control
+          chained event propagation.
+
+          .. versionchanged:: 0.9.0 the ``initiator`` argument is now
+             passed as a :class:`.attributes.Event` object, and may be modified
+             by backref handlers within a chain of backref-linked events.
+
         :return: if the event was registered with ``retval=True``,
          the given value, or a new effective value, should be returned.
 
@@ -1575,8 +1582,15 @@ class AttributeEvents(event.Events):
           If the listener is registered with ``raw=True``, this will
           be the :class:`.InstanceState` object.
         :param value: the value being removed.
-        :param initiator: the attribute implementation object
-          which initiated this event.
+        :param initiator: An instance of :class:`.attributes.Event`
+          representing the initiation of the event.  May be modified
+          from it's original value by backref handlers in order to control
+          chained event propagation.
+
+          .. versionchanged:: 0.9.0 the ``initiator`` argument is now
+             passed as a :class:`.attributes.Event` object, and may be modified
+             by backref handlers within a chain of backref-linked events.
+
         :return: No return value is defined for this event.
         """
 
@@ -1596,8 +1610,15 @@ class AttributeEvents(event.Events):
           the previous value of the attribute will be loaded from
           the database if the existing value is currently unloaded
           or expired.
-        :param initiator: the attribute implementation object
-          which initiated this event.
+        :param initiator: An instance of :class:`.attributes.Event`
+          representing the initiation of the event.  May be modified
+          from it's original value by backref handlers in order to control
+          chained event propagation.
+
+          .. versionchanged:: 0.9.0 the ``initiator`` argument is now
+             passed as a :class:`.attributes.Event` object, and may be modified
+             by backref handlers within a chain of backref-linked events.
+
         :return: if the event was registered with ``retval=True``,
          the given value, or a new effective value, should be returned.