From 5b0ad6042d0205c24ab211fc8fbaae0fa38426c7 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sat, 31 Jan 2026 08:39:01 -0300 Subject: [PATCH] fix: Don't set download state to Failure if message is available on another Session's transport (#7684) --- .../tests/test_multitransport.py | 28 ++++++++++++++++ src/download.rs | 33 +++++++++++-------- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/deltachat-rpc-client/tests/test_multitransport.py b/deltachat-rpc-client/tests/test_multitransport.py index fda851b48f..1a2ab6daaa 100644 --- a/deltachat-rpc-client/tests/test_multitransport.py +++ b/deltachat-rpc-client/tests/test_multitransport.py @@ -1,6 +1,7 @@ import pytest from deltachat_rpc_client import EventType +from deltachat_rpc_client.const import DownloadState from deltachat_rpc_client.rpc import JsonRpcError @@ -120,6 +121,33 @@ def test_change_address(acfactory) -> None: assert sender_addr2 == new_alice_addr +def test_download_on_demand(acfactory) -> None: + alice, bob = acfactory.get_online_accounts(2) + alice.set_config("download_limit", "1") + + alice.stop_io() + qr = acfactory.get_account_qr() + alice.add_transport_from_qr(qr) + alice.start_io() + + alice.create_chat(bob) + chat_bob_alice = bob.create_chat(alice) + chat_bob_alice.send_message(file="../test-data/image/screenshot.jpg") + msg = alice.wait_for_incoming_msg() + snapshot = msg.get_snapshot() + assert snapshot.download_state == DownloadState.AVAILABLE + chat_id = snapshot.chat_id + # Actually the message isn't available yet. Wait somehow for the post-message to arrive. + chat_bob_alice.send_message("Now you can download my previous message") + alice.wait_for_incoming_msg() + alice._rpc.download_full_message(alice.id, msg.id) + for dstate in [DownloadState.IN_PROGRESS, DownloadState.DONE]: + event = alice.wait_for_event(EventType.MSGS_CHANGED) + assert event.chat_id == chat_id + assert event.msg_id == msg.id + assert msg.get_snapshot().download_state == dstate + + @pytest.mark.parametrize("is_chatmail", ["0", "1"]) def test_mvbox_move_first_transport(acfactory, is_chatmail) -> None: """Test that mvbox_move is disabled by default even for non-chatmail accounts. diff --git a/src/download.rs b/src/download.rs index 8046540e2e..2ec995fd88 100644 --- a/src/download.rs +++ b/src/download.rs @@ -99,7 +99,8 @@ impl MsgId { Ok(()) } - /// Updates the message download state. Returns `Ok` if the message doesn't exist anymore. + /// Updates the message download state. Returns `Ok` if the message doesn't exist anymore or has + /// the download state up to date. pub(crate) async fn update_download_state( self, context: &Context, @@ -108,7 +109,7 @@ impl MsgId { if context .sql .execute( - "UPDATE msgs SET download_state=? WHERE id=?;", + "UPDATE msgs SET download_state=? WHERE id=? AND download_state<>?1", (download_state, self), ) .await? @@ -135,42 +136,46 @@ impl Message { } } -/// Actually download a message partially downloaded before. +/// Actually downloads a message partially downloaded before if the message is available on the +/// session transport, in which case returns `Some`. If the message is available on another +/// transport, returns `None`. /// /// Most messages are downloaded automatically on fetch instead. pub(crate) async fn download_msg( context: &Context, rfc724_mid: String, session: &mut Session, -) -> Result<()> { +) -> Result> { let transport_id = session.transport_id(); let row = context .sql .query_row_optional( - "SELECT uid, folder FROM imap - WHERE rfc724_mid=? - AND transport_id=? - AND target!=''", + "SELECT uid, folder, transport_id FROM imap + WHERE rfc724_mid=? AND target!='' + ORDER BY transport_id=? DESC LIMIT 1", (&rfc724_mid, transport_id), |row| { let server_uid: u32 = row.get(0)?; let server_folder: String = row.get(1)?; - Ok((server_uid, server_folder)) + let msg_transport_id: u32 = row.get(2)?; + Ok((server_uid, server_folder, msg_transport_id)) }, ) .await?; - let Some((server_uid, server_folder)) = row else { + let Some((server_uid, server_folder, msg_transport_id)) = row else { // No IMAP record found, we don't know the UID and folder. return Err(anyhow!( "IMAP location for {rfc724_mid:?} post-message is unknown" )); }; - + if msg_transport_id != transport_id { + return Ok(None); + } session .fetch_single_msg(context, &server_folder, server_uid, rfc724_mid) .await?; - Ok(()) + Ok(Some(())) } impl Session { @@ -272,7 +277,7 @@ pub(crate) async fn download_msgs(context: &Context, session: &mut Session) -> R for rfc724_mid in &rfc724_mids { let res = download_msg(context, rfc724_mid.clone(), session).await; - if res.is_ok() { + if let Ok(Some(())) = res { delete_from_downloads(context, rfc724_mid).await?; delete_from_available_post_msgs(context, rfc724_mid).await?; } @@ -327,7 +332,7 @@ pub(crate) async fn download_known_post_messages_without_pre_message( // The message may be in the wrong order, // but at least we have it at all. let res = download_msg(context, rfc724_mid.clone(), session).await; - if res.is_ok() { + if let Ok(Some(())) = res { delete_from_available_post_msgs(context, rfc724_mid).await?; } if let Err(err) = res {