From 4d50c51a75de70c0f30a196e1f128154ba1651fa Mon Sep 17 00:00:00 2001 From: fiaxh Date: Sun, 20 Nov 2022 22:12:38 +0100 Subject: [PATCH] Fix some MAM issues - Messages from MUCs weren't added to their respective MUC MAM ranges, thus re-fetched on rejoin - The earliest ('first') message of a mam page was used to update the to_id, but it should have been 'last'; also the other way around. - Duplicates weren't detected properly --- libdino/src/service/history_sync.vala | 58 ++++++++++++++++++--------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/libdino/src/service/history_sync.vala b/libdino/src/service/history_sync.vala index 5b963864..b819b90f 100644 --- a/libdino/src/service/history_sync.vala +++ b/libdino/src/service/history_sync.vala @@ -46,7 +46,7 @@ public class Dino.HistorySync { } public void update_latest_db_range(Account account, Xmpp.MessageStanza message_stanza) { - Jid mam_server = stream_interactor.get_module(MucManager.IDENTITY).might_be_groupchat(message_stanza.from, account) ? message_stanza.from.bare_jid : account.bare_jid; + Jid mam_server = stream_interactor.get_module(MucManager.IDENTITY).might_be_groupchat(message_stanza.from.bare_jid, account) ? message_stanza.from.bare_jid : account.bare_jid; if (!current_catchup_id.has_key(account) || !current_catchup_id[account].has_key(mam_server)) return; @@ -205,7 +205,12 @@ public class Dino.HistorySync { PageRequestResult page_result = yield get_mam_page(account, query_params, null); - if (page_result.page_result == PageResult.Error || page_result.page_result == PageResult.Duplicate) { + if (page_result.page_result == PageResult.Duplicate) { + // No new messages + return null; + } + + if (page_result.page_result == PageResult.Error) { debug("[%s | %s] Failed fetching latest page %s", mam_server.to_string(), mam_server.to_string(), page_result.page_result.to_string()); return null; } @@ -214,13 +219,13 @@ public class Dino.HistorySync { if (page_result.page_result in new PageResult[] { PageResult.TargetReached, PageResult.NoMoreMessages } && latest_row_id != -1) { if (page_result.stanzas == null || page_result.stanzas.is_empty) return null; - string first_mam_id = page_result.query_result.first; - long first_mam_time = (long) mam_times[account][first_mam_id].to_unix(); + string latest_mam_id = page_result.query_result.last; + long latest_mam_time = (long) mam_times[account][latest_mam_id].to_unix(); var query = db.mam_catchup.update() .with(db.mam_catchup.id, "=", latest_row_id) - .set(db.mam_catchup.to_time, first_mam_time) - .set(db.mam_catchup.to_id, first_mam_id); + .set(db.mam_catchup.to_time, latest_mam_time) + .set(db.mam_catchup.to_id, latest_mam_id); if (page_result.page_result == PageResult.NoMoreMessages) { // If the server doesn't have more messages, store that this range is at its end. @@ -326,14 +331,14 @@ public class Dino.HistorySync { if (page_result.page_result == PageResult.Error || page_result.stanzas == null) return page_result; - string last_mam_id = page_result.query_result.last; - long last_mam_time = (long)mam_times[account][last_mam_id].to_unix(); + string earliest_mam_id = page_result.query_result.first; + long earliest_mam_time = (long)mam_times[account][earliest_mam_id].to_unix(); - debug("Updating %s to %s, %s", query_params.mam_server.to_string(), last_mam_time.to_string(), last_mam_id); + debug("Updating %s to %s, %s", query_params.mam_server.to_string(), earliest_mam_time.to_string(), earliest_mam_id); var query = db.mam_catchup.update() .with(db.mam_catchup.id, "=", db_id) - .set(db.mam_catchup.from_time, last_mam_time) - .set(db.mam_catchup.from_id, last_mam_id); + .set(db.mam_catchup.from_time, earliest_mam_time) + .set(db.mam_catchup.from_id, earliest_mam_id); if (page_result.page_result == PageResult.NoMoreMessages) { // If the server doesn't have more messages, store that this range is at its end. @@ -349,7 +354,7 @@ public class Dino.HistorySync { MorePagesAvailable, TargetReached, NoMoreMessages, - Duplicate, // TODO additional boolean + Duplicate, Error } @@ -364,10 +369,10 @@ public class Dino.HistorySync { } else { query_result = yield Xmpp.MessageArchiveManagement.V2.page_through_results(stream, query_params, prev_page_result.query_result); } - return yield process_query_result(account, query_result, query_params.query_id, query_params.start_id); + return yield process_query_result(account, query_params, query_result); } - private async PageRequestResult process_query_result(Account account, Xmpp.MessageArchiveManagement.QueryResult query_result, string query_id, string? after_id) { + private async PageRequestResult process_query_result(Account account, Xmpp.MessageArchiveManagement.V2.MamQueryParams query_params, Xmpp.MessageArchiveManagement.QueryResult query_result) { PageResult page_result = PageResult.MorePagesAvailable; if (query_result.malformed || query_result.error) { @@ -387,6 +392,9 @@ public class Dino.HistorySync { string selection = null; string[] selection_args = {}; + string query_id = query_params.query_id; + string? after_id = query_params.start_id; + // Check the server id of all returned messages. Check if we've hit our target (from_id) or got a duplicate. if (stanzas.has_key(query_id) && !stanzas[query_id].is_empty) { foreach (Xmpp.MessageStanza message in stanzas[query_id]) { @@ -397,8 +405,12 @@ public class Dino.HistorySync { page_result = PageResult.TargetReached; } - if (selection != null) selection += " OR "; - selection = @"$(db.message.server_id) = ?"; + if (selection == null) { + selection = @"$(db.message.server_id) = ?"; + } else { + selection += @" OR $(db.message.server_id) = ?"; + } + selection_args += mam_message_flag.mam_id; } } @@ -407,8 +419,18 @@ public class Dino.HistorySync { page_result = PageResult.TargetReached; } - int64 duplicates_found = db.message.select().where(selection, selection_args).count(); - if (duplicates_found > 0) { + // Check for duplicates among the messages of the page. + var duplicates_qry = db.message.select() + .with(db.message.account_id, "=", account.id) + .where(selection, selection_args); + // We don't want messages from different MAM servers to interfere with each other. + if (!query_params.mam_server.equals_bare(account.bare_jid)) { + duplicates_qry.with(db.message.counterpart_id, "=", db.get_jid_id(query_params.mam_server)); + } else { + duplicates_qry.with(db.message.type_, "=", Message.Type.CHAT); + } + var duplicates_count = duplicates_qry.count(); + if (duplicates_count > 0) { // We got a duplicate although we thought we have to catch up. // There was a server bug where prosody would send all messages if it didn't know the after ID that was given page_result = PageResult.Duplicate;