From 29eef159d50c836fbd7a27770775d63700dc7f19 Mon Sep 17 00:00:00 2001 From: mathieui Date: Fri, 22 May 2020 17:09:17 +0200 Subject: [PATCH] Fix some edge cases of MAM history fetch - Wait until we receive our own MUC presence to fetch history - Fix /reconnect weirdness --- poezio/mam.py | 6 ++--- poezio/tabs/muctab.py | 2 +- poezio/text_buffer.py | 52 +++++++++++++++++++++++++++++----------- test/test_text_buffer.py | 41 +++++++++++++++++++++++++++++++ 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)