fix: Make calc_sort_timestamp() a continuous function of message timestamp and improve its performance

src/chat.rs

@@ -298,7 +298,10 @@ impl ChatId {
         let chat = Chat::load_from_db(context, chat_id).await?;
 
         if chat.is_encrypted(context).await? {
-            chat_id.add_encrypted_msg(context, timestamp).await?;
+            let respect_delayed_msgs = true;
+            chat_id
+                .add_encrypted_msg(context, timestamp, respect_delayed_msgs)
+                .await?;
         }
 
         info!(
@@ -459,15 +462,23 @@ impl ChatId {
     }
 
     /// Adds message "Messages are end-to-end encrypted".
-    async fn add_encrypted_msg(self, context: &Context, timestamp_sort: i64) -> Result<()> {
+    async fn add_encrypted_msg(
+        self,
+        context: &Context,
+        timestamp_sent: i64,
+        respect_delayed_msgs: bool,
+    ) -> Result<()> {
         let text = stock_str::messages_e2e_encrypted(context).await;
         add_info_msg_with_cmd(
             context,
             self,
             &text,
             SystemMessage::ChatE2ee,
-            timestamp_sort,
-            None,
+            // Create a time window for delayed encrypted messages so that they are sorted under
+            // "Messages are end-to-end encrypted." This way time still monotonically increases and
+            // there's no magic "N years ago" which should be adjusted in the future.
+            timestamp_sent / if respect_delayed_msgs { 2 } else { 1 },
+            Some(timestamp_sent),
             None,
             None,
             None,
@@ -1220,37 +1231,34 @@ impl ChatId {
                 )
                 .await?
         } 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 seen message, better sort the
-            // received message purely by timestamp. We could place it just before that seen
-            // message, but anyway the user may not notice it.
+            // Received incoming 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. Received
+            // outgoing messages are allowed to mingle with seen messages though to avoid seen
+            // replies appearing before messages sent from another device (cases like the user
+            // sharing the account with others or bots are rare, so let them break sometimes).
             //
             // NB: Received outgoing messages may break sorting of fresh incoming ones, but this
             // shouldn't happen frequently. Seen incoming messages don't really break sorting of
             // fresh ones, they rather mean that older incoming messages are actually seen as well.
+            let states = match incoming {
+                true => "13, 16, 18, 20, 24, 26", // `> MessageState::InFresh`
+                false => "18, 20, 24, 26",        // `> MessageState::InSeen`
+            };
             context
                 .sql
                 .query_row_optional(
-                    "SELECT MAX(timestamp), MAX(IIF(state=?,timestamp_sent,0))
-                     FROM msgs
-                     WHERE chat_id=? AND hidden=0 AND state>?
-                     HAVING COUNT(*) > 0",
-                    (MessageState::InSeen, self, MessageState::InFresh),
+                    &format!(
+                        "SELECT MAX(timestamp) FROM msgs
+                        WHERE state IN ({states}) AND hidden=0 AND chat_id=?
+                        HAVING COUNT(*) > 0"
+                    ),
+                    (self,),
                     |row| {
                         let ts: i64 = row.get(0)?;
-                        let ts_sent_seen: i64 = row.get(1)?;
-                        Ok((ts, ts_sent_seen))
+                        Ok(ts)
                     },
                 )
                 .await?
-                .and_then(|(ts, ts_sent_seen)| {
-                    match incoming || ts_sent_seen <= message_timestamp {
-                        true => Some(ts),
-                        false => None,
-                    }
-                })
         } else {
             None
         };
@@ -2425,7 +2433,10 @@ impl ChatIdBlocked {
             && !chat.param.exists(Param::Devicetalk)
             && !chat.param.exists(Param::Selftalk)
         {
-            chat_id.add_encrypted_msg(context, smeared_time).await?;
+            let respect_delayed_msgs = true;
+            chat_id
+                .add_encrypted_msg(context, smeared_time, respect_delayed_msgs)
+                .await?;
         }
 
         Ok(Self {
@@ -3488,8 +3499,10 @@ pub(crate) async fn create_group_ex(
     chatlist_events::emit_chatlist_item_changed(context, chat_id);
 
     if is_encrypted {
-        // Add "Messages are end-to-end encrypted." message.
-        chat_id.add_encrypted_msg(context, timestamp).await?;
+        let respect_delayed_msgs = false;
+        chat_id
+            .add_encrypted_msg(context, timestamp, respect_delayed_msgs)
+            .await?;
     }
 
     if !context.get_config_bool(Config::Bot).await?

src/message.rs

@@ -1446,7 +1446,7 @@ pub enum MessageState {
     OutDelivered = 26,
 
     /// Outgoing message read by the recipient (two checkmarks; this
-    /// requires goodwill on the receiver's side). Not used in the db for new messages.
+    /// requires goodwill on the receiver's side). API-only, not used in the db.
     OutMdnRcvd = 28,
 }
 

src/sql/migrations.rs

@@ -1271,6 +1271,18 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint);
         .await?;
     }
 
+    inc_and_check(&mut migration_version, 135)?;
+    if dbversion < migration_version {
+        // Tweak the index for `chat::calc_sort_timestamp()`.
+        sql.execute_migration(
+            "UPDATE msgs SET state=26 WHERE state=28;
+            DROP INDEX IF EXISTS msgs_index7;
+            CREATE INDEX msgs_index7 ON msgs (state, hidden, chat_id, timestamp);",
+            migration_version,
+        )
+        .await?;
+    }
+
     let new_version = sql
         .get_raw_config_int(VERSION_CFG)
         .await?

src/tests/verified_chats.rs

@@ -217,9 +217,11 @@ async fn test_degrade_verified_oneonone_chat() -> Result<()> {
 /// This test tests that the messages are still in the right order.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_old_message_4() -> Result<()> {
-    let alice = TestContext::new_alice().await;
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
     let msg_incoming = receive_imf(
-        &alice,
+        alice,
         b"From: Bob <bob@example.net>\n\
           To: alice@example.org\n\
           Message-ID: <1234-2-3@example.org>\n\
@@ -232,7 +234,7 @@ async fn test_old_message_4() -> Result<()> {
     .unwrap();
 
     let msg_sent = receive_imf(
-        &alice,
+        alice,
         b"From: alice@example.org\n\
           To: Bob <bob@example.net>\n\
           Message-ID: <1234-2-4@example.org>\n\
@@ -247,6 +249,16 @@ async fn test_old_message_4() -> Result<()> {
     // The "Happy birthday" message should be shown first, and then the "Thanks" message
     assert!(msg_sent.sort_timestamp < msg_incoming.sort_timestamp);
 
+    // And now the same for encrypted messages.
+    let msg_incoming = tcm.send_recv(bob, alice, "Thanks, Alice!").await;
+    message::markseen_msgs(alice, vec![msg_incoming.id]).await?;
+    let raw = include_bytes!("../../test-data/message/thunderbird_with_autocrypt.eml");
+    let msg_sent = receive_imf(alice, raw, true).await?.unwrap();
+    assert_eq!(msg_sent.chat_id, msg_incoming.chat_id);
+    assert!(msg_sent.sort_timestamp < msg_incoming.timestamp_sort);
+    alice
+        .golden_test_chat(msg_sent.chat_id, "test_old_message_4")
+        .await;
     Ok(())
 }
 
@@ -282,7 +294,7 @@ async fn test_old_message_5() -> Result<()> {
     .await?
     .unwrap();
 
-    assert!(msg_sent.sort_timestamp == msg_incoming.sort_timestamp);
+    assert_eq!(msg_sent.sort_timestamp, msg_incoming.sort_timestamp);
     alice
         .golden_test_chat(msg_sent.chat_id, "test_old_message_5")
         .await;

test-data/golden/test_old_message_4

@@ -0,0 +1,6 @@
+Single#Chat#11: bob@example.net [KEY bob@example.net]
+--------------------------------------------------------------------------------
+Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
+Msg#14🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual.  √
+Msg#13🔒:  (Contact#Contact#11): Thanks, Alice! [SEEN]
+--------------------------------------------------------------------------------

test-data/golden/two_group_securejoins

@@ -1,9 +1,9 @@
 Group#Chat#11: Group [3 member(s)]
 --------------------------------------------------------------------------------
+Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
 Msg#13: info (Contact#Contact#Info): alice@example.org invited you to join this group.
 
 Waiting for the device of alice@example.org to reply… [NOTICED][INFO]
 Msg#15: info (Contact#Contact#Info): alice@example.org replied, waiting for being added to the group… [NOTICED][INFO]
-Msg#12: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
 Msg#17🔒:  (Contact#Contact#10): Member Me added by alice@example.org. [FRESH][INFO]
 --------------------------------------------------------------------------------