From 2768dcb187394825036809043032cf4c36b4733b Mon Sep 17 00:00:00 2001 From: Christopher Astfalk Date: Mon, 14 Dec 2020 14:08:22 +0100 Subject: [PATCH 1/4] test(circuit): update tests to use EventTarget and CustomEvent --- src/network/circuit.test.js | 145 ++++++++++++++++++------------------ 1 file changed, 73 insertions(+), 72 deletions(-) diff --git a/src/network/circuit.test.js b/src/network/circuit.test.js index 692624b..a250826 100644 --- a/src/network/circuit.test.js +++ b/src/network/circuit.test.js @@ -107,15 +107,9 @@ test('it should create an instance', () => { test('circuit closes', () => { circuit = new Circuit('127.0.0.1', 8080, 123456, 'session id') - const removeAllListeners = circuit.removeAllListeners - circuit.removeAllListeners = jest.fn(() => { - removeAllListeners.call(circuit) - }) - circuit.close() expect(clearInterval).toBeCalled() - expect(circuit.removeAllListeners).toBeCalled() expect(circuit.websocket.close).toBeCalled() expect(circuit.websocket.close).lastCalledWith(1000, 'session end') }) @@ -184,58 +178,26 @@ test('parse a received package', () => { const messageBuffer = createTestMessage(false, false, false) const handler = jest.fn() - circuit.on('packetReceived', handler) + circuit.addEventListener('packetReceived', handler) circuit.websocket.onmessage({ data: messageBuffer }) - expect(handler).toBeCalledWith({ - frequency: 'Low', - from: { - ip: '127.0.0.1', - port: 33 - }, - isOld: undefined, - isReliable: false, - isResend: false, - name: 'TestMessage', - type: 'udp/TestMessage', - number: 1, - size: 52, - trusted: false, - NeighborBlock: [ - { - Test0: 0, - Test1: 1, - Test2: 2 + expect(handler).toBeCalledWith(new window.CustomEvent('packetReceived', { + detail: { + frequency: 'Low', + from: { + ip: '127.0.0.1', + port: 33 }, - { - Test0: 3, - Test1: 4, - Test2: 5 - }, - { - Test0: 6, - Test1: 7, - Test2: 8 - }, - { - Test0: 9, - Test1: 10, - Test2: 11 - } - ], - TestBlock1: [ - { - Test1: 0 - } - ], - blocks: [ - [ - { - Test1: 0 - } - ], - [ + isOld: undefined, + isReliable: false, + isResend: false, + name: 'TestMessage', + type: 'udp/TestMessage', + number: 1, + size: 52, + trusted: false, + NeighborBlock: [ { Test0: 0, Test1: 1, @@ -256,11 +218,44 @@ test('parse a received package', () => { Test1: 10, Test2: 11 } + ], + TestBlock1: [ + { + Test1: 0 + } + ], + blocks: [ + [ + { + Test1: 0 + } + ], + [ + { + Test0: 0, + Test1: 1, + Test2: 2 + }, + { + Test0: 3, + Test1: 4, + Test2: 5 + }, + { + Test0: 6, + Test1: 7, + Test2: 8 + }, + { + Test0: 9, + Test1: 10, + Test2: 11 + } + ] ] - ] - }) + } + })) expect(circuit.senderSequenceNumber).toBe(0) - circuit.removeAllListeners() }) test('save sender sequence number of reliable packages as ack', () => { @@ -766,7 +761,7 @@ describe('disconnection', () => { setTimeout.mockReset() const closeEvent = jest.fn() - circuit.on('close', closeEvent) + circuit.addEventListener('close', closeEvent) circuit.websocket.onclose(new window.CloseEvent('Policy Violation', { wasClean: true, @@ -778,10 +773,12 @@ describe('disconnection', () => { expect(window.WebSocket).toBeCalledTimes(1) expect(closeEvent).toBeCalled() - expect(closeEvent).toHaveBeenLastCalledWith({ - code: 1008, - reason: 'wrong session id' - }) + expect(closeEvent).toHaveBeenLastCalledWith(new window.CustomEvent('close', { + detail: { + code: 1008, + reason: 'wrong session id' + } + })) }) test('it should exponentially increase the reconnect timeout', () => { @@ -817,7 +814,7 @@ describe('disconnection', () => { openSocket() const closeEvent = jest.fn() - circuit.on('close', closeEvent) + circuit.addEventListener('close', closeEvent) expect(window.WebSocket).toBeCalledTimes(1) jest.clearAllTimers() @@ -836,10 +833,12 @@ describe('disconnection', () => { expect(reconnectCounts).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 11]) expect(window.WebSocket).toHaveBeenCalledTimes(12) - expect(closeEvent).toHaveBeenCalledWith({ - code: 1006, - reason: 'Max reconnection tries' - }) + expect(closeEvent).toHaveBeenCalledWith(new window.CustomEvent('close', { + detail: { + code: 1006, + reason: 'Max reconnection tries' + } + })) }) // This is for developing and if there will be a direct UDP connection in the future @@ -850,13 +849,15 @@ describe('disconnection', () => { jest.runOnlyPendingTimers() const closeHandler = jest.fn() - circuit.on('close', closeHandler) + circuit.addEventListener('close', closeHandler) jest.runTimersToTime(ms.minutes(1) + ms.seconds(45) + 150) - expect(closeHandler).toHaveBeenCalledWith({ - code: 1006, - reason: 'UDP disconnect' - }) + expect(closeHandler).toHaveBeenCalledWith(new window.CustomEvent('close', { + detail: { + code: 1006, + reason: 'UDP disconnect' + } + })) }) }) From 6ed1e4479517635286d388d2481c269d4f583c1f Mon Sep 17 00:00:00 2001 From: Christopher Astfalk Date: Mon, 14 Dec 2020 14:09:01 +0100 Subject: [PATCH 2/4] refactor(circuit): use EventTarget and CustomEvent --- src/network/circuit.js | 44 ++++++++++++++++++++++++------------------ src/testUtils.ts | 4 +--- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/network/circuit.js b/src/network/circuit.js index 7ed7fa8..3f23d7c 100644 --- a/src/network/circuit.js +++ b/src/network/circuit.js @@ -6,13 +6,12 @@ * and the event 'packetReceived' */ -import events from 'events' import Queue from 'double-ended-queue' import { parseBody, createBody } from './networkMessages' import { getValueOf, mapBlockOf } from './msgGetters' -export default class Circuit extends events.EventEmitter { +export default class Circuit extends window.EventTarget { /** * sequenceNumber is the id of a packet. * It will be increased for every packed. @@ -121,21 +120,23 @@ export default class Circuit extends events.EventEmitter { } if (event.code === 1008) { - this.emit('close', { - code: event.code, - reason: event.reason - }) - this.removeAllListeners() + this.dispatchEvent(new window.CustomEvent('close', { + detail: { + code: event.code, + reason: event.reason + } + })) clearInterval(this.acksProcessInterval) return } if (this.reconnectCount > 10) { - this.emit('close', { - code: 1006, - reason: 'Max reconnection tries' - }) - this.removeAllListeners() + this.dispatchEvent(new window.CustomEvent('close', { + detail: { + code: 1006, + reason: 'Max reconnection tries' + } + })) clearInterval(this.acksProcessInterval) return } @@ -149,7 +150,6 @@ export default class Circuit extends events.EventEmitter { close () { this.websocket.close(1000, 'session end') - this.removeAllListeners() clearInterval(this.acksProcessInterval) } @@ -217,8 +217,12 @@ export default class Circuit extends events.EventEmitter { this._resolveAcks(mapBlockOf(parsedBody, 'Packets', getValue => getValue('ID'))) } else { // For every message that is not a circuit control message - this.emit(parsedBody.name, parsedBody) - this.emit('packetReceived', parsedBody) + this.dispatchEvent(new window.CustomEvent(parsedBody.name, { + detail: parsedBody + })) + this.dispatchEvent(new window.CustomEvent('packetReceived', { + detail: parsedBody + })) } } @@ -339,10 +343,12 @@ export default class Circuit extends events.EventEmitter { this.lastReceivedCount += 1 if (this.lastReceivedCount > 1050) { - this.emit('close', { - code: 1006, - reason: 'UDP disconnect' - }) + this.dispatchEvent(new window.CustomEvent('close', { + detail: { + code: 1006, + reason: 'UDP disconnect' + } + })) this.close() return } diff --git a/src/testUtils.ts b/src/testUtils.ts index acb9fa8..bd70ceb 100644 --- a/src/testUtils.ts +++ b/src/testUtils.ts @@ -1,5 +1,3 @@ -import { EventEmitter } from 'events' - import { diff as getObjDiff } from 'deep-object-diff' import PouchDB from 'pouchdb-browser' import memoryAdapter from 'pouchdb-adapter-memory' @@ -480,6 +478,6 @@ async function setStateToConnectedToGrid ( store.dispatch(connectCircuit()) } -export class CircuitMock extends EventEmitter { +export class CircuitMock extends window.EventTarget { send = jest.fn(() => Promise.resolve()) } From 368b906654e51ce1625841533f122b87fc6970ea Mon Sep 17 00:00:00 2001 From: Christopher Astfalk Date: Mon, 14 Dec 2020 20:25:55 +0100 Subject: [PATCH 3/4] refactor(connectCircuit): use EventTarget methods --- src/actions/connectCircuit.js | 72 +++++++++++++++----- src/actions/connectCircuit.test.js | 103 +++++++++++++++++++++++++++++ src/actions/simAction.js | 22 +++--- src/actions/viewerAccount.test.js | 4 +- src/network/circuit.js | 7 ++ 5 files changed, 177 insertions(+), 31 deletions(-) create mode 100644 src/actions/connectCircuit.test.js diff --git a/src/actions/connectCircuit.js b/src/actions/connectCircuit.js index d1cefe1..04d5d45 100644 --- a/src/actions/connectCircuit.js +++ b/src/actions/connectCircuit.js @@ -1,35 +1,65 @@ -import createCallback from './simAction' +import simAction from './simAction' import { userWasKicked } from '../bundles/session' // Starts listening to packets on the circuit and dispatch a parsed action. export default function init () { return (dispatch, getState, { circuit }) => { - let callback = createCallback(dispatch) - circuit.on('packetReceived', callback) - - let closeHandler = getCloseHandler(dispatch) - circuit.on('close', closeHandler) + let listeners = connect(dispatch, circuit) if (process.env.NODE_ENV !== 'production') { if (module.hot) { // Replace simAction-callback if it changes module.hot.accept('./simAction', () => { - circuit.removeListener('packetReceived', callback) - callback = createCallback(dispatch) - circuit.on('packetReceived', callback) + removeEventListeners(circuit, listeners) + listeners = connect(dispatch, circuit) }) module.hot.accept('../bundles/session', () => { - circuit.removeListener('close', closeHandler) - closeHandler = getCloseHandler(dispatch) - circuit.on('close', closeHandler) + removeEventListeners(circuit, listeners) + listeners = connect(dispatch, circuit) }) } } } } -function getCloseHandler (dispatch) { +/** + * Connects the event listeners to the Circuit. + * @param {function} dispatch Redux Store dispatch + * @param {EventTarget} circuit Circuit from network/circuit. + * @returns {{simAction: EventListener, closeHandler: EventListener}} Event listeners. + */ +function connect (dispatch, circuit) { + const listeners = { + simAction: event => { + dispatch(simAction(event)) + }, + closeHandler: null + } + listeners.closeHandler = closeHandler(dispatch, listeners) + + circuit.addEventListener('packetReceived', listeners.simAction) + circuit.addEventListener('close', listeners.closeHandler) + + return listeners +} + +/** + * Remove all event listeners from the circuit. + * @param {EventTarget} circuit Circuit from network/circuit. + * @param {{simAction: EventListener, closeHandler: EventListener}} listeners Event listeners. + */ +function removeEventListeners (circuit, listeners) { + circuit.removeEventListener('packetReceived', listeners.simAction) + circuit.removeEventListener('close', listeners.closeHandler) +} + +/** + * Listens to close event. If the there is a reason, dispatch it. + * @param {function} dispatch Redux Store dispatch + * @param {simAction: EventListener, closeHandler: EventListener} listeners Event listeners. + */ +function closeHandler (dispatch, listeners) { const disconnectMessage = 'You have been disconnected!\n\n' + 'Please check if you have an Internet connection.\n' + 'This problem could also be on our or the grids side.' @@ -40,9 +70,17 @@ function getCloseHandler (dispatch) { } return event => { - const reason = event.reason in reasonTexts - ? reasonTexts[event.reason] - : event.reason - dispatch(userWasKicked({ reason })) + const reason = event.detail.reason in reasonTexts + ? reasonTexts[event.detail.reason] + : event.detail.reason + + dispatch((dispatch, getState, { circuit }) => { + removeEventListeners(circuit, listeners) + + if (event.detail.code !== 1000) { + // not normal circuit close + dispatch(userWasKicked({ reason })) + } + }) } } diff --git a/src/actions/connectCircuit.test.js b/src/actions/connectCircuit.test.js new file mode 100644 index 0000000..7532899 --- /dev/null +++ b/src/actions/connectCircuit.test.js @@ -0,0 +1,103 @@ +import connectCircuit from './connectCircuit' + +it('should listen to packages on the Circuit in the thunk extra argument', () => { + const innerFn = connectCircuit() + const dispatch = jest.fn() + const getState = jest.fn(() => ({})) + const circuitAddEventListener = jest.fn() + + innerFn(dispatch, getState, { + circuit: { + addEventListener: circuitAddEventListener + } + }) + + expect(dispatch).not.toBeCalled() + expect(getState).not.toBeCalled() + expect(circuitAddEventListener).toBeCalledTimes(2) + expect(circuitAddEventListener) + .toHaveBeenNthCalledWith(1, 'packetReceived', expect.any(Function)) + expect(circuitAddEventListener).toHaveBeenNthCalledWith(2, 'close', expect.any(Function)) +}) + +it('should close all event-listeners on the close event', () => { + const innerFn = connectCircuit() + const dispatch = jest.fn() + const getState = jest.fn(() => ({})) + const eventListeners = new Map() + const circuitRemoveEventListener = jest.fn() + const circuit = { + addEventListener (eventName, listener) { + eventListeners.set(eventName, listener) + }, + removeEventListener: circuitRemoveEventListener + } + + innerFn(dispatch, getState, { circuit }) + + const closeListener = eventListeners.get('close') + expect(typeof closeListener).toBe('function') + + closeListener(new window.CustomEvent('close', { + detail: { + code: 1006, + reason: 'Max reconnection tries' + } + })) + + expect(dispatch).toHaveBeenLastCalledWith(expect.any(Function)) + const cb = dispatch.mock.calls[0][0] + cb(dispatch, getState, { circuit }) + + expect(dispatch).toHaveBeenLastCalledWith({ + type: 'session/userWasKicked', + payload: { + reason: 'You have been disconnected!\n\n' + + 'Please check if you have an Internet connection.\n' + + 'This problem could also be on our or the grids side.' + } + }) + expect(getState).not.toHaveBeenCalled() + expect(circuitRemoveEventListener) + .toHaveBeenNthCalledWith(1, 'packetReceived', eventListeners.get('packetReceived')) + expect(circuitRemoveEventListener) + .toHaveBeenNthCalledWith(2, 'close', eventListeners.get('close')) +}) + +it('should not dispatch an kick event if the circuit did close with 1000', () => { + const innerFn = connectCircuit() + const dispatch = jest.fn() + const getState = jest.fn(() => ({})) + const eventListeners = new Map() + const circuitRemoveEventListener = jest.fn() + const circuit = { + addEventListener (eventName, listener) { + eventListeners.set(eventName, listener) + }, + removeEventListener: circuitRemoveEventListener + } + + innerFn(dispatch, getState, { circuit }) + + const closeListener = eventListeners.get('close') + expect(typeof closeListener).toBe('function') + + closeListener(new window.CustomEvent('close', { + detail: { + code: 1000, + reason: 'session end' + } + })) + + expect(dispatch).toBeCalledTimes(1) + expect(dispatch).toHaveBeenLastCalledWith(expect.any(Function)) + const cb = dispatch.mock.calls[0][0] + cb(dispatch, getState, { circuit }) + + expect(dispatch).toBeCalledTimes(1) + expect(getState).not.toHaveBeenCalled() + expect(circuitRemoveEventListener) + .toHaveBeenNthCalledWith(1, 'packetReceived', eventListeners.get('packetReceived')) + expect(circuitRemoveEventListener) + .toHaveBeenNthCalledWith(2, 'close', eventListeners.get('close')) +}) diff --git a/src/actions/simAction.js b/src/actions/simAction.js index b3b0723..a972be2 100644 --- a/src/actions/simAction.js +++ b/src/actions/simAction.js @@ -5,8 +5,13 @@ import { changeRights } from '../bundles/friends' import { selectAgentId, selectSessionId } from '../bundles/session' import { doHandleFriendOnlineStateChange } from './friendsActions' -// Gets all messages from the SIM and filters them, and if needed: calls their own actions. -function simActionFilter (msg) { +/** + * Gets all messages from the SIM and filters them, and if needed: calls their own actions. + * @param {CustomEvent} event Event from circuit with the Package data in `detail`. + */ +export default function simActionFilter (event) { + const msg = event.detail + switch (msg.name) { case 'ChatFromSimulator': return receiveChatFromSimulator(msg) @@ -37,7 +42,9 @@ function simActionFilter (msg) { if (process.env.NODE_ENV !== 'production' && window.debugDispatchAllMsg) { return msg } - break + // Don't dispatch an action. + // This is an empty thunk action. It will be called and does then nothing. + return () => {} } } @@ -81,12 +88,3 @@ function sendRegionHandshakeReply (RegionHandshake) { }) } } - -export default function createCallback (dispatch) { - return msg => { - const action = simActionFilter(msg) - if (action != null) { // If the packet is parsed, an action will be dispatched. - dispatch(action) - } - } -} diff --git a/src/actions/viewerAccount.test.js b/src/actions/viewerAccount.test.js index 2ae1fec..8f5068e 100644 --- a/src/actions/viewerAccount.test.js +++ b/src/actions/viewerAccount.test.js @@ -407,7 +407,7 @@ describe('grids', () => { isLLSDLogin: false }) - await new Promise(resolve => setTimeout(resolve, 5)) + await new Promise(resolve => setTimeout(resolve, 10)) expect(getDiff('A')).toEqual({ account: { @@ -455,7 +455,7 @@ describe('grids', () => { isLLSDLogin: false }) - await new Promise(resolve => setTimeout(resolve, 5)) + await new Promise(resolve => setTimeout(resolve, 10)) expect(getDiff('A')).toEqual({ account: { diff --git a/src/network/circuit.js b/src/network/circuit.js index 3f23d7c..589a13b 100644 --- a/src/network/circuit.js +++ b/src/network/circuit.js @@ -116,6 +116,13 @@ export default class Circuit extends window.EventTarget { if (event.code === 1000) { // Normal session end + this.dispatchEvent(new window.CustomEvent('close', { + detail: { + code: event.code, + reason: event.reason + } + })) + clearInterval(this.acksProcessInterval) return } From 9f90ff4b13a2078d5f00e4223995f23541fc7c51 Mon Sep 17 00:00:00 2001 From: Christopher Astfalk Date: Mon, 14 Dec 2020 21:09:22 +0100 Subject: [PATCH 4/4] fix(sessionActions): use new circuit methods --- src/actions/connectCircuit.js | 17 ++++++++--------- src/actions/connectCircuit.test.js | 11 +---------- src/actions/sessionActions.js | 15 +++++++++------ 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/actions/connectCircuit.js b/src/actions/connectCircuit.js index 04d5d45..75e2b83 100644 --- a/src/actions/connectCircuit.js +++ b/src/actions/connectCircuit.js @@ -36,7 +36,7 @@ function connect (dispatch, circuit) { }, closeHandler: null } - listeners.closeHandler = closeHandler(dispatch, listeners) + listeners.closeHandler = closeHandler(dispatch, circuit, listeners) circuit.addEventListener('packetReceived', listeners.simAction) circuit.addEventListener('close', listeners.closeHandler) @@ -57,9 +57,10 @@ function removeEventListeners (circuit, listeners) { /** * Listens to close event. If the there is a reason, dispatch it. * @param {function} dispatch Redux Store dispatch + * @param {EventTarget} circuit Circuit from network/circuit. * @param {simAction: EventListener, closeHandler: EventListener} listeners Event listeners. */ -function closeHandler (dispatch, listeners) { +function closeHandler (dispatch, circuit, listeners) { const disconnectMessage = 'You have been disconnected!\n\n' + 'Please check if you have an Internet connection.\n' + 'This problem could also be on our or the grids side.' @@ -74,13 +75,11 @@ function closeHandler (dispatch, listeners) { ? reasonTexts[event.detail.reason] : event.detail.reason - dispatch((dispatch, getState, { circuit }) => { - removeEventListeners(circuit, listeners) + removeEventListeners(circuit, listeners) - if (event.detail.code !== 1000) { - // not normal circuit close - dispatch(userWasKicked({ reason })) - } - }) + if (event.detail.code !== 1000) { + // not normal circuit close + dispatch(userWasKicked({ reason })) + } } } diff --git a/src/actions/connectCircuit.test.js b/src/actions/connectCircuit.test.js index 7532899..8e16489 100644 --- a/src/actions/connectCircuit.test.js +++ b/src/actions/connectCircuit.test.js @@ -45,10 +45,6 @@ it('should close all event-listeners on the close event', () => { } })) - expect(dispatch).toHaveBeenLastCalledWith(expect.any(Function)) - const cb = dispatch.mock.calls[0][0] - cb(dispatch, getState, { circuit }) - expect(dispatch).toHaveBeenLastCalledWith({ type: 'session/userWasKicked', payload: { @@ -89,12 +85,7 @@ it('should not dispatch an kick event if the circuit did close with 1000', () => } })) - expect(dispatch).toBeCalledTimes(1) - expect(dispatch).toHaveBeenLastCalledWith(expect.any(Function)) - const cb = dispatch.mock.calls[0][0] - cb(dispatch, getState, { circuit }) - - expect(dispatch).toBeCalledTimes(1) + expect(dispatch).toBeCalledTimes(0) expect(getState).not.toHaveBeenCalled() expect(circuitRemoveEventListener) .toHaveBeenNthCalledWith(1, 'packetReceived', eventListeners.get('packetReceived')) diff --git a/src/actions/sessionActions.js b/src/actions/sessionActions.js index 1ae602f..89a2803 100644 --- a/src/actions/sessionActions.js +++ b/src/actions/sessionActions.js @@ -244,10 +244,14 @@ export function logout () { dispatch(startLogout()) let isLoggedOut = false - const logoutHandler = msg => { + const logoutHandler = () => { if (isLoggedOut) return - isLoggedOut = true + + setTimeout(() => { + circuit.removeEventListener('packetReceived', console.log) + circuit.removeEventListener('LogoutReply', logoutHandler) + }, 0) dispatch(afterAvatarSessionEnds()) dispatch(didLogout()) @@ -255,8 +259,8 @@ export function logout () { resolve() } - circuit.on('packetReceived', console.log) - circuit.once('LogoutReply', logoutHandler) + circuit.addEventListener('packetReceived', console.log) + circuit.addEventListener('LogoutReply', logoutHandler, { once: true }) setTimeout(logoutHandler, ms.seconds(30)) // timeout for LogoutReply }) } @@ -281,7 +285,7 @@ function connectToSim (sessionInfo, circuit) { dispatch(connectCircuit()) // Connect message parsing with circuit. - activeCircuit.on('KickUser', msg => dispatch(getKicked(msg))) + activeCircuit.addEventListener('KickUser', msg => dispatch(getKicked(msg.detail))) await activeCircuit.send('UseCircuitCode', { CircuitCode: [ @@ -369,7 +373,6 @@ function getKicked (msg) { function afterAvatarSessionEnds () { return (dispatch, getState, extra) => { extra.circuit.close() - extra.circuit.removeAllListeners() extra.circuit = null for (const cb of extra.onAvatarLogout || []) {