mirror of https://github.com/zulip/zulip.git
Revert commits related to client_message_id.
I pushed a bunch of commits that attempted to introduce the concept of `client_message_id` into our server, as part of cleaning up our codepaths related to messages you sent (both for the locally echoed case and for the host case). When we deployed this, we had some strange failures involving double-echoed messages and issues advancing the pointer that appeared related to #5779. We didn't get to the bottom of exactly why the PR caused havoc, but I decided there was a cleaner approach, anyway.
This commit is contained in:
parent
d3cce041a4
commit
475eb21a5e
|
@ -56,7 +56,6 @@
|
|||
"typing_events": false,
|
||||
"typing_data": false,
|
||||
"typing_status": false,
|
||||
"sent_messages": false,
|
||||
"compose": false,
|
||||
"compose_actions": false,
|
||||
"compose_state": false,
|
||||
|
|
|
@ -45,7 +45,6 @@ add_dependencies({
|
|||
compose_ui: 'js/compose_ui.js',
|
||||
Handlebars: 'handlebars',
|
||||
people: 'js/people',
|
||||
sent_messages: 'js/sent_messages',
|
||||
stream_data: 'js/stream_data',
|
||||
util: 'js/util',
|
||||
});
|
||||
|
@ -255,8 +254,6 @@ people.add(bob);
|
|||
}());
|
||||
|
||||
(function test_send_message_success() {
|
||||
sent_messages.reset_id_state();
|
||||
|
||||
blueslip.error = noop;
|
||||
blueslip.log = noop;
|
||||
$("#new_message_content").val('foobarfoobar');
|
||||
|
@ -266,60 +263,72 @@ people.add(bob);
|
|||
$("#sending-indicator").show();
|
||||
global.feature_flags.log_send_times = true;
|
||||
global.feature_flags.collect_send_times = true;
|
||||
|
||||
var set_timeout_called = false;
|
||||
global.patch_builtin('setTimeout', function (func, delay) {
|
||||
assert.equal(delay, 5000);
|
||||
func();
|
||||
set_timeout_called = true;
|
||||
});
|
||||
var server_events_triggered;
|
||||
global.server_events = {
|
||||
restart_get_events: function () {
|
||||
server_events_triggered = true;
|
||||
},
|
||||
};
|
||||
var reify_message_id_checked;
|
||||
echo.reify_message_id = function (local_id, message_id) {
|
||||
assert.equal(local_id, 1001);
|
||||
assert.equal(message_id, 12);
|
||||
reify_message_id_checked = true;
|
||||
};
|
||||
var locally_echoed = false;
|
||||
compose.send_message_success(1001, 12, locally_echoed);
|
||||
var test_date = 'Wed Jun 28 2017 22:12:48 GMT+0000 (UTC)';
|
||||
compose.send_message_success(1001, 12, new Date(test_date), false);
|
||||
assert.equal($("#new_message_content").val(), '');
|
||||
assert($("#new_message_content").is_focused());
|
||||
assert(!$("#send-status").visible());
|
||||
assert.equal($("#compose-send-button").attr('disabled'), undefined);
|
||||
assert(!$("#sending-indicator").visible());
|
||||
|
||||
assert.equal(_.keys(compose.send_times_data).length, 1);
|
||||
assert.equal(compose.send_times_data[12].start.getTime(), new Date(test_date).getTime());
|
||||
assert(!compose.send_times_data[12].locally_echoed);
|
||||
assert(reify_message_id_checked);
|
||||
assert(server_events_triggered);
|
||||
assert(set_timeout_called);
|
||||
}());
|
||||
|
||||
(function test_mark_rendered_content_disparity() {
|
||||
compose.mark_rendered_content_disparity(13, true);
|
||||
assert.deepEqual(compose.send_times_data[13], { rendered_content_disparity: true });
|
||||
}());
|
||||
|
||||
(function test_report_as_received() {
|
||||
var msg = {
|
||||
id: 12,
|
||||
sent_by_me: true,
|
||||
};
|
||||
var set_timeout_called = false;
|
||||
global.patch_builtin('setTimeout', function (func, delay) {
|
||||
assert.equal(delay, 0);
|
||||
func();
|
||||
set_timeout_called = true;
|
||||
});
|
||||
|
||||
sent_messages.track_message({
|
||||
client_message_id: 12,
|
||||
});
|
||||
|
||||
var data = sent_messages.get_message_state(12).data;
|
||||
|
||||
data.locally_echoed = true;
|
||||
compose.send_times_data[12].locally_echoed = true;
|
||||
channel.post = function (payload) {
|
||||
assert.equal(payload.url, '/json/report_send_time');
|
||||
assert.equal(typeof(payload.data.time), 'string');
|
||||
assert(payload.data.locally_echoed);
|
||||
assert(!payload.data.rendered_content_disparity);
|
||||
};
|
||||
sent_messages.report_as_received(12);
|
||||
assert.equal(typeof(data.received), 'object');
|
||||
assert.equal(typeof(data.displayed), 'object');
|
||||
compose.report_as_received(msg);
|
||||
assert.equal(typeof(compose.send_times_data[12].received), 'object');
|
||||
assert.equal(typeof(compose.send_times_data[12].displayed), 'object');
|
||||
assert(set_timeout_called);
|
||||
|
||||
delete sent_messages.send_times_data[13];
|
||||
sent_messages.track_message({
|
||||
client_message_id: 13,
|
||||
});
|
||||
|
||||
|
||||
sent_messages.report_as_received(13);
|
||||
data = sent_messages.get_message_state(13).data;
|
||||
assert.equal(typeof(data.received), 'object');
|
||||
assert.equal(typeof(data.displayed), 'object');
|
||||
delete compose.send_times_data[13];
|
||||
msg.id = 13;
|
||||
compose.report_as_received(msg);
|
||||
assert.equal(typeof(compose.send_times_data[13].received), 'object');
|
||||
assert.equal(typeof(compose.send_times_data[13].displayed), 'object');
|
||||
}());
|
||||
|
||||
(function test_send_message() {
|
||||
|
@ -370,11 +379,10 @@ people.add(bob);
|
|||
reply_to: 'alice@example.com',
|
||||
private_message_recipient: 'alice@example.com',
|
||||
to_user_ids: '31',
|
||||
client_message_id: 1,
|
||||
local_id: 1,
|
||||
};
|
||||
assert.equal(payload.url, '/json/messages');
|
||||
assert.equal(_.keys(payload.data).length, 12);
|
||||
assert.equal(_.keys(payload.data).length, 11);
|
||||
assert.deepEqual(payload.data, single_msg);
|
||||
payload.data.id = stub_state.local_id_counter;
|
||||
payload.success(payload.data);
|
||||
|
@ -385,7 +393,7 @@ people.add(bob);
|
|||
assert.equal(typeof(message_id), 'number');
|
||||
stub_state.reify_message_id_checked += 1;
|
||||
};
|
||||
sent_messages.send_times_data = {};
|
||||
compose.send_times_data = {};
|
||||
// Setting message content with a host server link and we will assert
|
||||
// later that this has been converted to a relative link.
|
||||
$("#new_message_content").val('[foobar]' +
|
||||
|
@ -405,7 +413,7 @@ people.add(bob);
|
|||
server_events_triggered: 1,
|
||||
};
|
||||
assert.deepEqual(stub_state, state);
|
||||
assert.equal(_.keys(sent_messages.send_times_data).length, 1);
|
||||
assert.equal(_.keys(compose.send_times_data).length, 1);
|
||||
assert.equal($("#new_message_content").val(), '');
|
||||
assert($("#new_message_content").is_focused());
|
||||
assert(!$("#send-status").visible());
|
||||
|
@ -440,6 +448,7 @@ people.add(bob);
|
|||
server_events_triggered: 0,
|
||||
};
|
||||
assert.deepEqual(stub_state, state);
|
||||
assert.equal(_.keys(compose.send_times_data).length, 1);
|
||||
assert(server_error_triggered);
|
||||
assert(reload_initiate_triggered);
|
||||
}());
|
||||
|
@ -480,6 +489,7 @@ people.add(bob);
|
|||
server_events_triggered: 0,
|
||||
};
|
||||
assert.deepEqual(stub_state, state);
|
||||
assert.equal(_.keys(compose.send_times_data).length, 1);
|
||||
assert(server_error_triggered);
|
||||
assert(!reload_initiate_triggered);
|
||||
assert(xhr_error_msg_checked);
|
||||
|
@ -512,6 +522,7 @@ people.add(bob);
|
|||
server_events_triggered: 0,
|
||||
};
|
||||
assert.deepEqual(stub_state, state);
|
||||
assert.equal(_.keys(compose.send_times_data).length, 1);
|
||||
assert(server_error_triggered);
|
||||
assert(!reload_initiate_triggered);
|
||||
assert(xhr_error_msg_checked);
|
||||
|
@ -769,20 +780,18 @@ function test_with_mock_socket(test_params) {
|
|||
// socket_user_agent field will be added.
|
||||
var request = {foo: 'bar'};
|
||||
|
||||
var success_func_checked = false;
|
||||
var success = function () {
|
||||
success_func_checked = true;
|
||||
};
|
||||
// Our success function gets passed all the way through to
|
||||
// socket.send, so we can just use a stub to test that.
|
||||
var success = 'success-function-stub';
|
||||
|
||||
// Our error function gets wrapped, so we set up a real
|
||||
// function to test the wrapping mechanism.
|
||||
var error_func_checked = false;
|
||||
var error = function (error_msg) {
|
||||
assert.equal(error_msg, 'Error sending message: simulated_error');
|
||||
error_func_checked = true;
|
||||
};
|
||||
|
||||
sent_messages.send_times_data = {};
|
||||
sent_messages.reset_id_state();
|
||||
|
||||
test_with_mock_socket({
|
||||
run_code: function () {
|
||||
compose.transmit_message(request, success, error);
|
||||
|
@ -794,15 +803,11 @@ function test_with_mock_socket(test_params) {
|
|||
assert.equal(send_args.request, request);
|
||||
assert.deepEqual(send_args.request, {
|
||||
foo: 'bar',
|
||||
client_message_id: 1,
|
||||
socket_user_agent: 'unittest_transmit_message',
|
||||
});
|
||||
|
||||
// Just make sure our success function gets called.
|
||||
send_args.success({
|
||||
id: 42,
|
||||
});
|
||||
assert(success_func_checked);
|
||||
// Our success function never gets wrapped.
|
||||
assert.equal(send_args.success, success);
|
||||
|
||||
// Our error function does get wrapped, so we test by
|
||||
// using socket.send's error callback, which should
|
||||
|
@ -1240,6 +1245,26 @@ function test_with_mock_socket(test_params) {
|
|||
assert(!$("#preview_message_area").visible());
|
||||
assert($("#markdown_preview").visible());
|
||||
}());
|
||||
|
||||
(function test_message_id_changed_document() {
|
||||
var handler = $(document).get_on_handler('message_id_changed');
|
||||
compose.send_times_data = {
|
||||
1031: {
|
||||
data: 'Test data!',
|
||||
},
|
||||
};
|
||||
event.old_id = 1031;
|
||||
event.new_id = 1045;
|
||||
|
||||
handler(event);
|
||||
|
||||
var send_times_data = {
|
||||
1045: {
|
||||
data: 'Test data!',
|
||||
},
|
||||
};
|
||||
assert.deepEqual(compose.send_times_data, send_times_data);
|
||||
}());
|
||||
}());
|
||||
|
||||
(function test_upload_started() {
|
||||
|
|
|
@ -1,34 +0,0 @@
|
|||
var sent_messages = require('js/sent_messages.js');
|
||||
|
||||
(function test_ids() {
|
||||
sent_messages.reset_id_state();
|
||||
|
||||
var client_message_id = sent_messages.get_new_client_message_id({
|
||||
local_id: 42,
|
||||
});
|
||||
|
||||
assert.equal(client_message_id, 1);
|
||||
|
||||
var local_id = sent_messages.get_local_id({
|
||||
client_message_id: 1,
|
||||
});
|
||||
|
||||
assert.equal(local_id, 42);
|
||||
|
||||
|
||||
client_message_id = sent_messages.get_new_client_message_id({});
|
||||
|
||||
assert.equal(client_message_id, 2);
|
||||
|
||||
local_id = sent_messages.get_local_id({
|
||||
client_message_id: 2,
|
||||
});
|
||||
|
||||
assert.equal(local_id, undefined);
|
||||
|
||||
local_id = sent_messages.get_local_id({
|
||||
client_message_id: undefined,
|
||||
});
|
||||
assert.equal(local_id, undefined);
|
||||
|
||||
}());
|
|
@ -53,10 +53,6 @@ server_events.home_view_loaded();
|
|||
flags: [],
|
||||
};
|
||||
|
||||
set_global('sent_messages', {
|
||||
get_local_id: function () { return 9999; },
|
||||
});
|
||||
|
||||
var inserted;
|
||||
set_global('message_events', {
|
||||
insert_new_messages: function (messages) {
|
||||
|
|
|
@ -179,6 +179,20 @@ function send_message_ajax(request, success, error) {
|
|||
});
|
||||
}
|
||||
|
||||
function report_send_time(send_time, receive_time, display_time, locally_echoed, rendered_changed) {
|
||||
var data = {time: send_time.toString(),
|
||||
received: receive_time.toString(),
|
||||
displayed: display_time.toString(),
|
||||
locally_echoed: locally_echoed};
|
||||
if (locally_echoed) {
|
||||
data.rendered_content_disparity = rendered_changed;
|
||||
}
|
||||
channel.post({
|
||||
url: '/json/report_send_time',
|
||||
data: data,
|
||||
});
|
||||
}
|
||||
|
||||
var socket;
|
||||
if (page_params.use_websockets) {
|
||||
socket = new Socket("/sockjs");
|
||||
|
@ -197,6 +211,69 @@ function send_message_socket(request, success, error) {
|
|||
});
|
||||
}
|
||||
|
||||
exports.send_times_log = [];
|
||||
exports.send_times_data = {};
|
||||
function maybe_report_send_times(message_id) {
|
||||
var data = exports.send_times_data[message_id];
|
||||
if (data.send_finished === undefined || data.received === undefined ||
|
||||
data.displayed === undefined) {
|
||||
// We report the data once we have both the send and receive times
|
||||
return;
|
||||
}
|
||||
report_send_time(data.send_finished - data.start,
|
||||
data.received - data.start,
|
||||
data.displayed - data.start,
|
||||
data.locally_echoed,
|
||||
data.rendered_content_disparity || false);
|
||||
}
|
||||
|
||||
function mark_end_to_end_receive_time(message_id) {
|
||||
if (exports.send_times_data[message_id] === undefined) {
|
||||
exports.send_times_data[message_id] = {};
|
||||
}
|
||||
exports.send_times_data[message_id].received = new Date();
|
||||
maybe_report_send_times(message_id);
|
||||
}
|
||||
|
||||
function mark_end_to_end_display_time(message_id) {
|
||||
exports.send_times_data[message_id].displayed = new Date();
|
||||
maybe_report_send_times(message_id);
|
||||
}
|
||||
|
||||
exports.mark_rendered_content_disparity = function (message_id, changed) {
|
||||
if (exports.send_times_data[message_id] === undefined) {
|
||||
exports.send_times_data[message_id] = {};
|
||||
}
|
||||
exports.send_times_data[message_id].rendered_content_disparity = changed;
|
||||
};
|
||||
|
||||
exports.report_as_received = function report_as_received(message) {
|
||||
if (message.sent_by_me) {
|
||||
mark_end_to_end_receive_time(message.id);
|
||||
setTimeout(function () {
|
||||
mark_end_to_end_display_time(message.id);
|
||||
}, 0);
|
||||
}
|
||||
};
|
||||
|
||||
function process_send_time(message_id, start_time, locally_echoed) {
|
||||
var send_finished = new Date();
|
||||
var send_time = (send_finished - start_time);
|
||||
if (feature_flags.log_send_times) {
|
||||
blueslip.log("send time: " + send_time);
|
||||
}
|
||||
if (feature_flags.collect_send_times) {
|
||||
exports.send_times_log.push(send_time);
|
||||
}
|
||||
if (exports.send_times_data[message_id] === undefined) {
|
||||
exports.send_times_data[message_id] = {};
|
||||
}
|
||||
exports.send_times_data[message_id].start = start_time;
|
||||
exports.send_times_data[message_id].send_finished = send_finished;
|
||||
exports.send_times_data[message_id].locally_echoed = locally_echoed;
|
||||
maybe_report_send_times(message_id);
|
||||
}
|
||||
|
||||
function clear_compose_box() {
|
||||
$("#new_message_content").val('').focus();
|
||||
drafts.delete_draft_after_send();
|
||||
|
@ -207,40 +284,25 @@ function clear_compose_box() {
|
|||
resize.resize_bottom_whitespace();
|
||||
}
|
||||
|
||||
exports.send_message_success = function (local_id, message_id, locally_echoed) {
|
||||
exports.send_message_success = function (local_id, message_id, start_time, locally_echoed) {
|
||||
if (!locally_echoed) {
|
||||
clear_compose_box();
|
||||
}
|
||||
|
||||
process_send_time(message_id, start_time, locally_echoed);
|
||||
|
||||
echo.reify_message_id(local_id, message_id);
|
||||
|
||||
setTimeout(function () {
|
||||
if (exports.send_times_data[message_id].received === undefined) {
|
||||
blueslip.log("Restarting get_events due to delayed receipt of sent message " + message_id);
|
||||
server_events.restart_get_events();
|
||||
}
|
||||
}, 5000);
|
||||
};
|
||||
|
||||
exports.transmit_message = function (request, on_success, error) {
|
||||
var client_message_id = sent_messages.get_new_client_message_id({
|
||||
local_id: request.local_id,
|
||||
});
|
||||
|
||||
// The host will attach this to message events, and it will allow
|
||||
// us to match up the right messages to this request.
|
||||
request.client_message_id = client_message_id;
|
||||
|
||||
var local_id = request.local_id;
|
||||
var locally_echoed = local_id !== undefined;
|
||||
|
||||
var message_state = sent_messages.track_message({
|
||||
client_message_id: client_message_id,
|
||||
locally_echoed: locally_echoed,
|
||||
});
|
||||
|
||||
function success(data) {
|
||||
// Call back to our callers to do things like closing the compose
|
||||
// box and turning off spinners and reifying locally echoed messages.
|
||||
on_success(data);
|
||||
|
||||
// Once everything is done, get ready to report times to the server.
|
||||
message_state.process_success();
|
||||
}
|
||||
|
||||
exports.transmit_message = function (request, success, error) {
|
||||
delete exports.send_times_data[request.id];
|
||||
if (page_params.use_websockets) {
|
||||
send_message_socket(request, success, error);
|
||||
} else {
|
||||
|
@ -259,6 +321,7 @@ exports.send_message = function send_message(request) {
|
|||
request.to = JSON.stringify([request.to]);
|
||||
}
|
||||
|
||||
var start_time = new Date();
|
||||
var local_id;
|
||||
|
||||
local_id = echo.try_deliver_locally(request);
|
||||
|
@ -266,10 +329,11 @@ exports.send_message = function send_message(request) {
|
|||
// We delivered this message locally
|
||||
request.local_id = local_id;
|
||||
}
|
||||
|
||||
var locally_echoed = local_id !== undefined;
|
||||
|
||||
function success(data) {
|
||||
exports.send_message_success(local_id, data.id, locally_echoed);
|
||||
exports.send_message_success(local_id, data.id, start_time, locally_echoed);
|
||||
}
|
||||
|
||||
function error(response) {
|
||||
|
@ -835,6 +899,15 @@ exports.initialize = function () {
|
|||
compose_actions.start("stream", {});
|
||||
}
|
||||
}
|
||||
|
||||
$(document).on('message_id_changed', function (event) {
|
||||
if (exports.send_times_data[event.old_id] !== undefined) {
|
||||
var value = exports.send_times_data[event.old_id];
|
||||
delete exports.send_times_data[event.old_id];
|
||||
exports.send_times_data[event.new_id] =
|
||||
_.extend({}, exports.send_times_data[event.old_id], value);
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
return exports;
|
||||
|
|
|
@ -13,16 +13,14 @@ function resend_message(message, row) {
|
|||
// Always re-set queue_id if we've gotten a new one
|
||||
// since the time when the message object was initially created
|
||||
message.queue_id = page_params.queue_id;
|
||||
var start_time = new Date();
|
||||
compose.transmit_message(message, function success(data) {
|
||||
retry_spinner.toggleClass('rotating', false);
|
||||
|
||||
var message_id = data.id;
|
||||
|
||||
retry_spinner.toggleClass('rotating', false);
|
||||
|
||||
var locally_echoed = true;
|
||||
|
||||
compose.send_message_success(message.local_id, message_id, locally_echoed);
|
||||
compose.send_message_success(message.local_id, message_id, start_time, true);
|
||||
|
||||
// Resend succeeded, so mark as no longer failed
|
||||
message_store.get(message_id).failed_request = false;
|
||||
|
@ -173,14 +171,11 @@ exports.process_from_server = function process_from_server(messages) {
|
|||
if (client_message.content !== message.content) {
|
||||
client_message.content = message.content;
|
||||
updated = true;
|
||||
sent_messages.mark_rendered_content_disparity({
|
||||
client_message_id: message.client_message_id,
|
||||
changed: true,
|
||||
});
|
||||
compose.mark_rendered_content_disparity(message.id, true);
|
||||
}
|
||||
msgs_to_rerender.push(client_message);
|
||||
locally_processed_ids.push(client_message.id);
|
||||
sent_messages.report_as_received(message.client_message_id);
|
||||
compose.report_as_received(client_message);
|
||||
delete waiting_for_ack[client_message.id];
|
||||
return false;
|
||||
}
|
||||
|
|
|
@ -23,7 +23,7 @@ exports.add_messages = function add_messages(messages, msg_list, opts) {
|
|||
if (msg_list === home_msg_list && opts.messages_are_new) {
|
||||
_.each(messages, function (message) {
|
||||
if (message.local_id === undefined) {
|
||||
sent_messages.report_as_received(message.client_message_id);
|
||||
compose.report_as_received(message);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
|
@ -1,203 +0,0 @@
|
|||
var sent_messages = (function () {
|
||||
|
||||
var exports = {};
|
||||
|
||||
exports.send_times_log = [];
|
||||
exports.send_times_data = {};
|
||||
|
||||
exports.reset_id_state = function () {
|
||||
exports.local_id_dict = new Dict();
|
||||
exports.next_client_message_id = 0;
|
||||
};
|
||||
|
||||
exports.get_new_client_message_id = function (opts) {
|
||||
exports.next_client_message_id += 1;
|
||||
var client_message_id = exports.next_client_message_id;
|
||||
exports.local_id_dict.set(client_message_id, opts.local_id);
|
||||
return client_message_id;
|
||||
};
|
||||
|
||||
exports.get_local_id = function (opts) {
|
||||
var client_message_id = opts.client_message_id;
|
||||
|
||||
if (client_message_id === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return exports.local_id_dict.get(client_message_id);
|
||||
};
|
||||
|
||||
function report_send_time(send_time, receive_time, display_time, locally_echoed, rendered_changed) {
|
||||
var data = {time: send_time.toString(),
|
||||
received: receive_time.toString(),
|
||||
displayed: display_time.toString(),
|
||||
locally_echoed: locally_echoed};
|
||||
if (locally_echoed) {
|
||||
data.rendered_content_disparity = rendered_changed;
|
||||
}
|
||||
channel.post({
|
||||
url: '/json/report_send_time',
|
||||
data: data,
|
||||
});
|
||||
}
|
||||
|
||||
exports.track_message = function (opts) {
|
||||
var client_message_id = opts.client_message_id;
|
||||
|
||||
if (exports.send_times_data[client_message_id] !== undefined) {
|
||||
blueslip.error('We are re-using a client_message_id');
|
||||
return;
|
||||
}
|
||||
|
||||
var state = exports.message_state(opts);
|
||||
|
||||
exports.send_times_data[client_message_id] = state;
|
||||
|
||||
return state;
|
||||
};
|
||||
|
||||
exports.message_state = function (opts) {
|
||||
var self = {};
|
||||
self.data = {};
|
||||
|
||||
self.data.start = new Date();
|
||||
|
||||
self.data.client_message_id = opts.client_message_id;
|
||||
self.data.locally_echoed = opts.locally_echoed;
|
||||
|
||||
|
||||
self.data.received = undefined;
|
||||
self.data.displayed = undefined;
|
||||
self.data.send_finished = undefined;
|
||||
self.data.rendered_content_disparity = false;
|
||||
|
||||
self.maybe_restart_event_loop = function () {
|
||||
if (self.data.received) {
|
||||
// We got our event, no need to do anything
|
||||
return;
|
||||
}
|
||||
|
||||
blueslip.log("Restarting get_events due to " +
|
||||
"delayed receipt of sent message " +
|
||||
self.data.client_message_id);
|
||||
|
||||
server_events.restart_get_events();
|
||||
};
|
||||
|
||||
self.maybe_report_send_times = function () {
|
||||
if (!self.ready()) {
|
||||
return;
|
||||
}
|
||||
var data = self.data;
|
||||
report_send_time(data.send_finished - data.start,
|
||||
data.received - data.start,
|
||||
data.displayed - data.start,
|
||||
data.locally_echoed,
|
||||
data.rendered_content_disparity || false);
|
||||
};
|
||||
|
||||
self.mark_received = function () {
|
||||
self.data.received = new Date();
|
||||
self.maybe_report_send_times();
|
||||
};
|
||||
|
||||
self.mark_displayed = function () {
|
||||
self.data.displayed = new Date();
|
||||
self.maybe_report_send_times();
|
||||
};
|
||||
|
||||
self.mark_disparity = function (changed) {
|
||||
self.data.rendered_content_disparity = changed;
|
||||
};
|
||||
|
||||
self.process_success = function () {
|
||||
var send_finished = new Date();
|
||||
|
||||
// We only start our timer for events coming in here,
|
||||
// since it's plausible the server rejected our message,
|
||||
// or took a while to process it, but there is nothing
|
||||
// wrong with our event loop.
|
||||
|
||||
if (!self.data.received) {
|
||||
setTimeout(self.maybe_restart_event_loop, 5000);
|
||||
}
|
||||
|
||||
var send_time = (send_finished - self.data.start);
|
||||
if (feature_flags.log_send_times) {
|
||||
blueslip.log("send time: " + send_time);
|
||||
}
|
||||
if (feature_flags.collect_send_times) {
|
||||
exports.send_times_log.push(send_time);
|
||||
}
|
||||
|
||||
self.data.send_finished = send_finished;
|
||||
self.maybe_report_send_times();
|
||||
};
|
||||
|
||||
self.was_received = function () {
|
||||
return self.data.received !== undefined;
|
||||
};
|
||||
|
||||
self.ready = function () {
|
||||
return (self.data.send_finished !== undefined) &&
|
||||
(self.data.received !== undefined) &&
|
||||
(self.data.displayed !== undefined);
|
||||
};
|
||||
|
||||
return self;
|
||||
};
|
||||
|
||||
exports.get_message_state = function (client_message_id) {
|
||||
var state = exports.send_times_data[client_message_id];
|
||||
|
||||
if (!state) {
|
||||
blueslip.warn('Unknown client_message_id' + client_message_id);
|
||||
}
|
||||
|
||||
return state;
|
||||
};
|
||||
|
||||
|
||||
function mark_end_to_end_receive_time(client_message_id) {
|
||||
var state = exports.get_message_state(client_message_id);
|
||||
if (!state) {
|
||||
return;
|
||||
}
|
||||
state.mark_received();
|
||||
}
|
||||
|
||||
function mark_end_to_end_display_time(client_message_id) {
|
||||
var state = exports.get_message_state(client_message_id);
|
||||
if (!state) {
|
||||
return;
|
||||
}
|
||||
state.mark_displayed();
|
||||
}
|
||||
|
||||
exports.mark_rendered_content_disparity = function (opts) {
|
||||
var state = exports.get_message_state(opts.client_message_id);
|
||||
if (!state) {
|
||||
return;
|
||||
}
|
||||
state.mark_disparity(opts.changed);
|
||||
};
|
||||
|
||||
exports.report_as_received = function report_as_received(client_message_id) {
|
||||
if (client_message_id) {
|
||||
mark_end_to_end_receive_time(client_message_id);
|
||||
setTimeout(function () {
|
||||
mark_end_to_end_display_time(client_message_id);
|
||||
}, 0);
|
||||
}
|
||||
};
|
||||
|
||||
exports.initialize = function () {
|
||||
exports.reset_id_state();
|
||||
};
|
||||
|
||||
return exports;
|
||||
|
||||
}());
|
||||
if (typeof module !== 'undefined') {
|
||||
module.exports = sent_messages;
|
||||
}
|
|
@ -68,14 +68,9 @@ function get_events_success(events) {
|
|||
case 'message':
|
||||
var msg = event.message;
|
||||
msg.flags = event.flags;
|
||||
|
||||
// The server echos client_message_id to us, and then we
|
||||
// map it back to our local_id that was used for rendering.
|
||||
msg.local_id = sent_messages.get_local_id({
|
||||
client_message_id: event.client_message_id,
|
||||
});
|
||||
msg.client_message_id = event.client_message_id;
|
||||
|
||||
if (event.local_message_id !== undefined) {
|
||||
msg.local_id = event.local_message_id;
|
||||
}
|
||||
messages.push(msg);
|
||||
break;
|
||||
|
||||
|
|
|
@ -266,7 +266,6 @@ $(function () {
|
|||
pm_list.initialize();
|
||||
stream_list.initialize();
|
||||
drafts.initialize();
|
||||
sent_messages.initialize();
|
||||
compose.initialize();
|
||||
});
|
||||
|
||||
|
|
|
@ -753,7 +753,7 @@ def do_send_messages(messages_maybe_none):
|
|||
for message in messages:
|
||||
message['rendered_content'] = message.get('rendered_content', None)
|
||||
message['stream'] = message.get('stream', None)
|
||||
message['client_message_id'] = message.get('client_message_id', None)
|
||||
message['local_id'] = message.get('local_id', None)
|
||||
message['sender_queue_id'] = message.get('sender_queue_id', None)
|
||||
message['realm'] = message.get('realm', message['message'].sender.realm)
|
||||
|
||||
|
@ -899,8 +899,8 @@ def do_send_messages(messages_maybe_none):
|
|||
event['stream_name'] = message['stream'].name
|
||||
if message['stream'].invite_only:
|
||||
event['invite_only'] = True
|
||||
if message['client_message_id'] is not None:
|
||||
event['client_message_id'] = message['client_message_id']
|
||||
if message['local_id'] is not None:
|
||||
event['local_id'] = message['local_id']
|
||||
if message['sender_queue_id'] is not None:
|
||||
event['sender_queue_id'] = message['sender_queue_id']
|
||||
send_event(event, users)
|
||||
|
@ -1182,12 +1182,12 @@ def extract_recipients(s):
|
|||
# Returns the id of the sent message. Has same argspec as check_message.
|
||||
def check_send_message(sender, client, message_type_name, message_to,
|
||||
subject_name, message_content, realm=None, forged=False,
|
||||
forged_timestamp=None, forwarder_user_profile=None, client_message_id=None,
|
||||
forged_timestamp=None, forwarder_user_profile=None, local_id=None,
|
||||
sender_queue_id=None):
|
||||
# type: (UserProfile, Client, Text, Sequence[Text], Optional[Text], Text, Optional[Realm], bool, Optional[float], Optional[UserProfile], Optional[Text], Optional[Text]) -> int
|
||||
message = check_message(sender, client, message_type_name, message_to,
|
||||
subject_name, message_content, realm, forged, forged_timestamp,
|
||||
forwarder_user_profile, client_message_id, sender_queue_id)
|
||||
forwarder_user_profile, local_id, sender_queue_id)
|
||||
return do_send_messages([message])[0]
|
||||
|
||||
def check_stream_name(stream_name):
|
||||
|
@ -1252,7 +1252,7 @@ def send_pm_if_empty_stream(sender, stream, stream_name, realm):
|
|||
# Returns message ready for sending with do_send_message on success or the error message (string) on error.
|
||||
def check_message(sender, client, message_type_name, message_to,
|
||||
subject_name, message_content_raw, realm=None, forged=False,
|
||||
forged_timestamp=None, forwarder_user_profile=None, client_message_id=None,
|
||||
forged_timestamp=None, forwarder_user_profile=None, local_id=None,
|
||||
sender_queue_id=None):
|
||||
# type: (UserProfile, Client, Text, Sequence[Text], Optional[Text], Text, Optional[Realm], bool, Optional[float], Optional[UserProfile], Optional[Text], Optional[Text]) -> Dict[str, Any]
|
||||
stream = None
|
||||
|
@ -1347,7 +1347,7 @@ def check_message(sender, client, message_type_name, message_to,
|
|||
if id is not None:
|
||||
return {'message': id}
|
||||
|
||||
return {'message': message, 'stream': stream, 'client_message_id': client_message_id,
|
||||
return {'message': message, 'stream': stream, 'local_id': local_id,
|
||||
'sender_queue_id': sender_queue_id, 'realm': realm}
|
||||
|
||||
def _internal_prep_message(realm, sender, recipient_type_name, parsed_recipients,
|
||||
|
|
|
@ -272,9 +272,8 @@ class GetEventsTest(ZulipTestCase):
|
|||
self.assert_json_success(result)
|
||||
self.assert_length(events, 0)
|
||||
|
||||
client_message_id = 'opaque-id-1'
|
||||
self.send_message(email, recipient_email, Recipient.PERSONAL,
|
||||
"hello", client_message_id=client_message_id, sender_queue_id=queue_id)
|
||||
local_id = 10.01
|
||||
self.send_message(email, recipient_email, Recipient.PERSONAL, "hello", local_id=local_id, sender_queue_id=queue_id)
|
||||
|
||||
result = self.tornado_call(get_events_backend, user_profile,
|
||||
{"queue_id": queue_id,
|
||||
|
@ -287,14 +286,14 @@ class GetEventsTest(ZulipTestCase):
|
|||
self.assert_length(events, 1)
|
||||
self.assertEqual(events[0]["type"], "message")
|
||||
self.assertEqual(events[0]["message"]["sender_email"], email)
|
||||
self.assertEqual(events[0]["client_message_id"], client_message_id)
|
||||
self.assertEqual(events[0]["local_message_id"], local_id)
|
||||
self.assertEqual(events[0]["message"]["display_recipient"][0]["is_mirror_dummy"], False)
|
||||
self.assertEqual(events[0]["message"]["display_recipient"][1]["is_mirror_dummy"], False)
|
||||
|
||||
last_event_id = events[0]["id"]
|
||||
client_message_id = 'opaque-id-2'
|
||||
local_id += 0.01
|
||||
|
||||
self.send_message(email, recipient_email, Recipient.PERSONAL, "hello", client_message_id=client_message_id, sender_queue_id=queue_id)
|
||||
self.send_message(email, recipient_email, Recipient.PERSONAL, "hello", local_id=local_id, sender_queue_id=queue_id)
|
||||
|
||||
result = self.tornado_call(get_events_backend, user_profile,
|
||||
{"queue_id": queue_id,
|
||||
|
@ -307,7 +306,7 @@ class GetEventsTest(ZulipTestCase):
|
|||
self.assert_length(events, 1)
|
||||
self.assertEqual(events[0]["type"], "message")
|
||||
self.assertEqual(events[0]["message"]["sender_email"], email)
|
||||
self.assertEqual(events[0]["client_message_id"], client_message_id)
|
||||
self.assertEqual(events[0]["local_message_id"], local_id)
|
||||
|
||||
# Test that the received message in the receiver's event queue
|
||||
# exists and does not contain a local id
|
||||
|
@ -322,10 +321,10 @@ class GetEventsTest(ZulipTestCase):
|
|||
self.assertEqual(len(recipient_events), 2)
|
||||
self.assertEqual(recipient_events[0]["type"], "message")
|
||||
self.assertEqual(recipient_events[0]["message"]["sender_email"], email)
|
||||
self.assertTrue("client_message_id" not in recipient_events[0])
|
||||
self.assertTrue("local_message_id" not in recipient_events[0])
|
||||
self.assertEqual(recipient_events[1]["type"], "message")
|
||||
self.assertEqual(recipient_events[1]["message"]["sender_email"], email)
|
||||
self.assertTrue("client_message_id" not in recipient_events[1])
|
||||
self.assertTrue("local_message_id" not in recipient_events[1])
|
||||
|
||||
def test_get_events_narrow(self):
|
||||
# type: () -> None
|
||||
|
|
|
@ -356,7 +356,7 @@ class TornadoTestCase(WebSocketBaseTestCase):
|
|||
"queue_id": queue_events_data['response']['queue_id'],
|
||||
"to": ujson.dumps([self.example_email('othello')]),
|
||||
"reply_to": self.example_email('hamlet'),
|
||||
"client_message_id": 'id-42',
|
||||
"local_id": -1
|
||||
}
|
||||
}
|
||||
user_message_str = ujson.dumps(user_message)
|
||||
|
@ -391,7 +391,7 @@ class TornadoTestCase(WebSocketBaseTestCase):
|
|||
"queue_id": queue_events_data['response']['queue_id'],
|
||||
"to": ujson.dumps(["Denmark"]),
|
||||
"reply_to": self.example_email('hamlet'),
|
||||
"client_message_id": 'id-99',
|
||||
"local_id": -1
|
||||
}
|
||||
}
|
||||
user_message_str = ujson.dumps(user_message)
|
||||
|
|
|
@ -762,9 +762,9 @@ def process_message_event(event_template, users):
|
|||
user_event.update(extra_data)
|
||||
|
||||
if is_sender:
|
||||
client_message_id = event_template.get('client_message_id', None)
|
||||
if client_message_id is not None:
|
||||
user_event["client_message_id"] = client_message_id
|
||||
local_message_id = event_template.get('local_id', None)
|
||||
if local_message_id is not None:
|
||||
user_event["local_message_id"] = local_message_id
|
||||
|
||||
if not client.accepts_event(user_event):
|
||||
continue
|
||||
|
|
|
@ -262,7 +262,7 @@ def fake_message_sender(event):
|
|||
msg_id = check_send_message(sender, client, req['type'],
|
||||
extract_recipients(req['to']),
|
||||
req['subject'], req['content'],
|
||||
client_message_id=req.get('client_message_id', None),
|
||||
local_id=req.get('local_id', None),
|
||||
sender_queue_id=req.get('queue_id', None))
|
||||
resp = {"result": "success", "msg": "", "id": msg_id}
|
||||
except JsonableError as e:
|
||||
|
|
|
@ -118,7 +118,7 @@ class WebsocketClient(object):
|
|||
"queue_id": self.events_data['queue_id'],
|
||||
"to": ujson.dumps([private_message_recepient]),
|
||||
"reply_to": self.user_profile.email,
|
||||
"client_message_id": -1,
|
||||
"local_id": -1
|
||||
}
|
||||
}
|
||||
self.ws.write_message(ujson.dumps([ujson.dumps(user_message)]))
|
||||
|
|
|
@ -923,7 +923,7 @@ def send_message_backend(request, user_profile,
|
|||
subject_name = REQ('subject', lambda x: x.strip(), None),
|
||||
message_content = REQ('content'),
|
||||
realm_str = REQ('realm_str', default=None),
|
||||
client_message_id = REQ(default=None),
|
||||
local_id = REQ(default=None),
|
||||
queue_id = REQ(default=None)):
|
||||
# type: (HttpRequest, UserProfile, Text, List[Text], bool, Optional[Text], Text, Optional[Text], Optional[Text], Optional[Text]) -> HttpResponse
|
||||
client = request.client
|
||||
|
@ -979,7 +979,7 @@ def send_message_backend(request, user_profile,
|
|||
subject_name, message_content, forged=forged,
|
||||
forged_timestamp = request.POST.get('time'),
|
||||
forwarder_user_profile=user_profile, realm=realm,
|
||||
client_message_id=client_message_id, sender_queue_id=queue_id)
|
||||
local_id=local_id, sender_queue_id=queue_id)
|
||||
return json_success({"id": ret})
|
||||
|
||||
def fill_edit_history_entries(message_history, message):
|
||||
|
|
|
@ -869,7 +869,6 @@ JS_SPECS = {
|
|||
'js/markdown.js',
|
||||
'js/echo.js',
|
||||
'js/socket.js',
|
||||
'js/sent_messages.js',
|
||||
'js/compose_state.js',
|
||||
'js/compose_actions.js',
|
||||
'js/compose.js',
|
||||
|
|
Loading…
Reference in New Issue