Turn getComments() and getThreads() into generators.

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

plone/app/discussion/conversation.py

@@ -69,7 +69,7 @@ class Conversation(Traversable, Persistent, Explicit):
         # id -> comment - find comment by id
         self._comments = LOBTree()
 
-        # id -> IISet (children) - find all children for a given comment. 0 signifies root.
+        # id -> LLSet (children) - find all children for a given comment. 0 signifies root.
         self._children = LOBTree()
 
     def getId(self):
@@ -101,14 +101,44 @@ class Conversation(Traversable, Persistent, Explicit):
     def getComments(self, start=0, size=None):
         """Get unthreaded comments
         """
-        # TODO - batching
+        count = 0l
-        return self._comments.values()
+        for comment in self._comments.values(min=start):
+            yield comment
             
-    def getThreads(self, start=0, size=None, root=None, depth=None):
+            count += 1
+            if size and count > size:
+                return
+
+    def getThreads(self, start=0, size=None, root=0, depth=None):
         """Get threaded comments
         """
-        # TODO - build threads
+        
-        return []
+        def recurse(comment_id, d=0):
+            # Yield the current comment before we look for its children
+            yield {'id': comment_id, 'comment': self._comments[comment_id], 'depth': d}
+            
+            # Recurse if there are children and we are not out of our depth
+            if depth is None or d + 1 < depth:
+                children = self._children.get(comment_id, None)
+                if children is not None:
+                    for child_id in children:
+                        for value in recurse(child_id, d+1):
+                            yield value
+        
+        # Find top level threads
+        comments = self._children.get(root, None)
+        if comments is not None:
+            count = 0l
+            for comment_id in comments.keys(min=start):
+                
+                # Abort if we have found all the threads we want
+                count += 1
+                if size and count > size:
+                    return
+                
+                # Let the closure recurse
+                for value in recurse(comment_id):
+                    yield value
 
     def addComment(self, comment):
         """Add a new comment. The parent id should have been set already. The

plone/app/discussion/interfaces.py

@@ -53,31 +53,42 @@ class IConversation(IIterableMapping):
         """
     
     def getComments(start=0, size=None):
-        """Return a batch of comment objects for rendering. The 'start'
+        """Return an iterator of comment objects for rendering.
-        parameter is the id of the comment from which to start the batch.
+        
+        The 'start' parameter is the id of the comment from which to start the
+        batch. If no such comment exists, the next higher id will be used.
+        This means that you can use max key from a previous batch + 1 safely.
+        
         The 'size' parameter is the number of comments to return in the
         batch.
         
-        The comments are returned in creation date order, in the exact batche
+        The comments are returned in creation date order, in the exact batch
         size specified.
         """
     
-    def getThreads(start=0, size=None, root=None, depth=None):
+    def getThreads(start=0, size=None, root=0, depth=None):
-        """Return a batch of comment objects for rendering. The 'start'
+        """Return a batch of comment objects for rendering.
-        parameter is the id of the comment from which to start the batch.
+        
-        The 'size' parameter is the number of comments to return in the
+        The 'start' parameter is the id of the comment from which to start
-        batch. 'root', if given, is the id of the comment to which reply
+        the batch. If no such comment exists, the next higher id will be used.
-        threads will be found. If not given, all threads are returned.
+        This means that you can use max key from a previous batch + 1 safely.
+        This should be a root level comment.
+        
+        The 'size' parameter is the number of threads to return in the
+        batch. Full threads are always returned (although you can stop
+        consuming the iterator if you want to abort).
+        
+        'root', if given, is the id of the comment to which reply
+        threads will be found. 'root' itself is not included. If not given,
+        all threads are returned.
+        
         If 'depth' is given, it can be used to limit the depth of threads
         returned. For example, depth=1 will return only direct replies.
         
-        Comments are returned as a recursive list of '(comment, children)', 
+        Comments are returned as an iterator of dicts with keys 'comment',
-        where 'children' is a similar list of (comment, children), or an empty
+        the comment, 'id', the comment id, and 'depth', which is 0 for
-        list of a comment has no direct replies.
+        top-level comments, 1 for their replies, and so on. The list is
-        
+        returned in depth-first order.
-        The returned number of comments may be bigger than the batch size,
-        in order to give enough context to show the full  lineage of the
-        starting comment.
         """
 
 class IReplies(IIterableMapping):

plone/app/discussion/tests/test_conversation.py

@@ -10,8 +10,6 @@ from plone.app.discussion.tests.layer import DiscussionLayer
 
 from plone.app.discussion.interfaces import IConversation, IComment, IReplies
 
-from plone.app.discussion.conversation import ConversationReplies
-
 class ConversationTest(PloneTestCase):
 
     layer = DiscussionLayer
@@ -44,7 +42,7 @@ class ConversationTest(PloneTestCase):
         self.assert_(IComment.providedBy(conversation[new_id]))
         self.assertEquals(aq_base(conversation[new_id].__parent__), aq_base(conversation))
         self.assertEquals(new_id, comment.comment_id)
-        self.assertEquals(len(conversation.getComments()), 1)
+        self.assertEquals(len(list(conversation.getComments())), 1)
         # XXX: not yet implemented
         # self.assertEquals(len(conversation.getThreads()), 1)
         self.assertEquals(conversation.total_comments, 1)
@@ -68,7 +66,7 @@ class ConversationTest(PloneTestCase):
         new_id = conversation.addComment(comment)
 
         # make sure the comment has been added
-        self.assertEquals(len(conversation.getComments()), 1)
+        self.assertEquals(len(list(conversation.getComments())), 1)
         # XXX: not yet implemented
         # self.assertEquals(len(conversation.getThreads()), 1)
         self.assertEquals(conversation.total_comments, 1)
@@ -77,7 +75,7 @@ class ConversationTest(PloneTestCase):
         del conversation[new_id]
 
         # make sure there is no comment left in the conversation
-        self.assertEquals(len(conversation.getComments()), 0)
+        self.assertEquals(len(list(conversation.getComments())), 0)
 
         # XXX: not yet implemented
         # self.assertEquals(len(conversation.getThreads()), 0)
@@ -293,16 +291,88 @@ class ConversationTest(PloneTestCase):
         self.assert_(conversation.last_comment_date < datetime.now() - timedelta(days=3, hours=23, minutes=59, seconds=59))
         self.assert_(conversation.last_comment_date > datetime.now() - timedelta(days=4, seconds=1))
 
-    def test_get_comments_flat(self):
+    def test_get_comments_full(self):
         pass
 
     def test_get_comments_batched(self):
         pass
 
     def test_get_threads(self):
-        pass
+        
+        # Create a conversation. In this case we doesn't assign it to an
+        # object, as we just want to check the Conversation object API.
+        conversation = IConversation(self.portal.doc1)
+
+        # Pretend that we have traversed to the comment by aq wrapping it.
+        conversation = conversation.__of__(self.portal.doc1)
+
+        replies = IReplies(conversation)
+
+        # Create a nested comment structure:
+        #
+        # Conversation
+        # +- Comment 1
+        #    +- Comment 1_1
+        #    |  +- Comment 1_1_1
+        #    +- Comment 1_2
+        # +- Comment 2
+        #    +- Comment 2_1
+
+        # Create all comments
+        comment1 = createObject('plone.Comment')
+        comment1.title = 'Comment 1'
+        comment1.text = 'Comment text'
+
+        comment1_1 = createObject('plone.Comment')
+        comment1_1.title = 'Re: Comment 1'
+        comment1_1.text = 'Comment text'
+
+        comment1_1_1 = createObject('plone.Comment')
+        comment1_1_1.title = 'Re: Re: Comment 1'
+        comment1_1_1.text = 'Comment text'
+
+        comment1_2 = createObject('plone.Comment')
+        comment1_2.title = 'Re: Comment 1 (2)'
+        comment1_2.text = 'Comment text'
+
+        comment2 = createObject('plone.Comment')
+        comment2.title = 'Comment 2'
+        comment2.text = 'Comment text'
+
+        comment2_1 = createObject('plone.Comment')
+        comment2_1.title = 'Re: Comment 2'
+        comment2_1.text = 'Comment text'
+
+        # Create the nested comment structure
+        new_id_1 = conversation.addComment(comment1)
+        new_id_2 = conversation.addComment(comment2)
+        
+        comment1_1.in_reply_to = new_id_1
+        new_id_1_1 = conversation.addComment(comment1_1)
+        
+        comment1_1_1.in_reply_to = new_id_1_1
+        new_id_1_1_1 = conversation.addComment(comment1_1_1)
+        
+        comment1_2.in_reply_to = new_id_1
+        new_id_1_2 = conversation.addComment(comment1_2)
+        
+        comment2_1.in_reply_to = new_id_2
+        new_id_2_1 = conversation.addComment(comment2_1)
+        
+        # Get threads
+
+        self.assertEquals(
+            [{'comment': comment1,     'depth': 0, 'id': new_id_1},
+             {'comment': comment1_1,   'depth': 1, 'id': new_id_1_1},
+             {'comment': comment1_1_1, 'depth': 2, 'id': new_id_1_1_1},
+             {'comment': comment1_2,   'depth': 1, 'id': new_id_1_2},
+             {'comment': comment2,     'depth': 0, 'id': new_id_2},
+             {'comment': comment2_1,   'depth': 1, 'id': new_id_2_1},
+            ], list(conversation.getThreads()))
 
     def test_get_threads_batched(self):
+        # TODO: test start, size, root and depth arguments to getThreads()
+        #   - may want to split this into multiple tests
         pass
 
     def test_traversal(self):