diff --git a/chatmaild/src/chatmaild/metadata.py b/chatmaild/src/chatmaild/metadata.py index f5e93e2..da181f5 100644 --- a/chatmaild/src/chatmaild/metadata.py +++ b/chatmaild/src/chatmaild/metadata.py @@ -44,7 +44,7 @@ class Notifier: def get_metadata_dict(self, addr): return FileDict(self.vmail_dir / addr / "metadata.json") - def add_token(self, addr, token): + def add_token_to_addr(self, addr, token): with self.get_metadata_dict(addr).modify() as data: tokens = data.get(METADATA_TOKEN_KEY) if tokens is None: @@ -52,7 +52,7 @@ class Notifier: elif token not in tokens: tokens.append(token) - def remove_token(self, addr, token): + def remove_token_from_addr(self, addr, token): with self.get_metadata_dict(addr).modify() as data: tokens = data.get(METADATA_TOKEN_KEY, []) try: @@ -69,11 +69,15 @@ class Notifier: self.add_token_for_retry(token) def add_token_for_retry(self, token, numtries=0): + if numtries >= self.MAX_NUMBER_OF_TRIES: + return False + when = time.time() if numtries > 0: # backup exponentially with number of retries when += pow(self.NOTIFICATION_RETRY_DELAY, numtries) self.retry_queues[numtries].put((when, token)) + return True def requeue_persistent_pending_tokens(self): for token_path in self.notification_dir.iterdir(): @@ -120,19 +124,14 @@ class Notifier: except FileNotFoundError: logging.warning("no address for token %r:", token) return - self.remove_token(addr, token) + self.remove_token_from_addr(addr, token) token_path.unlink(missing_ok=True) return logging.warning("Notification request failed: %r", response) - numtries += 1 - if numtries < self.MAX_NUMBER_OF_TRIES: - self.add_token_for_retry(token, numtries=numtries) - else: + if not self.add_token_for_retry(token, numtries=numtries + 1): token_path.unlink(missing_ok=True) - logging.warning( - "giving up on token after %d tries: %r", numtries - 1, token - ) + logging.warning("dropping token after %d tries: %r", numtries - 1, token) def handle_dovecot_protocol(rfile, wfile, notifier): diff --git a/chatmaild/src/chatmaild/tests/test_metadata.py b/chatmaild/src/chatmaild/tests/test_metadata.py index ebbefaa..fe75f70 100644 --- a/chatmaild/src/chatmaild/tests/test_metadata.py +++ b/chatmaild/src/chatmaild/tests/test_metadata.py @@ -55,24 +55,24 @@ def test_notifier_persistence(tmp_path, testaddr, testaddr2): assert not notifier1.get_tokens(testaddr) assert not notifier2.get_tokens(testaddr) - notifier1.add_token(testaddr, "01234") - notifier1.add_token(testaddr2, "456") + notifier1.add_token_to_addr(testaddr, "01234") + notifier1.add_token_to_addr(testaddr2, "456") assert notifier2.get_tokens(testaddr) == ["01234"] assert notifier2.get_tokens(testaddr2) == ["456"] - notifier2.remove_token(testaddr, "01234") + notifier2.remove_token_from_addr(testaddr, "01234") assert not notifier1.get_tokens(testaddr) assert notifier1.get_tokens(testaddr2) == ["456"] def test_remove_nonexisting(tmp_path, testaddr): notifier1 = Notifier(tmp_path) - notifier1.add_token(testaddr, "123") - notifier1.remove_token(testaddr, "1l23k1l2k3") + notifier1.add_token_to_addr(testaddr, "123") + notifier1.remove_token_from_addr(testaddr, "1l23k1l2k3") assert notifier1.get_tokens(testaddr) == ["123"] -def test_notifier_delete_without_set(notifier, testaddr): - notifier.remove_token(testaddr, "123") +def test_notifier_remove_without_set(notifier, testaddr): + notifier.remove_token_from_addr(testaddr, "123") assert not notifier.get_tokens(testaddr) @@ -171,7 +171,7 @@ def test_handle_dovecot_protocol_iterate(notifier): def test_notifier_thread_firstrun(notifier, testaddr): reqmock = get_mocked_requests([200]) - notifier.add_token(testaddr, "01234") + notifier.add_token_to_addr(testaddr, "01234") notifier.new_message_for_addr(testaddr) notifier.thread_retry_one(reqmock, numtries=0) url, data, timeout = reqmock.requests[0] @@ -182,7 +182,7 @@ def test_notifier_thread_firstrun(notifier, testaddr): def test_notifier_thread_run(notifier, testaddr): - notifier.add_token(testaddr, "01234") + notifier.add_token_to_addr(testaddr, "01234") notifier.new_message_for_addr(testaddr) reqmock = get_mocked_requests([200]) notifier.thread_retry_one(reqmock, numtries=0) @@ -196,7 +196,7 @@ def test_notifier_thread_run(notifier, testaddr): @pytest.mark.parametrize("status", [requests.exceptions.RequestException(), 404, 500]) def test_notifier_thread_connection_failures(notifier, testaddr, status, caplog): """ test that tokens keep getting retried until they are given up. """ - notifier.add_token(testaddr, "01234") + notifier.add_token_to_addr(testaddr, "01234") notifier.new_message_for_addr(testaddr) notifier.NOTIFICATION_RETRY_DELAY = 5 for i in range(notifier.MAX_NUMBER_OF_TRIES): @@ -213,14 +213,14 @@ def test_notifier_thread_connection_failures(notifier, testaddr, status, caplog) assert len(caplog.records) == 1 else: assert len(caplog.records) == 2 - assert "giving up" in caplog.records[1].msg + assert "dropping token" in caplog.records[1].msg notifier.requeue_persistent_pending_tokens() assert notifier.retry_queues[0].qsize() == 0 def test_multi_device_notifier(notifier, testaddr): - notifier.add_token(testaddr, "01234") - notifier.add_token(testaddr, "56789") + notifier.add_token_to_addr(testaddr, "01234") + notifier.add_token_to_addr(testaddr, "56789") notifier.new_message_for_addr(testaddr) reqmock = get_mocked_requests([200, 200]) @@ -236,8 +236,8 @@ def test_multi_device_notifier(notifier, testaddr): def test_notifier_thread_run_gone_removes_token(notifier, testaddr): - notifier.add_token(testaddr, "01234") - notifier.add_token(testaddr, "45678") + notifier.add_token_to_addr(testaddr, "01234") + notifier.add_token_to_addr(testaddr, "45678") notifier.new_message_for_addr(testaddr) reqmock = get_mocked_requests([410, 200]) notifier.thread_retry_one(reqmock, numtries=0)