From dfdfae00801d1e2e84622825e67f34939d2750fc Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 20 Mar 2025 16:51:30 +0100 Subject: [PATCH] wip: derive the dirty status from the undoManager This should pave the way for removing the SaveManager. Co-authored-by: David Larlet --- umap/static/umap/css/bar.css | 12 +-- umap/static/umap/js/modules/saving.js | 2 +- umap/static/umap/js/modules/sync/engine.js | 95 ++++++++++++-------- umap/static/umap/js/modules/sync/undo.js | 70 +++++++++------ umap/static/umap/js/modules/sync/updaters.js | 15 +++- umap/static/umap/js/modules/ui/bar.js | 4 +- umap/static/umap/js/modules/umap.js | 5 +- 7 files changed, 123 insertions(+), 80 deletions(-) diff --git a/umap/static/umap/css/bar.css b/umap/static/umap/css/bar.css index 07356939..0f2a99ce 100644 --- a/umap/static/umap/css/bar.css +++ b/umap/static/umap/css/bar.css @@ -19,7 +19,7 @@ .leaflet-container .edit-disable, .leaflet-container .connected-peers { - display: block; + display: inline-block; height: 32px; line-height: 30px; padding: 0 20px; @@ -78,21 +78,13 @@ background: rgba(66, 236, 230, 0.10); } .leaflet-container .edit-save, -.leaflet-container .edit-undo, -.leaflet-container .edit-redo, -.leaflet-container .edit-disable, .umap-edit-enabled .edit-enable { display: none; } .umap-edit-enabled .edit-save, -.umap-edit-enabled .edit-disable, -.umap-edit-enabled.umap-is-dirty .edit-undo, -.umap-edit-enabled.umap-is-dirty .edit-redo { +.umap-edit-enabled .edit-disable { display: inline-block; } -.umap-is-dirty .edit-disable { - display: none; -} .umap-caption-bar { display: none; } diff --git a/umap/static/umap/js/modules/saving.js b/umap/static/umap/js/modules/saving.js index 501fe19a..d5181a37 100644 --- a/umap/static/umap/js/modules/saving.js +++ b/umap/static/umap/js/modules/saving.js @@ -31,7 +31,7 @@ function has(obj) { function onUpdate() { isDirty = Boolean(_queue.size) - document.body.classList.toggle('umap-is-dirty', isDirty) + // document.body.classList.toggle('umap-is-dirty', isDirty) } export class ServerStored { diff --git a/umap/static/umap/js/modules/sync/engine.js b/umap/static/umap/js/modules/sync/engine.js index bea881a0..8c1c2501 100644 --- a/umap/static/umap/js/modules/sync/engine.js +++ b/umap/static/umap/js/modules/sync/engine.js @@ -138,69 +138,90 @@ export class SyncEngine { verb: 'batch', operations: this._batch, }) - const syncOperations = this._batch.map((operation) => - this.convertToSyncOperation(operation) - ) - this._send({ verb: 'batch', operations: syncOperations, subject: 'batch' }) + const operations = this._batch.map((stage) => stage.operation) + this._send({ verb: 'batch', operations: operations, subject: 'batch' }) this._batch = null } - convertToSyncOperation(undoOperation) { - const syncOperation = { ...undoOperation, value: undoOperation.newValue } - delete syncOperation.oldValue - delete syncOperation.newValue - return syncOperation - } - upsert(subject, metadata, value, oldValue) { - const undoOperation = { + const operation = { verb: 'upsert', subject, metadata, + value, + } + const stage = { + operation, newValue: value, oldValue: oldValue, } if (this._batch) { - this._batch.push(undoOperation) + this._batch.push(stage) return } - this._undoManager.add(undoOperation) - const syncOperation = this.convertToSyncOperation(undoOperation) - this._send(syncOperation) + this._undoManager.add(stage) + this._send(operation) } update(subject, metadata, key, value, oldValue) { - const undoOperation = { + const operation = { verb: 'update', subject, metadata, key, + value, + } + const stage = { + operation, oldValue: oldValue, newValue: value, } if (this._batch) { - this._batch.push(undoOperation) + this._batch.push(stage) return } - this._undoManager.add(undoOperation) - const syncOperation = this.convertToSyncOperation(undoOperation) - this._send(syncOperation) + this._undoManager.add(stage) + this._send(operation) } delete(subject, metadata, oldValue) { - const undoOperation = { + const operation = { verb: 'delete', subject, metadata, + } + const stage = { + operation, oldValue: oldValue, } if (this._batch) { - this._batch.push(undoOperation) + this._batch.push(stage) return } - this._undoManager.add(undoOperation) - const syncOperation = this.convertToSyncOperation(undoOperation) - this._send(syncOperation) + this._undoManager.add(stage) + this._send(operation) + } + + async save() { + const needSave = new Map() + for (const operation of this._operations.sorted()) { + if (operation.dirty) { + const updater = this._getUpdater(operation.subject) + const obj = updater.getStoredObject(operation.metadata) + if (!needSave.has(obj)) { + needSave.set(obj, []) + } + needSave.get(obj).push(operation) + } + } + for (const [obj, operations] of needSave.entries()) { + await obj.save() + for (const operation of operations) { + operation.dirty = false + } + } + this.saved() + this._undoManager.toggleState() } saved() { @@ -213,8 +234,8 @@ export class SyncEngine { } } - _send(inputMessage) { - const message = this._operations.addLocal(inputMessage) + _send(operation) { + const message = this._operations.addLocal(operation) if (this.offline) return if (this.transport) { @@ -377,9 +398,7 @@ export class SyncEngine { onSavedMessage({ sender, lastKnownHLC }) { debug(`received saved message from peer ${sender}`, lastKnownHLC) - if (lastKnownHLC === this._operations.getLastKnownHLC() && SaveManager.isDirty) { - SaveManager.clear() - } + this._operations.saved(lastKnownHLC) } /** @@ -451,16 +470,22 @@ export class Operations { this._operations = new Array() } + saved(hlc) { + for (const operation of this.getOperationsSince(hlc)) { + operation.dirty = false + } + } + /** * Tick the clock and store the passed message in the operations list. * * @param {*} inputMessage * @returns {*} clock-aware message */ - addLocal(inputMessage) { - const message = { ...inputMessage, hlc: this._hlc.tick() } - this._operations.push(message) - return message + addLocal(operation) { + operation.hlc = this._hlc.tick() + this._operations.push(operation) + return operation } /** diff --git a/umap/static/umap/js/modules/sync/undo.js b/umap/static/umap/js/modules/sync/undo.js index f973febe..a9e7af2e 100644 --- a/umap/static/umap/js/modules/sync/undo.js +++ b/umap/static/umap/js/modules/sync/undo.js @@ -14,35 +14,53 @@ export class UndoManager { const redoButton = document.querySelector('.edit-redo') if (undoButton) undoButton.disabled = !this._undoStack.length if (redoButton) redoButton.disabled = !this._redoStack.length + const dirty = this.isDirty() + document.body.classList.toggle('umap-is-dirty', dirty) + for (const button of document.querySelectorAll('.disabled-on-dirty')) { + button.disabled = dirty + } } - add(operation) { - this._redoStack = [] - this._undoStack.push(operation) - this.toggleState() + isDirty() { + for (const stage of this._undoStack) { + if (stage.operation.dirty) return true + } + for (const stage of this._redoStack) { + if (stage.operation.dirty) return true + } + return false } - cleanOperation(operation, redo) { - const syncOperation = Utils.CopyJSON(operation) - delete syncOperation.oldValue - delete syncOperation.newValue - syncOperation.value = redo ? operation.newValue : operation.oldValue - return syncOperation + add(stage) { + // FIXME make it more generic + if (stage.operation.key !== '_referenceVersion') { + stage.operation.dirty = true + this._redoStack = [] + this._undoStack.push(stage) + this.toggleState() + } + } + + copyOperation(stage, redo) { + const operation = Utils.CopyJSON(stage.operation) + operation.value = redo ? stage.newValue : stage.oldValue + return operation } undo(redo = false) { const fromStack = redo ? this._redoStack : this._undoStack const toStack = redo ? this._undoStack : this._redoStack - const operation = fromStack.pop() - if (!operation) return - if (operation.verb === 'batch') { - for (const op of operation.operations) { - this.applyOperation(this.cleanOperation(op, redo)) + const stage = fromStack.pop() + if (!stage) return + stage.operation.dirty = !stage.operation.dirty + if (stage.operation.verb === 'batch') { + for (const op of stage.operations) { + this.applyOperation(this.copyOperation(op, redo)) } } else { - this.applyOperation(this.cleanOperation(operation, redo)) + this.applyOperation(this.copyOperation(stage, redo)) } - toStack.push(operation) + toStack.push(stage) this.toggleState() } @@ -50,21 +68,21 @@ export class UndoManager { this.undo(true) } - applyOperation(syncOperation) { - const updater = this._getUpdater(syncOperation.subject, syncOperation.metadata) - switch (syncOperation.verb) { + applyOperation(operation) { + const updater = this._getUpdater(operation.subject, operation.metadata) + switch (operation.verb) { case 'update': - updater.update(syncOperation) - this._syncEngine._send(syncOperation) + updater.update(operation) + this._syncEngine._send(operation) break case 'delete': case 'upsert': - if (syncOperation.value === null || syncOperation.value === undefined) { - updater.delete(syncOperation) + if (operation.value === null || operation.value === undefined) { + updater.delete(operation) } else { - updater.upsert(syncOperation) + updater.upsert(operation) } - this._syncEngine._send(syncOperation) + this._syncEngine._send(operation) break } } diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js index 0ea66d2e..082901bf 100644 --- a/umap/static/umap/js/modules/sync/updaters.js +++ b/umap/static/umap/js/modules/sync/updaters.js @@ -51,6 +51,10 @@ export class MapUpdater extends BaseUpdater { this._umap.onPropertiesUpdated([key]) this._umap.render([key]) } + + getStoredObject() { + return this._umap + } } export class DataLayerUpdater extends BaseUpdater { @@ -91,6 +95,10 @@ export class DataLayerUpdater extends BaseUpdater { datalayer.commitDelete() } } + + getStoredObject(metadata) { + return this.getDataLayerFromID(metadata.id) + } } export class FeatureUpdater extends BaseUpdater { @@ -101,14 +109,11 @@ export class FeatureUpdater extends BaseUpdater { // Create or update an object at a specific position upsert({ metadata, value }) { - console.log('updater.upsert for', metadata, value) const { id, layerId } = metadata const datalayer = this.getDataLayerFromID(layerId) const feature = this.getFeatureFromMetadata(metadata) - console.log('feature', feature) if (feature) { - console.log('changing feature geometry') feature.geometry = value.geometry } else { datalayer.makeFeature(value, false) @@ -139,4 +144,8 @@ export class FeatureUpdater extends BaseUpdater { const feature = this.getFeatureFromMetadata(metadata) if (feature) feature.del(false) } + + getStoredObject(metadata) { + return this.getDataLayerFromID(metadata.layerId) + } } diff --git a/umap/static/umap/js/modules/ui/bar.js b/umap/static/umap/js/modules/ui/bar.js index 484197fc..28eb3677 100644 --- a/umap/static/umap/js/modules/ui/bar.js +++ b/umap/static/umap/js/modules/ui/bar.js @@ -30,7 +30,7 @@ const TOP_BAR_TEMPLATE = ` ${translate('Redo')} - @@ -159,7 +159,7 @@ export class TopBar extends WithTemplate { redraw() { const syncEnabled = this._umap.getProperty('syncEnabled') this.elements.peers.hidden = !syncEnabled - // this.elements.cancel.hidden = syncEnabled + this.elements.view.disabled = this._umap.sync._undoManager.isDirty() this.elements.saveLabel.hidden = this._umap.permissions.isDraft() this.elements.saveDraftLabel.hidden = !this._umap.permissions.isDraft() } diff --git a/umap/static/umap/js/modules/umap.js b/umap/static/umap/js/modules/umap.js index a52e793a..6e9ca6d8 100644 --- a/umap/static/umap/js/modules/umap.js +++ b/umap/static/umap/js/modules/umap.js @@ -670,10 +670,10 @@ export default class Umap extends ServerStored { } async saveAll() { - if (!SAVEMANAGER.isDirty) return + // if (!SAVEMANAGER.isDirty) return if (this._defaultExtent) this._setCenterAndZoom() this.backup() - await SAVEMANAGER.save() + await this.sync.save() // Do a blind render for now, as we are not sure what could // have changed, we'll be more subtil when we'll remove the // save action @@ -684,7 +684,6 @@ export default class Umap extends ServerStored { Alert.success(translate('Map has been saved!')) }) } - this.sync.saved() this.fire('saved') }