wip: derive the dirty status from the undoManager

This should pave the way for removing the SaveManager.

Co-authored-by: David Larlet <david@larlet.fr>

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 .edit-disable {
-.umap-edit-enabled.umap-is-dirty .edit-undo,
-.umap-edit-enabled.umap-is-dirty .edit-redo {
     display: inline-block;
 }
-.umap-is-dirty .edit-disable {
-    display: none;
-}
 .umap-caption-bar {
     display: none;
 }

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 {

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) =>
+    const operations = this._batch.map((stage) => stage.operation)
-      this.convertToSyncOperation(operation)
+    this._send({ verb: 'batch', operations: operations, subject: 'batch' })
-    )
-    this._send({ verb: 'batch', operations: syncOperations, 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)
+    this._undoManager.add(stage)
-    const syncOperation = this.convertToSyncOperation(undoOperation)
+    this._send(operation)
-    this._send(syncOperation)
   }
 
   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)
+    this._undoManager.add(stage)
-    const syncOperation = this.convertToSyncOperation(undoOperation)
+    this._send(operation)
-    this._send(syncOperation)
   }
 
   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)
+    this._undoManager.add(stage)
-    const syncOperation = this.convertToSyncOperation(undoOperation)
+    this._send(operation)
-    this._send(syncOperation)
+  }
+
+  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) {
+  _send(operation) {
-    const message = this._operations.addLocal(inputMessage)
+    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) {
+    this._operations.saved(lastKnownHLC)
-      SaveManager.clear()
-    }
   }
 
   /**
@@ -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) {
+  addLocal(operation) {
-    const message = { ...inputMessage, hlc: this._hlc.tick() }
+    operation.hlc = this._hlc.tick()
-    this._operations.push(message)
+    this._operations.push(operation)
-    return message
+    return operation
   }
 
   /**

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) {
+  isDirty() {
-    this._redoStack = []
+    for (const stage of this._undoStack) {
-    this._undoStack.push(operation)
+      if (stage.operation.dirty) return true
-    this.toggleState()
+    }
+    for (const stage of this._redoStack) {
+      if (stage.operation.dirty) return true
+    }
+    return false
   }
 
-  cleanOperation(operation, redo) {
+  add(stage) {
-    const syncOperation = Utils.CopyJSON(operation)
+    // FIXME make it more generic
-    delete syncOperation.oldValue
+    if (stage.operation.key !== '_referenceVersion') {
-    delete syncOperation.newValue
+      stage.operation.dirty = true
-    syncOperation.value = redo ? operation.newValue : operation.oldValue
+      this._redoStack = []
-    return syncOperation
+      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()
+    const stage = fromStack.pop()
-    if (!operation) return
+    if (!stage) return
-    if (operation.verb === 'batch') {
+    stage.operation.dirty = !stage.operation.dirty
-      for (const op of operation.operations) {
+    if (stage.operation.verb === 'batch') {
-        this.applyOperation(this.cleanOperation(op, redo))
+      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) {
+  applyOperation(operation) {
-    const updater = this._getUpdater(syncOperation.subject, syncOperation.metadata)
+    const updater = this._getUpdater(operation.subject, operation.metadata)
-    switch (syncOperation.verb) {
+    switch (operation.verb) {
       case 'update':
-        updater.update(syncOperation)
+        updater.update(operation)
-        this._syncEngine._send(syncOperation)
+        this._syncEngine._send(operation)
         break
       case 'delete':
       case 'upsert':
-        if (syncOperation.value === null || syncOperation.value === undefined) {
+        if (operation.value === null || operation.value === undefined) {
-          updater.delete(syncOperation)
+          updater.delete(operation)
         } else {
-          updater.upsert(syncOperation)
+          updater.upsert(operation)
         }
-        this._syncEngine._send(syncOperation)
+        this._syncEngine._send(operation)
         break
     }
   }

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)
+  }
 }

umap/static/umap/js/modules/ui/bar.js

@@ -30,7 +30,7 @@ const TOP_BAR_TEMPLATE = `
             <i class="icon icon-16 icon-redo"></i>
             <span class="">${translate('Redo')}</span>
         </button>
-        <button class="edit-disable round" type="button" data-ref="view">
+        <button class="edit-disable round disabled-on-dirty" type="button" data-ref="view">
             <i class="icon icon-16 icon-eye"></i>
             <span class="">${translate('View')}</span>
         </button>
@@ -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()
   }

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')
   }