From a1e11cb5ee310e6fe58c5b3a94db19acd30c3e94 Mon Sep 17 00:00:00 2001 From: Martin Aspeli Date: Sun, 24 May 2009 16:04:49 +0000 Subject: [PATCH] Turn getComments() and getThreads() into generators. svn path=/plone.app.discussion/trunk/; revision=27090 --- plone/app/discussion/conversation.py | 42 ++++++++-- plone/app/discussion/interfaces.py | 43 ++++++---- .../app/discussion/tests/test_conversation.py | 84 +++++++++++++++++-- 3 files changed, 140 insertions(+), 29 deletions(-) diff --git a/plone/app/discussion/conversation.py b/plone/app/discussion/conversation.py index b0a5d05..3ac1fc2 100644 --- a/plone/app/discussion/conversation.py +++ b/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 - return self._comments.values() + count = 0l + for comment in self._comments.values(min=start): + yield comment + + count += 1 + if size and count > size: + return - def getThreads(self, start=0, size=None, root=None, depth=None): + 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 diff --git a/plone/app/discussion/interfaces.py b/plone/app/discussion/interfaces.py index 9c98a23..bb9e77b 100644 --- a/plone/app/discussion/interfaces.py +++ b/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' - parameter is the id of the comment from which to start the batch. + """Return an iterator of comment objects for rendering. + + 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): - """Return a batch of comment objects for rendering. The 'start' - 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 - batch. 'root', if given, is the id of the comment to which reply - threads will be found. If not given, all threads are returned. + def getThreads(start=0, size=None, root=0, depth=None): + """Return a batch of comment objects for rendering. + + 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. + 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)', - where 'children' is a similar list of (comment, children), or an empty - list of a comment has no direct replies. - - 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. + Comments are returned as an iterator of dicts with keys 'comment', + the comment, 'id', the comment id, and 'depth', which is 0 for + top-level comments, 1 for their replies, and so on. The list is + returned in depth-first order. """ class IReplies(IIterableMapping): diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py index 9f9bd28..4c6aa9f 100644 --- a/plone/app/discussion/tests/test_conversation.py +++ b/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):