From 25b59d00443723c85df51a6dbfee2a2545847e23 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 12 Jul 2017 10:12:00 -0400 Subject: [PATCH] Extract sent_messages.message_state class. This class helps us encapsulate the state of a message, with all the dates/flags that get sent as part of /json/report_send_time. --- frontend_tests/node_tests/compose.js | 23 ++++-- static/js/sent_messages.js | 114 +++++++++++++++++++-------- 2 files changed, 96 insertions(+), 41 deletions(-) diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js index 86c73778b2..b3dd303127 100644 --- a/frontend_tests/node_tests/compose.js +++ b/frontend_tests/node_tests/compose.js @@ -290,8 +290,11 @@ people.add(bob); assert.equal($("#compose-send-button").attr('disabled'), undefined); assert(!$("#sending-indicator").visible()); assert.equal(_.keys(sent_messages.send_times_data).length, 1); - assert.equal(sent_messages.send_times_data[12].start.getTime(), new Date(test_date).getTime()); - assert(!sent_messages.send_times_data[12].locally_echoed); + + var data = sent_messages.get_message_state(12).data; + assert.equal(data.start.getTime(), new Date(test_date).getTime()); + assert(!data.locally_echoed); + assert(reify_message_id_checked); assert(server_events_triggered); assert(set_timeout_called); @@ -299,7 +302,7 @@ people.add(bob); (function test_mark_rendered_content_disparity() { sent_messages.mark_rendered_content_disparity(13, true); - assert.deepEqual(sent_messages.send_times_data[13], { rendered_content_disparity: true }); + assert.deepEqual(sent_messages.send_times_data[13].data.rendered_content_disparity, true); }()); (function test_report_as_received() { @@ -313,7 +316,10 @@ people.add(bob); func(); set_timeout_called = true; }); - sent_messages.send_times_data[12].locally_echoed = true; + + var data = sent_messages.get_message_state(12).data; + + data.locally_echoed = true; channel.post = function (payload) { assert.equal(payload.url, '/json/report_send_time'); assert.equal(typeof(payload.data.time), 'string'); @@ -321,15 +327,16 @@ people.add(bob); assert(!payload.data.rendered_content_disparity); }; sent_messages.report_as_received(msg); - assert.equal(typeof(sent_messages.send_times_data[12].received), 'object'); - assert.equal(typeof(sent_messages.send_times_data[12].displayed), 'object'); + assert.equal(typeof(data.received), 'object'); + assert.equal(typeof(data.displayed), 'object'); assert(set_timeout_called); delete sent_messages.send_times_data[13]; msg.id = 13; sent_messages.report_as_received(msg); - assert.equal(typeof(sent_messages.send_times_data[13].received), 'object'); - assert.equal(typeof(sent_messages.send_times_data[13].displayed), 'object'); + data = sent_messages.get_message_state(13).data; + assert.equal(typeof(data.received), 'object'); + assert.equal(typeof(data.displayed), 'object'); }()); (function test_send_message() { diff --git a/static/js/sent_messages.js b/static/js/sent_messages.js index 6baf97ece3..3c0638cadc 100644 --- a/static/js/sent_messages.js +++ b/static/js/sent_messages.js @@ -19,38 +19,87 @@ function report_send_time(send_time, receive_time, display_time, locally_echoed, }); } -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; +exports.message_state = function () { + var self = {}; + self.data = {}; + + // TODO: Fix quirk that we don't know the start time sometime + // when we start recording message state. + self.data.start = undefined; + self.data.received = undefined; + self.data.displayed = undefined; + self.data.send_finished = undefined; + self.data.locally_echoed = false; + self.data.rendered_content_disparity = false; + + 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 (opts) { + self.data.start = opts.start; + self.data.send_finished = opts.send_finished; + self.data.locally_echoed = opts.locally_echoed; + 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 (message_id) { + if (exports.send_times_data[message_id] === undefined) { + exports.send_times_data[message_id] = exports.message_state(); } - report_send_time(data.send_finished - data.start, - data.received - data.start, - data.displayed - data.start, - data.locally_echoed, - data.rendered_content_disparity || false); -} + + return exports.send_times_data[message_id]; +}; + 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); + var state = exports.get_message_state(message_id); + state.mark_received(); } function mark_end_to_end_display_time(message_id) { - exports.send_times_data[message_id].displayed = new Date(); - maybe_report_send_times(message_id); + var state = exports.get_message_state(message_id); + state.mark_displayed(); } 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; + var state = exports.get_message_state(message_id); + state.mark_disparity(changed); }; exports.report_as_received = function report_as_received(message) { @@ -71,18 +120,18 @@ exports.process_success = function (message_id, start_time, locally_echoed) { 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); + + var state = exports.get_message_state(message_id); + state.process_success({ + start: start_time, + send_finished: send_finished, + locally_echoed: locally_echoed, + }); }; exports.set_timer_for_restarting_event_loop = function (message_id) { setTimeout(function () { - if (exports.send_times_data[message_id].received === undefined) { + if (!exports.send_times_data[message_id].was_received()) { blueslip.log("Restarting get_events due to delayed receipt of sent message " + message_id); server_events.restart_get_events(); } @@ -97,9 +146,8 @@ exports.initialize = function () { $(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]; + exports.send_times_data[event.new_id] = value; delete exports.send_times_data[event.old_id]; - exports.send_times_data[event.new_id] = - _.extend({}, exports.send_times_data[event.old_id], value); } }); };