Use move when available

Author: mrhiggi-close

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]

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

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]

pytest.ini

@@ -1,4 +1,5 @@
 [pytest]
+testpaths = tests
 norecursedirs = inbox tests/imap/network tests/data
 timeout = 60
 

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)