better preserve notification order, using a queue again

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## untagged 
 
+- better preserve notification order 
+  ([#263](https://github.com/deltachat/chatmail/pull/263))
+
 - re-enable running the CI in pull requests, but not concurrently 
   ([#258](https://github.com/deltachat/chatmail/pull/258))
 

chatmaild/src/chatmaild/metadata.py

@@ -1,7 +1,8 @@
 import pwd
 
 from pathlib import Path
-from threading import Thread, Event
+from threading import Thread
+from queue import Queue
 from socketserver import (
     UnixStreamServer,
     StreamRequestHandler,
@@ -34,7 +35,7 @@ class Notifier:
         self.notification_dir = vmail_dir / "pending_notifications"
         if not self.notification_dir.exists():
             self.notification_dir.mkdir()
-        self.message_arrived_event = Event()
+        self.notification_queue = Queue()
 
     def get_metadata_dict(self, addr):
         return FileDict(self.vmail_dir / addr / "metadata.json")
@@ -60,31 +61,37 @@ class Notifier:
 
     def new_message_for_addr(self, addr):
         self.notification_dir.joinpath(addr).touch()
-        self.message_arrived_event.set()
+        self.notification_queue.put(addr)
 
     def thread_run_loop(self):
         requests_session = requests.Session()
+        # on startup deliver all persisted notifications from last process run
+        self.notification_queue.put(None)
         while 1:
-            self.message_arrived_event.wait()
-            self.message_arrived_event.clear()
             self.thread_run_one(requests_session)
 
     def thread_run_one(self, requests_session):
-        for addr_path in self.notification_dir.iterdir():
-            addr = addr_path.name
-            if "@" not in addr:
-                continue
-            for token in self.get_tokens(addr):
-                response = requests_session.post(
-                    "https://notifications.delta.chat/notify",
-                    data=token,
-                    timeout=60,
-                )
-                if response.status_code == 410:
-                    # 410 Gone status code
-                    # means the token is no longer valid.
-                    self.remove_token(addr, token)
-            addr_path.unlink()
+        addr = self.notification_queue.get()
+        if addr is None:
+            # startup, notify any "pending" notifications from last run
+            for addr_path in self.notification_dir.iterdir():
+                if "@" in addr_path.name:
+                    self.notify_tokens_for(requests_session, addr_path.name)
+        else:
+            self.notify_tokens_for(requests_session, addr)
+
+    def notify_tokens_for(self, requests_session, addr):
+        for token in self.get_tokens(addr):
+            response = requests_session.post(
+                "https://notifications.delta.chat/notify",
+                data=token,
+                timeout=60,
+            )
+            if response.status_code == 410:
+                # 410 Gone status code
+                # means the token is no longer valid.
+                self.remove_token(addr, token)
+        self.notification_dir.joinpath(addr).unlink(missing_ok=True)
 
 
 def handle_dovecot_protocol(rfile, wfile, notifier):

chatmaild/src/chatmaild/tests/test_metadata.py

@@ -84,7 +84,7 @@ def test_handle_dovecot_request_happy_path(notifier, testaddr):
     assert handle_dovecot_request(f"B{tx2}\t{testaddr}", transactions, notifier) is None
     msg = f"S{tx2}\tpriv/guid00/messagenew"
     assert handle_dovecot_request(msg, transactions, notifier) is None
-    assert notifier.message_arrived_event.is_set()
+    assert notifier.notification_queue.get() == testaddr
     assert handle_dovecot_request(f"C{tx2}", transactions, notifier) == "O\n"
     assert not transactions
     assert notifier.notification_dir.joinpath(testaddr).exists()
@@ -159,8 +159,8 @@ def test_handle_dovecot_protocol_messagenew(notifier):
     wfile = io.BytesIO()
     handle_dovecot_protocol(rfile, wfile, notifier)
     assert wfile.getvalue() == b"O\n"
-    assert notifier.message_arrived_event.is_set()
-    assert notifier.notification_dir.joinpath("user@example.org").exists()
+    addr = notifier.notification_queue.get()
+    assert notifier.notification_dir.joinpath(addr).exists()
 
 
 def test_notifier_thread_run(notifier, testaddr):