Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

unread: Replace key_to_bucket Dict/FoldDict with Map. 

Fixes type confusion in unread_topic_counter, which uses stream IDs as
keys.

Since unread_topic_counter calls message_store.get now, update the
mocks so that message_store.get knows about our mocked messages.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit is contained in:
Anders Kaseorg 2020-02-01 00:47:58 +00:00 committed by Tim Abbott
parent fd9557aa0c
commit d84646f091
3 changed files with 58 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
frontend_tests/node_tests/topic_data.js
View File

@ -2,9 +2,11 @@ zrequire('unread');
zrequire('util');
zrequire('stream_data');
zrequire('topic_data');
const IntDict = zrequire('int_dict').IntDict;
set_global('channel', {});
set_global('message_list', {});
set_global('message_store', {});
run_test('basics', () => {
const stream_id = 55;
@ -216,10 +218,14 @@ run_test('test_unread_logic', () => {
{ id: 20, topic: 'UNREAD2' },
];
const message_dict = new IntDict();
message_store.get = msg_id => message_dict.get(msg_id);
_.each(msgs, (msg) => {
msg.type = 'stream';
msg.stream_id = stream_id;
msg.unread = true;
message_dict.set(msg.id, msg);
});
unread.process_loaded_messages(msgs);

85
frontend_tests/node_tests/unread.js
View File

@ -33,6 +33,13 @@ const social = {
};
stream_data.add_sub('social', social);
const message_dict = new IntDict();
message_store.get = msg_id => message_dict.get(msg_id);
function add_message(message) {
message_dict.set(message.id, message);
return message;
}
const zero_counts = {
private_message_count: 0,
home_unread_messages: 0,
@ -81,21 +88,21 @@ run_test('changing_topics', () => {
const stream_id = 100;
const wrong_stream_id = 110;
const message = {
const message = add_message({
id: 15,
type: 'stream',
stream_id: stream_id,
topic: 'luNch',
unread: true,
};
});
const other_message = {
const other_message = add_message({
id: 16,
type: 'stream',
stream_id: stream_id,
topic: 'lunCH',
unread: true,
};
});
assert.deepEqual(unread.get_unread_message_ids([15, 16]), []);
assert.deepEqual(unread.get_unread_messages([message, other_message]), []);
@ -163,22 +170,13 @@ run_test('changing_topics', () => {
unread.update_unread_topics(other_message, event);
// Update a message that was never marked as unread.
const sticky_message = {
const sticky_message = add_message({
id: 17,
type: 'stream',
stream_id: stream_id,
topic: 'sticky',
unread: true,
};
const message_dict = new Dict();
message_dict.set(message.id, message);
message_dict.set(other_message.id, other_message);
message_dict.set(sticky_message.id, sticky_message);
message_store.get = function (msg_id) {
return message_dict.get(msg_id);
};
});
unread.process_loaded_messages([sticky_message]);
count = unread.num_unread_for_topic(stream_id, 'sticky');
@ -217,13 +215,13 @@ run_test('muting', () => {
const stream_id = social.stream_id;
const unknown_stream_id = 555;
const message = {
const message = add_message({
id: 15,
type: 'stream',
stream_id: stream_id,
topic: 'test_muting',
unread: true,
};
});
unread.process_loaded_messages([message]);
let counts = unread.get_counts();
@ -263,19 +261,18 @@ run_test('num_unread_for_topic', () => {
let count = unread.num_unread_for_topic(stream_id, 'lunch');
assert.equal(count, 0);
const message = {
type: 'stream',
stream_id: stream_id,
topic: 'LuncH',
unread: true,
};
// Put messages into list in reverse order to try to confuse
// our sort.
const num_msgs = 500;
let i;
for (i = num_msgs; i > 0; i -= 1) {
message.id = i;
const message = add_message({
id: i,
type: 'stream',
stream_id: stream_id,
topic: 'LuncH',
unread: true,
});
unread.process_loaded_messages([message]);
}
@ -308,9 +305,8 @@ run_test('num_unread_for_topic', () => {
assert.deepEqual(missing_topics, []);
for (i = 0; i < num_msgs; i += 1) {
message.id = i + 1;
unread.mark_as_read(message.id);
for (i = 1; i <= num_msgs; i += 1) {
unread.mark_as_read(i);
}
count = unread.num_unread_for_topic(stream_id, 'lunch');
@ -340,13 +336,13 @@ run_test('home_messages', () => {
};
};
const message = {
const message = add_message({
id: 15,
type: 'stream',
stream_id: stream_id,
topic: 'lunch',
unread: true,
};
});
let counts = unread.get_counts();
assert.equal(counts.home_unread_messages, 0);
@ -363,6 +359,7 @@ run_test('home_messages', () => {
assert.equal(counts.home_unread_messages, 0);
test_notifiable_count(counts.home_unread_messages, 0);
message.unread = true;
unread.process_loaded_messages([message]);
counts = unread.get_counts();
assert.equal(counts.home_unread_messages, 1);
@ -379,12 +376,12 @@ run_test('home_messages', () => {
});
run_test('phantom_messages', () => {
const message = {
const message = add_message({
id: 999,
type: 'stream',
stream_id: 555,
topic: 'phantom',
};
});
stream_data.get_sub_by_id = function () { return; };
@ -405,7 +402,7 @@ run_test('private_messages', () => {
};
people.add_in_realm(anybody);
const message = {
const message = add_message({
id: 15,
type: 'private',
display_recipient: [
@ -413,7 +410,7 @@ run_test('private_messages', () => {
{id: me.user_id},
],
unread: true,
};
});
unread.process_loaded_messages([message]);
@ -450,12 +447,12 @@ run_test('private_messages', () => {
assert.deepEqual(unread.get_msg_ids_for_person(), []);
assert.deepEqual(unread.get_msg_ids_for_private(), []);
const message = {
const message = add_message({
id: 15,
display_recipient: [{id: alice.user_id}],
type: 'private',
unread: true,
};
});
const read_message = {
flags: ['read'],
@ -493,7 +490,7 @@ run_test('mentions', () => {
muting.add_muted_topic(401, 'lunch');
const mention_me_message = {
const mention_me_message = add_message({
id: 15,
type: 'stream',
stream_id: 400,
@ -501,9 +498,9 @@ run_test('mentions', () => {
mentioned: true,
mentioned_me_directly: true,
unread: true,
};
});
const mention_all_message = {
const mention_all_message = add_message({
id: 16,
type: 'stream',
stream_id: 400,
@ -511,10 +508,10 @@ run_test('mentions', () => {
mentioned: true,
mentioned_me_directly: false,
unread: true,
};
});
// This message shouldn't affect the unread mention counts.
const muted_mention_all_message = {
const muted_mention_all_message = add_message({
id: 17,
type: 'stream',
stream_id: muted_stream_id,
@ -522,7 +519,7 @@ run_test('mentions', () => {
mentioned: true,
mentioned_me_directly: false,
unread: true,
};
});
unread.process_loaded_messages([mention_me_message,
mention_all_message,
@ -551,13 +548,13 @@ run_test('starring', () => {
});
run_test('declare_bankruptcy', () => {
const message = {
const message = add_message({
id: 16,
type: 'whatever',
stream_id: 1999,
topic: 'whatever',
mentioned: true,
};
});
unread.process_loaded_messages([message]);

19
static/js/unread.js
View File

@ -67,7 +67,7 @@ const unread_messages = make_id_set();
function make_bucketer(options) {
const self = {};
const key_to_bucket = options.fold_case ? new FoldDict() : new Dict();
const key_to_bucket = new Map();
const reverse_lookup = new Dict();
self.clear = function () {
@ -80,10 +80,10 @@ function make_bucketer(options) {
const item_id = opts.item_id;
const add_callback = opts.add_callback;
let bucket = key_to_bucket.get(bucket_key);
let bucket = self.get_bucket(bucket_key);
if (!bucket) {
bucket = options.make_bucket();
key_to_bucket.set(bucket_key, bucket);
key_to_bucket.set(options.fold_case ? bucket_key.toLowerCase() : bucket_key, bucket);
}
if (add_callback) {
add_callback(bucket, item_id);
@ -102,15 +102,15 @@ function make_bucketer(options) {
};
self.get_bucket = function (bucket_key) {
return key_to_bucket.get(bucket_key);
return key_to_bucket.get(options.fold_case ? bucket_key.toLowerCase() : bucket_key);
};
self.each = function (callback) {
key_to_bucket.each(callback);
key_to_bucket.forEach(callback);
};
self.keys = function () {
return key_to_bucket.keys();
return [...key_to_bucket.keys()];
};
return self;
@ -330,10 +330,13 @@ exports.unread_topic_counter = (function () {
const result = _.map(topic_names, function (topic_name) {
const msgs = per_stream_bucketer.get_bucket(topic_name);
const message_id = msgs.max();
return {
pretty_name: topic_name,
message_id: msgs.max(),
// retrieve the topic with its original case, since topic_name
// has been lowercased
pretty_name: message_store.get(message_id).topic,
message_id,
};
});