Moved comment.list_to_root() to Bug.add_comments() with some cleanups.

This makes Bug.add_comment simpler.  Also makes Bug.from_xml() more
robust, since it no longer depends on the order in which the XML file
lists the comments.  The previous Bug.from_xml() would have choked on
  <be-xml>
    <bug>
      <comment>
        <uuid>B</uuid>
        <in-reply-to>A</in-reply-to>
      </comment>
      <comment>
        <uuid>A</uuid>
      </comment>
    </bug>
  </be-xml>
because when B was being added, the referenced A hadn't yet been
noticed.

2 files changed, 45 insertions, 65 deletions

diff --git a/libbe/bug.py b/libbe/bug.py
index 5f2cf54..d3dbe2d 100644
--- a/libbe/bug.py
+++ b/libbe/bug.py
@@ -228,7 +228,7 @@ class Bug(settings_object.SavedSettingsObject):
     @Property
     @cached_property(generator=_get_comment_root)
     @local_property("comment_root")
-    @doc_property(doc="The trunk of the comment tree")
+    @doc_property(doc="The trunk of the comment tree.  We use a dummy root comment by default, because there can be several comment threads rooted on the same parent bug.  To simplify comment interaction, we condense these threads into a single thread with a Comment dummy root.")
     def comment_root(): return {}
 
     def _get_vcs(self):
@@ -355,13 +355,14 @@ class Bug(settings_object.SavedSettingsObject):
         self.explicit_attrs = []
         uuid = None
         estrs = []
+        comments = []
         for child in bug.getchildren():
             if child.tag == 'short-name':
                 pass
             elif child.tag == 'comment':
                 comm = comment.Comment(bug=self)
                 comm.from_xml(child)
-                self.add_comment(comm)
+                comments.append(comm)
                 continue
             elif child.tag in tags:
                 if child.text == None or len(child.text) == 0:
@@ -385,8 +386,9 @@ class Bug(settings_object.SavedSettingsObject):
             if not hasattr(self, 'alt_id') or self.alt_id == None:
                 self.alt_id = uuid
         self.extra_strings = estrs
+        self.add_comments(comments)
 
-    def add_comment(self, new_comment):
+    def add_comment(self, comment, *args, **kwargs):
         """
         Add a comment too the current bug, under the parent specified
         by comment.in_reply_to.
@@ -438,22 +440,47 @@ class Bug(settings_object.SavedSettingsObject):
           </comment>
         </bug>
         """
+        self.add_comments([comment], **kwargs)
+
+    def add_comments(self, comments, default_parent=None,
+                     ignore_missing_references=False):
+        """
+        Convert a raw list of comments to single root comment.  If a
+        comment does not specify a parent with .in_reply_to, the
+        parent defaults to .comment_root, but you can specify another
+        default parent via default_parent.
+        """
         uuid_map = {}
-        for c in self.comments():
+        if default_parent == None:
+            default_parent = self.comment_root
+        for c in list(self.comments()) + comments:
+            assert c.uuid != None
+            assert c.uuid not in uuid_map
             uuid_map[c.uuid] = c
             if c.alt_id != None:
                 uuid_map[c.alt_id] = c
-        assert new_comment.uuid not in uuid_map
-        if new_comment.alt_id != None:
-            assert new_comment.alt_id not in uuid_map
-        if new_comment.in_reply_to == comment.INVALID_UUID:
-            new_comment.in_reply_to = None
-        if new_comment.in_reply_to == None:
-            parent = self.comment_root
-        else:
-            parent = uuid_map[new_comment.in_reply_to]
-        new_comment.bug = self
-        parent.append(new_comment)
+        uuid_map[None] = self.comment_root
+        if default_parent != self.comment_root:
+            assert default_parent.uuid in uuid_map, default_parent
+        for c in comments:
+            if c.in_reply_to == None \
+                    and default_parent.uuid != comment.INVALID_UUID:
+                c.in_reply_to = default_parent.uuid
+            elif c.in_reply_to == comment.INVALID_UUID:
+                c.in_reply_to = None
+            try:
+                parent = uuid_map[c.in_reply_to]
+            except KeyError:
+                if ignore_missing_references == True:
+                    print >> sys.stderr, \
+                        "Ignoring missing reference to %s" % c.in_reply_to
+                    parent = default_parent
+                    if parent.uuid != comment.INVALID_UUID:
+                        c.in_reply_to = parent.uuid
+                else:
+                    raise comment.MissingReference(c)
+            c.bug = self
+            parent.append(c)
 
     def merge(self, other, allow_changes=True, allow_new_comments=True):
         """
@@ -545,10 +572,8 @@ class Bug(settings_object.SavedSettingsObject):
                         % (o_comm.uuid, o_comm.alt_id, self.uuid)
                 o_comm_copy = copy.copy(o_comm)
                 o_comm_copy.bug = self
-                print >> sys.stderr, "add comment %s" % o_comm.uuid
                 self.comment_root.add_reply(o_comm_copy)
             else:
-                print >> sys.stderr, "merge comment %s into %s" % (o_comm.uuid, s_comm.uuid)
                 s_comm.merge(o_comm, allow_changes=allow_changes)
 
     def string(self, shortlist=False, show_comments=False):
diff --git a/libbe/comment.py b/libbe/comment.py
index e3dfea0..45134e0 100644
--- a/libbe/comment.py
+++ b/libbe/comment.py
@@ -65,53 +65,6 @@ class DiskAccessRequired (Exception):
 
 INVALID_UUID = "!!~~\n INVALID-UUID \n~~!!"
 
-def list_to_root(comments, bug, root=None,
-                 ignore_missing_references=False):
-    """
-    Convert a raw list of comments to single root comment.  We use a
-    dummy root comment by default, because there can be several
-    comment threads rooted on the same parent bug.  To simplify
-    comment interaction, we condense these threads into a single
-    thread with a Comment dummy root.  Can also be used to append
-    a list of subcomments to a non-dummy root comment, so long as
-    all the new comments are descendants of the root comment.
-    
-    No Comment method should use the dummy comment.
-    """
-    root_comments = []
-    uuid_map = {}
-    for comment in comments:
-        assert comment.uuid != None
-        uuid_map[comment.uuid] = comment
-    for comment in comments:
-        if comment.alt_id != None and comment.alt_id not in uuid_map:
-            uuid_map[comment.alt_id] = comment
-    if root == None:
-        root = Comment(bug, uuid=INVALID_UUID)
-    else:
-        uuid_map[root.uuid] = root
-    for comm in comments:
-        if comm.in_reply_to == INVALID_UUID:
-            comm.in_reply_to = None
-        rep = comm.in_reply_to
-        if rep == None or rep == bug.uuid:
-            root_comments.append(comm)
-        else:
-            parentUUID = comm.in_reply_to
-            try:
-                parent = uuid_map[parentUUID]
-                parent.add_reply(comm)
-            except KeyError, e:
-                if ignore_missing_references == True:
-                    print >> sys.stderr, \
-                        "Ignoring missing reference to %s" % parentUUID
-                    comm.in_reply_to = None
-                    root_comments.append(comm)
-                else:
-                    raise MissingReference(comm)
-    root.extend(root_comments)
-    return root
-
 def loadComments(bug, load_full=False):
     """
     Set load_full=True when you want to load the comment completely
@@ -132,7 +85,9 @@ def loadComments(bug, load_full=False):
             comm.load_settings()
             dummy = comm.body # force the body to load
         comments.append(comm)
-    return list_to_root(comments, bug)
+    bug.comment_root = Comment(bug, uuid=INVALID_UUID)
+    bug.add_comments(comments)
+    return bug.comment_root
 
 def saveComments(bug):
     if bug.sync_with_disk == False: