Location streaming fixes

src/chat.rs

@@ -2926,10 +2926,9 @@ pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -
 
     let now = time();
 
-    if rendered_msg.last_added_location_id.is_some()
-        && let Err(err) = location::set_kml_sent_timestamp(context, msg.chat_id, now).await
-    {
-        error!(context, "Failed to set kml sent_timestamp: {err:#}.");
+    if let Some(last_added_location_timestamp) = rendered_msg.last_added_location_timestamp {
+        location::set_kml_sent_timestamp(context, msg.chat_id, last_added_location_timestamp)
+            .await?;
     }
 
     if attach_selfavatar && let Err(err) = msg.chat_id.set_selfavatar_timestamp(context, now).await

src/location.rs

@@ -521,10 +521,9 @@ pub(crate) async fn delete_orphaned_poi(context: &Context) -> Result<()> {
     Ok(())
 }
 
-/// Returns `location.kml` contents.
-#[expect(clippy::arithmetic_side_effects)]
-pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<Option<(String, u32)>> {
-    let mut last_added_location_id = 0;
+/// Returns `location.kml` contents and the largest location timestamp, if any.
+pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<Option<(String, i64)>> {
+    let mut last_added_location_timestamp: Option<i64> = None;
 
     let self_addr = context.get_primary_self_addr().await?;
 
@@ -540,7 +539,6 @@ pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<Option<(Strin
         .await?;
 
     let now = time();
-    let mut location_count = 0;
     let mut ret = String::new();
     if locations_send_begin != 0 && now <= locations_send_until {
         ret += &format!(
@@ -551,10 +549,10 @@ pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<Option<(Strin
         context
             .sql
             .query_map(
-                "SELECT id, latitude, longitude, accuracy, timestamp \
+                "SELECT latitude, longitude, accuracy, timestamp \
              FROM locations  WHERE from_id=? \
              AND timestamp>=? \
-             AND (timestamp>=? OR \
+             AND (timestamp>? OR \
                   timestamp=(SELECT MAX(timestamp) FROM locations WHERE from_id=?)) \
              AND independent=0 \
              GROUP BY timestamp \
@@ -566,25 +564,24 @@ pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<Option<(Strin
                     ContactId::SELF
                 ),
                 |row| {
-                    let location_id: i32 = row.get(0)?;
-                    let latitude: f64 = row.get(1)?;
-                    let longitude: f64 = row.get(2)?;
-                    let accuracy: f64 = row.get(3)?;
-                    let timestamp = get_kml_timestamp(row.get(4)?);
+                    let latitude: f64 = row.get(0)?;
+                    let longitude: f64 = row.get(1)?;
+                    let accuracy: f64 = row.get(2)?;
+                    let timestamp: i64 = row.get(3)?;
 
-                    Ok((location_id, latitude, longitude, accuracy, timestamp))
+                    Ok((latitude, longitude, accuracy, timestamp))
                 },
                 |rows| {
                     for row in rows {
-                        let (location_id, latitude, longitude, accuracy, timestamp) = row?;
+                        let (latitude, longitude, accuracy, timestamp) = row?;
+                        let kml_timestamp = get_kml_timestamp(timestamp);
                         ret += &format!(
                             "<Placemark>\
-                <Timestamp><when>{timestamp}</when></Timestamp>\
+                <Timestamp><when>{kml_timestamp}</when></Timestamp>\
                 <Point><coordinates accuracy=\"{accuracy}\">{longitude},{latitude}</coordinates></Point>\
                 </Placemark>\n"
                         );
-                        location_count += 1;
-                        last_added_location_id = location_id as u32;
+                        last_added_location_timestamp = std::cmp::max(last_added_location_timestamp, Some(timestamp));
                     }
                     Ok(())
                 },
@@ -593,11 +590,7 @@ pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<Option<(Strin
         ret += "</Document>\n</kml>";
     }
 
-    if location_count > 0 {
-        Ok(Some((ret, last_added_location_id)))
-    } else {
-        Ok(None)
-    }
+    Ok(last_added_location_timestamp.map(|ts| (ret, ts)))
 }
 
 fn get_kml_timestamp(utc: i64) -> String {
@@ -1160,4 +1153,44 @@ Content-Disposition: attachment; filename="location.kml"
 
         Ok(())
     }
+
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_last_sent_location_timestamp() -> Result<()> {
+        let mut tcm = TestContextManager::new();
+        let alice = &tcm.alice().await;
+        let bob = &tcm.bob().await;
+
+        let alice_chat = alice.create_chat(bob).await;
+
+        send_to_chat(alice, alice_chat.id, 600).await?;
+        bob.recv_msg(&alice.pop_sent_msg().await).await;
+
+        assert_eq!(set(alice, 10.0, 20.0, 1.0).await?, true);
+
+        SystemTime::shift(Duration::from_secs(60));
+
+        maybe_send(alice).await?;
+        bob.recv_msg_opt(&alice.pop_sent_msg().await).await;
+
+        let alice_locations = get_range(alice, None, None, 0, 0).await?;
+        assert_eq!(alice_locations.len(), 1);
+        assert_eq!(get_range(bob, None, None, 0, 0).await?.len(), 1);
+
+        let last_sent = alice
+            .sql
+            .query_row(
+                "SELECT locations_last_sent FROM chats WHERE id = ?",
+                (alice_chat.id,),
+                |row| {
+                    let last_sent: i64 = row.get(0)?;
+
+                    Ok(last_sent)
+                },
+            )
+            .await?;
+
+        assert_eq!(alice_locations[0].timestamp, last_sent);
+
+        Ok(())
+    }
 }

src/mimefactory.rs

@@ -142,7 +142,8 @@ pub struct MimeFactory {
     /// using `Chat-Disposition-Notification-To` header.
     req_mdn: bool,
 
-    last_added_location_id: Option<u32>,
+    /// Largest timestamp of the location sent in `location.kml` in this message.
+    last_added_location_timestamp: Option<i64>,
 
     /// If the created mime-structure contains sync-items,
     /// the IDs of these items are listed here.
@@ -165,7 +166,9 @@ pub struct MimeFactory {
 pub struct RenderedEmail {
     pub message: String,
     pub is_encrypted: bool,
-    pub last_added_location_id: Option<u32>,
+
+    /// Largest timestamp of the location sent in `location.kml` in this message.
+    pub last_added_location_timestamp: Option<i64>,
 
     /// A comma-separated string of sync-IDs that are used by the rendered email and must be deleted
     /// from `multi_device_sync` once the message is actually queued for sending.
@@ -555,7 +558,7 @@ impl MimeFactory {
             in_reply_to,
             references,
             req_mdn,
-            last_added_location_id: None,
+            last_added_location_timestamp: None,
             sync_ids_to_delete: None,
             attach_selfavatar,
             webxdc_topic,
@@ -606,7 +609,7 @@ impl MimeFactory {
             in_reply_to: String::default(),
             references: Vec::new(),
             req_mdn: false,
-            last_added_location_id: None,
+            last_added_location_timestamp: None,
             sync_ids_to_delete: None,
             attach_selfavatar: false,
             webxdc_topic: None,
@@ -1222,7 +1225,7 @@ impl MimeFactory {
         };
 
         let MimeFactory {
-            last_added_location_id,
+            last_added_location_timestamp,
             ..
         } = self;
 
@@ -1231,7 +1234,7 @@ impl MimeFactory {
         Ok(RenderedEmail {
             message,
             is_encrypted,
-            last_added_location_id,
+            last_added_location_timestamp,
             sync_ids_to_delete: self.sync_ids_to_delete,
             rfc724_mid,
             subject: subject_str,
@@ -1253,28 +1256,25 @@ impl MimeFactory {
         Some(part)
     }
 
-    /// Returns MIME part with a `location.kml` attachment.
+    /// Returns MIME part with a `location.kml` attachment
+    /// and the timestamp of the latest location timestamp.
     async fn get_location_kml_part(
-        &mut self,
+        &self,
         context: &Context,
-    ) -> Result<Option<MimePart<'static>>> {
+    ) -> Result<Option<(MimePart<'static>, i64)>> {
         let Loaded::Message { msg, .. } = &self.loaded else {
             return Ok(None);
         };
 
-        let Some((kml_content, last_added_location_id)) =
+        let Some((kml_content, last_added_location_timestamp)) =
             location::get_kml(context, msg.chat_id).await?
         else {
             return Ok(None);
         };
 
         let part = MimePart::new("application/vnd.google-earth.kml+xml", kml_content)
             .attachment("location.kml");
-        if !msg.param.exists(Param::SetLatitude) {
-            // otherwise, the independent location is already filed
-            self.last_added_location_id = Some(last_added_location_id);
-        }
-        Ok(Some(part))
+        Ok(Some((part, last_added_location_timestamp)))
     }
 
     async fn render_message(
@@ -1769,14 +1769,18 @@ impl MimeFactory {
             }
         }
 
-        if let Some(msg_kml_part) = self.get_message_kml_part() {
+        if !matches!(self.pre_message_mode, PreMessageMode::Pre { .. })
+            && let Some(msg_kml_part) = self.get_message_kml_part()
+        {
             parts.push(msg_kml_part);
         }
 
-        if location::is_sending_to_chat(context, msg.chat_id).await?
-            && let Some(part) = self.get_location_kml_part(context).await?
+        if !matches!(self.pre_message_mode, PreMessageMode::Pre { .. })
+            && location::is_sending_to_chat(context, msg.chat_id).await?
+            && let Some((part, timestamp)) = self.get_location_kml_part(context).await?
         {
             parts.push(part);
+            self.last_added_location_timestamp = Some(timestamp);
         }
 
         // we do not piggyback sync-files to other self-sent-messages