From bee519db70878e0bb9901b9573bda5ef98c764a9 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Wed, 24 Jul 2024 21:14:00 -0300 Subject: [PATCH] feat: Sort received outgoing message down if it's fresher than all non fresh messages Received messages shouldn't mingle with just sent ones and appear somewhere in the middle of the chat, so we go after the newest non fresh message. But if a received outgoing message is older than some non fresh message, let it mingle with older messages. NB: received outgoing messages may break sorting of fresh incoming ones, but this shouldn't happen frequently. At least this fixes outgoing messages sorting for shared accounts where messages from other devices should be sorted the same way as incoming ones. --- src/chat.rs | 35 ++++++++++++++++++++++------------- src/receive_imf.rs | 2 ++ src/receive_imf/tests.rs | 35 +++++++++++++++++++++++++++++++++++ src/securejoin/bob.rs | 3 ++- 4 files changed, 61 insertions(+), 14 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 92f4902fb1..86d6bb5845 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -615,8 +615,9 @@ impl ChatId { contact_id: Option, ) -> Result<()> { let sort_to_bottom = true; + let (received, incoming) = (false, false); let ts = self - .calc_sort_timestamp(context, timestamp_sent, sort_to_bottom, false) + .calc_sort_timestamp(context, timestamp_sent, sort_to_bottom, received, incoming) .await? // Always sort protection messages below `SystemMessage::SecurejoinWait{,Timeout}` ones // in case of race conditions. @@ -1381,12 +1382,14 @@ impl ChatId { /// corresponding event in case of a system message (usually the current system time). /// `always_sort_to_bottom` makes this ajust the returned timestamp up so that the message goes /// to the chat bottom. + /// `received` -- whether the message is received. Otherwise being sent. /// `incoming` -- whether the message is incoming. pub(crate) async fn calc_sort_timestamp( self, context: &Context, message_timestamp: i64, always_sort_to_bottom: bool, + received: bool, incoming: bool, ) -> Result { let mut sort_timestamp = cmp::min(message_timestamp, smeared_time(context)); @@ -1404,22 +1407,28 @@ impl ChatId { (self, MessageState::OutDraft), ) .await? - } else if incoming { - // get newest non fresh message for this chat. - - // If a user hasn't been online for some time, the Inbox is fetched first and then the - // Sentbox. In order for Inbox and Sent messages to be allowed to mingle, outgoing - // messages are purely sorted by their sent timestamp. NB: The Inbox must be fetched - // first otherwise Inbox messages would be always below old Sentbox messages. We could - // take in the query below only incoming messages, but then new incoming messages would - // mingle with just sent outgoing ones and apear somewhere in the middle of the chat. + } else if received { + // Received messages shouldn't mingle with just sent ones and appear somewhere in the + // middle of the chat, so we go after the newest non fresh message. But if a received + // outgoing message is older than some non fresh message, let it mingle with older + // messages. NB: received outgoing messages may break sorting of fresh incoming ones, + // but this shouldn't happen frequently. context .sql - .query_get_value( - "SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND hidden=0 AND state>?", - (self, MessageState::InFresh), + .query_row_optional( + "SELECT MAX(timestamp), MAX(IIF(state?", + (MessageState::OutPreparing, self, MessageState::InFresh), + |row| { + let ts: i64 = row.get(0)?; + let ts_sent_incoming: i64 = row.get(1)?; + Ok((ts, ts_sent_incoming)) + }, ) .await? + .and_then(|(ts, ts_sent_incoming)| { + Some(ts).filter(|_| incoming || ts_sent_incoming <= message_timestamp) + }) } else { None }; diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 23496b3d3c..9b867009e2 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1255,11 +1255,13 @@ async fn add_parts( let in_fresh = state == MessageState::InFresh; let sort_to_bottom = false; + let received = true; let sort_timestamp = chat_id .calc_sort_timestamp( context, mime_parser.timestamp_sent, sort_to_bottom, + received, mime_parser.incoming, ) .await?; diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 68181be2cf..9fe273051e 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -4691,6 +4691,41 @@ async fn test_protected_group_add_remove_member_missing_key() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_older_message_from_2nd_device() -> Result<()> { + let alice = &TestContext::new_alice().await; + receive_imf( + alice, + b"From: alice@example.org\n\ + To: bob@example.net\n\ + Message-ID: <1234-2-3@example.org>\n\ + Date: Sat, 07 Dec 2019 19:00:27 +0000\n\ + \n\ + We share this account\n", + true, + ) + .await?; + let received = receive_imf( + alice, + b"From: alice@example.org\n\ + To: bob@example.net\n\ + Message-ID: <1234-2-4@example.org>\n\ + Date: Sat, 07 Dec 2019 19:00:26 +0000\n\ + \n\ + I'm Alice too\n", + true, + ) + .await? + .unwrap(); + alice + .golden_test_chat( + received.chat_id, + "receive_imf_older_message_from_2nd_device", + ) + .await; + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_dont_create_adhoc_group_on_member_removal() -> Result<()> { let mut tcm = TestContextManager::new(); diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index bd7a15e3ee..a461b43b06 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -73,8 +73,9 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul // Calculate the sort timestamp before checking the chat protection status so that if we // race with its change, we don't add our message below the protection message. let sort_to_bottom = true; + let (received, incoming) = (false, false); let ts_sort = chat_id - .calc_sort_timestamp(context, 0, sort_to_bottom, false) + .calc_sort_timestamp(context, 0, sort_to_bottom, received, incoming) .await?; if chat_id.is_protected(context).await? == ProtectionStatus::Unprotected { let ts_start = time();