mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-15 11:22:21 +02:00
fix(email): preserve mail when bulk Trash moves fail (#6633)
* fix(email): preserve mail when bulk Trash moves fail * fix(email): selective UID EXPUNGE and safe bulk delete cache invalidation * fix(email): eliminate mailbox-wide EXPUNGE TOCTOU race - Strictly prohibit mailbox-wide EXPUNGE in bulk email deletion fallback - Restrict cleanup exclusively to selective RFC 4315 UID EXPUNGE <set> - Track and preserve pre-existing \Deleted flags if selective expunge fails - Retain local index records and preserve source emails when cleanup fails - Ensure consistent partial-failure error semantics across all failure modes - Add comprehensive hermetic tests for concurrent clients, missing UIDPLUS, disconnects after COPY/STORE/EXPUNGE, and retry safety --------- Co-authored-by: PR #6166 Reviewer <reviewer@odysseus-review.local>
This commit is contained in:
Robert Vind-Gardoș
co-authored by
PR #6166 Reviewer
PR #6166 Reviewer
parent
c23b1be3f8
commit
0c56527d12
@@ -81,6 +81,7 @@ _SERVER_OWNED_OAUTH_FIELDS = {
|
||||
"oauth_refresh_token",
|
||||
"oauth_token_expiry",
|
||||
}
|
||||
_CACHE_HOOKS: dict = {"invalidate_list": None}
|
||||
|
||||
|
||||
def _normalized_mail_host(value) -> str:
|
||||
@@ -572,6 +573,23 @@ def _uid_exists(conn, uid: str, *, strict: bool = False) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _uid_has_deleted_flag(conn, uid: str) -> bool:
|
||||
r"""Return True if message already carries \Deleted flag."""
|
||||
try:
|
||||
status, data = conn.uid("FETCH", _uid_bytes(uid), "(FLAGS)")
|
||||
if status == "OK" and data:
|
||||
for part in data:
|
||||
if not part:
|
||||
continue
|
||||
meta = part[0] if isinstance(part, tuple) else part
|
||||
meta_b = meta if isinstance(meta, bytes) else str(meta).encode()
|
||||
if rb"\Deleted" in meta_b:
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
return False
|
||||
|
||||
|
||||
def _resolve_current_email_uid(conn, uid: str, message_id: str | None = None) -> str:
|
||||
"""Resolve a stale cached UID by the message's stable RFC Message-ID."""
|
||||
uid = str(uid or "").strip()
|
||||
@@ -1831,6 +1849,12 @@ def setup_email_routes():
|
||||
delete, move) so the UI doesn't show stale read/unread counts for up to
|
||||
the 8s TTL after a manual flag change. With no args, clears everything.
|
||||
"""
|
||||
hook = _CACHE_HOOKS.get("invalidate_list")
|
||||
if hook:
|
||||
try:
|
||||
hook(account_id, folder)
|
||||
except Exception:
|
||||
pass
|
||||
if account_id is None and folder is None:
|
||||
_LIST_CACHE.clear()
|
||||
return
|
||||
@@ -4764,6 +4788,7 @@ def setup_email_routes():
|
||||
return {"success": False, "error": "No email UIDs provided", "deleted_uids": [], "failed_uids": []}
|
||||
deleted_uids = []
|
||||
failed_uids = []
|
||||
trash_folder = None
|
||||
try:
|
||||
with _imap(account_id, owner=owner) as conn:
|
||||
select_status, _ = conn.select(_q(folder), readonly=False)
|
||||
@@ -4779,7 +4804,8 @@ def setup_email_routes():
|
||||
trash_folder = "Trash"
|
||||
except Exception:
|
||||
pass
|
||||
pending_expunge = False
|
||||
pending_expunge = []
|
||||
preexisting_deleted = set()
|
||||
for uid in uids:
|
||||
try:
|
||||
if not _uid_exists(conn, uid):
|
||||
@@ -4789,32 +4815,75 @@ def setup_email_routes():
|
||||
if status != "OK":
|
||||
copy_status, _ = conn.uid("COPY", _uid_bytes(uid), _q(trash_folder))
|
||||
if copy_status != "OK":
|
||||
# Keep deletion reliable even when this IMAP
|
||||
# server has no writable Trash folder.
|
||||
store_status, _ = conn.uid("STORE", _uid_bytes(uid), "+FLAGS", "\\Deleted")
|
||||
if store_status != "OK":
|
||||
failed_uids.append(uid)
|
||||
continue
|
||||
pending_expunge = True
|
||||
else:
|
||||
store_status, _ = conn.uid("STORE", _uid_bytes(uid), "+FLAGS", "\\Deleted")
|
||||
if store_status != "OK":
|
||||
failed_uids.append(uid)
|
||||
continue
|
||||
pending_expunge = True
|
||||
# Bulk Delete means move to Trash. Never remove
|
||||
# the original without a recoverable Trash copy.
|
||||
failed_uids.append(uid)
|
||||
continue
|
||||
if _uid_has_deleted_flag(conn, uid):
|
||||
preexisting_deleted.add(uid)
|
||||
store_status, _ = conn.uid("STORE", _uid_bytes(uid), "+FLAGS", "\\Deleted")
|
||||
if store_status != "OK":
|
||||
failed_uids.append(uid)
|
||||
continue
|
||||
# Keep the local index until the server confirms
|
||||
# that the copied originals have been removed.
|
||||
pending_expunge.append(uid)
|
||||
continue
|
||||
deleted_uids.append(uid)
|
||||
_email_index_delete(owner, account_id, folder, uid)
|
||||
except Exception:
|
||||
failed_uids.append(uid)
|
||||
logger.debug("Bulk email delete failed for uid=%s", uid, exc_info=True)
|
||||
if pending_expunge:
|
||||
conn.expunge()
|
||||
if deleted_uids:
|
||||
_invalidate_list_cache(account_id, folder)
|
||||
return {"success": True, "deleted_uids": deleted_uids, "failed_uids": failed_uids}
|
||||
expunge_ok = False
|
||||
# Attempt selective RFC 4315 UID EXPUNGE so unrelated messages
|
||||
# carrying \Deleted in the selected mailbox are never expunged.
|
||||
# Mailbox-wide EXPUNGE is strictly avoided to eliminate TOCTOU races
|
||||
# where concurrent clients mark unrelated messages \Deleted.
|
||||
try:
|
||||
uid_set_bytes = _uid_bytes(",".join(pending_expunge))
|
||||
uid_exp_status, _ = conn.uid("EXPUNGE", uid_set_bytes)
|
||||
if uid_exp_status == "OK":
|
||||
expunge_ok = True
|
||||
except Exception:
|
||||
logger.debug("Selective UID EXPUNGE failed", exc_info=True)
|
||||
expunge_ok = False
|
||||
|
||||
if expunge_ok:
|
||||
for uid in pending_expunge:
|
||||
deleted_uids.append(uid)
|
||||
_email_index_delete(owner, account_id, folder, uid)
|
||||
else:
|
||||
# Prioritize preservation over cleanup: unflag \Deleted from our
|
||||
# copied messages so future EXPUNGE operations do not destroy them.
|
||||
# Do not remove a pre-existing \Deleted flag present before this operation.
|
||||
for uid in pending_expunge:
|
||||
if uid not in preexisting_deleted:
|
||||
try:
|
||||
conn.uid("STORE", _uid_bytes(uid), "-FLAGS", "\\Deleted")
|
||||
except Exception:
|
||||
pass
|
||||
failed_uids.extend(pending_expunge)
|
||||
if not deleted_uids:
|
||||
return {"success": False, "error": "Failed to delete requested emails", "deleted_uids": [], "failed_uids": failed_uids}
|
||||
res = {"success": True, "deleted_uids": deleted_uids, "failed_uids": failed_uids}
|
||||
if failed_uids:
|
||||
res["error"] = "Mail operation partially failed"
|
||||
return res
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to bulk delete emails: {e}")
|
||||
return {"success": False, "error": "Mail operation failed", "deleted_uids": deleted_uids, "failed_uids": [u for u in uids if u not in deleted_uids]}
|
||||
failed_all = [u for u in uids if u not in deleted_uids]
|
||||
return {
|
||||
"success": bool(deleted_uids),
|
||||
"error": "Mail operation failed" if not deleted_uids else "Mail operation partially failed",
|
||||
"deleted_uids": deleted_uids,
|
||||
"failed_uids": failed_all,
|
||||
}
|
||||
finally:
|
||||
if deleted_uids:
|
||||
_invalidate_list_cache(account_id, folder)
|
||||
if trash_folder:
|
||||
_invalidate_list_cache(account_id, trash_folder)
|
||||
|
||||
@router.delete("/odysseus/reminders")
|
||||
async def delete_odysseus_reminder_emails(
|
||||
|
||||
@@ -0,0 +1,509 @@
|
||||
"""Bulk Delete must retain messages that could not be moved to Trash."""
|
||||
|
||||
from contextlib import contextmanager
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import Mock
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
|
||||
class Mailbox:
|
||||
def __init__(self):
|
||||
self.inbox = {"101", "102", "103"}
|
||||
self.trash = set()
|
||||
self.deleted = set()
|
||||
self.calls = []
|
||||
self.move_status = "NO"
|
||||
self.copy_status = {}
|
||||
self.store_status = {}
|
||||
self.expunge_status = "OK"
|
||||
self.uid_expunge_status = None
|
||||
self.select_status = "OK"
|
||||
|
||||
def select(self, folder, readonly=False):
|
||||
return self.select_status, [b"3"]
|
||||
|
||||
def uid(self, command, raw_uid, *args):
|
||||
uid = raw_uid.decode() if isinstance(raw_uid, bytes) else (str(raw_uid) if raw_uid is not None else "")
|
||||
self.calls.append((command, uid))
|
||||
if command == "FETCH":
|
||||
if uid not in self.inbox:
|
||||
return "OK", []
|
||||
if args and "(FLAGS)" in args:
|
||||
flags = rb"(\Deleted)" if uid in self.deleted else rb"()"
|
||||
return "OK", [(f"1 (UID {uid} FLAGS ".encode() + flags + b")")]
|
||||
return "OK", [f"1 (UID {uid})".encode()]
|
||||
if command == "SEARCH":
|
||||
if args and "DELETED" in args:
|
||||
return "OK", [b" ".join(u.encode() for u in sorted(self.deleted))]
|
||||
searched_uid = args[-1].split()[-1] if args else ""
|
||||
return "OK", [searched_uid.encode() if searched_uid in self.inbox else b""]
|
||||
if command == "MOVE":
|
||||
status = self.move_status.get(uid, "NO") if isinstance(self.move_status, dict) else self.move_status
|
||||
if isinstance(status, Exception):
|
||||
raise status
|
||||
if status == "OK":
|
||||
self.trash.add(uid)
|
||||
self.inbox.remove(uid)
|
||||
return status, []
|
||||
if command == "COPY":
|
||||
status = self.copy_status.get(uid, "OK")
|
||||
if isinstance(status, Exception):
|
||||
raise status
|
||||
if status == "OK":
|
||||
self.trash.add(uid)
|
||||
return status, []
|
||||
if command == "STORE":
|
||||
status = self.store_status.get(uid, "OK")
|
||||
if isinstance(status, Exception):
|
||||
raise status
|
||||
if status == "OK":
|
||||
if args and args[0] == "-FLAGS":
|
||||
self.deleted.discard(uid)
|
||||
else:
|
||||
self.deleted.add(uid)
|
||||
return status, []
|
||||
if command == "EXPUNGE":
|
||||
status = self.uid_expunge_status if self.uid_expunge_status is not None else self.expunge_status
|
||||
if isinstance(status, Exception):
|
||||
raise status
|
||||
if status == "OK":
|
||||
expunged = {u for u in uid.split(",") if u in self.deleted}
|
||||
self.inbox.difference_update(expunged)
|
||||
self.deleted.difference_update(expunged)
|
||||
return status, []
|
||||
raise AssertionError(f"Unexpected IMAP command: {command}")
|
||||
|
||||
def expunge(self):
|
||||
self.calls.append(("EXPUNGE", None))
|
||||
if isinstance(self.expunge_status, Exception):
|
||||
raise self.expunge_status
|
||||
if self.expunge_status == "OK":
|
||||
self.inbox.difference_update(self.deleted)
|
||||
self.deleted.clear()
|
||||
return self.expunge_status, []
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def bulk_delete(monkeypatch):
|
||||
from routes.email import email_routes as routes
|
||||
|
||||
mailbox = Mailbox()
|
||||
index_delete = Mock()
|
||||
invalidate_cache = Mock()
|
||||
|
||||
@contextmanager
|
||||
def imap(account_id=None, owner=""):
|
||||
assert owner == "alice"
|
||||
yield mailbox
|
||||
|
||||
monkeypatch.setattr(routes, "_start_poller", lambda: None)
|
||||
monkeypatch.setattr(routes, "_POOL_HOOKS", {})
|
||||
monkeypatch.setattr(routes, "_imap", imap)
|
||||
monkeypatch.setattr(routes, "_resolve_mail_folder", lambda *args: "Trash")
|
||||
monkeypatch.setattr(routes, "_list_imap_folders", lambda conn: ({}, ["INBOX", "Trash"]))
|
||||
monkeypatch.setattr(routes, "_email_index_delete", index_delete)
|
||||
monkeypatch.setitem(routes._CACHE_HOOKS, "invalidate_list", invalidate_cache)
|
||||
app = FastAPI()
|
||||
app.include_router(routes.setup_email_routes())
|
||||
app.dependency_overrides[routes.require_owner] = lambda: "alice"
|
||||
|
||||
with TestClient(app) as client:
|
||||
def delete(uids):
|
||||
response = client.post("/api/email/delete-bulk", json={"uids": uids})
|
||||
assert response.status_code == 200
|
||||
return response.json()
|
||||
|
||||
yield SimpleNamespace(
|
||||
mailbox=mailbox,
|
||||
delete=delete,
|
||||
index_delete=index_delete,
|
||||
invalidate_cache=invalidate_cache,
|
||||
routes=routes,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("copy_status", ["NO", "BAD"])
|
||||
def test_failed_trash_copy_preserves_original(bulk_delete, copy_status):
|
||||
bulk_delete.mailbox.copy_status["101"] = copy_status
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
assert bulk_delete.mailbox.deleted == set()
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert ("STORE", "101") not in bulk_delete.mailbox.calls
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_mixed_batch_only_deletes_messages_copied_to_trash(bulk_delete):
|
||||
bulk_delete.mailbox.copy_status["102"] = "NO"
|
||||
|
||||
result = bulk_delete.delete(["101", "102", "103"])
|
||||
|
||||
assert bulk_delete.mailbox.inbox == {"102"}
|
||||
assert bulk_delete.mailbox.trash == {"101", "103"}
|
||||
assert result["deleted_uids"] == ["101", "103"]
|
||||
assert result["failed_uids"] == ["102"]
|
||||
assert [call.args[-1] for call in bulk_delete.index_delete.call_args_list] == ["101", "103"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("expunge_status", ["NO", "BAD", RuntimeError("Disconnected")])
|
||||
def test_failed_expunge_keeps_index_and_reports_failure(bulk_delete, expunge_status):
|
||||
bulk_delete.mailbox.expunge_status = expunge_status
|
||||
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert bulk_delete.mailbox.inbox == {"101", "102", "103"}
|
||||
assert bulk_delete.mailbox.trash == {"101", "102"}
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101", "102"]
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_store_failure_retains_original_and_index(bulk_delete):
|
||||
bulk_delete.mailbox.store_status["101"] = "NO"
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
assert bulk_delete.mailbox.trash == {"101"}
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert result["deleted_uids"] == []
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_native_move_needs_no_copy_or_expunge(bulk_delete):
|
||||
bulk_delete.mailbox.move_status = "OK"
|
||||
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert result["deleted_uids"] == ["101", "102"]
|
||||
assert result["failed_uids"] == []
|
||||
assert bulk_delete.mailbox.inbox == {"103"}
|
||||
assert bulk_delete.mailbox.trash == {"101", "102"}
|
||||
assert not any(command in {"COPY", "STORE", "EXPUNGE"} for command, _ in bulk_delete.mailbox.calls)
|
||||
|
||||
|
||||
def test_copy_fallback_expunge_runs_once_for_the_batch(bulk_delete):
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert result["deleted_uids"] == ["101", "102"]
|
||||
assert result["failed_uids"] == []
|
||||
assert bulk_delete.mailbox.inbox == {"103"}
|
||||
assert bulk_delete.mailbox.trash == {"101", "102"}
|
||||
assert sum(1 for command, _ in bulk_delete.mailbox.calls if command == "EXPUNGE") == 1
|
||||
assert bulk_delete.index_delete.call_count == 2
|
||||
|
||||
|
||||
def test_failed_folder_selection_never_changes_messages(bulk_delete):
|
||||
bulk_delete.mailbox.select_status = "NO"
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert bulk_delete.mailbox.calls == []
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("expunge_status", ["NO", RuntimeError("Disconnected")])
|
||||
def test_completed_native_move_is_retained_when_fallback_expunge_fails(bulk_delete, expunge_status):
|
||||
bulk_delete.mailbox.move_status = {"101": "OK"}
|
||||
bulk_delete.mailbox.expunge_status = expunge_status
|
||||
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
assert result["failed_uids"] == ["102"]
|
||||
assert bulk_delete.mailbox.inbox == {"102", "103"}
|
||||
assert bulk_delete.mailbox.trash == {"101", "102"}
|
||||
bulk_delete.index_delete.assert_called_once_with("alice", None, "INBOX", "101")
|
||||
|
||||
|
||||
def test_absent_uid_is_reported_failed_without_deleting_another_message(bulk_delete):
|
||||
result = bulk_delete.delete(["999"])
|
||||
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["999"]
|
||||
assert bulk_delete.mailbox.inbox == {"101", "102", "103"}
|
||||
assert bulk_delete.mailbox.trash == set()
|
||||
assert not any(command in {"MOVE", "COPY", "STORE", "EXPUNGE"} for command, _ in bulk_delete.mailbox.calls)
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_duplicate_and_invalid_uids_do_not_repeat_mail_operations(bulk_delete):
|
||||
result = bulk_delete.delete(["101", "101", "", None, "invalid"])
|
||||
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
assert result["failed_uids"] == []
|
||||
assert bulk_delete.mailbox.calls.count(("COPY", "101")) == 1
|
||||
bulk_delete.index_delete.assert_called_once_with("alice", None, "INBOX", "101")
|
||||
|
||||
|
||||
def test_bulk_delete_invalidates_source_and_trash_caches(bulk_delete):
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
bulk_delete.invalidate_cache.assert_any_call(None, "INBOX")
|
||||
bulk_delete.invalidate_cache.assert_any_call(None, "Trash")
|
||||
|
||||
|
||||
def test_partial_failure_with_exception_invalidates_caches(bulk_delete):
|
||||
bulk_delete.mailbox.move_status = {"101": "OK"}
|
||||
bulk_delete.mailbox.uid_expunge_status = RuntimeError("Socket reset during expunge")
|
||||
bulk_delete.mailbox.expunge_status = RuntimeError("Socket reset during expunge")
|
||||
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
assert result["failed_uids"] == ["102"]
|
||||
assert result["error"] == "Mail operation partially failed"
|
||||
bulk_delete.invalidate_cache.assert_any_call(None, "INBOX")
|
||||
bulk_delete.invalidate_cache.assert_any_call(None, "Trash")
|
||||
|
||||
|
||||
def test_total_failure_returns_success_false(bulk_delete):
|
||||
bulk_delete.mailbox.copy_status["101"] = "NO"
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["error"] == "Failed to delete requested emails"
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
|
||||
|
||||
def test_uid_expunge_preserves_unrelated_deleted_message(bulk_delete):
|
||||
# Unrelated message 999 is already marked \Deleted before bulk delete
|
||||
bulk_delete.mailbox.inbox.add("999")
|
||||
bulk_delete.mailbox.deleted.add("999")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
# 101 was expunged, 999 remains in inbox and remains marked deleted
|
||||
assert "101" not in bulk_delete.mailbox.inbox
|
||||
assert "999" in bulk_delete.mailbox.inbox
|
||||
assert bulk_delete.mailbox.deleted == {"999"}
|
||||
assert ("EXPUNGE", "101") in bulk_delete.mailbox.calls
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
|
||||
|
||||
def test_server_lacking_uidplus_never_calls_mailbox_expunge(bulk_delete):
|
||||
# Server does not support UIDPLUS (UID EXPUNGE returns BAD)
|
||||
# Even if no other deleted messages exist, mailbox-wide EXPUNGE is strictly forbidden.
|
||||
bulk_delete.mailbox.uid_expunge_status = "BAD"
|
||||
assert bulk_delete.mailbox.deleted == set()
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
assert "101" not in bulk_delete.mailbox.deleted
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_concurrent_client_marks_unrelated_deleted_message_during_operation(bulk_delete):
|
||||
# Another client marks unrelated UID 999 \Deleted before/during the operation
|
||||
bulk_delete.mailbox.inbox.add("999")
|
||||
bulk_delete.mailbox.deleted.add("999")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
# 101 was expunged, unrelated 999 is NEVER expunged
|
||||
assert "101" not in bulk_delete.mailbox.inbox
|
||||
assert "999" in bulk_delete.mailbox.inbox
|
||||
assert bulk_delete.mailbox.deleted == {"999"}
|
||||
assert ("EXPUNGE", "101") in bulk_delete.mailbox.calls
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
|
||||
|
||||
def test_preexisting_deleted_flag_preserved_when_expunge_fails(bulk_delete):
|
||||
# Message 101 already had \Deleted flag before the bulk operation
|
||||
bulk_delete.mailbox.deleted.add("101")
|
||||
bulk_delete.mailbox.uid_expunge_status = "NO"
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
# Pre-existing \Deleted flag must NOT be removed
|
||||
assert "101" in bulk_delete.mailbox.deleted
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_preexisting_deleted_flag_expunged_on_success(bulk_delete):
|
||||
# Message 101 already had \Deleted flag before the bulk operation
|
||||
bulk_delete.mailbox.deleted.add("101")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
assert "101" not in bulk_delete.mailbox.inbox
|
||||
assert "101" not in bulk_delete.mailbox.deleted
|
||||
assert "101" in bulk_delete.mailbox.trash
|
||||
bulk_delete.index_delete.assert_called_once_with("alice", None, "INBOX", "101")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", ["NO", "BAD"])
|
||||
def test_uid_expunge_status_failures(bulk_delete, status):
|
||||
bulk_delete.mailbox.uid_expunge_status = status
|
||||
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101", "102"]
|
||||
assert bulk_delete.mailbox.inbox == {"101", "102", "103"}
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_uid_expunge_connection_exception_handled_conservatively(bulk_delete):
|
||||
bulk_delete.mailbox.uid_expunge_status = ConnectionResetError("Connection lost during selective expunge")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_disconnect_after_copy_before_store_preserves_message(bulk_delete):
|
||||
# COPY succeeds, but STORE raises a disconnect
|
||||
bulk_delete.mailbox.store_status["101"] = ConnectionResetError("Disconnected")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
assert "101" in bulk_delete.mailbox.trash
|
||||
assert bulk_delete.mailbox.deleted == set()
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_disconnect_after_store_preserves_messages_and_index(bulk_delete):
|
||||
# COPY and STORE succeed, but UID EXPUNGE disconnects
|
||||
bulk_delete.mailbox.uid_expunge_status = ConnectionResetError("Disconnected during expunge")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert "101" in bulk_delete.mailbox.inbox
|
||||
assert "101" in bulk_delete.mailbox.trash
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_disconnect_with_partial_move_success(bulk_delete):
|
||||
# 101 succeeds via MOVE, 102 falls back to COPY+STORE and UID EXPUNGE disconnects
|
||||
bulk_delete.mailbox.move_status = {"101": "OK", "102": "NO"}
|
||||
bulk_delete.mailbox.uid_expunge_status = ConnectionResetError("Disconnected during expunge")
|
||||
|
||||
result = bulk_delete.delete(["101", "102"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
assert result["failed_uids"] == ["102"]
|
||||
assert "101" not in bulk_delete.mailbox.inbox
|
||||
assert "102" in bulk_delete.mailbox.inbox
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_called_once_with("alice", None, "INBOX", "101")
|
||||
bulk_delete.invalidate_cache.assert_any_call(None, "INBOX")
|
||||
bulk_delete.invalidate_cache.assert_any_call(None, "Trash")
|
||||
|
||||
|
||||
def test_lost_acknowledgement_retains_local_index_and_permits_safe_retry(bulk_delete):
|
||||
# Server succeeds in moving, but connection drops during acknowledgement
|
||||
bulk_delete.mailbox.uid_expunge_status = OSError("Connection dropped")
|
||||
bulk_delete.mailbox.expunge_status = OSError("Connection dropped")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is False
|
||||
assert result["deleted_uids"] == []
|
||||
assert result["failed_uids"] == ["101"]
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
# Retry when server is reconnected and 101 was already moved (absent from source)
|
||||
bulk_delete.mailbox.inbox.discard("101")
|
||||
bulk_delete.mailbox.uid_expunge_status = "OK"
|
||||
bulk_delete.mailbox.expunge_status = "OK"
|
||||
|
||||
retry_result = bulk_delete.delete(["101"])
|
||||
assert retry_result["deleted_uids"] == []
|
||||
assert retry_result["failed_uids"] == ["101"]
|
||||
bulk_delete.index_delete.assert_not_called()
|
||||
|
||||
|
||||
def test_repeated_requests_after_partial_failure(bulk_delete):
|
||||
# Initial request: 101 moves, 102 fails copy
|
||||
bulk_delete.mailbox.move_status = {"101": "OK", "102": "NO"}
|
||||
bulk_delete.mailbox.copy_status["102"] = "NO"
|
||||
|
||||
res1 = bulk_delete.delete(["101", "102"])
|
||||
assert res1["deleted_uids"] == ["101"]
|
||||
assert res1["failed_uids"] == ["102"]
|
||||
|
||||
# Retry with failed UID 102 now succeeding via copy
|
||||
bulk_delete.mailbox.copy_status["102"] = "OK"
|
||||
res2 = bulk_delete.delete(["102"])
|
||||
assert res2["deleted_uids"] == ["102"]
|
||||
assert res2["failed_uids"] == []
|
||||
|
||||
assert bulk_delete.mailbox.inbox == {"103"}
|
||||
assert bulk_delete.mailbox.trash == {"101", "102"}
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
|
||||
|
||||
def test_unrelated_messages_never_expunged(bulk_delete):
|
||||
# Mailbox contains unrelated 888 (with \Deleted) and 999 (without \Deleted)
|
||||
bulk_delete.mailbox.inbox.update({"888", "999"})
|
||||
bulk_delete.mailbox.deleted.add("888")
|
||||
|
||||
result = bulk_delete.delete(["101"])
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["deleted_uids"] == ["101"]
|
||||
assert "101" not in bulk_delete.mailbox.inbox
|
||||
assert "101" in bulk_delete.mailbox.trash
|
||||
# 888 and 999 are completely untouched in inbox
|
||||
assert "888" in bulk_delete.mailbox.inbox
|
||||
assert "999" in bulk_delete.mailbox.inbox
|
||||
assert "888" in bulk_delete.mailbox.deleted
|
||||
assert "888" not in bulk_delete.mailbox.trash
|
||||
assert "999" not in bulk_delete.mailbox.trash
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
|
||||
|
||||
def test_standard_expunge_never_invoked_by_fallback(bulk_delete):
|
||||
# Perform operations under multiple fallback conditions; standard expunge is never called
|
||||
bulk_delete.delete(["101"])
|
||||
assert ("EXPUNGE", None) not in bulk_delete.mailbox.calls
|
||||
Reference in New Issue
Block a user