From b94995120a4c9cfb72eaa8ef43c5d75b4faf4116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20M=C3=A9taireau?= Date: Mon, 10 Jun 2024 19:29:56 +0200 Subject: [PATCH] refactoring(sync): Simplify the design of the sync engine. Removes the concept of message dispatcher and message sender, all of it can be done by the syncengine class, making it easier to grasp. --- umap/static/umap/js/modules/sync/engine.js | 113 +++++++----------- umap/static/umap/js/modules/sync/updaters.js | 2 +- umap/static/umap/js/modules/sync/websocket.js | 2 +- umap/static/umap/unittests/sync.js | 58 +++++---- 4 files changed, 72 insertions(+), 103 deletions(-) diff --git a/umap/static/umap/js/modules/sync/engine.js b/umap/static/umap/js/modules/sync/engine.js index 186615b6..2af5dfb6 100644 --- a/umap/static/umap/js/modules/sync/engine.js +++ b/umap/static/umap/js/modules/sync/engine.js @@ -3,15 +3,12 @@ import { MapUpdater, DataLayerUpdater, FeatureUpdater } from './updaters.js' export class SyncEngine { constructor(map) { - this.map = map - this.dispatcher = new MessagesDispatcher(this.map) - this._initialize() - } - _initialize() { + this.updaters = { + map: new MapUpdater(map), + feature: new FeatureUpdater(map), + datalayer: new DataLayerUpdater(map), + } this.transport = undefined - const noop = () => {} - // by default, all operations do nothing, until the engine is started. - this.upsert = this.update = this.delete = noop } async authenticate(tokenURI, webSocketURI, server) { @@ -22,16 +19,47 @@ export class SyncEngine { } start(webSocketURI, authToken) { - this.transport = new WebSocketTransport(webSocketURI, authToken, this.dispatcher) - this.sender = new MessagesSender(this.transport) - this.upsert = this.sender.upsert.bind(this.sender) - this.update = this.sender.update.bind(this.sender) - this.delete = this.sender.delete.bind(this.sender) + this.transport = new WebSocketTransport(webSocketURI, authToken, this) } stop() { if (this.transport) this.transport.close() - this._initialize() + this.transport = undefined + } + + _getUpdater(subject, metadata) { + if (Object.keys(this.updaters).includes(subject)) { + return this.updaters[subject] + } + throw new Error(`Unknown updater ${subject}, ${metadata}`) + } + + // This method is called by the transport layer on new messages + receive({ kind, ...payload }) { + if (kind == 'operation') { + let updater = this._getUpdater(payload.subject, payload.metadata) + updater.applyMessage(payload) + } else { + throw new Error(`Unknown dispatch kind: ${kind}`) + } + } + + _send(message) { + if (this.transport) { + this.transport.send('operation', message) + } + } + + upsert(subject, metadata, value) { + this._send({ verb: 'upsert', subject, metadata, value }) + } + + update(subject, metadata, key, value) { + this._send({ verb: 'update', subject, metadata, key, value }) + } + + delete(subject, metadata, key) { + this._send({ verb: 'delete', subject, metadata, key }) } /** @@ -63,60 +91,3 @@ export class SyncEngine { return new Proxy(this, handler) } } - -export class MessagesDispatcher { - constructor(map) { - this.map = map - this.updaters = { - map: new MapUpdater(this.map), - feature: new FeatureUpdater(this.map), - datalayer: new DataLayerUpdater(this.map), - } - } - - getUpdater(subject, metadata) { - if (Object.keys(this.updaters).includes(subject)) { - return this.updaters[subject] - } - throw new Error(`Unknown updater ${subject}, ${metadata}`) - } - - dispatch({ kind, ...payload }) { - if (kind == 'operation') { - let updater = this.getUpdater(payload.subject, payload.metadata) - updater.applyMessage(payload) - } else { - throw new Error(`Unknown dispatch kind: ${kind}`) - } - } -} - -/** - * Send messages to other connected peers, using the specified transport. - * - * - `subject` is the type of object this is referering to (map, feature, layer) - * - `metadata` contains information about the object we're refering to (id, layerId for instance) - * - `key` and - * - `value` are the keys and values that are being modified. - */ -export class MessagesSender { - constructor(transport) { - this._transport = transport - } - - send(message) { - this._transport.send('operation', message) - } - - upsert(subject, metadata, value) { - this.send({ verb: 'upsert', subject, metadata, value }) - } - - update(subject, metadata, key, value) { - this.send({ verb: 'update', subject, metadata, key, value }) - } - - delete(subject, metadata, key) { - this.send({ verb: 'delete', subject, metadata, key }) - } -} diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js index 9d83e001..a64e351d 100644 --- a/umap/static/umap/js/modules/sync/updaters.js +++ b/umap/static/umap/js/modules/sync/updaters.js @@ -15,7 +15,7 @@ class BaseUpdater { // Reduce the current list of attributes, // to find the object to set the property onto const objectToSet = parts.reduce((currentObj, part) => { - if (part in currentObj) return currentObj[part] + if (currentObj !== undefined && part in currentObj) return currentObj[part] }, obj) // In case the given path doesn't exist, stop here diff --git a/umap/static/umap/js/modules/sync/websocket.js b/umap/static/umap/js/modules/sync/websocket.js index e575bed6..45854dc1 100644 --- a/umap/static/umap/js/modules/sync/websocket.js +++ b/umap/static/umap/js/modules/sync/websocket.js @@ -9,7 +9,7 @@ export class WebSocketTransport { } onMessage(wsMessage) { - this.receiver.dispatch(JSON.parse(wsMessage.data)) + this.receiver.receive(JSON.parse(wsMessage.data)) } send(kind, payload) { diff --git a/umap/static/umap/unittests/sync.js b/umap/static/umap/unittests/sync.js index a7eb464b..e20a5153 100644 --- a/umap/static/umap/unittests/sync.js +++ b/umap/static/umap/unittests/sync.js @@ -5,7 +5,7 @@ import pkg from 'chai' const { expect } = pkg import { MapUpdater } from '../js/modules/sync/updaters.js' -import { MessagesDispatcher, SyncEngine } from '../js/modules/sync/engine.js' +import { SyncEngine } from '../js/modules/sync/engine.js' describe('SyncEngine', () => { it('should initialize methods even before start', function () { @@ -16,35 +16,33 @@ describe('SyncEngine', () => { }) }) -describe('MessageDispatcher', () => { - describe('#dispatch', function () { - it('should raise an error on unknown updater', function () { - const dispatcher = new MessagesDispatcher({}) - expect(() => { - dispatcher.dispatch({ - kind: 'operation', - subject: 'unknown', - metadata: {}, - }) - }).to.throw(Error) - }) - it('should produce an error on malformated messages', function () { - const dispatcher = new MessagesDispatcher({}) - expect(() => { - dispatcher.dispatch({ - yeah: 'yeah', - payload: { foo: 'bar' }, - }) - }).to.throw(Error) - }) - it('should raise an unknown operations', function () { - const dispatcher = new MessagesDispatcher({}) - expect(() => { - dispatcher.dispatch({ - kind: 'something-else', - }) - }).to.throw(Error) - }) +describe('#dispatch', function () { + it('should raise an error on unknown updater', function () { + const dispatcher = new SyncEngine({}) + expect(() => { + dispatcher.dispatch({ + kind: 'operation', + subject: 'unknown', + metadata: {}, + }) + }).to.throw(Error) + }) + it('should produce an error on malformated messages', function () { + const dispatcher = new SyncEngine({}) + expect(() => { + dispatcher.dispatch({ + yeah: 'yeah', + payload: { foo: 'bar' }, + }) + }).to.throw(Error) + }) + it('should raise an unknown operations', function () { + const dispatcher = new SyncEngine({}) + expect(() => { + dispatcher.dispatch({ + kind: 'something-else', + }) + }).to.throw(Error) }) })