From 5b7014e7d6855c4e0e0563066c0ed40b6b4afb09 Mon Sep 17 00:00:00 2001
From: Prakhar Pratyush <prakhar@zulip.com>
Date: Mon, 9 Oct 2023 19:49:56 +0530
Subject: [PATCH] server_history: Prevent concurrent requests for the same
 stream_id.

This commit updates the 'get_server_history' function to return
early if a request is already in progress for a given stream_id,
thus preventing concurrent requests for a single stream_id.

We maintain a set 'request_pending_stream_ids', which contains all
the stream IDs for whom requests are in progress.

Using this set, we return early.

Fixes #26915.
---
 web/src/stream_topic_history.js        | 14 ++++++++++++++
 web/src/stream_topic_history_util.js   |  8 ++++++++
 web/tests/stream_topic_history.test.js | 15 ++++++++++++++-
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/web/src/stream_topic_history.js b/web/src/stream_topic_history.js
index 2340b99b36..6c408369f9 100644
--- a/web/src/stream_topic_history.js
+++ b/web/src/stream_topic_history.js
@@ -6,6 +6,7 @@ import * as unread from "./unread";
 
 const stream_dict = new Map(); // stream_id -> PerStreamHistory object
 const fetched_stream_ids = new Set();
+const request_pending_stream_ids = new Set();
 
 export function all_topics_in_cache(sub) {
     // Checks whether this browser's cache of contiguous messages
@@ -312,4 +313,17 @@ export function reset() {
     // This is only used by tests.
     stream_dict.clear();
     fetched_stream_ids.clear();
+    request_pending_stream_ids.clear();
+}
+
+export function is_request_pending_for(stream_id) {
+    return request_pending_stream_ids.has(stream_id);
+}
+
+export function add_request_pending_for(stream_id) {
+    request_pending_stream_ids.add(stream_id);
+}
+
+export function remove_request_pending_for(stream_id) {
+    request_pending_stream_ids.delete(stream_id);
 }
diff --git a/web/src/stream_topic_history_util.js b/web/src/stream_topic_history_util.js
index c6d37758f5..377af8e7d7 100644
--- a/web/src/stream_topic_history_util.js
+++ b/web/src/stream_topic_history_util.js
@@ -6,7 +6,11 @@ export function get_server_history(stream_id, on_success) {
         on_success();
         return;
     }
+    if (stream_topic_history.is_request_pending_for(stream_id)) {
+        return;
+    }
 
+    stream_topic_history.add_request_pending_for(stream_id);
     const url = "/json/users/me/" + stream_id + "/topics";
 
     channel.get({
@@ -15,7 +19,11 @@ export function get_server_history(stream_id, on_success) {
         success(data) {
             const server_history = data.topics;
             stream_topic_history.add_history(stream_id, server_history);
+            stream_topic_history.remove_request_pending_for(stream_id);
             on_success();
         },
+        error() {
+            stream_topic_history.remove_request_pending_for(stream_id);
+        },
     });
 }
diff --git a/web/tests/stream_topic_history.test.js b/web/tests/stream_topic_history.test.js
index e7ac9e9827..530f86207b 100644
--- a/web/tests/stream_topic_history.test.js
+++ b/web/tests/stream_topic_history.test.js
@@ -309,21 +309,34 @@ test("server_history_end_to_end", () => {
     ];
 
     let get_success_callback;
+    let get_error_callback;
     let on_success_called;
 
     channel.get = (opts) => {
         assert.equal(opts.url, "/json/users/me/99/topics");
         assert.deepEqual(opts.data, {});
+        assert.ok(stream_topic_history.is_request_pending_for(stream_id));
         get_success_callback = opts.success;
+        get_error_callback = opts.error;
     };
 
+    stream_topic_history_util.get_server_history(stream_id, () => {});
+
+    // Another call. Early return because a request is already in progress
+    // for stream_id = 99. This function call adds coverage.
+    stream_topic_history_util.get_server_history(stream_id, () => {});
+    assert.ok(stream_topic_history.is_request_pending_for(stream_id));
+
+    get_error_callback();
+    assert.ok(!stream_topic_history.is_request_pending_for(stream_id));
+
     stream_topic_history_util.get_server_history(stream_id, () => {
         on_success_called = true;
     });
 
     get_success_callback({topics});
-
     assert.ok(on_success_called);
+    assert.ok(!stream_topic_history.is_request_pending_for(stream_id));
 
     const history = stream_topic_history.get_recent_topic_names(stream_id);
     assert.deepEqual(history, ["topic3", "topic2", "topic1"]);