[WIP] POC structured merge with array double-push handling

source/server/package.json

@@ -7,6 +7,7 @@
     "scripts": {
         "start": "ROOT_DIR=\"$(pwd)/../..\" node ./dist/server/index.js",
         "test": "NODE_NO_WARNINGS=1 mocha",
+        "test-watch": "NODE_NO_WARNINGS=1 mocha --parallel --watch --reporter min",
         "build": "tsc -b ."
     },
     "nodemonConfig": {

source/server/utils/merge/apply.ts

@@ -3,7 +3,7 @@ import {Diff, DELETE_KEY} from "./types.js";
 
 /**
  * Deep assign two or more objects
- * Like `Object.assign()` but recursive
+ * Like `Object.assign()` but recursive (modifies objects in-place)
  * It's currently quite simplified and doesn't handle strings splicing
  * @param into Object to merge into (will be mutated in-place)
  * @returns into, merged with source(s)
@@ -29,7 +29,13 @@ export default function apply<T extends Record<string, any>>(into :T, ...diffs :
   return into;
 }
 
-//Handle special merge conditions for arrays
+/**
+ * Handle special merge conditions for arrays
+ * In particular:
+ *  - for arrays of native types, replace the whole array
+ *  - for arrays of objects with a "name" or "id", tries to merge objects with a matching id/name, appends the rest
+ * 
+ */
 function apply_array<T extends Array<any>>(into :T, ...diffs :Diff<T>[]):T{
   for (const diff of diffs){
     for(const idx in diff){
@@ -38,17 +44,18 @@ function apply_array<T extends Array<any>>(into :T, ...diffs :Diff<T>[]):T{
         continue;
       }
       if(typeof into[idx] !== undefined){
-        if(value.id){
-          const ref_id = into[idx].id;
-          if(ref_id && ref_id != value.id){
-            into.push(value);
-            continue;
-          }
+        //When an item already exist, check
+        let append = ["id", "name"].some(key =>{
+          if(!value[key]) return false;
+          const ref_id = into[idx][key];
+          if(!ref_id || ref_id == value[key]) return false;
+          return true;
+        });
+
+        if(append){
+          into.push(value);
+          continue;
         }
-        /** 
-         * @todo names should more or less follow the same rules but are trickier
-         * Because they require a parent lookup to also reorder affected nodes.
-         */
       }
       //Default case : recurse as apply() would.
       into[idx]  ??= {} as any;
@@ -58,7 +65,9 @@ function apply_array<T extends Array<any>>(into :T, ...diffs :Diff<T>[]):T{
   return into;
 }
 
-
+/**
+ * Return true if the value was applied, false if it needs further processing
+ */
 function apply_core<T extends Record<string, any>>(into :T, key:keyof T, value :any):boolean{
 
   if(value === DELETE_KEY){

source/server/utils/merge/merge.test.ts

@@ -7,7 +7,7 @@ const thisDir = path.dirname(fileURLToPath(import.meta.url));
 
 
 import { IDocument, INode } from "../schema/document.js";
-import {DELETE_KEY, apply, applyDoc, diff} from "./index.js";
+import {DELETE_KEY, apply, applyDoc, diff, diffDoc} from "./index.js";
 
 
 
@@ -124,18 +124,69 @@ describe("three-way merge", function(){
     expect(apply(current, diff(ref, next))).to.deep.equal({greet:"Hi", name:"Universe"});
   });
 
-  it.skip("ignores camera translation/rotation changes", function(){
-    //This is a potential optimization
-    //The camera node's position can safely be ignored as it will get reset on load, in favor of `scene.setups[].navigation`
-  });
+  describe("Arrays", function(){
+
+    it("Overrides array double-push for native types", function(){
+      const ref = {a:[1]};
+      const current = {a:[1, 2]};
+      const next = {a:[1,3]};
+      expect(apply(current, diff(ref, next))).to.deep.equal({a:[1,3]});
+    });
+
+    describe("deduplicates by ID", function(){
+      //Here it would be possible to differentiate annotations or articles by their id
+      //thus detecting a double-push and resolving it
+      let A:{}[] = [];
+      beforeEach(function(){
+        A.push(
+          { "id": "mMzG2tLjmIst", "titles": { "EN": "A1"} },
+          { "id": "FiIaONzRIbL4", "titles": { "EN": "A2"} },
+          { "id": "rJpCltJjxyyL", "titles": { "EN": "A3"} }
+        );
+      });
+
+      it("handle double array push (annotations)", function(){
+        const ref = {annotations: [A[0]]};
+        const current = {annotations: [A[0], A[1]]};
+        const next = {annotations: [A[0], A[2]]};
+
+        const d = diff(ref, next);
+        expect(d).to.deep.equal({annotations: {1: A[2]}});
+        expect(apply(current, d)).to.deep.equal({annotations: [A[0], A[1], A[2]]});
+      });
+
+      it("handle array modify in place", function(){
+        const m = {...A[0], titles: {EN: "A1 modified"}}
+        const ref = {annotations: [A[0]]};
+        const current = {annotations: [A[0], A[1]]};
+        const next = {annotations: [m]};
+
+        const d = diff(ref, next);
+        expect(apply(current, d)).to.deep.equal({annotations: [m, A[1]]});
 
-  describe("fields ignore optimization", function(){
-    //This is a potential optimization
-    //The camera node's position can safely be ignored as it will get reset on load, in favor of `scene.setups[].navigation`
-    it("ignores camera translation changes", function(){
-      const ref = {nodes:[{name:"camera", translation:[1,0,0]}]};
-      const next = {nodes:[{name:"camera", translation:[0,1,0]}]};
-      expect(apply(ref, diff(ref, next))).to.deep.equal({nodes:[{name:"camera", translation:[1,0,0]}]});
+      });
+    });
+    describe("deduplicates by name", function(){
+      //Here it would be possible to differentiate annotations or articles by their id
+      //thus detecting a double-push and resolving it
+      let N:{}[] = [];
+      beforeEach(function(){
+        N.push(
+          { "name": "Node1"},
+          { "name": "Node2"},
+          { "name": "Node3"},
+        );
+      });
+
+      it("handle double array push", function(){
+        const ref = {nodes: [N[0]]};
+        const current = {nodes: [N[0], N[1]]};
+        const next = {nodes: [N[0], N[2]]};
+
+        const d = diff(ref, next);
+        expect(d).to.deep.equal({nodes: {1: N[2]}});
+        expect(apply(current, d)).to.deep.equal({nodes: [N[0], N[1], N[2]]});
+      });
     });
   });
 });
@@ -151,7 +202,7 @@ describe("merge documents", function(){
   });
   it("merge simple document changes", function(){
     const current = JSON.parse(docString);
-    current.lights.push({type:"ambient"});
+    current.lights.push({type:"ambiant"});
 
     current.nodes.push({
       "name": "Ambiant Light",
@@ -168,19 +219,28 @@ describe("merge documents", function(){
       "light": 1,
     });
     next.nodes.find( (n:INode) =>n.name=="Lights").children.push(next.nodes.length-1);
-    const d = diff(doc, next);
-    console.log("Diff : ", d);
+    const d = diffDoc(doc, next);
+
     const result = applyDoc(current, d);
-    expect(result.nodes).to.deep.equal([
-      ...(doc as any).nodes,
-      {
+
+    expect(result.nodes, "merged nodes").to.deep.equal([
+      (doc.nodes as any)[0], //The camera
+      {name: "Lights", children: [2, 3, 4]},
+      (doc.nodes as any)[2], //The base light (index 2)
+      { //index 3
         "name": "Ambiant Light",
         "light": 1,
       },
-      {
+      { //index 4
         "name": "Directional Light",
-        "light": 1,
-      }
+        "light": 2,
+      },
+      (doc.nodes as any)[3], //The model
     ]);
+    expect(result.lights).to.deep.equal([
+      ...(doc.lights as any),
+      {type: "ambiant"},
+      {type:"directional"}
+    ])
   })
 })