- specify dependencies

- set profile version to 1 (profile version != package version)
- complete dict APIs and IReplies adapters (not yet fully tested)
- tidy up addComment() and __delitem__ w.r.t events
- sync interfaces with actual code
- move tests to use collective.testcaselayer

svn path=/plone.app.discussion/trunk/; revision=27010

plone/app/discussion/TODO.txt

@@ -0,0 +1,37 @@
+==========================
+plone.app.discussion to-do
+==========================
+
+ [ ] Thread building in conversation.getThreads()
+ [ ] Batching in conversation.getComments()
+
+ [ ] ++comments++ namespace traversal adapter
+
+ [ ] Acquisition wrapping - should it happen on:
+ 
+    - Conversation retrieval methods (get, __getitem__, values etc)?
+    - IConversation adapter lookup?
+
+ [ ] Implement plone.indexer indexers for comments, filling standard metadata
+    
+    - Note discrepancy between Python datetime and indexing expecting a Zope 2
+        DateTime field
+
+ [ ] Implement plone.indexer indexers for commented-upon content
+ 
+    - Unique set of commentators
+    - Number of comments
+    - Date/time of most recent comment
+    
+    Needs to reindex when comment is added/removed (IContainerModifiedEvent)
+ 
+ [ ] Add tests for conversation dict API
+ [ ] Add tests for IReplies adapters
+ 
+ [ ] Make sure a catalog Clear & Rebuild doesn't lose all comments
+
+ [ ] Add BBB support for the existing portal_discussion interface
+    
+    - implement in BBB package
+    - mix into tool.CommentingTool
+    - emit deprecation warnings

plone/app/discussion/comment.py

@@ -1,6 +1,5 @@
 """The default comment class and factory.
 """
-import time
 from datetime import datetime
 from zope.interface import implements
 from zope.component.factory import Factory
@@ -26,8 +25,8 @@ class Comment(Explicit, Traversable, RoleManager, Owned):
     
     __parent__ = None
     
-    comment_id = None # int
+    comment_id = None # long
-    in_reply_to = None # int
+    in_reply_to = None # long
 
     title = u""
     
@@ -44,7 +43,7 @@ class Comment(Explicit, Traversable, RoleManager, Owned):
     author_email = None
     
     def __init__(self, conversation=None, **kw):
-        self.comment_id = long(time.time() * 1e6)
+        self.comment_id = None # will be set by IConversation.addComment()
 
         self.__parent__ = conversation
         self.creation_date = self.modification_date = datetime.now()
@@ -54,11 +53,11 @@ class Comment(Explicit, Traversable, RoleManager, Owned):
 
     @property
     def __name__(self):
-        return unicode(self.comment_id)
+        return self.comment_id and unicode(self.comment_id) or None
     
     @property
     def id(self):
-        return str(self.comment_id)
+        return self.comment_id and str(self.comment_id) or None
     
     def getId(self):
         """The id of the comment, as a string

plone/app/discussion/configure.zcml

@@ -10,9 +10,10 @@
     <genericsetup:registerProfile
         name="default"
         title="Plone Discussions"
+        description="Commenting infrastructure for Plone"
         directory="profiles/default"
-      description="Setup plone.app.discussions."
         provides="Products.GenericSetup.interfaces.EXTENSION"
+        for="Products.CMFPlone.interfaces.IPloneSiteRoot"
         />
     
     <!-- Comments -->

plone/app/discussion/conversation.py

@@ -10,6 +10,8 @@ manipulating the comments directly in reply to a particular comment or at the
 top level of the conversation.
 """
 
+import time
+
 from persistent import Persistent
 
 from zope.interface import implements, implementer
@@ -17,18 +19,22 @@ from zope.component import adapts, adapter
 from zope.annotation.interfaces import IAnnotations
 
 from zope.event import notify
-from zope.app.container.interfaces import IObjectAddedEvent
+
+from Acquisition import Explicit
+from OFS.Traversable import Traversable
+
 from OFS.event import ObjectWillBeAddedEvent
 from OFS.event import ObjectWillBeRemovedEvent
+
 from zope.app.container.contained import ContainerModifiedEvent
+
 from zope.app.container.contained import ObjectAddedEvent
 from zope.app.container.contained import ObjectRemovedEvent
 
 from zope.annotation.interfaces import IAnnotatable
 
 from BTrees.OIBTree import OIBTree
-from BTrees.IOBTree import IOBTree
+
-from BTrees.IIBTree import IIBTree, IISet
 try:
     # These exist in new versions, but not in the one that comes with Zope 2.10.
     from BTrees.LOBTree import LOBTree
@@ -37,13 +43,13 @@ except ImportError:
     from BTrees.OOBTree import OOBTree as LOBTree
     from BTrees.OOBTree import OOSet as LLSet
 
-
-from Acquisition import Explicit
 from plone.app.discussion.interfaces import IConversation, IComment, IReplies
 
+from Acquisition import aq_base
+
 ANNO_KEY = 'plone.app.discussion:conversation'
 
-class Conversation(Persistent, Explicit):
+class Conversation(Traversable, Persistent, Explicit):
     """A conversation is a container for all comments on a content object.
     
     It manages internal data structures for comment threading and efficient
@@ -57,7 +63,6 @@ class Conversation(Persistent, Explicit):
         
         # username -> count of comments; key is removed when count reaches 0
         self._commentators = OIBTree()
-        self._last_comment_date = None
         
         # id -> comment - find comment by id
         self._comments = LOBTree()  
@@ -66,55 +71,76 @@ class Conversation(Persistent, Explicit):
         self._children = LOBTree()
         
     def getId(self):
-        """
+        """Get the id of 
         """
         return self.id
     
     @property
     def enabled(self):
-        # TODO
+        # TODO - check __parent__'s settings + global settings
         return True
     
     @property
     def total_comments(self):
-        # TODO
         return len(self._comments)
     
     @property
     def last_comment_date(self):
-        return self._last_comment_date
+        try:
+            return self._comments[self._comments.maxKey()].creation_date
+        except (ValueError, KeyError, AttributeError,):
+            return None
     
     @property
     def commentators(self):
-        # TODO:
+        return self._commentators.keys()
-        return set()
     
     def getComments(self, start=0, size=None):
+        """Get unthreaded comments
+        """
+        # TODO - batching
         return self._comments.values()
     
     def getThreads(self, start=0, size=None, root=None, depth=None):
-        # TODO:
+        """Get threaded comments
+        """
+        # TODO - build threads
         return self._comments.values()
     
     def addComment(self, comment):
-        id = comment.comment_id
+        """Add a new comment. The parent id should have been set already. The
-        if id in self._comments:
+        comment id may be modified to find a free key. The id used will be
-            id = max(self._comments.keys()) + 1
+        returned.
+        """
+        
+        # Make sure we don't have a wrapped object
+        
+        comment = aq_base(comment)
+        
+        id = long(time.time() * 1e6)
+        while id in self._comments:
+            id += 1
+        
+        comment.comment_id = id
         notify(ObjectWillBeAddedEvent(comment, self, id))
         self._comments[id] = comment
-        comment.comment_id = id
         
-        commentator = comment.creator
+        # for logged in users only
+        commentator = comment.author_username
+        if commentator:
             if not commentator in self._commentators:
                 self._commentators[commentator] = 0
             self._commentators[commentator] += 1
         
-        self._last_comment_date = comment.creation_date
-        
         reply_to = comment.in_reply_to
+        if not reply_to:
+            # top level comments are in reply to the faux id 0
+            comment.in_reply_to = reply_to = 0
+        
         if not reply_to in self._children:
             self._children[reply_to] = LLSet()
         self._children[reply_to].insert(id)
+        
         # Notify that the object is added. The object must here be
         # acquisition wrapped or the indexing will fail.
         notify(ObjectAddedEvent(comment.__of__(self), self, id))
@@ -122,27 +148,63 @@ class Conversation(Persistent, Explicit):
         
     # Dict API
     
-    def __getitem__(self, key):
+    def __len__(self):
-        return self._comments[key]
+        return len(self._comments)
     
-    def __setitem__(self, key, value):
+    def __contains__(self, key):
-        # XXX Check that it implements the commenting interface
+        return long(key) in self._comments
-        if value.comment_id in self._comments:
+    
-            raise ValueError("Can not replace an existing comment")
+    # TODO: Should __getitem__, get, __iter__, values(), items() and iter* return aq-wrapped comments?
-        # Note that we ignore the key completely:
+    
-        self.addComment(comment)
+    def __getitem__(self, key):
+        """Get an item by its long key
+        """
+        return self._comments[long(key)]
     
     def __delitem__(self, key):
-        # TODO unindex everything
+        """Delete an item by its long key
-        return self._comments.remove(key)
+        """
+        
+        key = long(key)
+        
+        comment = self[key]
+        commentator = comment.author_username
+        
+        notify(ObjectWillBeRemovedEvent(comment, self, key))
+        self._comments.remove(key)
+        notify(ObjectRemovedEvent(comment, self, key))
+        
+        if commentator and commentator in self._commentators:
+            if self._commentators[commentator] <= 1:
+                del self._commentators[commentator]
+            else:
+                self._commentators[commentator] -= 1
+        
+        notify(ContainerModifiedEvent(self))
+    
+    def __iter__(self):
+        return iter(self._comments)
+    
+    def get(self, key, default=None):
+        return self._comments.get(long(key), default)
     
     def keys(self):
         return self._comments.keys()
     
-    def getPhysicalPath(self):
+    def items(self):
-        return self.aq_parent.getPhysicalPath() + (self.id,)
+        return self._comments.items()
     
-    # TODO: Update internal data structures when items added or removed
+    def values(self):
+        return self._comments.values()
+    
+    def iterkeys(self):
+        return self._comments.iterkeys()
+    
+    def itervalues(self):
+        return self._comments.itervalues()
+    
+    def iteritems(self):
+        return self._comments.iteritems()
 
 @implementer(IConversation)
 @adapter(IAnnotatable)
@@ -155,7 +217,6 @@ def conversationAdapterFactory(content):
         conversation._parent_uid = content.UID()
         annotions[ANNO_KEY] = conversation
     conversation = annotions[ANNO_KEY]
-    # Probably this needs an acquisition wrapper
     return conversation
 
 class ConversationReplies(object):
@@ -165,14 +226,70 @@ class ConversationReplies(object):
     """
     
     implements(IReplies)
-    adapts(Conversation)
+    adapts(Conversation) # relies on implementation details
     
     def __init__(self, context):
         self.conversation = context
-        self.root = 0
+        self.children = self.conversation._children.get(0, LLSet())
     
-    # TODO: dict interface - generalise to work with any starting point, so
+    def addComment(self, comment):
-    # that the subclassing below works
+        comment.in_reply_to = None
+        return self.conversation.addComment(comment)
+        
+    # Dict API
+    
+    def __len__(self):
+        return len(self.children)
+    
+    def __contains__(self, key):
+        return long(key) in self.children
+    
+    # TODO: Should __getitem__, get, __iter__, values(), items() and iter* return aq-wrapped comments?
+    
+    def __getitem__(self, key):
+        """Get an item by its long key
+        """
+        key = long(key)
+        if key not in self.children:
+            raise KeyError(key)
+        return self.conversation[key]
+    
+    def __delitem__(self, key):
+        """Delete an item by its long key
+        """
+        key = long(key)
+        if key not in self.children:
+            raise KeyError(key)
+        del self.conversation[key]
+    
+    def __iter__(self):
+        return iter(self.children)
+    
+    def get(self, key, default=None):
+        key = long(key)
+        if key not in self.children:
+            return default
+        return self.conversation.get(key)
+    
+    def keys(self):
+        return self.children
+    
+    def items(self):
+        return [(k, self.conversation[k]) for k in self.children]
+    
+    def values(self):
+        return [self.conversation[k] for k in self.children]
+    
+    def iterkeys(self):
+        return iter(self.children)
+    
+    def itervalues(self):
+        for key in self.children:
+            yield self.conversation[key]
+    
+    def iteritems(self):
+        for key in self.children:
+            yield (key, self.conversation[key],)
 
 class CommentReplies(ConversationReplies):
     """An IReplies adapter for comments.
@@ -184,5 +301,12 @@ class CommentReplies(ConversationReplies):
     adapts(IComment)
     
     def __init__(self, context):
-        self.conversation = context.__parent__
+        self.comment = context
-        self.root = context.comment_id
+        self.comment_id = context.comment_id
+        self.children = self.conversation._children.get(0, LLSet())
+    
+    def addComment(self, comment):
+        comment.in_reply_to = self.comment_id
+        return self.conversation.addComment(comment)
+    
+    # Dict API is inherited

plone/app/discussion/interfaces.py

@@ -1,5 +1,5 @@
 from zope.interface import Interface
-from zope.interface.common.mapping import IIterableMapping, IWriteMapping
+from zope.interface.common.mapping import IIterableMapping
 from zope import schema
 
 from zope.i18nmessageid import MessageFactory
@@ -22,13 +22,16 @@ class IDiscussionSettings(Interface):
                                                 "the standard search tools."),
                                  default=True)
 
-class IConversation(IIterableMapping, IWriteMapping):
+class IConversation(IIterableMapping):
     """A conversation about a content object.
     
     This is a persistent object in its own right and manages all comments.
     
     The dict interface allows access to all comments. They are stored by
-    integer key, in the order they were added.
+    long integer key, in the order they were added.
+    
+    Note that __setitem__() is not supported - use addComment() instead.
+    However, comments can be deleted using __delitem__().
     
     To get replies at the top level, adapt the conversation to IReplies.
     
@@ -46,6 +49,16 @@ class IConversation(IIterableMapping, IWriteMapping):
     last_comment_date = schema.Date(title=_(u"Date of the most recent comment"), readonly=True)
     commentators = schema.Set(title=_(u"The set of unique commentators (usernames)"), readonly=True)
     
+    def __delitem__(key):
+        """Delete the comment with the given key. The key is a long id.
+        """
+    
+    def addComment(comment):
+        """Adds a new comment to the list of comments, and returns the 
+        comment id that was assigned. The comment_id property on the comment
+        will be set accordingly.
+        """
+    
     def getComments(start=0, size=None):
         """Return a batch of comment objects for rendering. The 'start'
         parameter is the id of the comment from which to start the batch.
@@ -74,12 +87,7 @@ class IConversation(IIterableMapping, IWriteMapping):
         starting comment.
         """
 
-    def addComment(comment):
+class IReplies(IIterableMapping):
-        """Adds a new comment to the list of comments, and returns the 
-        comment id that was assigned.
-        """
-
-class IReplies(IIterableMapping, IWriteMapping):
     """A set of related comments in reply to a given content object or
     another comment.
     
@@ -87,6 +95,16 @@ class IReplies(IIterableMapping, IWriteMapping):
     direct replies.
     """
     
+    def addComment(comment):
+        """Adds a new comment as a child of this comment, and returns the 
+        comment id that was assigned. The comment_id property on the comment
+        will be set accordingly.
+        """
+    
+    def __delitem__(key):
+        """Delete the comment with the given key. The key is a long id.
+        """
+
 class IComment(Interface):
     """A comment.
     
@@ -120,16 +138,30 @@ class IComment(Interface):
 class ICommentingTool(Interface):
     """A tool that indexes all comments for usage by the management interface.
     
-    This was the management interface can still work even though we don't
+    This means the management interface can still work even though we don't
     index the comments in portal_catalog.
+    
+    The default implementation of this interface simply defers to
+    portal_catalog, but a custom version of the tool can be used to provide
+    an alternate indexing mechanism.
     """
     
-    def index(comment):
+    def indexObject(comment):
-        """Indexes a comment"""
+        """Indexes a comment
+        """
         
-    def unindex(comment):
+    def reindexObject(comment):
-        """Removes a comment from the indexes"""
+        """Reindex a comment
+        """
         
-    def search(username=None, wfstate=None):
+    def unindexObject(comment):
-        """Get all comments with a certain username of wfstate"""
+        """Removes a comment from the indexes
+        """
         
+    def uniqueValuesFor(name):
+        """Get unique values for FieldIndex name
+        """
+        
+    def searchResults(REQUEST=None, **kw):
+        """Perform a search over all indexed comments.
+        """

plone/app/discussion/profiles/default/metadata.xml

@@ -1,3 +1,3 @@
 <metadata>
-    <version>1.0a1</version>
+    <version>1</version>
 </metadata>

plone/app/discussion/tests/base.py

@@ -1,29 +0,0 @@
-import unittest
-
-from zope.testing import doctestunit
-from zope.component import testing, getMultiAdapter
-from zope.publisher.browser import TestRequest
-from zope.publisher.interfaces.browser import IBrowserView
-from Testing import ZopeTestCase as ztc
-
-from Products.Five import zcml
-from Products.Five import fiveconfigure
-from Products.PloneTestCase import PloneTestCase as ptc
-from Products.PloneTestCase.layer import PloneSite
-ptc.setupPloneSite(extension_profiles=['plone.app.discussion:default'])
-
-import plone.app.discussion
-
-class TestCase(ptc.PloneTestCase):
-    class layer(PloneSite):
-        @classmethod
-        def setUp(cls):
-            fiveconfigure.debug_mode = True
-            zcml.load_config('configure.zcml',
-                             plone.app.discussion)
-            fiveconfigure.debug_mode = False
-
-        @classmethod
-        def tearDown(cls):
-            pass
-

plone/app/discussion/tests/layer.py

@@ -0,0 +1,12 @@
+from Products.PloneTestCase import ptc
+import collective.testcaselayer.ptc
+
+ptc.setupPloneSite()
+
+class Layer(collective.testcaselayer.ptc.BasePTCLayer):
+    """Install collective.flowplayer"""
+
+    def afterSetUp(self):
+        self.addProfile('plone.app.discussion:default')
+
+DiscussionLayer = Layer([collective.testcaselayer.ptc.ptc_layer])

plone/app/discussion/tests/test_api.py

@@ -1,23 +1,17 @@
 import unittest
 from datetime import datetime, timedelta
-from base import TestCase
 
-from zope.testing import doctestunit
+from Products.PloneTestCase.ptc import PloneTestCase
-from zope.component import testing, getMultiAdapter
+from plone.app.discussion.tests.layer import DiscussionLayer
-from zope.publisher.browser import TestRequest
-from zope.publisher.interfaces.browser import IBrowserView
-from Testing import ZopeTestCase as ztc
-
-from Products.Five import zcml
-from Products.Five import fiveconfigure
-from Products.PloneTestCase import PloneTestCase as ptc
-from Products.PloneTestCase.layer import PloneSite
 
 from plone.app.discussion.conversation import Conversation
 from plone.app.discussion.comment import Comment
 from plone.app.discussion.interfaces import ICommentingTool, IConversation
 
-class ConversationTest(TestCase):
+class ConversationTest(PloneTestCase):
+    
+    layer = DiscussionLayer
+    
     def afterSetUp(self):
         # XXX If we make this a layer, it only get run once...
         # First we need to create some content.
@@ -48,9 +42,4 @@ class ConversationTest(TestCase):
         self.assert_(conversation.last_comment_date - datetime.now() < timedelta(seconds=1))
     
 def test_suite():
-    return unittest.TestSuite([
+    return unittest.defaultTestLoader.loadTestsFromName(__name__)
-        unittest.makeSuite(ConversationTest),
-        ])
-
-if __name__ == '__main__':
-    unittest.main(defaultTest='test_suite')

plone/app/discussion/tests/test_tool.py

@@ -1,23 +1,17 @@
 import unittest
-from datetime import datetime, timedelta
-from base import TestCase
 
-from zope.testing import doctestunit
+from zope.component import getUtility
-from zope.component import testing, getMultiAdapter, getUtility
-from zope.publisher.browser import TestRequest
-from zope.publisher.interfaces.browser import IBrowserView
-from Testing import ZopeTestCase as ztc
 
-from Products.Five import zcml
+from Products.PloneTestCase.ptc import PloneTestCase
-from Products.Five import fiveconfigure
+from plone.app.discussion.tests.layer import DiscussionLayer
-from Products.PloneTestCase import PloneTestCase as ptc
-from Products.PloneTestCase.layer import PloneSite
 
-from plone.app.discussion.conversation import Conversation
 from plone.app.discussion.comment import Comment
 from plone.app.discussion.interfaces import ICommentingTool, IConversation
 
-class ToolTest(TestCase):
+class ToolTest(PloneTestCase):
+    
+    layer = DiscussionLayer
+    
     def afterSetUp(self):
         # XXX If we make this a layer, it only get run once...
         # First we need to create some content.
@@ -47,11 +41,5 @@ class ToolTest(TestCase):
                      " %s results in the search" % len(comment))
         self.assertEquals(comment[0].Title, 'Comment 1')
     
-    
 def test_suite():
-    return unittest.TestSuite([
+    return unittest.defaultTestLoader.loadTestsFromName(__name__)
-        unittest.makeSuite(ToolTest),
-        ])
-
-if __name__ == '__main__':
-    unittest.main(defaultTest='test_suite')

plone/app/discussion/tool.py

@@ -1,9 +1,14 @@
-import time
+"""The portal_discussion tool, usually accessed via
+getUtility(ICommentingTool). The default implementation delegates to the
+standard portal_catalog for indexing comments.
+
+BBB support for the old portal_discussion is provided in the bbb package.
+"""
+
 from zope import interface
 from zope.component import getUtility
 
 from interfaces import ICommentingTool, IComment
-# The commenting tool, which is a local utility
 
 from Products.CMFCore.utils import UniqueObject, getToolByName
 from OFS.SimpleItem import SimpleItem
@@ -41,12 +46,14 @@ class CommentingTool(UniqueObject, SimpleItem):
         """
         catalog = getToolByName(self, 'portal_catalog')
         object_provides = [IComment.__identifier__]
+        
         if 'object_provides' in kw:
             kw_provides = kw['object_provides']
             if isinstance(str, kw_provides):
                 object_provides.append(kw_provides)
             else:
                 object_provides.extend(kw_provides)
+        
         if REQUEST is not None and 'object_provides' in REQUEST.form:
             rq_provides = REQUEST.form['object_provides']
             del REQUEST.form['object_provides']
@@ -54,6 +61,7 @@ class CommentingTool(UniqueObject, SimpleItem):
                 object_provides.append(rq_provides)
             else:
                 object_provides.extend(rq_provides)
+        
         kw['object_provides'] = object_provides
         return catalog.searchResults(REQUEST, **kw)
 

setup.py

@@ -26,8 +26,17 @@ setup(name='plone.app.discussion',
       install_requires=[
           'setuptools',
           'collective.autopermission',
+          'collective.testcaselayer',
+          'plone.indexer',
+          'ZODB3',
+          'zope.interface',
+          'zope.component',
+          'zope.annotation',
+          'zope.event',
+          'zope.app.container', # XXX: eventually should change to zope.container
       ],
       entry_points="""
-      # -*- Entry points: -*-
+      [z3c.autoinclude.plugin]
+      target = plone
       """,
       )