From bb5c732daeba3ce7e796478bf334b97ae86515e4 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Sat, 29 Jun 2024 10:00:56 +0000 Subject: [PATCH] stream_topic_history: Ask server for last msg id for historical topics. Historical topics = Topics which we received from server and have no messages cached for them to make any sense for the removed messages. --- web/src/stream_topic_history.ts | 10 +++------- web/tests/stream_topic_history.test.js | 14 +++++++++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/web/src/stream_topic_history.ts b/web/src/stream_topic_history.ts index 328a8ee36d..dc3157b81b 100644 --- a/web/src/stream_topic_history.ts +++ b/web/src/stream_topic_history.ts @@ -185,18 +185,14 @@ export class PerStreamHistory { maybe_remove(topic_name: string, num_messages: number): void { const existing = this.topics.get(topic_name); - if (!existing || existing.count === 0) { + if (!existing) { return; } if (existing.count <= num_messages) { this.topics.delete(topic_name); - if (!is_complete_for_stream_id(this.stream_id)) { - // Request server for latest message in topic if we - // cannot be sure that we have all messages in the topic. - update_topic_last_message_id(this.stream_id, topic_name); - return; - } + // Verify if this topic still has messages from the server. + update_topic_last_message_id(this.stream_id, topic_name); } existing.count -= num_messages; diff --git a/web/tests/stream_topic_history.test.js b/web/tests/stream_topic_history.test.js index a2c0c42945..42b33b95a2 100644 --- a/web/tests/stream_topic_history.test.js +++ b/web/tests/stream_topic_history.test.js @@ -214,15 +214,23 @@ test("server_history", () => { history = stream_topic_history.get_recent_topic_names(stream_id); assert.deepEqual(history, ["hist2", "hist1"]); - // We can try to remove a historical message, but it should - // have no effect. + // Removing message from a topic fetched from server history, will send + // query to the server to get the latest message id in the topic. + let update_topic_called = false; + stream_topic_history.set_update_topic_last_message_id((stream_id, topic_name) => { + assert.equal(stream_id, 66); + assert.equal(topic_name, "hist2"); + update_topic_called = true; + }); stream_topic_history.remove_messages({ stream_id, topic_name: "hist2", num_messages: 1, }); + assert.equal(update_topic_called, true); history = stream_topic_history.get_recent_topic_names(stream_id); - assert.deepEqual(history, ["hist2", "hist1"]); + assert.deepEqual(history, ["hist1"]); + stream_topic_history.set_update_topic_last_message_id(noop); // If we call back to the server for history, the // effect is always additive. We may decide to prune old