From 7cb6b97de29adbf02c07d2244b3e15b07a853991 Mon Sep 17 00:00:00 2001 From: Matt Higgins Date: Fri, 23 Jan 2026 12:30:02 -0500 Subject: [PATCH 1/3] Use move when available --- inbox/actions/backends/generic.py | 8 ++++-- inbox/crispin.py | 4 +++ inbox/util/testutils.py | 17 +++++++++++-- pytest.ini | 1 + tests/imap/test_actions.py | 42 +++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-) diff --git a/inbox/actions/backends/generic.py b/inbox/actions/backends/generic.py index caa962789..a9f1541c2 100644 --- a/inbox/actions/backends/generic.py +++ b/inbox/actions/backends/generic.py @@ -111,8 +111,12 @@ def remote_move( # type: ignore[no-untyped-def] for folder_name, uids in uids_for_message.items(): crispin_client.select_folder_if_necessary(folder_name, uidvalidity_cb) - crispin_client.conn.copy(uids, destination) - crispin_client.delete_uids(uids) + + if crispin_client.move_supported(): + crispin_client.conn.move(uids, destination) + else: + crispin_client.conn.copy(uids, destination) + crispin_client.delete_uids(uids) def remote_create_folder( # type: ignore[no-untyped-def] diff --git a/inbox/crispin.py b/inbox/crispin.py index 89c41e29a..c9bd94534 100644 --- a/inbox/crispin.py +++ b/inbox/crispin.py @@ -874,6 +874,10 @@ def idle_supported(self) -> bool: return b"IDLE" in self.conn.capabilities() + def move_supported(self) -> bool: + interruptible_threading.check_interrupted() + return b"MOVE" in self.conn.capabilities() + def search_uids(self, criteria: list[str]) -> Iterable[int]: """ Find UIDs in this folder matching the criteria. See diff --git a/inbox/util/testutils.py b/inbox/util/testutils.py index c04016958..188534991 100644 --- a/inbox/util/testutils.py +++ b/inbox/util/testutils.py @@ -263,8 +263,21 @@ def copy( # type: ignore[no-untyped-def] self, matching_uids, folder_name ) -> None: """ - Note: _moves_ one or more messages from the currently selected folder - to folder_name + Copies one or more messages from the currently selected folder + to folder_name. + + Note: Also deletes from source to simulate existing test expectations. + """ + for u in matching_uids: + self._data[folder_name][u] = self._data[self.selected_folder][u] + self.delete_messages(matching_uids) + + def move( # type: ignore[no-untyped-def] + self, matching_uids, folder_name + ) -> None: + """ + Atomically moves one or more messages from the currently selected folder + to folder_name (RFC 6851). """ for u in matching_uids: self._data[folder_name][u] = self._data[self.selected_folder][u] diff --git a/pytest.ini b/pytest.ini index 5a2ea1d7e..2886d3a49 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,4 +1,5 @@ [pytest] +testpaths = tests norecursedirs = inbox tests/imap/network tests/data timeout = 60 diff --git a/tests/imap/test_actions.py b/tests/imap/test_actions.py index 064e4b527..ce42ef15a 100644 --- a/tests/imap/test_actions.py +++ b/tests/imap/test_actions.py @@ -13,6 +13,7 @@ delete_label, mark_starred, mark_unread, + move, save_draft, update_draft, update_folder, @@ -275,3 +276,44 @@ def test_failed_event_creation( assert all(a.status == "failed" for a in q) service.stop() + + +def test_move_uses_imap_move_when_supported( + db, default_account, message, folder, mock_imapclient +) -> None: + """Test that IMAP MOVE command is used when the server supports it.""" + mock_imapclient.add_folder_data(folder.name, {}) + mock_imapclient.add_folder_data("Archive", {}) + mock_imapclient.capabilities = mock.Mock(return_value=[b"IMAP4rev1", b"MOVE"]) + mock_imapclient.move = mock.Mock() + mock_imapclient.copy = mock.Mock() + mock_imapclient.delete_messages = mock.Mock() + add_fake_imapuid(db.session, default_account.id, message, folder, 42) + + with writable_connection_pool(default_account.id).get() as crispin_client: + move(crispin_client, default_account.id, message.id, {"destination": "Archive"}) + + mock_imapclient.move.assert_called_once_with([42], "Archive") + mock_imapclient.copy.assert_not_called() + + +def test_move_falls_back_to_copy_delete_when_move_not_supported( + db, default_account, message, folder, mock_imapclient +) -> None: + """Test fallback to COPY+DELETE when MOVE is not supported.""" + mock_imapclient.add_folder_data(folder.name, {}) + mock_imapclient.add_folder_data("Archive", {}) + mock_imapclient.capabilities = mock.Mock(return_value=[b"IMAP4rev1"]) + mock_imapclient.move = mock.Mock() + mock_imapclient.copy = mock.Mock() + mock_imapclient.delete_messages = mock.Mock() + mock_imapclient.expunge = mock.Mock() + add_fake_imapuid(db.session, default_account.id, message, folder, 42) + + with writable_connection_pool(default_account.id).get() as crispin_client: + move(crispin_client, default_account.id, message.id, {"destination": "Archive"}) + + mock_imapclient.move.assert_not_called() + mock_imapclient.copy.assert_called_once_with([42], "Archive") + # delete_uids converts UIDs to strings before calling delete_messages + mock_imapclient.delete_messages.assert_called_once_with(["42"], silent=True) From 943044a919974bb4b355ff4b8a07e18596f06588 Mon Sep 17 00:00:00 2001 From: Matt Higgins Date: Fri, 23 Jan 2026 12:39:58 -0500 Subject: [PATCH 2/3] Format --- tests/imap/test_actions.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/imap/test_actions.py b/tests/imap/test_actions.py index ce42ef15a..9c0a4cf41 100644 --- a/tests/imap/test_actions.py +++ b/tests/imap/test_actions.py @@ -284,14 +284,21 @@ def test_move_uses_imap_move_when_supported( """Test that IMAP MOVE command is used when the server supports it.""" mock_imapclient.add_folder_data(folder.name, {}) mock_imapclient.add_folder_data("Archive", {}) - mock_imapclient.capabilities = mock.Mock(return_value=[b"IMAP4rev1", b"MOVE"]) + mock_imapclient.capabilities = mock.Mock( + return_value=[b"IMAP4rev1", b"MOVE"] + ) mock_imapclient.move = mock.Mock() mock_imapclient.copy = mock.Mock() mock_imapclient.delete_messages = mock.Mock() add_fake_imapuid(db.session, default_account.id, message, folder, 42) with writable_connection_pool(default_account.id).get() as crispin_client: - move(crispin_client, default_account.id, message.id, {"destination": "Archive"}) + move( + crispin_client, + default_account.id, + message.id, + {"destination": "Archive"}, + ) mock_imapclient.move.assert_called_once_with([42], "Archive") mock_imapclient.copy.assert_not_called() @@ -311,9 +318,16 @@ def test_move_falls_back_to_copy_delete_when_move_not_supported( add_fake_imapuid(db.session, default_account.id, message, folder, 42) with writable_connection_pool(default_account.id).get() as crispin_client: - move(crispin_client, default_account.id, message.id, {"destination": "Archive"}) + move( + crispin_client, + default_account.id, + message.id, + {"destination": "Archive"}, + ) mock_imapclient.move.assert_not_called() mock_imapclient.copy.assert_called_once_with([42], "Archive") # delete_uids converts UIDs to strings before calling delete_messages - mock_imapclient.delete_messages.assert_called_once_with(["42"], silent=True) + mock_imapclient.delete_messages.assert_called_once_with( + ["42"], silent=True + ) From 624a62b8711b4a16638779316f6980dfdb79fc1f Mon Sep 17 00:00:00 2001 From: Matt Higgins Date: Fri, 23 Jan 2026 12:45:20 -0500 Subject: [PATCH 3/3] Rephrase docstrings --- inbox/util/testutils.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/inbox/util/testutils.py b/inbox/util/testutils.py index 188534991..8b6849ca1 100644 --- a/inbox/util/testutils.py +++ b/inbox/util/testutils.py @@ -263,8 +263,7 @@ def copy( # type: ignore[no-untyped-def] self, matching_uids, folder_name ) -> None: """ - Copies one or more messages from the currently selected folder - to folder_name. + Copy one or more messages from the currently selected folder. Note: Also deletes from source to simulate existing test expectations. """ @@ -276,8 +275,7 @@ def move( # type: ignore[no-untyped-def] self, matching_uids, folder_name ) -> None: """ - Atomically moves one or more messages from the currently selected folder - to folder_name (RFC 6851). + Atomically move one or more messages to folder_name (RFC 6851). """ for u in matching_uids: self._data[folder_name][u] = self._data[self.selected_folder][u]