Fixed MongoContainer vs IdNamesMongoContainer add and ``_…

…_setitem__`` behavour on ``None`` keys.

CHANGES.txt

@@ -5,7 +5,17 @@ CHANGES
 0.8.3 (unreleased)
 ------------------
 
-- Nothing changed yet.
+- Fixed ``MongoContainer`` vs ``IdNamesMongoContainer``
+  ``add`` and ``__setitem__`` behavour on ``None`` keys.
+  ``ObjectAddedEvent`` or ``ObjectMovedEvent`` were not fired because
+  ``zope.container.contained.setitem`` got the just inserted object back.
+
+  ``MongoContainer`` always requires ``_m_mapping_key`` and uses that attribute
+  of the object to determine the new key.
+
+  ``IdNamesMongoContainer`` requires ``_m_mapping_key`` ``None`` and uses
+  ``_id`` to determine the new key.
+
 
 
 0.8.2 (2013-04-03)

src/mongopersist/zope/container.py

@@ -218,27 +218,36 @@ def __getitem__(self, key):
         return obj
 
     def _real_setitem(self, key, value):
+        # Make sure the value is in the database, since we might want
+        # to use its oid.
+        if value._p_oid is None:
+            self._m_jar.insert(value)
+
         # This call by itself causes the state to change _p_changed to True.
         if self._m_mapping_key is not None:
             setattr(value, self._m_mapping_key, key)
         if self._m_parent_key is not None:
             setattr(value, self._m_parent_key, self._m_get_parent_key_value())
 
     def __setitem__(self, key, value):
-        # Make sure the value is in the database, since we might want to use
-        # its oid.
-        if value._p_oid is None:
-            self._m_jar.insert(value)
-        # When the key is None, we use the object id as name.
+        # When the key is None, we need to determine it.
         if key is None:
-            key = unicode(value._p_oid.id)
+            if self._m_mapping_key is None:
+                # Make sure the value is in the database, since we might want
+                # to use its oid.
+                if value._p_oid is None:
+                    self._m_jar.insert(value)
+                key = unicode(value._p_oid.id)
+            else:
+                # we have _m_mapping_key, use that attribute
+                key = getattr(value, self._m_mapping_key)
         # We want to be as close as possible to using the Zope semantics.
         contained.setitem(self, self._real_setitem, key, value)
 
     def add(self, value, key=None):
         # We are already supporting ``None`` valued keys, which prompts the key
-        # to be the OID. But people felt that a more explicit interface would
-        # be better in this case.
+        # to be determined here. But people felt that a more explicit
+        # interface would be better in this case.
         self[key] = value
 
     def __delitem__(self, key):
@@ -315,6 +324,8 @@ class IdNamesMongoContainer(MongoContainer):
     """A container that uses the Mongo ObjectId as the name/key."""
     _m_mapping_key = None
 
+    def __init__(self, collection=None, database=None, parent_key=None):
+        super(IdNamesMongoContainer, self).__init__(collection, database, parent_key)
 
     @property
     def _m_remove_documents(self):

src/mongopersist/zope/interfaces.py

@@ -110,6 +110,7 @@ def find_one(spec_or_id=None, fields=None, *args, **kwargs):
     def add(value, key=None):
         """Add an object without necessarily knowing the key of the object.
 
-        It is up to the implementation to determine a key, if none is passed
-        in. One approach would be to use the object's OID.
+        Either pass in a valid key or the key will be:
+        - in case _m_mapping_key is None: the object's OID
+        - otherwise getattr(value, _m_mapping_key)
         """

src/mongopersist/zope/tests/test_container.py

@@ -547,13 +547,32 @@ def doctest_MongoContainer_many_items():
       <Person Adam>
 """
 
-def doctest_MongoContainer_setitem_with_no_key():
+def doctest_MongoContainer_setitem_with_no_key_MongoContainer():
     """MongoContainer: __setitem__(None, obj)
 
+    Whenever an item is added with no key, getattr(obj, _m_mapping_key) is used.
+
+      >>> transaction.commit()
+      >>> dm.root['people'] = container.MongoContainer(
+      ...     'person', mapping_key='name')
+      >>> dm.root['people'][None] = Person(u'Stephan')
+
+    Let's now search and receive documents as result:
+
+      >>> sorted(dm.root['people'].keys())
+      [u'...']
+      >>> stephan = dm.root['people'].values()[0]
+      >>> stephan.__name__ == str(stephan.name)
+      True
+"""
+
+def doctest_MongoContainer_setitem_with_no_key_IdNamesMongoContainer():
+    """IdNamesMongoContainer: __setitem__(None, obj)
+
     Whenever an item is added with no key, the OID is used.
 
       >>> transaction.commit()
-      >>> dm.root['people'] = container.MongoContainer('person')
+      >>> dm.root['people'] = container.IdNamesMongoContainer('person')
       >>> dm.root['people'][None] = Person(u'Stephan')
 
     Let's now search and receive documents as result:
@@ -565,15 +584,36 @@ def doctest_MongoContainer_setitem_with_no_key():
       True
 """
 
-def doctest_MongoContainer_add():
+def doctest_MongoContainer_add_MongoContainer():
     """MongoContainer: add(value, key=None)
 
+    Sometimes we just do not want to be responsible to determine the name of
+    the object to be added. This method makes this optional. The default
+    implementation assigns getattr(obj, _m_mapping_key) as name:
+
+      >>> transaction.commit()
+      >>> dm.root['people'] = container.MongoContainer(
+      ...     'person', mapping_key='name')
+      >>> dm.root['people'].add(Person(u'Stephan'))
+
+    Let's now search and receive documents as result:
+
+      >>> sorted(dm.root['people'].keys())
+      [u'...']
+      >>> stephan = dm.root['people'].values()[0]
+      >>> stephan.__name__ == str(stephan.name)
+      True
+"""
+
+def doctest_MongoContainer_add_IdNamesMongoContainer():
+    """IdNamesMongoContainer: add(value, key=None)
+
     Sometimes we just do not want to be responsible to determine the name of
     the object to be added. This method makes this optional. The default
     implementation assigns the OID as name:
 
       >>> transaction.commit()
-      >>> dm.root['people'] = container.MongoContainer('person')
+      >>> dm.root['people'] = container.IdNamesMongoContainer('person')
       >>> dm.root['people'].add(Person(u'Stephan'))
 
     Let's now search and receive documents as result:
@@ -1064,6 +1104,7 @@ def doctest_Realworldish():
 
 
 class People(container.AllItemsMongoContainer):
+    _m_mapping_key = 'name'
     _p_mongo_collection = 'people'
     _m_collection = 'person'
 
@@ -1121,6 +1162,111 @@ def doctest_load_does_not_set_p_changed():
 
     """
 
+
+def doctest_firing_events_MongoContainer():
+    """Events need to be fired when _m_mapping_key is already set on the object
+    and the object gets added to the container
+
+      >>> @zope.component.adapter(zope.component.interfaces.IObjectEvent)
+      ... def eventHandler(event):
+      ...     print event
+
+      >>> zope.component.provideHandler(eventHandler)
+
+    Let's add some objects:
+
+      >>> transaction.commit()
+      >>> dm.root['people'] = people = People()
+      >>> x = transaction.begin()
+      >>> for idx in xrange(2):
+      ...     people[None] = PeoplePerson('Mr Number %.5i' %idx, random.randint(0, 100))
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      >>> transaction.commit()
+      >>> list(people.keys())
+      [u'Mr Number 00000', u'Mr Number 00001']
+
+      >>> for idx in xrange(2):
+      ...     name = 'Mr Number %.5i' % (idx+10, )
+      ...     people.add(PeoplePerson(name, random.randint(0, 100)))
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      >>> transaction.commit()
+      >>> list(people.keys())
+      [u'Mr Number 00000', u'Mr Number 00001', u'Mr Number 00010', u'Mr Number 00011']
+
+      >>> for idx in xrange(2):
+      ...     name = 'Mr Number %.5i' % (idx+20, )
+      ...     people[name] = PeoplePerson(name, random.randint(0, 100))
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      >>> transaction.commit()
+      >>> list(people.keys())
+      [u'Mr Number 00000', u'Mr Number 00001', u'Mr Number 00010', u'Mr Number 00011',
+       u'Mr Number 00020', u'Mr Number 00021']
+
+    """
+
+
+class PeopleWithIDKeys(container.IdNamesMongoContainer):
+    _p_mongo_collection = 'people'
+    _m_collection = 'person'
+
+
+def doctest_firing_events_IdNamesMongoContainer():
+    """Events need to be fired when the object gets added to the container
+
+      >>> @zope.component.adapter(zope.component.interfaces.IObjectEvent)
+      ... def eventHandler(event):
+      ...     print event
+
+      >>> zope.component.provideHandler(eventHandler)
+
+    Let's add some objects:
+
+      >>> transaction.commit()
+      >>> dm.root['people'] = people = PeopleWithIDKeys()
+      >>> x = transaction.begin()
+      >>> for idx in xrange(2):
+      ...     people[None] = PeoplePerson('Mr Number %.5i' %idx, random.randint(0, 100))
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      >>> transaction.commit()
+      >>> list(people.keys())
+      [u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000']
+
+      >>> for idx in xrange(2):
+      ...     name = 'Mr Number %.5i' % (idx+10, )
+      ...     people.add(PeoplePerson(name, random.randint(0, 100)))
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      >>> transaction.commit()
+      >>> list(people.keys())
+      [u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000']
+
+      >>> for idx in xrange(2):
+      ...     name = 'Mr Number %.5i' % (idx+20, )
+      ...     people[name] = PeoplePerson(name, random.randint(0, 100))
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      <zope.lifecycleevent.ObjectAddedEvent object at ...>
+      <zope.container.contained.ContainerModifiedEvent object at ...>
+      >>> transaction.commit()
+      >>> list(people.keys())
+      [u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000', u'4e7ddf12e138237403000000']
+
+    """
+
 checker = renormalizing.RENormalizing([
     (re.compile(r'datetime.datetime(.*)'),
      'datetime.datetime(2011, 10, 1, 9, 45)'),