Keep the reply graph acyclic

2 files changed, 35 insertions, 1 deletions

diff --git a/kittystore/analysis.py b/kittystore/analysis.py
index 0f49b78..9e3471a 100644
--- a/kittystore/analysis.py
+++ b/kittystore/analysis.py
@@ -37,11 +37,14 @@ def compute_thread_order_and_depth(thread):
         thread_pos["d"] += 1
         thread_pos["o"] += 1
         for succ in sorted(graph.successors(msgid),
-                        key=lambda m: graph.node[m]["num"]):
+                           key=lambda m: graph.node[m]["num"]):
             walk_successors(succ)
         thread_pos["d"] -= 1
     for index, email in enumerate(thread.emails):
         graph.add_node(email.message_id, num=index, obj=email)
         if email.in_reply_to is not None:
             graph.add_edge(email.in_reply_to, email.message_id)
+            if not nx.is_directed_acyclic_graph(graph):
+                # I don't want reply loops in my graph, thank you very much
+                graph.remove_edge(email.in_reply_to, email.message_id)
     walk_successors(thread.starting_email.message_id)
diff --git a/kittystore/test/test_analysis.py b/kittystore/test/test_analysis.py
index 16d0d01..bcaf3bc 100644
--- a/kittystore/test/test_analysis.py
+++ b/kittystore/test/test_analysis.py
@@ -127,3 +127,34 @@ class TestThreadOrderDepth(unittest.TestCase):
         self.assertEqual(msg3.thread_depth, 1)
         self.assertEqual(msg4.thread_order, 2)
         self.assertEqual(msg4.thread_depth, 2)
+
+    def test_reply_to_oneself(self):
+        # A message replying to itself (yes, it's been spotted in the wild)
+        thread = Thread("example-list", "<msg1>")
+        self.store.db.add(thread)
+        msg1 = make_fake_email(1)
+        msg1.in_reply_to = u"<msg1>"
+        msg1.thread_order = msg1.thread_depth = 42
+        self.store.db.add(msg1)
+        self.store.flush()
+        compute_thread_order_and_depth(thread)
+        # Don't traceback with a "maximum recursion depth exceeded" error
+        self.assertEqual(msg1.thread_order, 0)
+        self.assertEqual(msg1.thread_depth, 0)
+
+    def test_reply_loops(self):
+        """Loops in message replies"""
+        # This implies that someone replies to a message not yet sent, but you
+        # never know, Dr Who can be on your mailing-list.
+        thread = Thread("example-list", "<msg1>")
+        self.store.db.add(thread)
+        msg1 = make_fake_email(1)
+        msg1.in_reply_to = u"<msg2>"
+        self.store.db.add(msg1)
+        msg2 = make_fake_email(2)
+        msg2.thread_id = u"<msg1>"
+        msg2.in_reply_to = u"<msg1>"
+        self.store.db.add(msg2)
+        self.store.flush()
+        compute_thread_order_and_depth(thread)
+        # Don't traceback with a "maximum recursion depth exceeded" error