Batch operations

Co-authored-by: Alexis Métaireau <alexis@notmyidea.org>
Co-authored-by: David Larlet <david@larlet.fr>

umap/static/umap/js/modules/data/layer.js

@@ -464,7 +464,10 @@ export class DataLayer extends ServerStored {
     try {
       // Do not fail if remote data is somehow invalid,
       // otherwise the layer becomes uneditable.
-      return this.makeFeatures(geojson, sync)
+      this.sync.startBatch()
+      const features = this.makeFeatures(geojson, sync)
+      this.sync.commitBatch()
+      return features
     } catch (err) {
       console.debug('Error with DataLayer', this.id)
       console.error(err)
@@ -522,7 +525,7 @@ export class DataLayer extends ServerStored {
     }
     if (feature && !feature.isEmpty()) {
       this.addFeature(feature)
-      if (sync) feature.onCommit()
+      if (sync) feature.sync.upsert(feature.toGeoJSON(), null)
       return feature
     }
   }

umap/static/umap/js/modules/sync/engine.js

@@ -124,38 +124,83 @@ export class SyncEngine {
       await this.authenticate()
     }, this._reconnectDelay)
   }
-  upsert(subject, metadata, value, oldValue) {
+
+  startBatch() {
+    this._batch = []
+  }
+
+  commitBatch() {
+    if (!this._batch.length) {
+      this._batch = null
+      return
+    }
     this._undoManager.add({
+      verb: 'batch',
+      operations: this._batch,
+    })
+    const syncOperations = this._batch.map((operation) =>
+      this.convertToSyncOperation(operation)
+    )
+    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 = {
       verb: 'upsert',
       subject,
       metadata,
-      oldValue: oldValue,
       newValue: value,
-    })
-    this._send({ verb: 'upsert', subject, metadata, value })
+      oldValue: oldValue,
+    }
+    if (this._batch) {
+      this._batch.push(undoOperation)
+      return
+    }
+    this._undoManager.add(undoOperation)
+    const syncOperation = this.convertToSyncOperation(undoOperation)
+    this._send(syncOperation)
   }
 
   update(subject, metadata, key, value, oldValue) {
-    this._undoManager.add({
+    const undoOperation = {
       verb: 'update',
       subject,
       metadata,
       key,
       oldValue: oldValue,
       newValue: value,
-    })
-    this._send({ verb: 'update', subject, metadata, key, value })
+    }
+    if (this._batch) {
+      this._batch.push(undoOperation)
+      return
+    }
+    this._undoManager.add(undoOperation)
+    const syncOperation = this.convertToSyncOperation(undoOperation)
+    this._send(syncOperation)
   }
 
   delete(subject, metadata, oldValue) {
-    console.log('oldValue', oldValue)
-    this._undoManager.add({
+    const undoOperation = {
       verb: 'delete',
       subject,
       metadata,
       oldValue: oldValue,
-    })
-    this._send({ verb: 'delete', subject, metadata })
+    }
+    if (this._batch) {
+      this._batch.push(undoOperation)
+      return
+    }
+    this._undoManager.add(undoOperation)
+    const syncOperation = this.convertToSyncOperation(undoOperation)
+    this._send(syncOperation)
   }
 
   saved() {
@@ -185,6 +230,10 @@ export class SyncEngine {
   }
 
   _applyOperation(operation) {
+    if (operation.verb === 'batch') {
+      operation.operations.map((op) => this._applyOperation(op))
+      return
+    }
     const updater = this._getUpdater(operation.subject, operation.metadata)
     updater.applyMessage(operation)
   }

umap/static/umap/js/modules/sync/undo.js

@@ -10,28 +10,38 @@ export class UndoManager {
   }
 
   toggleState() {
-    document.querySelector('.edit-undo').disabled = !this._undoStack.length
-    document.querySelector('.edit-redo').disabled = !this._redoStack.length
+    const undoButton = document.querySelector('.edit-undo')
+    const redoButton = document.querySelector('.edit-redo')
+    if (undoButton) undoButton.disabled = !this._undoStack.length
+    if (redoButton) redoButton.disabled = !this._redoStack.length
   }
 
   add(operation) {
-    console.debug('New entry in undo stack', operation)
     this._redoStack = []
     this._undoStack.push(operation)
     this.toggleState()
   }
 
+  cleanOperation(operation, redo) {
+    const syncOperation = Utils.CopyJSON(operation)
+    delete syncOperation.oldValue
+    delete syncOperation.newValue
+    syncOperation.value = redo ? operation.newValue : operation.oldValue
+    return syncOperation
+  }
+
   undo(redo = false) {
     const fromStack = redo ? this._redoStack : this._undoStack
     const toStack = redo ? this._undoStack : this._redoStack
     const operation = fromStack.pop()
     if (!operation) return
-    const syncOperation = Utils.CopyJSON(operation)
-    console.log('old/new', syncOperation.oldValue, syncOperation.newValue)
-    delete syncOperation.oldValue
-    delete syncOperation.newValue
-    syncOperation.value = redo ? operation.newValue : operation.oldValue
-    this.applyOperation(syncOperation)
+    if (operation.verb === 'batch') {
+      for (const op of operation.operations) {
+        this.applyOperation(this.cleanOperation(op, redo))
+      }
+    } else {
+      this.applyOperation(this.cleanOperation(operation, redo))
+    }
     toStack.push(operation)
     this.toggleState()
   }
@@ -49,12 +59,9 @@ export class UndoManager {
         break
       case 'delete':
       case 'upsert':
-        console.log('undo upsert/delete', syncOperation.value)
         if (syncOperation.value === null || syncOperation.value === undefined) {
-          console.log('case delete')
           updater.delete(syncOperation)
         } else {
-          console.log('case upsert')
           updater.upsert(syncOperation)
         }
         this._syncEngine._send(syncOperation)

umap/sync/payloads.py

@@ -1,4 +1,4 @@
-from typing import Literal, Optional, Union
+from typing import List, Literal, Optional, Union
 
 from pydantic import BaseModel, Field, RootModel
 
@@ -14,10 +14,11 @@ class OperationMessage(BaseModel):
     """Message sent from one peer to all the others"""
 
     kind: Literal["OperationMessage"] = "OperationMessage"
-    verb: Literal["upsert", "update", "delete"]
-    subject: Literal["map", "datalayer", "feature"]
+    verb: Literal["upsert", "update", "delete", "batch"]
+    subject: Literal["map", "datalayer", "feature", "batch"]
     metadata: Optional[dict] = None
     key: Optional[str] = None
+    operations: Optional[List] = None
 
 
 class PeerMessage(BaseModel):

umap/tests/integration/test_undo_redo.py

@@ -89,8 +89,8 @@ def test_can_undo_redo_layer_color_change(
 ):
     page.goto(f"{live_server.url}{map_with_polygon.get_absolute_url()}?edit")
 
-    expect(page.locator(".edit-undo")).to_be_disabled()
-    expect(page.locator(".edit-redo")).to_be_disabled()
+    expect(page.locator(".edit-undo")).to_be_hidden()
+    expect(page.locator(".edit-redo")).to_be_hidden()
     page.get_by_role("button", name="Manage layers").click()
     page.locator(".panel").get_by_title("Edit", exact=True).click()
     page.get_by_text("Shape properties").click()