From f6d670ae3da3cc0084d2c9e6b8a32cf1570abdbd Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 12 Jul 2017 06:13:56 -0400 Subject: [PATCH] Extract sent_messages.js. This is mostly straightforward moving of code out of compose.js. The code that was moved currently supports sending time reports for sent messages, but we intend to grow out the new module to track more state about sent messages. The following function names in this commit are new, but their code was basically pulled over verbatim: process_success (was process_send_time) set_timer_for_restarting_event_loop clear initialize All the code in the new module is covered by previous tests that had been written for compose.js. This commit only modifies a few things to keep those tests. The new module has 100% node coverage, so we updated `enforce_fully_covered`. --- .eslintrc.json | 1 + frontend_tests/node_tests/compose.js | 42 +++++----- static/js/compose.js | 100 ++---------------------- static/js/echo.js | 4 +- static/js/message_util.js | 2 +- static/js/sent_messages.js | 112 +++++++++++++++++++++++++++ static/js/ui_init.js | 1 + tools/test-js-with-node | 1 + zproject/settings.py | 1 + 9 files changed, 147 insertions(+), 117 deletions(-) create mode 100644 static/js/sent_messages.js diff --git a/.eslintrc.json b/.eslintrc.json index 15056240e0..8719b37f93 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -56,6 +56,7 @@ "typing_events": false, "typing_data": false, "typing_status": false, + "sent_messages": false, "compose": false, "compose_actions": false, "compose_state": false, diff --git a/frontend_tests/node_tests/compose.js b/frontend_tests/node_tests/compose.js index 1dee23515e..86c73778b2 100644 --- a/frontend_tests/node_tests/compose.js +++ b/frontend_tests/node_tests/compose.js @@ -45,6 +45,7 @@ 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', }); @@ -288,17 +289,17 @@ people.add(bob); 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.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); 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 }); + sent_messages.mark_rendered_content_disparity(13, true); + assert.deepEqual(sent_messages.send_times_data[13], { rendered_content_disparity: true }); }()); (function test_report_as_received() { @@ -312,23 +313,23 @@ people.add(bob); func(); set_timeout_called = true; }); - compose.send_times_data[12].locally_echoed = true; + sent_messages.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); }; - 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'); + 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(set_timeout_called); - delete compose.send_times_data[13]; + delete sent_messages.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'); + 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'); }()); (function test_send_message() { @@ -394,7 +395,7 @@ people.add(bob); assert.equal(typeof(message_id), 'number'); stub_state.reify_message_id_checked += 1; }; - compose.send_times_data = {}; + sent_messages.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]' + @@ -414,7 +415,7 @@ people.add(bob); server_events_triggered: 1, }; assert.deepEqual(stub_state, state); - assert.equal(_.keys(compose.send_times_data).length, 1); + assert.equal(_.keys(sent_messages.send_times_data).length, 1); assert.equal($("#new_message_content").val(), ''); assert($("#new_message_content").is_focused()); assert(!$("#send-status").visible()); @@ -449,7 +450,7 @@ people.add(bob); server_events_triggered: 0, }; assert.deepEqual(stub_state, state); - assert.equal(_.keys(compose.send_times_data).length, 1); + assert.equal(_.keys(sent_messages.send_times_data).length, 1); assert(server_error_triggered); assert(reload_initiate_triggered); }()); @@ -490,7 +491,7 @@ people.add(bob); server_events_triggered: 0, }; assert.deepEqual(stub_state, state); - assert.equal(_.keys(compose.send_times_data).length, 1); + assert.equal(_.keys(sent_messages.send_times_data).length, 1); assert(server_error_triggered); assert(!reload_initiate_triggered); assert(xhr_error_msg_checked); @@ -523,7 +524,7 @@ people.add(bob); server_events_triggered: 0, }; assert.deepEqual(stub_state, state); - assert.equal(_.keys(compose.send_times_data).length, 1); + assert.equal(_.keys(sent_messages.send_times_data).length, 1); assert(server_error_triggered); assert(!reload_initiate_triggered); assert(xhr_error_msg_checked); @@ -1249,8 +1250,9 @@ function test_with_mock_socket(test_params) { }()); (function test_message_id_changed_document() { + sent_messages.initialize(); var handler = $(document).get_on_handler('message_id_changed'); - compose.send_times_data = { + sent_messages.send_times_data = { 1031: { data: 'Test data!', }, @@ -1265,7 +1267,7 @@ function test_with_mock_socket(test_params) { data: 'Test data!', }, }; - assert.deepEqual(compose.send_times_data, send_times_data); + assert.deepEqual(sent_messages.send_times_data, send_times_data); }()); }()); diff --git a/static/js/compose.js b/static/js/compose.js index b9b2217c73..8d65c104d9 100644 --- a/static/js/compose.js +++ b/static/js/compose.js @@ -179,20 +179,6 @@ 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"); @@ -211,69 +197,6 @@ 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(); @@ -289,16 +212,13 @@ exports.send_message_success = function (local_id, message_id, start_time, local clear_compose_box(); } - process_send_time(message_id, start_time, locally_echoed); + sent_messages.process_success(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); + /* This next line is kind of suspect, since we are processing success here. */ + sent_messages.set_timer_for_restarting_event_loop(message_id); + }; exports.transmit_message = function (request, success, error) { @@ -309,7 +229,8 @@ exports.transmit_message = function (request, success, error) { // its lifecycle of being posted/received/acked/sent/displayed. request.client_message_id = request.local_id; - delete exports.send_times_data[request.id]; + sent_messages.clear(request.id); + if (page_params.use_websockets) { send_message_socket(request, success, error); } else { @@ -906,15 +827,6 @@ 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; diff --git a/static/js/echo.js b/static/js/echo.js index a60d80584c..c75bc03070 100644 --- a/static/js/echo.js +++ b/static/js/echo.js @@ -171,11 +171,11 @@ exports.process_from_server = function process_from_server(messages) { if (client_message.content !== message.content) { client_message.content = message.content; updated = true; - compose.mark_rendered_content_disparity(message.id, true); + sent_messages.mark_rendered_content_disparity(message.id, true); } msgs_to_rerender.push(client_message); locally_processed_ids.push(client_message.id); - compose.report_as_received(client_message); + sent_messages.report_as_received(client_message); delete waiting_for_ack[client_message.id]; return false; } diff --git a/static/js/message_util.js b/static/js/message_util.js index 16c74fc7af..a5afb654a0 100644 --- a/static/js/message_util.js +++ b/static/js/message_util.js @@ -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) { - compose.report_as_received(message); + sent_messages.report_as_received(message); } }); } diff --git a/static/js/sent_messages.js b/static/js/sent_messages.js new file mode 100644 index 0000000000..6baf97ece3 --- /dev/null +++ b/static/js/sent_messages.js @@ -0,0 +1,112 @@ +var sent_messages = (function () { + +var exports = {}; + +exports.send_times_log = []; +exports.send_times_data = {}; + +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, + }); +} + +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); + } +}; + +exports.process_success = function (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); +}; + +exports.set_timer_for_restarting_event_loop = function (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.clear = function (message_id) { + delete exports.send_times_data[message_id]; +}; + +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]; + 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; + +}()); +if (typeof module !== 'undefined') { + module.exports = sent_messages; +} diff --git a/static/js/ui_init.js b/static/js/ui_init.js index c4a9cd75c8..f60e9a355f 100644 --- a/static/js/ui_init.js +++ b/static/js/ui_init.js @@ -266,6 +266,7 @@ $(function () { pm_list.initialize(); stream_list.initialize(); drafts.initialize(); + sent_messages.initialize(); compose.initialize(); }); diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 45a52406ec..775567d621 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -47,6 +47,7 @@ enforce_fully_covered = { 'static/js/recent_senders.js', 'static/js/rtl.js', 'static/js/search_suggestion.js', + 'static/js/sent_messages.js', 'static/js/stream_events.js', 'static/js/stream_sort.js', 'static/js/topic_generator.js', diff --git a/zproject/settings.py b/zproject/settings.py index 95e6475a76..be1b763f7f 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -869,6 +869,7 @@ 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',