Skip to content

Commit bb0fd7c

Browse files
Simon-LauxHocuri
authored andcommitted
feat: message previews
Part of #7367 - Remove partial downloads (remove creation of the stub messages) (#7373) - Remove "Download maximum available until" and remove stock string `DC_STR_DOWNLOAD_AVAILABILITY` (#7369) - Send pre-message on messages with large attachments (#7410) - Pre messages can now get read receipts (#7433) progress / what's to do: - [x] send pre-message - [x] The message's state must be set to MessageState::OutDelivered only after both messages are sent. If a read receipt is received, the message can be OutMdnRcvd or OutPending; let's just do whatever is easiest for now. Take care not to revert from OutMdnReceived to OutDelivered if we first receive a read receipt and then deliver the full message. - this is already the case: - `OutDelivered` is set when a message is sent out and has no remaining send jobs in the smtp table for this message id - so already works since full message and pre message have same msgId in that table. - `OutMdnRcvd` is a "virtual" state (#7367 (comment)), so going back to `OutDelivered` can't happen anymore - [x] delimit `ChatFullMessageId` with `<` and `>` like the other message ids - [x] add tests - [x] test that pre message is sent for attachment larger than X - test that correct headers are present on both messages - also test that Autocrypt-gossip and selfavatar should never go into full-messages - [x] test that no pre message is sent for attachment smaller than X - no "is full message" header should be present - [x] test that pre message is not send for large webxdc update or large text - [x] fix test `receive_imf::receive_imf_tests::test_dont_reverify_by_self_on_outgoing_msg` feat: receive pre-messages and adapt download on demand fix python lint errors receive pre-mesages, start with changes to imap loop. refactor: move download code from `scheduler.rs` to `download.rs`, also move `get_msg_id_by_rfc724_mid` to `MsgId::get_by_rfc724_mid` `MAX_FETCH_MSG_SIZE` is no longer unused Parse if it is a pre-message or full-message start with receiving logic get rid of `MsgId::get_by_rfc724_mid` because it was a duplicate of `message::rfc724_mid_exists` docs: add hint to `MimeMessage::from_bytes` stating that it has side-effects. receiving full message send and receive `attachment_size` and set viewtype to text in pre_message metadata as struct in pre-message in header. And fill params that we can already fill from the metadata. Also add a new api to check what viewtype the message will have once downloaded. api: jsonrpc: add `full_message_view_type` to `Message` and `MessageInfo` make PreMsgMetadata.to_header_value not consume self/PreMsgMetadata add api to merge params on download full message: merge new params into old params and remove full-message metadata params move tests to `src/tests/pre_messages.rs` dynamically allocate test attachment bytes fix detection of pre-messages. (it looked for the ChatFullMessageId header in the unencrypted headers before) fix setting dl state to avaiable on pre-messages fix: save pre message with rfc724_mid of full message als disable replacement for full messages add some receiving tests and update test todo for premessage metadata test: process full message before pre-message test receive normal message some serialization tests for PreMsgMetadata remove outdated todo comment test that pre-message contains message text PreMsgMetadata: test_build_from_file_msg and test_build_from_file_msg test: test_receive_pre_message_image Test receiving the full message after receiving an edit after receiving the pre-message test_reaction_on_pre_message test_full_download_after_trashed test_webxdc_update_for_not_downloaded_instance simplify fake webxdc generation in test_webxdc_update_for_not_downloaded_instance test_markseen_pre_msg test_pre_msg_can_start_chat and test_full_msg_can_start_chat test_download_later_keeps_message_order test_chatlist_event_on_full_msg_download fix download not working log splitting into pre-message add pre-message info to text when loading from db. this can be disabled with config key `hide_pre_message_metadata_text` if ui wants to display it in a prettier way. update `download_limit` documentation more logging: log size of pre and post messages rename full message to Post-Message split up the pre-message tests into multiple files dedup test code by extracting code to create test messages into util methods remove post_message_view_type from api, now it is only used internally for tests remove `hide_pre_message_metadata_text` config option, as there currently is no way to get the full message viewtype anymore Update src/download.rs resolve comment use `parse_message_id` instead of removing `<>`parenthesis it manually fix available_post_msgs gets no entries handle forwarding and add a test for it. convert comment to log warning event on unexpected download failure add doc comment to `simple_imap_loop` more logging handle saving pre-message to self messages and test. refactor: Simplify the logic for downloading a bit Immediately fully download all messages that do not have the `Chat-Is-Post-Message` header. This way, we simplify the logic for when which messages are downloaded, there are no differences at all anymore between 'background_fetch' and 'normal fetch'. Also, we prevent message reordering when reveiving a message from a legacy client. Messages larger than 1MB without `Chat-Is-Post-Message` should be rare, so, we do not expect this to really worsen things. Update deltachat-ffi/deltachat.h less .clone() clippy fixes rustfmt restore test_download_on_demand use anyhow remove TODO remove PartialDownloadMsgBody Add DC_STR_CONTACT stock string remove stock string TODOs remove optional todo feat: Don't put text into post-message This way, there will be no duplicate text if a chat partner is using an older DC version. If the chat partner is using a new DC version, then this will have no effect. fix: Make pre-messages db migration backwards-compatible See this comment that described the problem: https://github.com/chatmail/core/pull/7371/files#r2612182387 > The migration removes the `msg_id` column from the `download` table, so that, if you import the backup into an older DC version, you get the error `Failed inbox fetch_idle: Failed to download messages: no such column: msg_id in SELECT msg_id FROM download at offset 7: Error code 1: SQL error or missing database.` > > So, we need: > - either to make sure that things somewhat work if the user downgrades > - or to again increase `backup_version`, and then again coordinate that releases are done on all platforms simultaneously (which I would like to avoid). This PR here implements the first solution: Re-add `download.msg_id`, so that an older core can handle the database scheme. Co-authored-by: Hocuri <hocuri@gmx.de>
1 parent 5925f72 commit bb0fd7c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+2411
-1446
lines changed

deltachat-ffi/deltachat.h

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -488,11 +488,11 @@ char* dc_get_blobdir (const dc_context_t* context);
488488
* 0=use IMAP IDLE if the server supports it.
489489
* This is a developer option used for testing polling used as an IDLE fallback.
490490
* - `download_limit` = Messages up to this number of bytes are downloaded automatically.
491-
* For larger messages, only the header is downloaded and a placeholder is shown.
491+
* For messages with large attachments, two messages are sent:
492+
* a Pre-Message containing metadata and a Post-Message containing the attachment.
493+
* Pre-Messages are always downloaded and show a placeholder message.
492494
* These messages can be downloaded fully using dc_download_full_msg() later.
493-
* The limit is compared against raw message sizes, including headers.
494-
* The actually used limit may be corrected
495-
* to not mess up with non-delivery-reports or read-receipts.
495+
* Post-Messages are automatically downloaded if they are smaller than the download_limit.
496496
* 0=no limit (default).
497497
* Changes affect future messages only.
498498
* - `protect_autocrypt` = Enable Header Protection for Autocrypt header.
@@ -4310,7 +4310,8 @@ char* dc_msg_get_webxdc_info (const dc_msg_t* msg);
43104310

43114311
/**
43124312
* Get the size of the file. Returns the size of the file associated with a
4313-
* message, if applicable.
4313+
* message, if applicable.
4314+
* If message is a pre-message, then this returns size of the to be downloaded file.
43144315
*
43154316
* Typically, this is used to show the size of document files, e.g. a PDF.
43164317
*
@@ -7263,22 +7264,9 @@ void dc_event_unref(dc_event_t* event);
72637264
/// `%1$s` will be replaced by the percentage used
72647265
#define DC_STR_QUOTA_EXCEEDING_MSG_BODY 98
72657266

7266-
/// "%1$s message"
7267-
///
7268-
/// Used as the message body when a message
7269-
/// was not yet downloaded completely
7270-
/// (dc_msg_get_download_state() is e.g. @ref DC_DOWNLOAD_AVAILABLE).
7271-
///
7272-
/// `%1$s` will be replaced by human-readable size (e.g. "1.2 MiB").
7267+
/// @deprecated Deprecated 2025-11-12, this string is no longer needed.
72737268
#define DC_STR_PARTIAL_DOWNLOAD_MSG_BODY 99
72747269

7275-
/// "Download maximum available until %1$s"
7276-
///
7277-
/// Appended after some separator to @ref DC_STR_PARTIAL_DOWNLOAD_MSG_BODY.
7278-
///
7279-
/// `%1$s` will be replaced by human-readable date and time.
7280-
#define DC_STR_DOWNLOAD_AVAILABILITY 100
7281-
72827270
/// "Multi Device Synchronization"
72837271
///
72847272
/// Used in subjects of outgoing sync messages.
@@ -7776,6 +7764,11 @@ void dc_event_unref(dc_event_t* event);
77767764
/// Used as the first info messages in newly created classic email threads.
77777765
#define DC_STR_CHAT_UNENCRYPTED_EXPLANATON 230
77787766

7767+
/// "Contact"
7768+
///
7769+
/// Used in summaries.
7770+
#define DC_STR_CONTACT 231
7771+
77797772
/**
77807773
* @}
77817774
*/

deltachat-jsonrpc/src/api/types/message.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ pub struct MessageObject {
9292

9393
file: Option<String>,
9494
file_mime: Option<String>,
95+
96+
/// The size of the file in bytes, if applicable.
97+
/// If message is a pre-message, then this is the size of the to be downloaded file.
9598
file_bytes: u64,
9699
file_name: Option<String>,
97100

deltachat-rpc-client/tests/test_chatlist_events.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
from __future__ import annotations
22

3-
import base64
4-
import os
53
from typing import TYPE_CHECKING
64

75
from deltachat_rpc_client import Account, EventType, const
@@ -129,7 +127,7 @@ def test_download_on_demand(acfactory: ACFactory) -> None:
129127
msg.get_snapshot().chat.accept()
130128
bob.get_chat_by_id(chat_id).send_message(
131129
"Hello World, this message is bigger than 5 bytes",
132-
html=base64.b64encode(os.urandom(300000)).decode("utf-8"),
130+
file="../test-data/image/screenshot.jpg",
133131
)
134132

135133
message = alice.wait_for_incoming_msg()

deltachat-rpc-client/tests/test_something.py

Lines changed: 5 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,12 @@
55
import os
66
import socket
77
import subprocess
8-
import time
98
from unittest.mock import MagicMock
109

1110
import pytest
1211

13-
from deltachat_rpc_client import Contact, EventType, Message, events
14-
from deltachat_rpc_client.const import DownloadState, MessageState
12+
from deltachat_rpc_client import EventType, events
13+
from deltachat_rpc_client.const import MessageState
1514
from deltachat_rpc_client.pytestplugin import E2EE_INFO_MSGS
1615
from deltachat_rpc_client.rpc import JsonRpcError
1716

@@ -333,7 +332,7 @@ def test_receive_imf_failure(acfactory) -> None:
333332
alice_contact_bob = alice.create_contact(bob, "Bob")
334333
alice_chat_bob = alice_contact_bob.create_chat()
335334

336-
bob.set_config("fail_on_receiving_full_msg", "1")
335+
bob.set_config("simulate_receive_imf_error", "1")
337336
alice_chat_bob.send_text("Hello!")
338337
event = bob.wait_for_event(EventType.MSGS_CHANGED)
339338
assert event.chat_id == bob.get_device_chat().id
@@ -342,17 +341,16 @@ def test_receive_imf_failure(acfactory) -> None:
342341
snapshot = message.get_snapshot()
343342
assert (
344343
snapshot.text == "❌ Failed to receive a message:"
345-
" Condition failed: `!context.get_config_bool(Config::FailOnReceivingFullMsg).await?`."
344+
" Condition failed: `!context.get_config_bool(Config::SimulateReceiveImfError).await?`."
346345
" Please report this bug to delta@merlinux.eu or https://support.delta.chat/."
347346
)
348347

349348
# The failed message doesn't break the IMAP loop.
350-
bob.set_config("fail_on_receiving_full_msg", "0")
349+
bob.set_config("simulate_receive_imf_error", "0")
351350
alice_chat_bob.send_text("Hello again!")
352351
message = bob.wait_for_incoming_msg()
353352
snapshot = message.get_snapshot()
354353
assert snapshot.text == "Hello again!"
355-
assert snapshot.download_state == DownloadState.DONE
356354
assert snapshot.error is None
357355

358356

@@ -588,94 +586,6 @@ def test_mdn_doesnt_break_autocrypt(acfactory) -> None:
588586
assert snapshot.show_padlock
589587

590588

591-
def test_reaction_to_partially_fetched_msg(acfactory, tmp_path):
592-
"""See https://github.com/deltachat/deltachat-core-rust/issues/3688 "Partially downloaded
593-
messages are received out of order".
594-
595-
If the Inbox contains X small messages followed by Y large messages followed by Z small
596-
messages, Delta Chat first downloaded a batch of X+Z messages, and then a batch of Y messages.
597-
598-
This bug was discovered by @Simon-Laux while testing reactions PR #3644 and can be reproduced
599-
with online test as follows:
600-
- Bob enables download limit and goes offline.
601-
- Alice sends a large message to Bob and reacts to this message with a thumbs-up.
602-
- Bob goes online
603-
- Bob first processes a reaction message and throws it away because there is no corresponding
604-
message, then processes a partially downloaded message.
605-
- As a result, Bob does not see a reaction
606-
"""
607-
download_limit = 300000
608-
ac1, ac2 = acfactory.get_online_accounts(2)
609-
ac1_addr = ac1.get_config("addr")
610-
chat = ac1.create_chat(ac2)
611-
ac2.set_config("download_limit", str(download_limit))
612-
ac2.stop_io()
613-
614-
logging.info("sending small+large messages from ac1 to ac2")
615-
msgs = []
616-
msgs.append(chat.send_text("hi"))
617-
path = tmp_path / "large"
618-
path.write_bytes(os.urandom(download_limit + 1))
619-
msgs.append(chat.send_file(str(path)))
620-
for m in msgs:
621-
m.wait_until_delivered()
622-
623-
logging.info("sending a reaction to the large message from ac1 to ac2")
624-
# TODO: Find the reason of an occasional message reordering on the server (so that the reaction
625-
# has a lower UID than the previous message). W/a is to sleep for some time to let the reaction
626-
# have a later INTERNALDATE.
627-
time.sleep(1.1)
628-
react_str = "\N{THUMBS UP SIGN}"
629-
msgs.append(msgs[-1].send_reaction(react_str))
630-
msgs[-1].wait_until_delivered()
631-
632-
ac2.start_io()
633-
634-
logging.info("wait for ac2 to receive a reaction")
635-
msg2 = Message(ac2, ac2.wait_for_reactions_changed().msg_id)
636-
assert msg2.get_sender_contact().get_snapshot().address == ac1_addr
637-
assert msg2.get_snapshot().download_state == DownloadState.AVAILABLE
638-
reactions = msg2.get_reactions()
639-
contacts = [Contact(ac2, int(i)) for i in reactions.reactions_by_contact]
640-
assert len(contacts) == 1
641-
assert contacts[0].get_snapshot().address == ac1_addr
642-
assert list(reactions.reactions_by_contact.values())[0] == [react_str]
643-
644-
645-
@pytest.mark.parametrize("n_accounts", [3, 2])
646-
def test_download_limit_chat_assignment(acfactory, tmp_path, n_accounts):
647-
download_limit = 300000
648-
649-
alice, *others = acfactory.get_online_accounts(n_accounts)
650-
bob = others[0]
651-
652-
alice_group = alice.create_group("test group")
653-
for account in others:
654-
chat = account.create_chat(alice)
655-
chat.send_text("Hello Alice!")
656-
assert alice.wait_for_incoming_msg().get_snapshot().text == "Hello Alice!"
657-
658-
contact = alice.create_contact(account)
659-
alice_group.add_contact(contact)
660-
661-
bob.set_config("download_limit", str(download_limit))
662-
663-
alice_group.send_text("hi")
664-
snapshot = bob.wait_for_incoming_msg().get_snapshot()
665-
assert snapshot.text == "hi"
666-
bob_group = snapshot.chat
667-
668-
path = tmp_path / "large"
669-
path.write_bytes(os.urandom(download_limit + 1))
670-
671-
for i in range(10):
672-
logging.info("Sending message %s", i)
673-
alice_group.send_file(str(path))
674-
snapshot = bob.wait_for_incoming_msg().get_snapshot()
675-
assert snapshot.download_state == DownloadState.AVAILABLE
676-
assert snapshot.chat == bob_group
677-
678-
679589
def test_markseen_contact_request(acfactory):
680590
"""
681591
Test that seen status is synchronized for contact request messages

python/tests/test_1_online.py

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import os
22
import queue
33
import sys
4-
import base64
54
from datetime import datetime, timezone
65

76
import pytest
@@ -222,38 +221,6 @@ def test_webxdc_huge_update(acfactory, data, lp):
222221
assert update["payload"] == payload
223222

224223

225-
def test_webxdc_download_on_demand(acfactory, data, lp):
226-
ac1, ac2 = acfactory.get_online_accounts(2)
227-
acfactory.introduce_each_other([ac1, ac2])
228-
chat = acfactory.get_accepted_chat(ac1, ac2)
229-
230-
msg1 = Message.new_empty(ac1, "webxdc")
231-
msg1.set_text("message1")
232-
msg1.set_file(data.get_path("webxdc/minimal.xdc"))
233-
msg1 = chat.send_msg(msg1)
234-
assert msg1.is_webxdc()
235-
assert msg1.filename
236-
237-
msg2 = ac2._evtracker.wait_next_incoming_message()
238-
assert msg2.is_webxdc()
239-
240-
lp.sec("ac2 sets download limit")
241-
ac2.set_config("download_limit", "100")
242-
assert msg1.send_status_update({"payload": base64.b64encode(os.urandom(300000))}, "some test data")
243-
ac2_update = ac2._evtracker.wait_next_incoming_message()
244-
assert ac2_update.download_state == dc.const.DC_DOWNLOAD_AVAILABLE
245-
assert not msg2.get_status_updates()
246-
247-
ac2_update.download_full()
248-
ac2._evtracker.get_matching("DC_EVENT_WEBXDC_STATUS_UPDATE")
249-
assert msg2.get_status_updates()
250-
251-
# Get a event notifying that the message disappeared from the chat.
252-
msgs_changed_event = ac2._evtracker.get_matching("DC_EVENT_MSGS_CHANGED")
253-
assert msgs_changed_event.data1 == msg2.chat.id
254-
assert msgs_changed_event.data2 == 0
255-
256-
257224
def test_enable_mvbox_move(acfactory, lp):
258225
(ac1,) = acfactory.get_online_accounts(1)
259226

src/calls/calls_tests.rs

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::*;
22
use crate::chat::forward_msgs;
33
use crate::config::Config;
44
use crate::constants::DC_CHAT_ID_TRASH;
5-
use crate::receive_imf::{receive_imf, receive_imf_from_inbox};
5+
use crate::receive_imf::receive_imf;
66
use crate::test_utils::{TestContext, TestContextManager};
77

88
struct CallSetup {
@@ -610,65 +610,3 @@ async fn test_end_text_call() -> Result<()> {
610610

611611
Ok(())
612612
}
613-
614-
/// Tests that partially downloaded "call ended"
615-
/// messages are not processed.
616-
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
617-
async fn test_no_partial_calls() -> Result<()> {
618-
let mut tcm = TestContextManager::new();
619-
let alice = &tcm.alice().await;
620-
621-
let seen = false;
622-
623-
// The messages in the test
624-
// have no `Date` on purpose,
625-
// so they are treated as new.
626-
let received_call = receive_imf(
627-
alice,
628-
b"From: bob@example.net\n\
629-
To: alice@example.org\n\
630-
Message-ID: <first@example.net>\n\
631-
Chat-Version: 1.0\n\
632-
Chat-Content: call\n\
633-
Chat-Webrtc-Room: YWFhYWFhYWFhCg==\n\
634-
\n\
635-
Hello, this is a call\n",
636-
seen,
637-
)
638-
.await?
639-
.unwrap();
640-
assert_eq!(received_call.msg_ids.len(), 1);
641-
let call_msg = Message::load_from_db(alice, received_call.msg_ids[0])
642-
.await
643-
.unwrap();
644-
assert_eq!(call_msg.viewtype, Viewtype::Call);
645-
assert_eq!(call_state(alice, call_msg.id).await?, CallState::Alerting);
646-
647-
let imf_raw = b"From: bob@example.net\n\
648-
To: alice@example.org\n\
649-
Message-ID: <second@example.net>\n\
650-
In-Reply-To: <first@example.net>\n\
651-
Chat-Version: 1.0\n\
652-
Chat-Content: call-ended\n\
653-
\n\
654-
Call ended\n";
655-
receive_imf_from_inbox(
656-
alice,
657-
"second@example.net",
658-
imf_raw,
659-
seen,
660-
Some(imf_raw.len().try_into().unwrap()),
661-
)
662-
.await?;
663-
664-
// The call is still not ended.
665-
assert_eq!(call_state(alice, call_msg.id).await?, CallState::Alerting);
666-
667-
// Fully downloading the message ends the call.
668-
receive_imf_from_inbox(alice, "second@example.net", imf_raw, seen, None)
669-
.await
670-
.context("Failed to fully download end call message")?;
671-
assert_eq!(call_state(alice, call_msg.id).await?, CallState::Missed);
672-
673-
Ok(())
674-
}

0 commit comments

Comments
 (0)