- AttributeListener has been refined such that the event

is fired before the mutation actually occurs.  Addtionally,
the append() and set() methods must now return the given value,
which is used as the value to be used in the mutation operation.
This allows creation of validating AttributeListeners which
raise before the action actually occurs, and which can change
the given value into something else before its used.
A new example "validate_attributes.py" shows one such recipe
for doing this.   AttributeListener helper functions are
also on the way.

4 files changed, 55 insertions, 30 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index ddebc563f..17fea7854 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -382,8 +382,8 @@ class ScalarAttributeImpl(AttributeImpl):
         state.modified_event(self, False, old)
 
         if self.extensions:
-            del state.dict[self.key]
             self.fire_remove_event(state, old, None)
+            del state.dict[self.key]
         else:
             del state.dict[self.key]
 
@@ -403,14 +403,15 @@ class ScalarAttributeImpl(AttributeImpl):
         state.modified_event(self, False, old)
 
         if self.extensions:
+            value = self.fire_replace_event(state, value, old, initiator)
             state.dict[self.key] = value
-            self.fire_replace_event(state, value, old, initiator)
         else:
             state.dict[self.key] = value
 
     def fire_replace_event(self, state, value, previous, initiator):
         for ext in self.extensions:
-            ext.set(state, value, previous, initiator or self)
+            value = ext.set(state, value, previous, initiator or self)
+        return value
 
     def fire_remove_event(self, state, value, initiator):
         for ext in self.extensions:
@@ -457,8 +458,8 @@ class MutableScalarAttributeImpl(ScalarAttributeImpl):
 
         if self.extensions:
             old = self.get(state)
+            value = self.fire_replace_event(state, value, old, initiator)
             state.dict[self.key] = value
-            self.fire_replace_event(state, value, old, initiator)
         else:
             state.dict[self.key] = value
 
@@ -483,9 +484,8 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
 
     def delete(self, state):
         old = self.get(state)
-        # TODO: catch key errors, convert to attributeerror?
-        del state.dict[self.key]
         self.fire_remove_event(state, old, self)
+        del state.dict[self.key]
 
     def get_history(self, state, passive=False):
         if self.key in state.dict:
@@ -510,8 +510,8 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
 
         # may want to add options to allow the get() here to be passive
         old = self.get(state)
+        value = self.fire_replace_event(state, value, old, initiator)
         state.dict[self.key] = value
-        self.fire_replace_event(state, value, old, initiator)
 
     def fire_remove_event(self, state, value, initiator):
         state.modified_event(self, False, value)
@@ -532,7 +532,8 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
                 self.sethasparent(instance_state(previous), False)
 
         for ext in self.extensions:
-            ext.set(state, value, previous, initiator or self)
+            value = ext.set(state, value, previous, initiator or self)
+        return value
 
 
 class CollectionAttributeImpl(AttributeImpl):
@@ -582,7 +583,8 @@ class CollectionAttributeImpl(AttributeImpl):
             self.sethasparent(instance_state(value), True)
 
         for ext in self.extensions:
-            ext.append(state, value, initiator or self)
+            value = ext.append(state, value, initiator or self)
+        return value
 
     def fire_pre_remove_event(self, state, initiator):
         state.modified_event(self, True, NEVER_SET, passive=True)
@@ -624,8 +626,8 @@ class CollectionAttributeImpl(AttributeImpl):
 
         collection = self.get_collection(state, passive=passive)
         if collection is PASSIVE_NORESULT:
+            value = self.fire_append_event(state, value, initiator)
             state.get_pending(self.key).append(value)
-            self.fire_append_event(state, value, initiator)
         else:
             collection.append_with_event(value, initiator)
 
@@ -635,8 +637,8 @@ class CollectionAttributeImpl(AttributeImpl):
 
         collection = self.get_collection(state, passive=passive)
         if collection is PASSIVE_NORESULT:
-            state.get_pending(self.key).remove(value)
             self.fire_remove_event(state, value, initiator)
+            state.get_pending(self.key).remove(value)
         else:
             collection.remove_with_event(value, initiator)
 
@@ -745,7 +747,7 @@ class GenericBackrefExtension(interfaces.AttributeExtension):
 
     def set(self, state, child, oldchild, initiator):
         if oldchild is child:
-            return
+            return child
         if oldchild is not None:
             # With lazy=None, there's no guarantee that the full collection is
             # present when updating via a backref.
@@ -758,11 +760,13 @@ class GenericBackrefExtension(interfaces.AttributeExtension):
         if child is not None:
             new_state = instance_state(child)
             new_state.get_impl(self.key).append(new_state, state.obj(), initiator, passive=True)
-
+        return child
+        
     def append(self, state, child, initiator):
         child_state = instance_state(child)
         child_state.get_impl(self.key).append(child_state, state.obj(), initiator, passive=True)
-
+        return child
+        
     def remove(self, state, child, initiator):
         if child is not None:
             child_state = instance_state(child)
diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py
index f8570dd5f..497ef5941 100644
--- a/lib/sqlalchemy/orm/collections.py
+++ b/lib/sqlalchemy/orm/collections.py
@@ -584,7 +584,9 @@ class CollectionAdapter(object):
 
         """
         if initiator is not False and item is not None:
-            self.attr.fire_append_event(self.owner_state, item, initiator)
+            return self.attr.fire_append_event(self.owner_state, item, initiator)
+        else:
+            return item
 
     def fire_remove_event(self, item, initiator=None):
         """Notify that a entity has been removed from the collection.
@@ -881,11 +883,13 @@ def _instrument_membership_mutator(method, before, argument, after):
 
 def __set(collection, item, _sa_initiator=None):
     """Run set events, may eventually be inlined into decorators."""
+
     if _sa_initiator is not False and item is not None:
         executor = getattr(collection, '_sa_adapter', None)
         if executor:
-            getattr(executor, 'fire_append_event')(item, _sa_initiator)
-
+            item = getattr(executor, 'fire_append_event')(item, _sa_initiator)
+    return item
+    
 def __del(collection, item, _sa_initiator=None):
     """Run del events, may eventually be inlined into decorators."""
     if _sa_initiator is not False and item is not None:
@@ -908,7 +912,7 @@ def _list_decorators():
 
     def append(fn):
         def append(self, item, _sa_initiator=None):
-            __set(self, item, _sa_initiator)
+            item = __set(self, item, _sa_initiator)
             fn(self, item)
         _tidy(append)
         return append
@@ -924,7 +928,7 @@ def _list_decorators():
 
     def insert(fn):
         def insert(self, index, value):
-            __set(self, value)
+            value = __set(self, value)
             fn(self, index, value)
         _tidy(insert)
         return insert
@@ -935,7 +939,7 @@ def _list_decorators():
                 existing = self[index]
                 if existing is not None:
                     __del(self, existing)
-                __set(self, value)
+                value = __set(self, value)
                 fn(self, index, value)
             else:
                 # slice assignment requires __delitem__, insert, __len__
@@ -985,8 +989,7 @@ def _list_decorators():
         def __setslice__(self, start, end, values):
             for value in self[start:end]:
                 __del(self, value)
-            for value in values:
-                __set(self, value)
+            values = [__set(self, value) for value in values]
             fn(self, start, end, values)
         _tidy(__setslice__)
         return __setslice__
@@ -1047,7 +1050,7 @@ def _dict_decorators():
         def __setitem__(self, key, value, _sa_initiator=None):
             if key in self:
                 __del(self, self[key], _sa_initiator)
-            __set(self, value, _sa_initiator)
+            value = __set(self, value, _sa_initiator)
             fn(self, key, value)
         _tidy(__setitem__)
         return __setitem__
@@ -1154,7 +1157,7 @@ def _set_decorators():
     def add(fn):
         def add(self, value, _sa_initiator=None):
             if value not in self:
-                __set(self, value, _sa_initiator)
+                value = __set(self, value, _sa_initiator)
             # testlib.pragma exempt:__hash__
             fn(self, value)
         _tidy(add)
diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 6dd2225c8..495d22be1 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -705,18 +705,35 @@ class AttributeExtension(object):
     """An event handler for individual attribute change events.
     
     AttributeExtension is assembled within the descriptors associated 
-    with a mapped class.
+    with a mapped class. 
     
     """
 
     def append(self, state, value, initiator):
-        pass
+        """Receive a collection append event.
+        
+        The returned value will be used as the actual value to be
+        appended.
+        
+        """
+        return value
 
     def remove(self, state, value, initiator):
+        """Receive a remove event.
+        
+        No return value is defined.
+        
+        """
         pass
 
     def set(self, state, value, oldvalue, initiator):
-        pass
+        """Receive a set event.
+        
+        The returned value will be used as the actual value to be
+        set.
+        
+        """
+        return value
 
 
 class StrategizedOption(PropertyOption):
diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index f4d2b51bd..67a886306 100644
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -46,7 +46,8 @@ class UOWEventHandler(interfaces.AttributeExtension):
             prop = _state_mapper(state).get_property(self.key)
             if prop.cascade.save_update and item not in sess:
                 sess.save_or_update(item)
-
+        return item
+        
     def remove(self, state, item, initiator):
         sess = _state_session(state)
         if sess:
@@ -60,7 +61,7 @@ class UOWEventHandler(interfaces.AttributeExtension):
     def set(self, state, newvalue, oldvalue, initiator):
         # process "save_update" cascade rules for when an instance is attached to another instance
         if oldvalue is newvalue:
-            return
+            return newvalue
         sess = _state_session(state)
         if sess:
             prop = _state_mapper(state).get_property(self.key)
@@ -68,7 +69,7 @@ class UOWEventHandler(interfaces.AttributeExtension):
                 sess.save_or_update(newvalue)
             if prop.cascade.delete_orphan and oldvalue in sess.new:
                 sess.expunge(oldvalue)
-
+        return newvalue
 
 def register_attribute(class_, key, *args, **kwargs):
     """overrides attributes.register_attribute() to add UOW event handlers