Skip to content

Commit bb816ff

Browse files
committed
fix: do not sort prefetched messages by INTERNALDATE
Messages are iterated over in fetch_new_msg_batch() and largest_uid_fetched variable is updated there assuming that messages come in the order of increasing UID. If UIDs are not increasing, it is possible that largest_uid_fetched will be updated even though smaller UID is not fetched yet and the message will be lost. INTERNALDATE sorting was introduced to deal with email providers such as Gmail that keep INTERNALDATE but not the UID order when moving the messages. Since we don't move the messages anymore after commit 04c0e7d, there is no need for ordering by INTERNALDATE.
1 parent 9fcb26c commit bb816ff

2 files changed

Lines changed: 5 additions & 56 deletions

File tree

deltachat-rpc-client/tests/test_folders.py

Lines changed: 1 addition & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,8 @@
1-
import logging
21
import re
3-
import time
42

53
from imap_tools import AND, U
64

7-
from deltachat_rpc_client import Contact, EventType, Message
8-
9-
10-
def test_reactions_for_a_reordering_move(acfactory, direct_imap):
11-
"""When a batch of messages is moved from Inbox to another folder with a single MOVE command,
12-
their UIDs may be reordered (e.g. Gmail is known for that) which led to that messages were
13-
processed by receive_imf in the wrong order, and, particularly, reactions were processed before
14-
messages they refer to and thus dropped.
15-
"""
16-
(ac1,) = acfactory.get_online_accounts(1)
17-
18-
addr, password = acfactory.get_credentials()
19-
ac2 = acfactory.get_unconfigured_account()
20-
ac2.add_or_update_transport({"addr": addr, "password": password})
21-
assert ac2.is_configured()
22-
23-
ac2.bring_online()
24-
chat1 = acfactory.get_accepted_chat(ac1, ac2)
25-
ac2.stop_io()
26-
27-
logging.info("sending message + reaction from ac1 to ac2")
28-
msg1 = chat1.send_text("hi")
29-
msg1.wait_until_delivered()
30-
# It's is sad, but messages must differ in their INTERNALDATEs to be processed in the correct
31-
# order by DC, and most (if not all) mail servers provide only seconds precision.
32-
time.sleep(1.1)
33-
react_str = "\N{THUMBS UP SIGN}"
34-
msg1.send_reaction(react_str).wait_until_delivered()
35-
36-
logging.info("moving messages to ac2's movebox folder in the reverse order")
37-
ac2_direct_imap = direct_imap(ac2)
38-
ac2_direct_imap.create_folder("Movebox")
39-
ac2_direct_imap.connect()
40-
for uid in sorted([m.uid for m in ac2_direct_imap.get_all_messages()], reverse=True):
41-
ac2_direct_imap.conn.move(uid, "Movebox")
42-
43-
logging.info("moving messages back")
44-
ac2_direct_imap.select_folder("Movebox")
45-
for uid in sorted([m.uid for m in ac2_direct_imap.get_all_messages()]):
46-
ac2_direct_imap.conn.move(uid, "INBOX")
47-
48-
logging.info("receiving messages by ac2")
49-
ac2.start_io()
50-
msg2 = Message(ac2, ac2.wait_for_reactions_changed().msg_id)
51-
assert msg2.get_snapshot().text == msg1.get_snapshot().text
52-
reactions = msg2.get_reactions()
53-
contacts = [Contact(ac2, int(i)) for i in reactions.reactions_by_contact]
54-
assert len(contacts) == 1
55-
assert contacts[0].get_snapshot().address == ac1.get_config("addr")
56-
assert list(reactions.reactions_by_contact.values())[0] == [react_str]
5+
from deltachat_rpc_client import EventType
576

587

598
def test_moved_markseen(acfactory, direct_imap, log):

src/imap/session.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::net::session::SessionStream;
1616
/// - Autocrypt-Setup-Message to check if a message is an autocrypt setup message,
1717
/// not necessarily sent by Delta Chat.
1818
/// - Chat-Is-Post-Message to skip it in background fetch or when it is > `DownloadLimit`.
19-
const PREFETCH_FLAGS: &str = "(UID INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (\
19+
const PREFETCH_FLAGS: &str = "(UID RFC822.SIZE BODY.PEEK[HEADER.FIELDS (\
2020
MESSAGE-ID \
2121
DATE \
2222
X-MICROSOFT-ORIGINAL-MESSAGE-ID \
@@ -124,7 +124,7 @@ impl Session {
124124
}
125125

126126
/// Prefetch `n_uids` messages starting from `uid_next`. Returns a list of fetch results in the
127-
/// order of ascending delivery time to the server (INTERNALDATE).
127+
/// order of ascending UIDs.
128128
#[expect(clippy::arithmetic_side_effects)]
129129
pub(crate) async fn prefetch(
130130
&mut self,
@@ -142,10 +142,10 @@ impl Session {
142142
let mut msgs = BTreeMap::new();
143143
while let Some(msg) = list.try_next().await? {
144144
if let Some(msg_uid) = msg.uid {
145-
msgs.insert((msg.internal_date(), msg_uid), msg);
145+
msgs.insert(msg_uid, msg);
146146
}
147147
}
148148

149-
Ok(msgs.into_iter().map(|((_, uid), msg)| (uid, msg)).collect())
149+
Ok(Vec::from_iter(msgs))
150150
}
151151
}

0 commit comments

Comments
 (0)