From c8b0f8fed40c183b3632915b0398c375ce1d2f88 Mon Sep 17 00:00:00 2001 From: Guilherme Goncalves Date: Wed, 24 Mar 2021 17:17:43 -0300 Subject: [PATCH] [fix] Dequeue messages when committing --- src/static/js/collab_client.js | 82 +++++----- src/tests/frontend/helper/multipleUsers.js | 182 +++++++++++++++++++++ src/tests/frontend/index.html | 1 + src/tests/frontend/specs/collab_client.js | 130 +++++++++++++++ 4 files changed, 358 insertions(+), 37 deletions(-) create mode 100644 src/tests/frontend/helper/multipleUsers.js create mode 100644 src/tests/frontend/specs/collab_client.js diff --git a/src/static/js/collab_client.js b/src/static/js/collab_client.js index a4cbafd92..3e1246d91 100644 --- a/src/static/js/collab_client.js +++ b/src/static/js/collab_client.js @@ -97,6 +97,10 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) setChannelState('DISCONNECTED', 'slowcommit'); } else if (state === 'COMMITTING' && msgQueue.length === 0 && (t - lastCommitTime) > 5000) { callbacks.onConnectionTrouble('SLOW'); + } else if (msgQueue.length > 0) { + // in slow or bad network conditions, there might be enqueued messages + // while the state is still COMMITTING + dequeueMessages(); } else { // run again in a few seconds, to detect a disconnect setTimeout(wrapRecordingErrors('setTimeout(handleUserChanges)', handleUserChanges), 3000); @@ -114,31 +118,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) // apply msgQueue changeset. if (msgQueue.length !== 0) { - let msg; - while ((msg = msgQueue.shift())) { - const newRev = msg.newRev; - rev = newRev; - if (msg.type === 'ACCEPT_COMMIT') { - editor.applyPreparedChangesetToBase(); - setStateIdle(); - callCatchingErrors('onInternalAction', () => { - callbacks.onInternalAction('commitAcceptedByServer'); - }); - callCatchingErrors('onConnectionTrouble', () => { - callbacks.onConnectionTrouble('OK'); - }); - handleUserChanges(); - } else if (msg.type === 'NEW_CHANGES') { - const changeset = msg.changeset; - const author = (msg.author || ''); - const apool = msg.apool; - - editor.applyChangesToBase(changeset, author, apool); - } - } - if (isPendingRevision) { - setIsPendingRevision(false); - } + dequeueMessages(); } let sentMessage = false; @@ -170,6 +150,42 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) } }; + const acceptCommit = () => { + editor.applyPreparedChangesetToBase(); + setStateIdle(); + callCatchingErrors('onInternalAction', () => { + callbacks.onInternalAction('commitAcceptedByServer'); + }); + callCatchingErrors('onConnectionTrouble', () => { + callbacks.onConnectionTrouble('OK'); + }); + handleUserChanges(); + }; + + const enqueueMessage = (msg) => { + msgQueue.push(msg); + }; + + const dequeueMessages = () => { + let msg; + while ((msg = msgQueue.shift())) { + const newRev = msg.newRev; + rev = newRev; + if (msg.type === 'ACCEPT_COMMIT') { + acceptCommit(); + } else if (msg.type === 'NEW_CHANGES') { + const changeset = msg.changeset; + const author = (msg.author || ''); + const apool = msg.apool; + + editor.applyChangesToBase(changeset, author, apool); + } + } + if (isPendingRevision) { + setIsPendingRevision(false); + } + }; + const setUpSocket = () => { setChannelState('CONNECTED'); doDeferredActions(); @@ -226,7 +242,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) // setChannelState("DISCONNECTED", "badmessage_newchanges"); return; } - msgQueue.push(msg); + enqueueMessage(msg); return; } @@ -247,7 +263,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) // setChannelState("DISCONNECTED", "badmessage_acceptcommit"); return; } - msgQueue.push(msg); + enqueueMessage(msg); return; } @@ -257,15 +273,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) return; } rev = newRev; - editor.applyPreparedChangesetToBase(); - setStateIdle(); - callCatchingErrors('onInternalAction', () => { - callbacks.onInternalAction('commitAcceptedByServer'); - }); - callCatchingErrors('onConnectionTrouble', () => { - callbacks.onConnectionTrouble('OK'); - }); - handleUserChanges(); + acceptCommit(); } else if (msg.type === 'CLIENT_RECONNECT') { // Server sends a CLIENT_RECONNECT message when there is a client reconnect. // Server also returns all pending revisions along with this CLIENT_RECONNECT message @@ -289,7 +297,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) return; } msg.type = 'NEW_CHANGES'; - msgQueue.push(msg); + enqueueMessage(msg); return; } diff --git a/src/tests/frontend/helper/multipleUsers.js b/src/tests/frontend/helper/multipleUsers.js new file mode 100644 index 000000000..644cfe15d --- /dev/null +++ b/src/tests/frontend/helper/multipleUsers.js @@ -0,0 +1,182 @@ +'use strict'; + +helper.multipleUsers = { + thisUser: null, + otherUser: null, +}; + +// open the same pad on the same frame (does not allow concurrent editions to pad) +helper.multipleUsers.loadSamePadAsAnotherUser = (done, tokenForOtherUser) => { + // change user + const token = tokenForOtherUser || _createTokenForAnotherUser(); + _removeExistingTokensFromCookie(); + _setTokenOnCookie(token); + + // reload pad + const padId = helper.padChrome$.window.clientVars.padId; + helper.newPad(done, padId); +}; + +// open the same pad on different frames (allows concurrent editions to pad) +helper.multipleUsers.openSamePadOnWithAnotherUser = (done) => { + const self = helper.multipleUsers; + + // do some cleanup, in case any of the tests failed on the previous run + const currentToken = _createTokenForCurrentUser(); + const otherToken = _createTokenForAnotherUser(); + _removeExistingTokensFromCookie(); + + self.thisUser = { + $frame: $('#iframe-container iframe'), + token: currentToken, + // we'll switch between pads, need to store current values of helper.pad* + // to be able to restore those values later + padChrome$: helper.padChrome$, + padOuter$: helper.padOuter$, + padInner$: helper.padInner$, + }; + + self.otherUser = { + token: otherToken, + }; + + // need to perform as the other user, otherwise we'll get the userdup error message + self.performAsOtherUser(_createFrameForOtherUser, done); +}; + +helper.multipleUsers.performAsOtherUser = (action, done) => { + const self = helper.multipleUsers; + + self.startActingLikeOtherUser(); + action(() => { + // go back to initial state when we're done + self.startActingLikeThisUser(); + done(); + }); +}; + +helper.multipleUsers.closePadForOtherUser = () => { + const self = helper.multipleUsers; + + self.thisUser.$frame.attr('style', ''); // make the default ocopy the full height + self.otherUser.$frame.remove(); +}; + +helper.multipleUsers.startActingLikeOtherUser = () => { + const self = helper.multipleUsers; + _startActingLike(self.otherUser); +}; + +helper.multipleUsers.startActingLikeThisUser = () => { + const self = helper.multipleUsers; + _startActingLike(self.thisUser); +}; + +// adapted form helper.js on Etherpad code +const _getFrameJQuery = (codesToLoad, $iframe) => { + const win = $iframe[0].contentWindow; + const doc = win.document; + + for (let i = 0; i < codesToLoad.length; i++) { + win.eval(codesToLoad[i]); + } + + win.$.window = win; + win.$.document = doc; + + return win.$; +}; + +const _loadJQueryCodeForOtherFrame = (done) => { + const self = helper.multipleUsers; + + $.get('/static/js/jquery.js').done((code) => { + // make sure we don't override existing jquery + const jQueryCode = `if(typeof $ === "undefined") {\n${code}\n}`; + $.get('/tests/frontend/lib/sendkeys.js').done((sendkeysCode) => { + const codesToLoad = [jQueryCode, sendkeysCode]; + + self.otherUser.padChrome$ = _getFrameJQuery(codesToLoad, self.otherUser.$frame); + self.otherUser.padOuter$ = _getFrameJQuery(codesToLoad, self.otherUser.padChrome$('iframe[name="ace_outer"]')); + self.otherUser.padInner$ = _getFrameJQuery(codesToLoad, self.otherUser.padOuter$('iframe[name="ace_inner"]')); + + // update helper vars now that they are available + helper.padChrome$ = self.otherUser.padChrome$; + helper.padOuter$ = self.otherUser.padOuter$; + helper.padInner$ = self.otherUser.padInner$; + + done(); + }); + }); +}; + +const _createFrameForOtherUser = (done) => { + const self = helper.multipleUsers; + + // create the iframe + const padUrl = self.thisUser.$frame.attr('src'); + self.otherUser.$frame = $(``); + + // place one iframe (visually) below the other + self.thisUser.$frame.attr('style', 'height: 50%'); + self.otherUser.$frame.attr('style', 'height: 50%; top: 50%'); + self.otherUser.$frame.insertAfter(self.thisUser.$frame); + + // wait for other pad to load + self.otherUser.$frame.one('load', () => { + const $editorLoadingMessage = self.otherUser.$frame.contents().find('#editorloadingbox'); + const $errorMessageModal = self.thisUser.$frame.contents().find('#connectivity .userdup'); + + helper.waitFor(() => { + const finishedLoadingOtherFrame = !$editorLoadingMessage.is(':visible'); + // make sure we don't get the userdup by mistake + const didNotDetectUserDup = !$errorMessageModal.is(':visible'); + + return finishedLoadingOtherFrame && didNotDetectUserDup; + }, 50000).done(() => { + // need to get values for self.otherUser.pad* vars + _loadJQueryCodeForOtherFrame(done); + }); + }); +}; + +const _getDocumentWithCookie = () => ( + helper.padChrome$ + ? helper.padChrome$.document + : helper.multipleUsers.thisUser.$frame.get(0).contentDocument +); + +const _setTokenOnCookie = (token) => { + _getDocumentWithCookie().cookie = `token=${token};secure`; +}; + +const _getTokenFromCookie = () => { + const fullCookie = _getDocumentWithCookie().cookie; + return fullCookie.replace(/.*token=([^;]*).*/, '$1').trim(); +}; + +const _createTokenForCurrentUser = () => ( + _getTokenFromCookie().replace(/-other_user.*/g, '') +); + +const _createTokenForAnotherUser = () => { + const currentToken = _createTokenForCurrentUser(); + return `${currentToken}-other_user${helper.randomString(4)}`; +}; + +const _startActingLike = (user) => { + // update helper references, so other methods will act as if the main frame + // was the one we're using from now on + helper.padChrome$ = user.padChrome$; + helper.padOuter$ = user.padOuter$; + helper.padInner$ = user.padInner$; + + _setTokenOnCookie(user.token); +}; + +const _removeExistingTokensFromCookie = () => { + // Expire cookie, to make sure it is removed by the browser. + // See https://developer.mozilla.org/en-US/docs/Web/API/Document/cookie#Example_4_Reset_the_previous_cookie + _getDocumentWithCookie().cookie = 'token=foo;expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/p'; + _getDocumentWithCookie().cookie = 'token=foo;expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/'; +}; diff --git a/src/tests/frontend/index.html b/src/tests/frontend/index.html index 3d806f626..e8a8012fb 100644 --- a/src/tests/frontend/index.html +++ b/src/tests/frontend/index.html @@ -20,6 +20,7 @@ + diff --git a/src/tests/frontend/specs/collab_client.js b/src/tests/frontend/specs/collab_client.js new file mode 100644 index 000000000..7d613eb21 --- /dev/null +++ b/src/tests/frontend/specs/collab_client.js @@ -0,0 +1,130 @@ +'use strict'; + +describe('Messages in the COLLABROOM', function () { + const newTextOfUser1 = 'text created by user 1'; + const newTextOfUser2 = 'text created by user 2'; + + before(function (done) { + helper.newPad(() => { + helper.multipleUsers.openSamePadOnWithAnotherUser(done); + }); + this.timeout(60000); + }); + + context('when user 1 is with slow or bad network conditions', function () { + before(function (done) { + helper.multipleUsers.startActingLikeThisUser(); + simulateSlowOrBadNetworkConditions(); + done(); + }); + + context('and user 1 is in international composition', function () { + before(function (done) { + replaceLineText(1, newTextOfUser1, () => { + simulateInternationalCompositionStartEvent(); + done(); + }); + }); + + context('and user 2 edits the text', function () { + before(function (done) { + this.timeout(4000); + helper.multipleUsers.startActingLikeOtherUser(); + replaceLineText(2, newTextOfUser2, () => { + helper.multipleUsers.startActingLikeThisUser(); + done(); + }); + }); + + context('and user 1 ends the international composition', function () { + before(function (done) { + simulateInternationalCompositionEndEvent(); + done(); + }); + + context('and both users add more editions', function () { + before(function (done) { + this.timeout(10000); + + helper.multipleUsers.startActingLikeOtherUser(); + replaceLineText(4, newTextOfUser2, () => { + helper.multipleUsers.startActingLikeThisUser(); + replaceLineText(3, newTextOfUser1, done); + }); + }); + + it('user 1 has all editions of user 2', function (done) { + this.timeout(5000); + helper.multipleUsers.startActingLikeThisUser(); + helper.waitFor(() => { + const inner$ = helper.padInner$; + const expectedLines = [2, 4]; + return expectedLines.every((line) => ( + inner$('div').eq(line).text() === newTextOfUser2) + ); + }, 4000).done(done); + }); + + it('user 2 has all editions of user 1', function (done) { + this.timeout(5000); + helper.multipleUsers.startActingLikeOtherUser(); + helper.waitFor(() => { + const inner$ = helper.padInner$; + const expectedLines = [1, 3]; + return expectedLines.every((line) => ( + inner$('div').eq(line).text() === newTextOfUser1) + ); + }, 4000).done(done); + }); + }); + }); + }); + }); + }); + + const triggerEvent = (eventName) => { + const event = new helper.padInner$.Event(eventName); + helper.padInner$('#innerdocbody').trigger(event); + }; + + const simulateInternationalCompositionStartEvent = () => { + triggerEvent('compositionstart'); + }; + + const simulateInternationalCompositionEndEvent = () => { + triggerEvent('compositionend'); + }; + + const simulateSlowOrBadNetworkConditions = () => { + // to simulate slow or bad network conditions (packet loss), we delay the + // sending of messages through the socket. + const originalFunction = helper.padChrome$.window.pad.socket.json.send; + const mockedFunction = function (...args) { + const context = this; + setTimeout(() => { + originalFunction.apply(context, args); + }, 4000); + }; + mockedFunction.bind(helper.padChrome$.window.pad.socket); + helper.padChrome$.window.pad.socket.json.send = mockedFunction; + }; + + const replaceLineText = (lineNumber, newText, done) => { + const inner$ = helper.padInner$; + + // get the line element + const $line = inner$('div').eq(lineNumber); + + // simulate key presses to delete content + $line.sendkeys('{selectall}'); // select all + $line.sendkeys('{del}'); // clear the first line + $line.sendkeys(newText); // insert the string + + helper.waitFor(() => ( + inner$('div').eq(lineNumber).text() === newText + ), 2000).done(() => { + // give some time to receive NEW_CHANGES message + setTimeout(done, 2000); + }); + }; +});