- [feature] Adding/removing None from a mapped collection

now generates attribute events.  Previously, a None
append would be ignored in some cases.  Related
to [ticket:2229].

- [feature] The presence of None in a mapped collection
now raises an error during flush.   Previously,
None values in collections would be silently ignored.
[ticket:2229]

4 files changed, 23 insertions, 13 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index d26ee61c3..08e536f71 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -1022,6 +1022,9 @@ def backref_listeners(attribute, key, uselist):
         return child
 
     def emit_backref_from_collection_append_event(state, child, initiator):
+        if child is None:
+            return
+
         child_state, child_dict = instance_state(child), \
                                     instance_dict(child)
         child_impl = child_state.manager[key].impl
@@ -1207,7 +1210,6 @@ class History(History):
     def from_collection(cls, attribute, state, current):
         original = state.committed_state.get(attribute.key, _NO_HISTORY)
         current = getattr(current, '_sa_adapter')
-
         if original is NO_VALUE:
             return cls(list(current), (), ())
         elif original is _NO_HISTORY:
diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py
index 93b99d83c..a57ef5e68 100644
--- a/lib/sqlalchemy/orm/collections.py
+++ b/lib/sqlalchemy/orm/collections.py
@@ -716,7 +716,7 @@ class CollectionAdapter(object):
         operation.
 
         """
-        if initiator is not False and item is not None:
+        if initiator is not False:
             if self.invalidated:
                 self._warn_invalidated()
             return self.attr.fire_append_event(
@@ -734,7 +734,7 @@ class CollectionAdapter(object):
         an initiator value from a chained operation.
 
         """
-        if initiator is not False and item is not None:
+        if initiator is not False:
             if self.invalidated:
                 self._warn_invalidated()
             self.attr.fire_remove_event(
@@ -1032,7 +1032,7 @@ 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:
+    if _sa_initiator is not False:
         executor = getattr(collection, '_sa_adapter', None)
         if executor:
             item = getattr(executor, 'fire_append_event')(item, _sa_initiator)
@@ -1040,7 +1040,7 @@ def __set(collection, item, _sa_initiator=None):
 
 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:
+    if _sa_initiator is not False:
         executor = getattr(collection, '_sa_adapter', None)
         if executor:
             getattr(executor, 'fire_remove_event')(item, _sa_initiator)
diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index 881a7bb62..5c9efb398 100644
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -247,7 +247,11 @@ class DependencyProcessor(object):
                 self.mapper in uowcommit.mappers
 
     def _verify_canload(self, state):
-        if state is not None and \
+        if self.prop.uselist and state is None:
+            raise exc.FlushError(
+                    "Can't flush None value found in "
+                    "collection %s" % (self.prop, ))
+        elif state is not None and \
             not self.mapper._canload(state,
                             allow_subtypes=not self.enable_typechecks):
             if self.mapper._canload(state, allow_subtypes=True):
@@ -559,10 +563,10 @@ class OneToManyDP(DependencyProcessor):
                             pks_changed):
         source = state
         dest = child
+        self._verify_canload(child)
         if dest is None or \
                 (not self.post_update and uowcommit.is_deleted(dest)):
             return
-        self._verify_canload(child)
         if clearkeys:
             sync.clear(dest, self.mapper, self.prop.synchronize_pairs)
         else:
@@ -1032,8 +1036,7 @@ class ManyToManyDP(DependencyProcessor):
                                                 passive)
             if history:
                 for child in history.added:
-                    if child is None or \
-                            (processed is not None and
+                    if (processed is not None and
                                 (state, child) in processed):
                         continue
                     associationrow = {}
@@ -1044,8 +1047,7 @@ class ManyToManyDP(DependencyProcessor):
                         continue
                     secondary_insert.append(associationrow)
                 for child in history.deleted:
-                    if child is None or \
-                            (processed is not None and
+                    if (processed is not None and
                             (state, child) in processed):
                         continue
                     associationrow = {}
@@ -1130,6 +1132,8 @@ class ManyToManyDP(DependencyProcessor):
         if associationrow is None:
             return
 
+        self._verify_canload(child)
+
         if child is not None and not uowcommit.session._contains_state(child):
             if not child.deleted:
                 util.warn(
@@ -1138,8 +1142,6 @@ class ManyToManyDP(DependencyProcessor):
                     (mapperutil.state_class_str(child), operation, self.prop))
             return False
 
-        self._verify_canload(child)
-
         sync.populate_dict(state, self.parent, associationrow,
                                         self.prop.synchronize_pairs)
         sync.populate_dict(child, self.mapper, associationrow,
diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index c0423939f..84c9f647c 100644
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -29,6 +29,9 @@ def track_cascade_events(descriptor, prop):
         # process "save_update" cascade rules for when
         # an instance is appended to the list of another instance
 
+        if item is None:
+            return
+
         sess = sessionlib._state_session(state)
         if sess:
             prop = state.manager.mapper._props[key]
@@ -40,6 +43,9 @@ def track_cascade_events(descriptor, prop):
         return item
 
     def remove(state, item, initiator):
+        if item is None:
+            return
+
         sess = sessionlib._state_session(state)
         if sess:
             prop = state.manager.mapper._props[key]