Fix some edge cases of MAM history fetch

- Wait until we receive our own MUC presence to fetch history
- Fix /reconnect weirdness

4 files changed, 83 insertions, 18 deletions

diff --git a/poezio/mam.py b/poezio/mam.py
index edd1e462..ee5b1be8 100644
--- a/poezio/mam.py
+++ b/poezio/mam.py
@@ -247,10 +247,10 @@ def schedule_tab_open(tab: tabs.Tab) -> None:
 
 async def on_tab_open(tab: tabs.Tab) -> None:
     gap = tab._text_buffer.find_last_gap_muc()
-    if gap is not None:
-        await fill_missing_history(tab, gap)
-    else:
+    if gap is None or not gap.leave_message:
         await on_new_tab_open(tab)
+    else:
+        await fill_missing_history(tab, gap)
 
 
 def schedule_scroll_up(tab: tabs.Tab) -> None:
diff --git a/poezio/tabs/muctab.py b/poezio/tabs/muctab.py
index edf80bb6..0cb3d859 100644
--- a/poezio/tabs/muctab.py
+++ b/poezio/tabs/muctab.py
@@ -170,7 +170,6 @@ class MucTab(ChatTab):
             status=status.message,
             show=status.show,
             seconds=seconds)
-        mam.schedule_tab_open(self)
 
     def leave_room(self, message: str):
         if self.joined:
@@ -601,6 +600,7 @@ class MucTab(ChatTab):
                     },
                 ),
                 typ=0)
+        mam.schedule_tab_open(self)
 
     def handle_presence_joined(self, presence: Presence, status_codes) -> None:
         """
diff --git a/poezio/text_buffer.py b/poezio/text_buffer.py
index 03ad2f1b..f5886ae0 100644
--- a/poezio/text_buffer.py
+++ b/poezio/text_buffer.py
@@ -44,8 +44,8 @@ class AckError(Exception):
 @dataclass
 class HistoryGap:
     """Class representing a period of non-presence inside a MUC"""
-    leave_message: Optional[MucOwnLeaveMessage]
-    join_message: Optional[MucOwnJoinMessage]
+    leave_message: Optional[BaseMessage]
+    join_message: Optional[BaseMessage]
     last_timestamp_before_leave: Optional[datetime]
     first_timestamp_after_join: Optional[datetime]
 
@@ -78,33 +78,57 @@ class TextBuffer:
         leave, join = None, None
         for i, item in enumerate(reversed(self.messages)):
             if isinstance(item, MucOwnLeaveMessage):
-                leave = (i, item)
+                leave = (len(self.messages) - i - 1, item)
+                break
+            elif join and isinstance(item, MucOwnJoinMessage):
+                leave = (len(self.messages) - i - 1, item)
                 break
             if isinstance(item, MucOwnJoinMessage):
-                join = (i, item)
-        if join and leave:  # Skip if we find a message in the interval
-            real_leave = len(self.messages) - leave[0] - 1
-            real_join = len(self.messages) - join[0] - 1
-            for i in range(real_leave, real_join, 1):
+                join = (len(self.messages) - i - 1, item)
+
+        last_timestamp = None
+        first_timestamp = datetime.now()
+
+        # Identify the special case when we got disconnected from a chatroom
+        # without receiving or sending the relevant presence, therefore only
+        # having two joins with no leave, and messages in the middle.
+        if leave and isinstance(leave[1], MucOwnJoinMessage):
+            for i in range(join[0] - 1, leave[0], - 1):
+                if isinstance(self.messages[i], Message):
+                    leave = (
+                        i,
+                        self.messages[i]
+                    )
+                    last_timestamp = self.messages[i].time
+                    break
+        # If we have a normal gap but messages inbetween, it probably
+        # already has history, so abort there without returning it.
+        if join and leave:
+            for i in range(leave[0] + 1, join[0], 1):
                 if isinstance(self.messages[i], Message):
                     return None
         elif not (join or leave):
             return None
+
+        # If a leave message is found, get the last Message timestamp
+        # before it.
         if leave is None:
-            last_timestamp, leave_msg = None, None
-        else:
-            last_timestamp = None
+            leave_msg = None
+        elif last_timestamp is None:
             leave_msg = leave[1]
-            for i in range(len(self.messages) - leave[0] - 1, 0, -1):
+            for i in range(leave[0], 0, -1):
                 if isinstance(self.messages[i], Message):
                     last_timestamp = self.messages[i].time
                     break
-        first_timestamp = datetime.now()
+        else:
+            leave_msg = leave[1]
+        # If a join message is found, get the first Message timestamp
+        # after it, or the current time.
         if join is None:
             join_msg = None
         else:
             join_msg = join[1]
-            for i in range(len(self.messages) - join[0], len(self.messages)):
+            for i in range(join[0], len(self.messages)):
                 msg = self.messages[i]
                 if isinstance(msg, Message) and msg.time < first_timestamp:
                     first_timestamp = msg.time
diff --git a/test/test_text_buffer.py b/test/test_text_buffer.py
index 8e9829f4..65c6d9bf 100644
--- a/test/test_text_buffer.py
+++ b/test/test_text_buffer.py
@@ -35,6 +35,13 @@ def msgs_noleave():
     msg4 = Message('4', 'f')
     return [join, msg3, msg4]
 
+@fixture(scope='function')
+def msgs_doublejoin():
+    join = MucOwnJoinMessage('join')
+    msg1 = Message('1', 'd')
+    msg2 = Message('2', 'f')
+    join2 = MucOwnJoinMessage('join')
+    return [join, msg1, msg2, join2]
 
 def test_last_message(buf2048):
     msg = BaseMessage('toto')
@@ -67,6 +74,24 @@ def test_find_gap(buf2048, msgs_noleave):
     assert gap.first_timestamp_after_join == msg3.time
 
 
+def test_find_gap_doublejoin(buf2048, msgs_doublejoin):
+    for msg in msgs_doublejoin:
+        buf2048.add_message(msg)
+    gap = buf2048.find_last_gap_muc()
+    assert gap.leave_message == msgs_doublejoin[2]
+    assert gap.join_message == msgs_doublejoin[3]
+
+
+def test_find_gap_doublejoin_no_msg(buf2048):
+    join1 = MucOwnJoinMessage('join')
+    join2 = MucOwnJoinMessage('join')
+    for msg in [join1, join2]:
+        buf2048.add_message(msg)
+    gap = buf2048.find_last_gap_muc()
+    assert gap.leave_message is join1
+    assert gap.join_message is join2
+
+
 def test_find_gap_already_filled(buf2048):
     msg1 = Message('1', 'q')
     msg2 = Message('2', 's')
@@ -115,6 +140,22 @@ def test_get_gap_index(buf2048):
     assert buf2048.get_gap_index(gap) == 3
 
 
+def test_get_gap_index_doublejoin(buf2048, msgs_doublejoin):
+    for msg in msgs_doublejoin:
+        buf2048.add_message(msg)
+    gap = buf2048.find_last_gap_muc()
+    assert buf2048.get_gap_index(gap) == 3
+
+
+def test_get_gap_index_doublejoin_no_msg(buf2048):
+    join1 = MucOwnJoinMessage('join')
+    join2 = MucOwnJoinMessage('join')
+    for msg in [join1, join2]:
+        buf2048.add_message(msg)
+    gap = buf2048.find_last_gap_muc()
+    assert buf2048.get_gap_index(gap) == 1
+
+
 def test_get_gap_index_nojoin(buf2048, msgs_nojoin):
     for msg in msgs_nojoin:
         buf2048.add_message(msg)