From 2be28f13111ba8854b09bf7fd41bd5f3778436a0 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sun, 7 Apr 2024 18:58:30 +0000 Subject: [PATCH] test: move reaction tests to JSON-RPC --- deltachat-rpc-client/tests/test_something.py | 101 +++++++++++++++++- python/tests/test_1_online.py | 102 ------------------- 2 files changed, 100 insertions(+), 103 deletions(-) diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 6e1b08998..fbf869d1a 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -1,10 +1,15 @@ import concurrent.futures import json +import logging +import os import subprocess +import time from unittest.mock import MagicMock import pytest -from deltachat_rpc_client import EventType, events +from deltachat_rpc_client import Contact, EventType, Message, events +from deltachat_rpc_client.const import DownloadState +from deltachat_rpc_client.direct_imap import DirectImap from deltachat_rpc_client.rpc import JsonRpcError @@ -439,3 +444,97 @@ def test_mdn_doesnt_break_autocrypt(acfactory) -> None: message = alice.get_message_by_id(msg_id) snapshot = message.get_snapshot() assert snapshot.show_padlock + + +def test_reaction_to_partially_fetched_msg(acfactory, tmp_path): + """See https://github.com/deltachat/deltachat-core-rust/issues/3688 "Partially downloaded + messages are received out of order". + + If the Inbox contains X small messages followed by Y large messages followed by Z small + messages, Delta Chat first downloaded a batch of X+Z messages, and then a batch of Y messages. + + This bug was discovered by @Simon-Laux while testing reactions PR #3644 and can be reproduced + with online test as follows: + - Bob enables download limit and goes offline. + - Alice sends a large message to Bob and reacts to this message with a thumbs-up. + - Bob goes online + - Bob first processes a reaction message and throws it away because there is no corresponding + message, then processes a partially downloaded message. + - As a result, Bob does not see a reaction + """ + download_limit = 300000 + ac1, ac2 = acfactory.get_online_accounts(2) + ac1_addr = ac1.get_config("addr") + chat = ac1.create_chat(ac2) + ac2.set_config("download_limit", str(download_limit)) + ac2.stop_io() + + logging.info("sending small+large messages from ac1 to ac2") + msgs = [] + msgs.append(chat.send_text("hi")) + path = tmp_path / "large" + path.write_bytes(os.urandom(download_limit + 1)) + msgs.append(chat.send_file(str(path))) + for m in msgs: + m.wait_until_delivered() + + logging.info("sending a reaction to the large message from ac1 to ac2") + # TODO: Find the reason of an occasional message reordering on the server (so that the reaction + # has a lower UID than the previous message). W/a is to sleep for some time to let the reaction + # have a later INTERNALDATE. + time.sleep(1.1) + react_str = "\N{THUMBS UP SIGN}" + msgs.append(msgs[-1].send_reaction(react_str)) + msgs[-1].wait_until_delivered() + + ac2.start_io() + + logging.info("wait for ac2 to receive a reaction") + msg2 = Message(ac2, ac2.wait_for_reactions_changed().msg_id) + assert msg2.get_sender_contact().get_snapshot().address == ac1_addr + assert msg2.get_snapshot().download_state == DownloadState.AVAILABLE + reactions = msg2.get_reactions() + contacts = [Contact(ac2, int(i)) for i in reactions.reactions_by_contact] + assert len(contacts) == 1 + assert contacts[0].get_snapshot().address == ac1_addr + assert list(reactions.reactions_by_contact.values())[0] == [react_str] + + +def test_reactions_for_a_reordering_move(acfactory): + """When a batch of messages is moved from Inbox to DeltaChat folder with a single MOVE command, + their UIDs may be reordered (e.g. Gmail is known for that) which led to that messages were + processed by receive_imf in the wrong order, and, particularly, reactions were processed before + messages they refer to and thus dropped. + """ + (ac1,) = acfactory.get_online_accounts(1) + ac2 = acfactory.new_preconfigured_account() + ac2.set_config("mvbox_move", "1") + ac2.configure() + ac2.bring_online() + chat1 = acfactory.get_accepted_chat(ac1, ac2) + ac2.stop_io() + + logging.info("sending message + reaction from ac1 to ac2") + msg1 = chat1.send_text("hi") + msg1.wait_until_delivered() + # It's is sad, but messages must differ in their INTERNALDATEs to be processed in the correct + # order by DC, and most (if not all) mail servers provide only seconds precision. + time.sleep(1.1) + react_str = "\N{THUMBS UP SIGN}" + msg1.send_reaction(react_str).wait_until_delivered() + + logging.info("moving messages to ac2's DeltaChat folder in the reverse order") + ac2_direct_imap = DirectImap(ac2) + ac2_direct_imap.connect() + for uid in sorted([m.uid for m in ac2_direct_imap.get_all_messages()], reverse=True): + ac2_direct_imap.conn.move(uid, "DeltaChat") + + logging.info("receiving messages by ac2") + ac2.start_io() + msg2 = Message(ac2, ac2.wait_for_reactions_changed().msg_id) + assert msg2.get_snapshot().text == msg1.get_snapshot().text + reactions = msg2.get_reactions() + contacts = [Contact(ac2, int(i)) for i in reactions.reactions_by_contact] + assert len(contacts) == 1 + assert contacts[0].get_snapshot().address == ac1.get_config("addr") + assert list(reactions.reactions_by_contact.values())[0] == [react_str] diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index cb7a2cfb0..a3185db82 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -1,7 +1,6 @@ import os import queue import sys -import time import base64 from datetime import datetime, timezone @@ -1493,107 +1492,6 @@ def test_send_and_receive_image(acfactory, lp, data): assert m == msg_in -def test_reaction_to_partially_fetched_msg(acfactory, lp, tmp_path): - """See https://github.com/deltachat/deltachat-core-rust/issues/3688 "Partially downloaded - messages are received out of order". - - If the Inbox contains X small messages followed by Y large messages followed by Z small - messages, Delta Chat first downloaded a batch of X+Z messages, and then a batch of Y messages. - - This bug was discovered by @Simon-Laux while testing reactions PR #3644 and can be reproduced - with online test as follows: - - Bob enables download limit and goes offline. - - Alice sends a large message to Bob and reacts to this message with a thumbs-up. - - Bob goes online - - Bob first processes a reaction message and throws it away because there is no corresponding - message, then processes a partially downloaded message. - - As a result, Bob does not see a reaction - """ - download_limit = 300000 - ac1, ac2 = acfactory.get_online_accounts(2) - ac1_addr = ac1.get_config("addr") - chat = ac1.create_chat(ac2) - ac2.set_config("download_limit", str(download_limit)) - ac2.stop_io() - - reactions_queue = queue.Queue() - - class InPlugin: - @account_hookimpl - def ac_reactions_changed(self, message): - reactions_queue.put(message) - - ac2.add_account_plugin(InPlugin()) - - lp.sec("sending small+large messages from ac1 to ac2") - msgs = [] - msgs.append(chat.send_text("hi")) - path = tmp_path / "large" - path.write_bytes(os.urandom(download_limit + 1)) - msgs.append(chat.send_file(str(path))) - for m in msgs: - ac1._evtracker.wait_msg_delivered(m) - - lp.sec("sending a reaction to the large message from ac1 to ac2") - # TODO: Find the reason of an occasional message reordering on the server (so that the reaction - # has a lower UID than the previous message). W/a is to sleep for some time to let the reaction - # have a later INTERNALDATE. - time.sleep(1.1) - react_str = "\N{THUMBS UP SIGN}" - msgs.append(msgs[-1].send_reaction(react_str)) - ac1._evtracker.wait_msg_delivered(msgs[-1]) - - ac2.start_io() - - lp.sec("wait for ac2 to receive a reaction") - msg2 = ac2._evtracker.wait_next_reactions_changed() - assert msg2.get_sender_contact().addr == ac1_addr - assert msg2.download_state == dc.const.DC_DOWNLOAD_AVAILABLE - assert reactions_queue.get() == msg2 - reactions = msg2.get_reactions() - contacts = reactions.get_contacts() - assert len(contacts) == 1 - assert contacts[0].addr == ac1_addr - assert reactions.get_by_contact(contacts[0]) == react_str - - -def test_reactions_for_a_reordering_move(acfactory, lp): - """When a batch of messages is moved from Inbox to DeltaChat folder with a single MOVE command, - their UIDs may be reordered (e.g. Gmail is known for that) which led to that messages were - processed by receive_imf in the wrong order, and, particularly, reactions were processed before - messages they refer to and thus dropped. - """ - (ac1,) = acfactory.get_online_accounts(1) - ac2 = acfactory.new_online_configuring_account(mvbox_move=True) - acfactory.bring_accounts_online() - chat1 = acfactory.get_accepted_chat(ac1, ac2) - ac2.stop_io() - - lp.sec("sending message + reaction from ac1 to ac2") - msg1 = chat1.send_text("hi") - ac1._evtracker.wait_msg_delivered(msg1) - # It's is sad, but messages must differ in their INTERNALDATEs to be processed in the correct - # order by DC, and most (if not all) mail servers provide only seconds precision. - time.sleep(1.1) - react_str = "\N{THUMBS UP SIGN}" - ac1._evtracker.wait_msg_delivered(msg1.send_reaction(react_str)) - - lp.sec("moving messages to ac2's DeltaChat folder in the reverse order") - ac2.direct_imap.connect() - for uid in sorted([m.uid for m in ac2.direct_imap.get_all_messages()], reverse=True): - ac2.direct_imap.conn.move(uid, "DeltaChat") - - lp.sec("receiving messages by ac2") - ac2.start_io() - msg2 = ac2._evtracker.wait_next_reactions_changed() - assert msg2.text == msg1.text - reactions = msg2.get_reactions() - contacts = reactions.get_contacts() - assert len(contacts) == 1 - assert contacts[0].addr == ac1.get_config("addr") - assert reactions.get_by_contact(contacts[0]) == react_str - - def test_import_export_online_all(acfactory, tmp_path, data, lp): (ac1, some1) = acfactory.get_online_accounts(2)