From 5ef152fd84bfe3ae46bae0cb33c25304f13e54d9 Mon Sep 17 00:00:00 2001 From: missytake Date: Thu, 28 Apr 2022 16:48:28 +0200 Subject: [PATCH 1/8] replaced imapclient python library with imap-tools in the tests. works with testrun.org locally --- python/setup.py | 2 +- python/src/deltachat/direct_imap.py | 129 ++++++++++++---------------- python/tests/test_account.py | 20 ++--- 3 files changed, 66 insertions(+), 85 deletions(-) diff --git a/python/setup.py b/python/setup.py index 728772d6d..b50fcba78 100644 --- a/python/setup.py +++ b/python/setup.py @@ -11,7 +11,7 @@ def main(): description='Python bindings for the Delta Chat Core library using CFFI against the Rust-implemented libdeltachat', long_description=long_description, author='holger krekel, Floris Bruynooghe, Bjoern Petersen and contributors', - install_requires=['cffi>=1.0.0', 'pluggy', 'imapclient', 'requests'], + install_requires=['cffi>=1.0.0', 'pluggy', 'imap-tools', 'requests'], setup_requires=[ 'setuptools_scm', # required for compatibility with `python3 setup.py sdist` 'pkgconfig', diff --git a/python/src/deltachat/direct_imap.py b/python/src/deltachat/direct_imap.py index 080d2e836..de5227d4f 100644 --- a/python/src/deltachat/direct_imap.py +++ b/python/src/deltachat/direct_imap.py @@ -4,25 +4,23 @@ and for cleaning up inbox/mvbox for each test function run. """ import io -import email import ssl import pathlib -from imapclient import IMAPClient -from imapclient.exceptions import IMAPClientError +from imap_tools import MailBox, MailBoxTls, errors, AND, Header, MailMessageFlags, MailMessage import imaplib import deltachat from deltachat import const, Account -SEEN = b'\\Seen' -DELETED = b'\\Deleted' +SEEN = MailMessageFlags.SEEN +DELETED = MailMessageFlags.DELETED FLAGS = b'FLAGS' FETCH = b'FETCH' ALL = "1:*" @deltachat.global_hookimpl -def dc_account_extra_configure(account): +def dc_account_extra_configure(account: Account): """ Reset the account (we reuse accounts across tests) and make 'account.direct_imap' available for direct IMAP ops. """ @@ -36,7 +34,7 @@ def dc_account_extra_configure(account): assert imap.select_folder(folder) imap.delete(ALL, expunge=True) else: - imap.conn.delete_folder(folder) + imap.conn.folder.delete(folder) # We just deleted the folder, so we have to make DC forget about it, too if account.get_config("configured_sentbox_folder") == folder: account.set_config("configured_sentbox_folder", None) @@ -86,37 +84,34 @@ class DirectImap: ssl_context.verify_mode = ssl.CERT_NONE if security == const.DC_SOCKET_STARTTLS: - self.conn = IMAPClient(host, port, ssl=False) - self.conn.starttls(ssl_context) - elif security == const.DC_SOCKET_PLAIN: - self.conn = IMAPClient(host, port, ssl=False) - elif security == const.DC_SOCKET_SSL: - self.conn = IMAPClient(host, port, ssl_context=ssl_context) + self.conn = MailBoxTls(host, port, ssl_context=ssl_context) + elif security == const.DC_SOCKET_PLAIN or security == const.DC_SOCKET_SSL: + self.conn = MailBox(host, port, ssl_context=ssl_context) self.conn.login(user, pw) self.select_folder("INBOX") def shutdown(self): try: - self.conn.idle_done() - except (OSError, IMAPClientError): + self.idle_done() + except (OSError, imaplib.IMAP4.abort): pass try: self.conn.logout() - except (OSError, IMAPClientError): + except (OSError, imaplib.IMAP4.abort): print("Could not logout direct_imap conn") def create_folder(self, foldername): try: - self.conn.create_folder(foldername) - except imaplib.IMAP4.error as e: + self.conn.folder.create(foldername) + except errors.MailboxFolderCreateError as e: print("Can't create", foldername, "probably it already exists:", str(e)) - def select_folder(self, foldername): + def select_folder(self, foldername: str) -> tuple: assert not self._idling - return self.conn.select_folder(foldername) + return self.conn.folder.set(foldername) - def select_config_folder(self, config_name): + def select_config_folder(self, config_name: str): """ Return info about selected folder if it is configured, otherwise None. """ if "_" not in config_name: @@ -125,50 +120,36 @@ class DirectImap: if foldername: return self.select_folder(foldername) - def list_folders(self): + def list_folders(self) -> [str]: """ return list of all existing folder names""" assert not self._idling - folders = [] - for meta, sep, foldername in self.conn.list_folders(): - folders.append(foldername) - return folders + return [folder.name for folder in self.conn.folder.list()] - def delete(self, range, expunge=True): + def delete(self, uid_list: str, expunge=True): """ delete a range of messages (imap-syntax). If expunge is true, perform the expunge-operation to make sure the messages are really gone and not just flagged as deleted. """ - self.conn.set_flags(range, [DELETED]) + self.conn.client.uid('STORE', uid_list, '+FLAGS', r'(\Deleted)') if expunge: self.conn.expunge() - def get_all_messages(self): + def get_all_messages(self) -> [MailMessage]: assert not self._idling + return [mail for mail in self.conn.fetch()] - # Flush unsolicited responses. IMAPClient has problems - # dealing with them: https://github.com/mjs/imapclient/issues/334 - # When this NOOP was introduced, next FETCH returned empty - # result instead of a single message, even though IMAP server - # can only return more untagged responses than required, not - # less. - self.conn.noop() - - return self.conn.fetch(ALL, [FLAGS]) - - def get_unread_messages(self): + def get_unread_messages(self) -> [str]: assert not self._idling - res = self.conn.fetch(ALL, [FLAGS]) - return [uid for uid in res - if SEEN not in res[uid][FLAGS]] + return [msg.uid for msg in self.conn.fetch(AND(seen=False))] def mark_all_read(self): messages = self.get_unread_messages() if messages: - res = self.conn.set_flags(messages, [SEEN]) + res = self.conn.flag(messages, SEEN, True) print("marked seen:", messages, res) - def get_unread_cnt(self): + def get_unread_cnt(self) -> int: return len(self.get_unread_messages()) def dump_imap_structures(self, dir, logfile): @@ -191,20 +172,20 @@ class DirectImap: # get message content without auto-marking it as seen # fetching 'RFC822' would mark it as seen. requested = [b'BODY.PEEK[]', FLAGS] - for uid, data in self.conn.fetch(messages, requested).items(): - body_bytes = data[b'BODY[]'] - if not body_bytes: - log("Message", uid, "has empty body") + for msg in self.conn.fetch(mark_seen=False): + body = getattr(msg.obj, "text", None) + if not body: + body = getattr(msg.obj, "html", None) + if not body: + log("Message", msg.uid, "has empty body") continue - flags = data[FLAGS] path = pathlib.Path(str(dir)).joinpath("IMAP", self.logid, imapfolder) path.mkdir(parents=True, exist_ok=True) - fn = path.joinpath(str(uid)) - fn.write_bytes(body_bytes) - log("Message", uid, fn) - email_message = email.message_from_bytes(body_bytes) - log("Message", uid, flags, "Message-Id:", email_message.get("Message-Id")) + fn = path.joinpath(str(msg.uid)) + fn.write_bytes(body) + log("Message", msg.uid, fn) + log("Message", msg.uid, msg.flags, "Message-Id:", msg.obj.get("Message-Id")) if empty_folders: log("--------- EMPTY FOLDERS:", empty_folders) @@ -214,59 +195,59 @@ class DirectImap: def idle_start(self): """ switch this connection to idle mode. non-blocking. """ assert not self._idling - res = self.conn.idle() + res = self.conn.idle.start() self._idling = True return res - def idle_check(self, terminate=False): + def idle_check(self, terminate=False, timeout=60) -> [bytes]: """ (blocking) wait for next idle message from server. """ assert self._idling self.account.log("imap-direct: calling idle_check") - res = self.conn.idle_check() + res = self.conn.idle.poll(timeout=timeout) if terminate: self.idle_done() self.account.log("imap-direct: idle_check returned {!r}".format(res)) return res - def idle_wait_for_new_message(self, terminate=False): + def idle_wait_for_new_message(self, terminate=False, timeout=60) -> bytes: while 1: - for item in self.idle_check(): - if item[1] in (b'EXISTS', b'RECENT'): + for item in self.idle_check(timeout=timeout): + if b'EXISTS' in item or b'RECENT' in item: if terminate: self.idle_done() return item - def idle_wait_for_seen(self, terminate=False): + def idle_wait_for_seen(self, terminate=False, timeout=60) -> int: """ Return first message with SEEN flag from a running idle-stream REtiurn. """ while 1: - for item in self.idle_check(): - if item[1] == FETCH: - if item[2][0] == FLAGS: - if SEEN in item[2][1]: - if terminate: - self.idle_done() - return item[0] + for item in self.idle_check(timeout=timeout): + if FETCH in item: + self.account.log(str(item)) + if FLAGS in item and bytes(SEEN, encoding='ascii') in item: + if terminate: + self.idle_done() + return int(item.split(b' ')[1]) def idle_done(self): """ send idle-done to server if we are currently in idle mode. """ if self._idling: - res = self.conn.idle_done() + res = self.conn.idle.stop() self._idling = False return res - def append(self, folder, msg): + def append(self, folder: str, msg: str): """Upload a message to *folder*. Trailing whitespace or a linebreak at the beginning will be removed automatically. """ if msg.startswith("\n"): msg = msg[1:] msg = '\n'.join([s.lstrip() for s in msg.splitlines()]) - self.conn.append(folder, msg) + self.conn.append(bytes(msg, encoding='ascii'), folder) - def get_uid_by_message_id(self, message_id): - msgs = self.conn.search(['HEADER', 'MESSAGE-ID', message_id]) + def get_uid_by_message_id(self, message_id) -> str: + msgs = [msg.uid for msg in self.conn.fetch(AND(header=Header('MESSAGE-ID', message_id)))] if len(msgs) == 0: raise Exception("Did not find message " + message_id + ", maybe you forgot to select the correct folder?") return msgs[0] diff --git a/python/tests/test_account.py b/python/tests/test_account.py index 2483c7d51..345216a40 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -11,6 +11,7 @@ from deltachat.hookspec import account_hookimpl from deltachat.capi import ffi, lib from deltachat.cutil import iter_array from datetime import datetime, timedelta, timezone +from imap_tools import AND, U @pytest.mark.parametrize("msgtext,res", [ @@ -1058,12 +1059,9 @@ class TestOnlineAccount: # Accept the contact request. msg.chat.accept() ac2.mark_seen_messages([msg]) - ac2.direct_imap.idle_wait_for_seen(terminate=True) + uid = ac2.direct_imap.idle_wait_for_seen(terminate=True) - fetch = list(ac2.direct_imap.conn.fetch("*", b'FLAGS').values()) - flags = fetch[-1][b'FLAGS'] - is_seen = b'\\Seen' in flags - assert is_seen + assert len([a for a in ac2.direct_imap.conn.fetch(AND(seen=True, uid=U(uid, "*")))]) == 1 def test_multidevice_sync_seen(self, acfactory, lp): """Test that message marked as seen on one device is marked as seen on another.""" @@ -2296,7 +2294,7 @@ class TestOnlineAccount: ac1.direct_imap.delete("1:*", expunge=False) ac1.start_io() - for ev in ac1._evtracker.iter_events(): + for ev in ac1._evtracker.iter_events(timeout=60): if ev.name == "DC_EVENT_MSGS_CHANGED": pytest.fail("A deleted message was shown to the user") @@ -2552,7 +2550,9 @@ class TestOnlineAccount: lp.sec("imap2: test that only one message is left") imap2 = ac2.direct_imap - + imap2.idle_start() + imap2.idle_wait_for_new_message(timeout=600) + imap2.idle_done() assert len(imap2.get_all_messages()) == 1 def test_configure_error_msgs(self, acfactory): @@ -2858,15 +2858,15 @@ class TestOnlineAccount: ac2 = acfactory.get_online_configuring_account() acfactory.wait_configure(ac1) - ac1.direct_imap.conn.delete_folder("DeltaChat") - assert len(ac1.direct_imap.conn.list_folders(pattern="DeltaChat")) == 0 + ac1.direct_imap.conn.folder.delete("DeltaChat") + assert "DeltaChat" not in ac1.direct_imap.list_folders() acfactory.wait_configure_and_start_io() ac2.create_chat(ac1).send_text("hello") msg = ac1._evtracker.wait_next_incoming_message() assert msg.text == "hello" - assert len(ac1.direct_imap.conn.list_folders(pattern="DeltaChat")) == 1 + assert "DeltaChat" in ac1.direct_imap.list_folders() class TestGroupStressTests: From a2e5c60683ed2fb1add2454c338b76ea9bff5864 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Fri, 29 Apr 2022 09:42:05 +0200 Subject: [PATCH 2/8] - remove one unncessary usage of imap idle - simplify SEEN bytes/unicode flag issue - fix a lint issue and a docstring --- python/src/deltachat/direct_imap.py | 10 +++------- python/tests/test_account.py | 9 +++------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/python/src/deltachat/direct_imap.py b/python/src/deltachat/direct_imap.py index de5227d4f..c862eb990 100644 --- a/python/src/deltachat/direct_imap.py +++ b/python/src/deltachat/direct_imap.py @@ -12,8 +12,6 @@ import deltachat from deltachat import const, Account -SEEN = MailMessageFlags.SEEN -DELETED = MailMessageFlags.DELETED FLAGS = b'FLAGS' FETCH = b'FETCH' ALL = "1:*" @@ -146,7 +144,7 @@ class DirectImap: def mark_all_read(self): messages = self.get_unread_messages() if messages: - res = self.conn.flag(messages, SEEN, True) + res = self.conn.flag(messages, MailMessageFlags.SEEN, True) print("marked seen:", messages, res) def get_unread_cnt(self) -> int: @@ -171,7 +169,6 @@ class DirectImap: log("---------", imapfolder, len(messages), "messages ---------") # get message content without auto-marking it as seen # fetching 'RFC822' would mark it as seen. - requested = [b'BODY.PEEK[]', FLAGS] for msg in self.conn.fetch(mark_seen=False): body = getattr(msg.obj, "text", None) if not body: @@ -218,14 +215,13 @@ class DirectImap: return item def idle_wait_for_seen(self, terminate=False, timeout=60) -> int: - """ Return first message with SEEN flag - from a running idle-stream REtiurn. + """ Return first message with SEEN flag from a running idle-stream. """ while 1: for item in self.idle_check(timeout=timeout): if FETCH in item: self.account.log(str(item)) - if FLAGS in item and bytes(SEEN, encoding='ascii') in item: + if FLAGS in item and rb'\Seen' in item: if terminate: self.idle_done() return int(item.split(b' ')[1]) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index 345216a40..a8d797bd4 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -2548,12 +2548,9 @@ class TestOnlineAccount: ac2._evtracker.get_info_contains("close/expunge succeeded") - lp.sec("imap2: test that only one message is left") - imap2 = ac2.direct_imap - imap2.idle_start() - imap2.idle_wait_for_new_message(timeout=600) - imap2.idle_done() - assert len(imap2.get_all_messages()) == 1 + lp.sec("ac2: test that only one message is left") + ac2.direct_imap.select_config_folder("inbox") + assert len(ac2.direct_imap.get_all_messages()) == 1 def test_configure_error_msgs(self, acfactory): ac1, configdict = acfactory.get_online_config() From 521fa58b75b544996f1d622d0092e8bb4cb5d61b Mon Sep 17 00:00:00 2001 From: holger krekel Date: Fri, 29 Apr 2022 10:00:43 +0200 Subject: [PATCH 3/8] remove timeout --- python/tests/test_account.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index a8d797bd4..cbe5bceab 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -2294,7 +2294,7 @@ class TestOnlineAccount: ac1.direct_imap.delete("1:*", expunge=False) ac1.start_io() - for ev in ac1._evtracker.iter_events(timeout=60): + for ev in ac1._evtracker.iter_events(): if ev.name == "DC_EVENT_MSGS_CHANGED": pytest.fail("A deleted message was shown to the user") From 4c7c4e2a814bdeb1df65ef1827b184aa94723ae5 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Fri, 29 Apr 2022 10:06:02 +0200 Subject: [PATCH 4/8] better document one sometimes failing test --- python/tests/test_account.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index cbe5bceab..45ea3605f 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -2769,10 +2769,7 @@ class TestOnlineAccount: acfactory.wait_configure_and_start_io() assert_folders_configured(ac1) - if mvbox_move: - ac1.direct_imap.select_config_folder("mvbox") - else: - ac1.direct_imap.select_folder("INBOX") + assert ac1.direct_imap.select_config_folder("mvbox" if mvbox_move else "inbox") ac1.direct_imap.idle_start() lp.sec("send out message with bcc to ourselves") @@ -2781,22 +2778,24 @@ class TestOnlineAccount: chat.send_text("message text") assert_folders_configured(ac1) - # now wait until the bcc_self message arrives - # Also test that bcc_self messages moved to the mvbox are marked as read. + lp.sec("wait until the bcc_self message arrives in correct folder and is marked seen") assert ac1.direct_imap.idle_wait_for_seen() assert_folders_configured(ac1) + lp.sec("create a cloned ac1 and fetch contact history during configure") ac1_clone = acfactory.clone_online_account(ac1) ac1_clone.set_config("fetch_existing_msgs", "1") ac1_clone._configtracker.wait_finish() ac1_clone.start_io() assert_folders_configured(ac1_clone) + lp.sec("check that ac2 contact was fetchted during configure") ac1_clone._evtracker.get_matching("DC_EVENT_CONTACTS_CHANGED") ac2_addr = ac2.get_config("addr") assert any(c.addr == ac2_addr for c in ac1_clone.get_contacts()) assert_folders_configured(ac1_clone) + lp.sec("check that messages changed events arrive for the correct message") msg = ac1_clone._evtracker.wait_next_messages_changed() assert msg.text == "message text" assert_folders_configured(ac1) From d59aa35b2f7ca719b34249dac5026cf2c386b4bd Mon Sep 17 00:00:00 2001 From: missytake Date: Fri, 29 Apr 2022 11:14:19 +0200 Subject: [PATCH 5/8] fix mypy errors --- python/src/deltachat/direct_imap.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/python/src/deltachat/direct_imap.py b/python/src/deltachat/direct_imap.py index c862eb990..c8d5f846b 100644 --- a/python/src/deltachat/direct_imap.py +++ b/python/src/deltachat/direct_imap.py @@ -10,6 +10,7 @@ from imap_tools import MailBox, MailBoxTls, errors, AND, Header, MailMessageFlag import imaplib import deltachat from deltachat import const, Account +from typing import List FLAGS = b'FLAGS' @@ -118,7 +119,7 @@ class DirectImap: if foldername: return self.select_folder(foldername) - def list_folders(self) -> [str]: + def list_folders(self) -> List[str]: """ return list of all existing folder names""" assert not self._idling return [folder.name for folder in self.conn.folder.list()] @@ -133,11 +134,11 @@ class DirectImap: if expunge: self.conn.expunge() - def get_all_messages(self) -> [MailMessage]: + def get_all_messages(self) -> List[MailMessage]: assert not self._idling return [mail for mail in self.conn.fetch()] - def get_unread_messages(self) -> [str]: + def get_unread_messages(self) -> List[str]: assert not self._idling return [msg.uid for msg in self.conn.fetch(AND(seen=False))] @@ -196,7 +197,7 @@ class DirectImap: self._idling = True return res - def idle_check(self, terminate=False, timeout=60) -> [bytes]: + def idle_check(self, terminate=False, timeout=60) -> List[bytes]: """ (blocking) wait for next idle message from server. """ assert self._idling self.account.log("imap-direct: calling idle_check") From 032e644b2ba63be578dea57ff91c6e0e252253e5 Mon Sep 17 00:00:00 2001 From: missytake Date: Fri, 29 Apr 2022 11:30:06 +0200 Subject: [PATCH 6/8] set default timeout to None --- python/src/deltachat/direct_imap.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/src/deltachat/direct_imap.py b/python/src/deltachat/direct_imap.py index c8d5f846b..4647b6bb9 100644 --- a/python/src/deltachat/direct_imap.py +++ b/python/src/deltachat/direct_imap.py @@ -197,7 +197,7 @@ class DirectImap: self._idling = True return res - def idle_check(self, terminate=False, timeout=60) -> List[bytes]: + def idle_check(self, terminate=False, timeout=None) -> List[bytes]: """ (blocking) wait for next idle message from server. """ assert self._idling self.account.log("imap-direct: calling idle_check") @@ -207,7 +207,7 @@ class DirectImap: self.account.log("imap-direct: idle_check returned {!r}".format(res)) return res - def idle_wait_for_new_message(self, terminate=False, timeout=60) -> bytes: + def idle_wait_for_new_message(self, terminate=False, timeout=None) -> bytes: while 1: for item in self.idle_check(timeout=timeout): if b'EXISTS' in item or b'RECENT' in item: @@ -215,7 +215,7 @@ class DirectImap: self.idle_done() return item - def idle_wait_for_seen(self, terminate=False, timeout=60) -> int: + def idle_wait_for_seen(self, terminate=False, timeout=None) -> int: """ Return first message with SEEN flag from a running idle-stream. """ while 1: From e27345e4899adc9f2c673217a6719956333dc7aa Mon Sep 17 00:00:00 2001 From: missytake Date: Fri, 29 Apr 2022 15:19:48 +0200 Subject: [PATCH 7/8] python bindings: ignore mypy errors for imap_tools --- python/pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/pyproject.toml b/python/pyproject.toml index a3e714f31..a2b67e8e6 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -6,3 +6,6 @@ build-backend = "setuptools.build_meta" root = ".." tag_regex = '^(?Ppy-)?(?P[^\+]+)(?P.*)?$' git_describe_command = "git describe --dirty --tags --long --match py-*.*" + +[mypy-imap_tools] +ignore_missing_imports = true From b97b374487c848a6eecb38ca734175e5a9ea26b0 Mon Sep 17 00:00:00 2001 From: missytake Date: Fri, 29 Apr 2022 16:01:48 +0200 Subject: [PATCH 8/8] move imap_tools mypy ignore to mypy.ini --- python/mypy.ini | 4 ++++ python/pyproject.toml | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/python/mypy.ini b/python/mypy.ini index 7fbceefa1..e84e40280 100644 --- a/python/mypy.ini +++ b/python/mypy.ini @@ -17,3 +17,7 @@ ignore_missing_imports = True [mypy-_pytest.*] ignore_missing_imports = True + +[mypy-imap_tools.*] +ignore_missing_imports = True + diff --git a/python/pyproject.toml b/python/pyproject.toml index a2b67e8e6..a3e714f31 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -6,6 +6,3 @@ build-backend = "setuptools.build_meta" root = ".." tag_regex = '^(?Ppy-)?(?P[^\+]+)(?P.*)?$' git_describe_command = "git describe --dirty --tags --long --match py-*.*" - -[mypy-imap_tools] -ignore_missing_imports = true