mirror of https://github.com/zulip/zulip.git
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.
This commit is contained in:
parent
f214ba7acc
commit
5b7014e7d6
|
@ -6,6 +6,7 @@ import * as unread from "./unread";
|
||||||
|
|
||||||
const stream_dict = new Map(); // stream_id -> PerStreamHistory object
|
const stream_dict = new Map(); // stream_id -> PerStreamHistory object
|
||||||
const fetched_stream_ids = new Set();
|
const fetched_stream_ids = new Set();
|
||||||
|
const request_pending_stream_ids = new Set();
|
||||||
|
|
||||||
export function all_topics_in_cache(sub) {
|
export function all_topics_in_cache(sub) {
|
||||||
// Checks whether this browser's cache of contiguous messages
|
// Checks whether this browser's cache of contiguous messages
|
||||||
|
@ -312,4 +313,17 @@ export function reset() {
|
||||||
// This is only used by tests.
|
// This is only used by tests.
|
||||||
stream_dict.clear();
|
stream_dict.clear();
|
||||||
fetched_stream_ids.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);
|
||||||
}
|
}
|
||||||
|
|
|
@ -6,7 +6,11 @@ export function get_server_history(stream_id, on_success) {
|
||||||
on_success();
|
on_success();
|
||||||
return;
|
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";
|
const url = "/json/users/me/" + stream_id + "/topics";
|
||||||
|
|
||||||
channel.get({
|
channel.get({
|
||||||
|
@ -15,7 +19,11 @@ export function get_server_history(stream_id, on_success) {
|
||||||
success(data) {
|
success(data) {
|
||||||
const server_history = data.topics;
|
const server_history = data.topics;
|
||||||
stream_topic_history.add_history(stream_id, server_history);
|
stream_topic_history.add_history(stream_id, server_history);
|
||||||
|
stream_topic_history.remove_request_pending_for(stream_id);
|
||||||
on_success();
|
on_success();
|
||||||
},
|
},
|
||||||
|
error() {
|
||||||
|
stream_topic_history.remove_request_pending_for(stream_id);
|
||||||
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
@ -309,21 +309,34 @@ test("server_history_end_to_end", () => {
|
||||||
];
|
];
|
||||||
|
|
||||||
let get_success_callback;
|
let get_success_callback;
|
||||||
|
let get_error_callback;
|
||||||
let on_success_called;
|
let on_success_called;
|
||||||
|
|
||||||
channel.get = (opts) => {
|
channel.get = (opts) => {
|
||||||
assert.equal(opts.url, "/json/users/me/99/topics");
|
assert.equal(opts.url, "/json/users/me/99/topics");
|
||||||
assert.deepEqual(opts.data, {});
|
assert.deepEqual(opts.data, {});
|
||||||
|
assert.ok(stream_topic_history.is_request_pending_for(stream_id));
|
||||||
get_success_callback = opts.success;
|
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, () => {
|
stream_topic_history_util.get_server_history(stream_id, () => {
|
||||||
on_success_called = true;
|
on_success_called = true;
|
||||||
});
|
});
|
||||||
|
|
||||||
get_success_callback({topics});
|
get_success_callback({topics});
|
||||||
|
|
||||||
assert.ok(on_success_called);
|
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);
|
const history = stream_topic_history.get_recent_topic_names(stream_id);
|
||||||
assert.deepEqual(history, ["topic3", "topic2", "topic1"]);
|
assert.deepEqual(history, ["topic3", "topic2", "topic1"]);
|
||||||
|
|
Loading…
Reference in New Issue