fix: replace last_added_location_id with last_added_location_timestamp

Stored location ID was not actually used
and locations_last_sent timestamp was updated
with the current time, possibly skipping
locations.
This commit is contained in:
link2xt
2026-06-18 00:03:04 +02:00
committed by l
parent a22ba021e5
commit 5eb5d22d7b
3 changed files with 75 additions and 42 deletions

View File

@@ -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

View File

@@ -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(())
}
}

View File

@@ -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,16 +1256,17 @@ 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);
@@ -1270,11 +1274,7 @@ impl MimeFactory {
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(
@@ -1774,9 +1774,10 @@ impl MimeFactory {
}
if location::is_sending_to_chat(context, msg.chat_id).await?
&& let Some(part) = self.get_location_kml_part(context).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