From c2fec5509c6e0bba261345f161a87a46cb43a3f7 Mon Sep 17 00:00:00 2001 From: Sebastien DUMETZ Date: Wed, 15 Nov 2023 09:00:50 +0100 Subject: [PATCH] WIP --- source/server/utils/merge.test.ts | 83 ++++++++++++++++++----- source/server/utils/merge.ts | 107 ++++++++++++++++++++++++------ 2 files changed, 156 insertions(+), 34 deletions(-) diff --git a/source/server/utils/merge.test.ts b/source/server/utils/merge.test.ts index 43d8e69d..82705e6d 100644 --- a/source/server/utils/merge.test.ts +++ b/source/server/utils/merge.test.ts @@ -117,29 +117,55 @@ describe("three-way merge", function(){ //The camera node's position can safely be ignored as it will get reset on load, in favor of `scene.setups[].navigation` }); - describe.skip("conflict resolution", function(){ + 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("conflict resolution", function(){ //Test only cases where "smart" conflict resolution is possible describe("by ID", function(){ //Here it would be possible to differentiate annotations or articles by their id //thus detecting a double-push and resolving it - const A1 = { "id": "mMzG2tLjmIst", "titles": { "EN": "A1"} } - const A2 = { "id": "FiIaONzRIbL4", "titles": { "EN": "A2"} } - const A3 = { "id": "rJpCltJjxyyL", "titles": { "EN": "A3"} } + let A1: any, A2: any, A3 : any; + beforeEach(function(){ + A1 = { "id": "mMzG2tLjmIst", "titles": { "EN": "A1"} } + A2 = { "id": "FiIaONzRIbL4", "titles": { "EN": "A2"} } + A3 = { "id": "rJpCltJjxyyL", "titles": { "EN": "A3"} } + }); + it("handle double array push (annotations)", function(){ - const ref = {annotations: [A1], title: "foo"}; - const current = {annotations: [A1,A2]}; - const next = {annotations: [A1,A3]}; + const ref = {annotations: [A1]}; + const current = {annotations: [A1, A2]}; + const next = {annotations: [A1, A3]}; const d = diff(ref, next); expect(d).to.deep.equal({annotations: {1: A3}}); - expect(apply(current, d)).to.deep.equal({annotations: [A1,A2,A3], title: "foo"}); + expect(apply(current, d)).to.deep.equal({annotations: [A1, A2, A3]}); }); + + it("can still modify annotations in place", function(){ + const A1b = {"id": "mMzG2tLjmIst", "titles": { "EN": "A1b", "FR": "A1b"} } + const ref = {annotations: [A1]}; + const current = {annotations: [A1, A2]}; + const next = {annotations: [A1b]}; + expect(apply(current, diff(ref, next))).to.deep.equal({annotations: [A1b, A2]}); + + }) - it.skip("handle double array push (articles)", function(){ - const ref = {articles: [A1], title: "foo"}; - const current = {articles: [A1,A2]}; - const next = {articles: [A1,A3]}; - expect(apply(current, diff(ref, next))).to.deep.equal({articles: [A1,A2,A3], title: "foo"}); + it("handle double array push (articles)", function(){ + const ref = {articles: [A1]}; + const current = {articles: [A1, A2]}; + const next = {articles: [A1, A3]}; + const d = diff(ref, next); + expect(d).to.deep.equal({articles: {1: A3}}); + const patch = apply(current, d) + expect(patch, JSON.stringify(patch)).to.deep.equal({articles: [A1, A2, A3]}); }); }) @@ -156,7 +182,6 @@ describe("three-way merge", function(){ const next = {nodes: [N1,N3], scenes:[{nodes:[0, 1]}]}; expect(apply(current, diff(ref, next))).to.deep.equal({nodes: [N1,N2,N3], scenes:[{nodes:[0, 1, 2]}]}); - //FIXME a node's children might also be affected. }); it("reassign nodes children appropriately", function(){ @@ -165,8 +190,36 @@ describe("three-way merge", function(){ ["lights", "cameras", "models"].forEach(type=>{ it(`reassign ${type} appropriately`, function(){ - //Lights have no property that could be used to differentiate them + const stype = type.slice(0, -1); + function make(nodes: object[]=[], types: object[]=[]){ + return { + nodes: [ + {name: `${stype}.0`, [stype]: 0}, + ...nodes, + ], [type]:[ + {reference: "default item"}, + [...types] + ] + }; + } + //Lights, cameras and models have no property that could be used to differentiate them //We can only suppose that if someone adds a node, the light attached to this node is unique to this one and should stay with it. + const ref = make(); + const current = make([{name: `${stype}.current`, [stype]: 1}], [{reference: "current_item"}]); + const next = make([{name: `${stype}.next`, [stype]: 1}], [{reference: "next_item"}]); + + expect(apply(current, diff(ref, next))).to.deep.equal({ + nodes: [ + {name: `${stype}.0`, [stype]: 0}, + {name: `${stype}.current`, [stype]: 1}, + {name: `${stype}.next`, [stype]: 2}, + ], [type]:[ + {reference: "default item"}, + {reference: "current_item"}, + {reference: "next_item"}, + ] + }); + }); }); }); diff --git a/source/server/utils/merge.ts b/source/server/utils/merge.ts index ce36e079..cbe9888e 100644 --- a/source/server/utils/merge.ts +++ b/source/server/utils/merge.ts @@ -1,4 +1,14 @@ +/** + * Special symbol to mark a field for deletion in a diff object + */ +export const DELETE_KEY = Symbol("_DELETE_KEY"); + +type Diff = { + [K in keyof T]?: typeof DELETE_KEY|Diff|T[K]|Record; +} + + /** * Deep assign two or more objects * Like `Object.assign()` but recursive @@ -10,26 +20,68 @@ export function apply>(into :T, ...diffs :Diff[ for(const diff of diffs){ for(const key in diff){ const value = diff[key] as T[Extract]; - if(value === DELETE_KEY) delete into[key]; - else if(typeof diff[key] == "object"){ - if(typeof into[key] !== "object") into[key] = {} as any; - apply(into[key], value); - }else{ - into[key] = value; + if(apply_core(into, key, value)){ + continue; } + + //Then handle arrays to apply edge cases + if(Array.isArray(into[key])){ + apply_array(into[key], value); + continue; + } + //Default case : recurse. + into[key] ??= {} as any; + apply(into[key], value); } } return into; } +//Handle special merge conditions for arrays +function apply_array>(into :T, ...diffs :Diff[]):T{ + for (const diff of diffs){ + for(const idx in diff){ + const value = diff[idx]; + if(apply_core(into, idx, value)){ + 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; + } + } + /** + * @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; + apply(into[idx], value); + } + } + return into; +} -export const DELETE_KEY = Symbol("MERGE_DELETE_KEY"); +function apply_core>(into :T, key:keyof T, value :any):boolean{ -type Diff = { - [K in keyof T]?: typeof DELETE_KEY|Diff|T[K]|Record; -} + if(value === DELETE_KEY){ + delete into[key]; + return true; + } + + if(typeof value !== "object"){ + //Handle primitives + into[key] = value; + return true; + } + return false; +} /** * Computes a diff between two objects. @@ -46,17 +98,34 @@ export function diff>(from :T, to :T) :Diff{ let r :Diff = {} as any; const keys :Set= new Set([...Object.keys(from), ...Object.keys(to)]); for(const key of keys.values()){ - if(typeof to[key] == "undefined") r[key] = DELETE_KEY; - else if(typeof from[key] == "object"){ - if(to[key] == null){ - if(from[key] != null ) r[key] = null as T[Extract]; - }else{ - const d = diff(from[key] as any, to[key] as any); - if(Object.keys(d).length) r[key] = d; - } - }else if(from[key] !== to[key]){ + + if(typeof to[key] == "undefined"){ + r[key] = DELETE_KEY; + continue; + } + + if(typeof from[key] != "object"){ + if(from[key] === to[key]) continue; + //Simple case with primitive values + //console.log("Assigning ", key, to[key]); r[key] = to[key]; + continue; + } + + //Handle cases where `typeof from[key] === "object"` + if(to[key] == null){ + //Null is special because it can't be fed back to diff() recursively + if(from[key] != null ) r[key] = null as T[Extract]; + continue; } + + const d = diff(from[key] as any, to[key] as any); + if(Object.keys(d).length){ + //console.log("Diffing", key, from[key], to[key]); + r[key] = d; + } + + } return r; }