- perform direct_imap init in testplugin instead of global deltachat

plugin, probably also helping to avoid some segfeaults during teardown

- some API renaming on the side (too hard to split into separate commit, sorry)

python/src/deltachat/direct_imap.py

@@ -8,7 +8,6 @@ import ssl
 import pathlib
 from imap_tools import MailBox, MailBoxTls, errors, AND, Header, MailMessageFlags, MailMessage
 import imaplib
-import deltachat
 from deltachat import const, Account
 from typing import List
 
@@ -18,44 +17,6 @@ FETCH = b'FETCH'
 ALL = "1:*"
 
 
-@deltachat.global_hookimpl
-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.
-    """
-    try:
-
-        if not hasattr(account, "direct_imap"):
-            imap = DirectImap(account)
-
-            for folder in imap.list_folders():
-                if folder.lower() == "inbox" or folder.lower() == "deltachat":
-                    assert imap.select_folder(folder)
-                    imap.delete(ALL, expunge=True)
-                else:
-                    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)
-
-            setattr(account, "direct_imap", imap)
-
-    except Exception as e:
-        # Uncaught exceptions here would lead to a timeout without any note written to the log
-        # start with DC_EVENT_WARNING so that the line is printed in yellow and won't be overlooked when reading
-        account.log("DC_EVENT_WARNING =================== DIRECT_IMAP CAN'T RESET ACCOUNT: ===================")
-        account.log("DC_EVENT_WARNING =================== " + str(e) + " ===================")
-
-
-@deltachat.global_hookimpl
-def dc_account_after_shutdown(account):
-    """ shutdown the imap connection if there is one. """
-    imap = getattr(account, "direct_imap", None)
-    if imap is not None:
-        imap.shutdown()
-        del account.direct_imap
-
-
 class DirectImap:
     def __init__(self, account: Account) -> None:
         self.account = account

python/src/deltachat/testplugin.py

@@ -17,7 +17,7 @@ import requests
 from . import Account, const
 from .events import FFIEventLogger, FFIEventTracker
 from _pytest._code import Source
-from deltachat import direct_imap
+from deltachat.direct_imap import DirectImap
 
 import deltachat
 
@@ -215,7 +215,6 @@ class ACFactory:
         self._preconfigured_keys = ["alice", "bob", "charlie",
                                     "dom", "elena", "fiona"]
         self.set_logging_default(False)
-        deltachat.register_global_plugin(direct_imap)
 
     def finalize(self):
         while self._finalizers:
@@ -224,9 +223,12 @@ class ACFactory:
 
         while self._accounts:
             acc = self._accounts.pop()
+            imap = getattr(acc, "direct_imap", None)
+            if imap is not None:
+                imap.shutdown()
+                del acc.direct_imap
             acc.shutdown()
             acc.disable_logging()
-        deltachat.unregister_global_plugin(direct_imap)
 
     def get_next_liveconfig(self):
         """ Base function to get functional online configurations
@@ -303,13 +305,7 @@ class ACFactory:
         self._preconfigure_key(ac, configdict["addr"])
         ac.update_config(configdict)
 
-    def get_online_accounts(self, num):
-        # to reduce number of log events logging starts after accounts can receive
-        accounts = [self.get_online_configuring_account() for i in range(num)]
-        self.wait_configure_and_start_io(logstart="after_inbox_idle_ready")
-        return accounts
-
-    def clone_online_account(self, account):
+    def get_cloned_configuring_account(self, account):
         """ Clones addr, mail_pw, mvbox_move, sentbox_watch and the
         direct_imap object of an online account. This simulates the user setting
         up a new device without importing a backup.
@@ -348,14 +344,21 @@ class ACFactory:
             acc._evtracker.consume_events()
             acc.get_device_chat().mark_noticed()
             del acc._configtracker
+            if not hasattr(acc, "direct_imap"):
+                self.init_direct_imap(acc)
+
+    def get_online_accounts(self, num):
+        # to reduce number of log events logging starts after accounts can receive
+        accounts = [self.get_online_configuring_account() for i in range(num)]
+        self.wait_configure_and_start_io(logstart="after_inbox_idle_ready")
+        return accounts
 
     def run_bot_process(self, module, ffi=True):
         fn = module.__file__
 
         bot_cfg = self.get_next_liveconfig()
         bot_ac = self.get_unconfigured_account()
-        bot_ac.update_config(bot_cfg)
-        self._preconfigure_key(bot_ac, bot_cfg["addr"])
+        self.prepare_account_with_liveconfig(bot_ac, bot_cfg)
 
         # Avoid starting ac so we don't interfere with the bot operating on
         # the same database.
@@ -385,6 +388,16 @@ class ACFactory:
         self._finalizers.append(bot.kill)
         return bot
 
+    def init_direct_imap(self, acc):
+        if not hasattr(acc, "direct_imap"):
+            acc.direct_imap = imap = DirectImap(acc)
+            for folder in imap.list_folders():
+                if folder.lower() == "inbox" or folder.lower() == "deltachat":
+                    assert imap.select_folder(folder)
+                    imap.delete("1:*", expunge=True)
+                else:
+                    imap.conn.folder.delete(folder)
+
     def dump_imap_summary(self, logfile):
         for ac in self._accounts:
             ac.dump_account_info(logfile=logfile)

python/tests/test_account.py

@@ -727,15 +727,12 @@ class TestOnlineAccount:
     def test_one_account_send_bcc_setting(self, acfactory, lp):
         ac1 = acfactory.get_online_configuring_account()
         ac2 = acfactory.get_online_configuring_account()
-
-        # Clone the first account: we will test if sent messages
-        # are copied to it via BCC.
-        ac1_clone = acfactory.clone_online_account(ac1)
-
+        ac1_clone = acfactory.get_cloned_configuring_account(ac1)
         acfactory.wait_configure_and_start_io()
 
-        chat = acfactory.get_accepted_chat(ac1, ac2)
+        # test if sent messages are copied to it via BCC.
 
+        chat = acfactory.get_accepted_chat(ac1, ac2)
         self_addr = ac1.get_config("addr")
         other_addr = ac2.get_config("addr")
 
@@ -1094,7 +1091,7 @@ class TestOnlineAccount:
         """Test that message marked as seen on one device is marked as seen on another."""
         ac1 = acfactory.get_online_configuring_account()
         ac2 = acfactory.get_online_configuring_account()
-        ac1_clone = acfactory.clone_online_account(ac1)
+        ac1_clone = acfactory.get_cloned_configuring_account(ac1)
         acfactory.wait_configure_and_start_io()
 
         ac1.set_config("bcc_self", "1")
@@ -1535,7 +1532,7 @@ class TestOnlineAccount:
     def test_no_old_msg_is_fresh(self, acfactory, lp):
         ac1 = acfactory.get_online_configuring_account()
         ac2 = acfactory.get_online_configuring_account()
-        ac1_clone = acfactory.clone_online_account(ac1)
+        ac1_clone = acfactory.get_cloned_configuring_account(ac1)
         acfactory.wait_configure_and_start_io()
 
         ac1.set_config("e2ee_enabled", "0")
@@ -1899,7 +1896,7 @@ class TestOnlineAccount:
         # before ther setup message is send. DC does not read old messages
         # as of Jul2019
         ac1 = acfactory.get_online_configuring_account()
-        ac2 = acfactory.clone_online_account(ac1)
+        ac2 = acfactory.get_cloned_configuring_account(ac1)
         acfactory.wait_configure_and_start_io()
 
         lp.sec("trigger ac setup message and return setupcode")
@@ -1920,7 +1917,7 @@ class TestOnlineAccount:
 
     def test_ac_setup_message_twice(self, acfactory, lp):
         ac1 = acfactory.get_online_configuring_account()
-        ac2 = acfactory.clone_online_account(ac1)
+        ac2 = acfactory.get_cloned_configuring_account(ac1)
         acfactory.wait_configure_and_start_io()
 
         lp.sec("trigger ac setup message but ignore")
@@ -2404,7 +2401,7 @@ class TestOnlineAccount:
         lp.sec("ac3 reinstalls DC and generates a new key")
         ac3.stop_io()
         acfactory.remove_preconfigured_keys()
-        ac4 = acfactory.clone_online_account(ac3)
+        ac4 = acfactory.get_cloned_configuring_account(ac3)
         ac4._configtracker.wait_finish()
         # Create contacts to make sure incoming messages are not treated as contact requests
         chat41 = ac4.create_chat(ac1)
@@ -2810,7 +2807,7 @@ class TestOnlineAccount:
         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 = acfactory.get_cloned_configuring_account(ac1)
         ac1_clone.set_config("fetch_existing_msgs", "1")
         ac1_clone._configtracker.wait_finish()
         ac1_clone.start_io()
@@ -2855,7 +2852,7 @@ class TestOnlineAccount:
         assert ac1.direct_imap.idle_wait_for_seen()
 
         lp.sec("Clone online account and let it fetch the existing messages")
-        ac1_clone = acfactory.clone_online_account(ac1)
+        ac1_clone = acfactory.get_cloned_configuring_account(ac1)
         ac1_clone.set_config("fetch_existing_msgs", "1")
         ac1_clone._configtracker.wait_finish()