mirror of
https://github.com/zulip/zulip.git
synced 2025-10-23 04:52:12 +00:00
2126478867
This reduces our dependency on message_list code (via
message_util), and it makes moving streams/topics and
deleting messages more performant.
For every single message that was being updated or
deleted, the previous code was basically re-computing
lots of things, including having to iterate through
every message in memory to find the messages matching
your topic.
Now everything basically happens in O(1) time.
The only O(N) computation is that we now lazily
re-compute the max message id every time you need it
for typeahead logic, and then we cache it for
subsequent use. The N here is the number of messages
that the particular sender has sent to the particular
stream/topic combination, so it should always be quite
small, except for certain spammy bots.
Once the max has been calculated, the common operation
of adding a message doesn't invalidate our cached
value. We only invalidate the cache on deletes.
The main change that we make here from a data
standpoint is that we just keep track of all
message_ids for all senders. The storage overhead here
should be negligible. By keeping track of our own
messages, we don't have to punt to other code for
update/delete situations.
There is similar code in recent_topics that I think can
be improved in similar ways, and it would allow us to
eliminate functions like this one:
export function get_messages_in_topic(stream_id, topic) {
return message_list.all
.all_messages()
.filter(
(x) =>
x.type === "stream" &&
x.stream_id === stream_id &&
x.topic.toLowerCase() === topic.toLowerCase(),
);
}
313 lines
8.9 KiB
JavaScript
313 lines
8.9 KiB
JavaScript
"use strict";
|
|
|
|
const {strict: assert} = require("assert");
|
|
|
|
const {mock_esm, zrequire} = require("../zjsunit/namespace");
|
|
const {run_test} = require("../zjsunit/test");
|
|
|
|
let next_id = 0;
|
|
const messages = new Map();
|
|
|
|
function make_stream_message({stream_id, topic, sender_id}) {
|
|
next_id += 1;
|
|
|
|
const message = {
|
|
type: "stream",
|
|
stream_id,
|
|
id: next_id,
|
|
topic,
|
|
sender_id,
|
|
};
|
|
messages.set(message.id, message);
|
|
|
|
return message;
|
|
}
|
|
|
|
mock_esm("../../static/js/message_store", {
|
|
get: (message_id) => messages.get(message_id),
|
|
});
|
|
|
|
const rs = zrequire("recent_senders");
|
|
zrequire("message_util.js");
|
|
|
|
function test(label, f) {
|
|
run_test(label, (override) => {
|
|
messages.clear();
|
|
next_id = 0;
|
|
rs.clear_for_testing();
|
|
f(override);
|
|
});
|
|
}
|
|
|
|
test("IdTracker", () => {
|
|
const id_tracker = new rs.IdTracker();
|
|
|
|
function test_add(id, expected_max_id) {
|
|
id_tracker.add(id);
|
|
assert.equal(id_tracker.max_id(), expected_max_id);
|
|
}
|
|
|
|
test_add(5, 5);
|
|
test_add(7, 7);
|
|
test_add(3, 7);
|
|
test_add(10, 10);
|
|
test_add(12, 12);
|
|
test_add(11, 12);
|
|
|
|
function test_remove(id, expected_max_id) {
|
|
id_tracker.remove(id);
|
|
assert.equal(id_tracker.max_id(), expected_max_id);
|
|
}
|
|
|
|
test_remove(10, 12);
|
|
test_remove(999999, 12); // bogus id has no effect
|
|
test_remove(3, 12);
|
|
test_remove(12, 11);
|
|
|
|
test_add(3, 11);
|
|
test_add(7, 11);
|
|
test_add(13, 13);
|
|
|
|
test_remove(3, 13);
|
|
test_remove(13, 11);
|
|
});
|
|
|
|
test("noop process_topic_edit", () => {
|
|
// Just get line coverage on defensive code.
|
|
const bogus_ids = [333, 444];
|
|
rs.process_topic_edit({message_ids: bogus_ids});
|
|
});
|
|
|
|
test("update_topics_of_deleted_message_ids", () => {
|
|
// Just get line coverage on defensive code.
|
|
const stream_id = 555;
|
|
const topic = "whatever";
|
|
const sender_id = 999;
|
|
|
|
const message = make_stream_message({
|
|
stream_id,
|
|
topic,
|
|
sender_id,
|
|
});
|
|
rs.update_topics_of_deleted_message_ids([message.id]);
|
|
assert.deepEqual(rs.get_topic_recent_senders(stream_id, topic), []);
|
|
|
|
rs.process_message_for_senders(message);
|
|
assert.deepEqual(rs.get_topic_recent_senders(stream_id, topic), [sender_id]);
|
|
});
|
|
|
|
test("process_message_for_senders", () => {
|
|
const stream1 = 1;
|
|
const stream2 = 2;
|
|
const stream3 = 3;
|
|
|
|
const topic1 = "topic-1";
|
|
const topic2 = "topic-2";
|
|
const topic3 = "topic-3";
|
|
const topic4 = "topic-4";
|
|
|
|
const sender1 = 1;
|
|
const sender2 = 2;
|
|
const sender3 = 3;
|
|
const stream4 = 4;
|
|
const stream5 = 5;
|
|
|
|
// New stream
|
|
const message1 = make_stream_message({
|
|
stream_id: stream1,
|
|
topic: topic1,
|
|
sender_id: sender1,
|
|
});
|
|
|
|
const message2 = make_stream_message({
|
|
stream_id: stream2,
|
|
topic: topic1,
|
|
sender_id: sender2,
|
|
});
|
|
|
|
rs.process_message_for_senders(message1);
|
|
rs.process_message_for_senders(message2);
|
|
|
|
// Users have posted in only one of the streams
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender1}, {user_id: sender2}, stream1, topic1) < 0,
|
|
true,
|
|
);
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender1}, {user_id: sender2}, stream2, topic1) > 0,
|
|
true,
|
|
);
|
|
|
|
// Users haven't posted in this stream, return zero
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender1}, {user_id: sender2}, stream3, undefined) === 0,
|
|
true,
|
|
);
|
|
|
|
// New topic
|
|
const message3 = make_stream_message({
|
|
stream_id: stream1,
|
|
topic: topic2,
|
|
sender_id: sender3,
|
|
});
|
|
|
|
rs.process_message_for_senders(message3);
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender3}, {user_id: sender2}, stream1, topic2) < 0,
|
|
true,
|
|
);
|
|
|
|
// New sender
|
|
const message4 = make_stream_message({
|
|
stream_id: stream1,
|
|
topic: topic1,
|
|
sender_id: sender2,
|
|
});
|
|
|
|
rs.process_message_for_senders(message4);
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender1}, {user_id: sender2}, stream1, topic1) > 0,
|
|
true,
|
|
);
|
|
|
|
// More recent message
|
|
const message5 = make_stream_message({
|
|
stream_id: stream1,
|
|
topic: topic1,
|
|
sender_id: sender1,
|
|
});
|
|
|
|
rs.process_message_for_senders(message5);
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender1}, {user_id: sender2}, stream1, topic1) < 0,
|
|
true,
|
|
);
|
|
|
|
// Same stream, but different topics
|
|
const message6 = make_stream_message({
|
|
stream_id: stream3,
|
|
topic: topic1,
|
|
sender_id: sender1,
|
|
});
|
|
const message7 = make_stream_message({
|
|
stream_id: stream3,
|
|
topic: topic2,
|
|
sender_id: sender2,
|
|
});
|
|
const message8 = make_stream_message({
|
|
stream_id: stream3,
|
|
topic: topic3,
|
|
sender_id: sender3,
|
|
});
|
|
|
|
rs.process_message_for_senders(message6);
|
|
rs.process_message_for_senders(message7);
|
|
rs.process_message_for_senders(message8);
|
|
|
|
// topic3 has a message in it, but sender1 nor sender2 have participated, so sort by stream
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender1}, {user_id: sender2}, stream3, topic3) > 0,
|
|
true,
|
|
);
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender2}, {user_id: sender1}, stream3, topic3) < 0,
|
|
true,
|
|
);
|
|
|
|
assert.equal(rs.compare_by_recency({}, {}, (next_id += 1), ""), 0);
|
|
|
|
// new message in topic2
|
|
const message9 = make_stream_message({
|
|
stream_id: stream3,
|
|
topic: topic2,
|
|
sender_id: sender3,
|
|
});
|
|
|
|
rs.process_message_for_senders(message9);
|
|
|
|
// Test topic change
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic3).toString(), "3");
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic2).toString(), "2,3");
|
|
|
|
// message7's topic was changed by user
|
|
message7.topic = topic3;
|
|
|
|
rs.process_topic_edit({
|
|
message_ids: [message7.id],
|
|
old_stream_id: stream3,
|
|
new_stream_id: stream3,
|
|
old_topic: topic2,
|
|
new_topic: topic3,
|
|
});
|
|
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic3).toString(), "2,3");
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic2).toString(), "3");
|
|
|
|
// Test stream change
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic3).toString(), "2,3");
|
|
assert.equal(rs.get_topic_recent_senders(stream4, topic3).toString(), "");
|
|
|
|
message7.stream_id = stream4;
|
|
message8.stream_id = stream4;
|
|
rs.process_topic_edit({
|
|
message_ids: [message7.id, message8.id],
|
|
old_stream_id: stream3,
|
|
new_stream_id: stream4,
|
|
old_topic: topic3,
|
|
new_topic: topic3,
|
|
});
|
|
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic3).toString(), "");
|
|
assert.equal(rs.get_topic_recent_senders(stream4, topic3).toString(), "2,3");
|
|
|
|
// Test stream & topic change
|
|
assert.equal(rs.get_topic_recent_senders(stream4, topic3).toString(), "2,3");
|
|
assert.equal(rs.get_topic_recent_senders(stream5, topic4).toString(), "");
|
|
|
|
message7.stream_id = stream5;
|
|
message7.topic = topic4;
|
|
|
|
message8.stream_id = stream5;
|
|
message8.topic = topic4;
|
|
|
|
rs.process_topic_edit({
|
|
message_ids: [message7.id, message8.id],
|
|
old_stream_id: stream4,
|
|
new_stream_id: stream5,
|
|
old_topic: topic3,
|
|
new_topic: topic4,
|
|
});
|
|
|
|
assert.equal(rs.get_topic_recent_senders(stream4, topic3).toString(), "");
|
|
assert.equal(rs.get_topic_recent_senders(stream5, topic4).toString(), "2,3");
|
|
assert.equal(rs.get_topic_recent_senders(stream1, topic1).toString(), "2,1");
|
|
|
|
// delete message1 and message5 sent by sender1
|
|
rs.update_topics_of_deleted_message_ids([message1.id, message5.id]);
|
|
assert.equal(rs.get_topic_recent_senders(stream1, topic1).toString(), "2");
|
|
|
|
// test that we can remove again, harmlessly
|
|
rs.update_topics_of_deleted_message_ids([message1.id, message5.id]);
|
|
assert.equal(rs.get_topic_recent_senders(stream1, topic1).toString(), "2");
|
|
|
|
// remove some more senders
|
|
rs.update_topics_of_deleted_message_ids([message2.id, message3.id, message4.id, message5.id]);
|
|
assert.equal(rs.get_topic_recent_senders(stream1, topic1).toString(), "");
|
|
|
|
rs.update_topics_of_deleted_message_ids([message6.id, message7.id, message8.id, message9.id]);
|
|
assert.equal(rs.get_topic_recent_senders(stream1, topic1).toString(), "");
|
|
assert.equal(rs.get_topic_recent_senders(stream2, topic2).toString(), "");
|
|
assert.equal(rs.get_topic_recent_senders(stream3, topic3).toString(), "");
|
|
|
|
// deleting an old message which isn't locally stored.
|
|
// We are just testing that it doesn't raise an error;
|
|
// no changes should take place in this case.
|
|
rs.update_topics_of_deleted_message_ids([-1]);
|
|
|
|
// Comparing on a non-existent topic doesn't crash.
|
|
assert.equal(
|
|
rs.compare_by_recency({user_id: sender2}, {user_id: sender1}, stream3, "bogus") < 0,
|
|
true,
|
|
);
|
|
});
|