diff --git a/source/server/__test_fixtures/documents/04_tours.svx.json b/source/server/__test_fixtures/documents/04_tours.svx.json index 04d952e8..c76d1f34 100644 --- a/source/server/__test_fixtures/documents/04_tours.svx.json +++ b/source/server/__test_fixtures/documents/04_tours.svx.json @@ -129,6 +129,31 @@ "duration": 1.5 } ] - } + }, + "tours": [ + { + "id": "9Ya68S8n2tnl", + "steps": [ + { + "id": "MJ05xF", + "titles": { + "EN": "New Step #0" + } + }, + { + "id": "xyhq5j", + "titles": { + "EN": "New Step #1" + } + } + ], + "titles": { + "EN": "New Tour #0" + }, + "leads": { + "EN": "" + } + } + ] }] } diff --git a/source/server/routes/history/diff/get.test.ts b/source/server/routes/history/diff/get.test.ts index 69add293..d9e756d0 100644 --- a/source/server/routes/history/diff/get.test.ts +++ b/source/server/routes/history/diff/get.test.ts @@ -1,9 +1,12 @@ +import path from "path"; +import { readFile } from "fs/promises"; import request from "supertest"; import Vfs from "../../../vfs/index.js"; import User from "../../../auth/User.js"; import UserManager from "../../../auth/UserManager.js"; +import { fixturesDir } from "../../../__test_fixtures/fixtures.js"; /** @@ -36,16 +39,37 @@ describe("GET /history/:scene/:id/diff", function(){ .expect("Content-Type", "text/plain; charset=utf-8"); expect(res.text).to.equal(`--- hello.txt\n+++ hello.txt\n@@ -1 +1 @@\n-Hello\n+Hello World\n`); }); + describe("documents", function(){ + it("get diff summary", async function(){ + await vfs.writeDoc(`{"label":"foo"}`, {scene: scene_id, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json", user_id: 0}); + let ref = await vfs.writeDoc(`{"label":"bar"}`, {scene: scene_id, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json", user_id: 0}); + let res = await request(this.server).get(`/history/foo/${ref.id}/diff`) + .set("Accept", "application/json") + .expect(200) + .expect("Content-Type", "application/json; charset=utf-8"); + expect(res.body).to.have.property("diff").match(/Couldn't compile .*diff/); + expect(res.body).to.have.property("src"); + expect(res.body).to.have.property("dst"); + }); + it("stringifies DELETED_KEY symbol", async function(){ + const docString = await readFile(path.join(fixturesDir, "documents", "01_simple.svx.json"), {encoding: "utf-8"}); + let doc2 = JSON.parse(docString); + delete doc2.asset.version + await vfs.writeDoc(docString, {scene: scene_id, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json", user_id: 0}); + let ref = await vfs.writeDoc(JSON.stringify(doc2), {scene: scene_id, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json", user_id: 0}); + let res = await request(this.server).get(`/history/foo/${ref.id}/diff`) + .set("Accept", "application/json") + .expect(200) + .expect("Content-Type", "application/json; charset=utf-8"); + + expect(res.body).to.have.property("diff", "STRUCTURED CHANGES SUMMARY\n"+JSON.stringify({ + "asset": { + "version": "*DELETED*" + } + }, null, 2)); + expect(res.body).to.have.property("src"); + expect(res.body).to.have.property("dst"); + }); - it("get diff summary for documents", async function(){ - await vfs.writeDoc(`{"label":"foo"}`, {scene: scene_id, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json", user_id: 0}); - let ref = await vfs.writeDoc(`{"label":"bar"}`, {scene: scene_id, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json", user_id: 0}); - let res = await request(this.server).get(`/history/foo/${ref.id}/diff`) - .set("Accept", "application/json") - .expect(200) - .expect("Content-Type", "application/json; charset=utf-8"); - expect(res.body).to.have.property("diff").match(/Couldn't compile diff/); - expect(res.body).to.have.property("src"); - expect(res.body).to.have.property("dst"); }) }); diff --git a/source/server/routes/history/diff/get.ts b/source/server/routes/history/diff/get.ts index 167f05c6..042320d0 100644 --- a/source/server/routes/history/diff/get.ts +++ b/source/server/routes/history/diff/get.ts @@ -5,11 +5,13 @@ import {Request, Response} from "express"; import { getVfs } from "../../../utils/locals.js"; import { BadRequestError } from "../../../utils/errors.js"; import { FileProps } from "../../../vfs/types.js"; -import { diffDoc } from "../../../utils/merge/index.js"; +import { DELETE_KEY, diffDoc } from "../../../utils/merge/index.js"; import { fromPointers } from "../../../utils/merge/pointers/index.js"; const sizeMax = 400*1000; + + /** * Computes a somewhat arbitrary text representation of the difference between */ @@ -52,7 +54,10 @@ export default async function handleGetDiff(req :Request, res :Response){ try{ let fromDoc = JSON.parse(fromFile.data); let toDoc = JSON.parse(dstFile.data); - diff = `STRUCTURED CHANGES SUMMARY\n`+JSON.stringify(fromPointers(diffDoc(fromDoc, toDoc) as any), null, 2); + diff = `STRUCTURED CHANGES SUMMARY\n`+JSON.stringify(fromPointers(diffDoc(fromDoc, toDoc) as any), (key, value)=>{ + if(value === DELETE_KEY) return "*DELETED*"; + return value; + }, 2); }catch(e:any){ console.error("Failed to compile scene diff : ", e); diff = `Couldn't compile diff from #${fromFile.id} to #${dstFile.id}: ${e.message}`; diff --git a/source/server/routes/scenes/post.ts b/source/server/routes/scenes/post.ts index ac9ad78e..5fb0ce5a 100644 --- a/source/server/routes/scenes/post.ts +++ b/source/server/routes/scenes/post.ts @@ -42,7 +42,7 @@ export default async function postScenes(req :Request, res :Response){ let m = /^(?scenes\/)?(?[^/]+)(?:\/(?.+))?\/?$/.exec(record.filename); const scene :string|undefined = m?.groups?.scene; const name :string|undefined = m?.groups?.name; - console.log("Parse %s %s from zip", scene, name); + if(!scene){ results.fail.push(`${record.filename}: not matching pattern`); continue; diff --git a/source/server/routes/scenes/scene/files/put/document.ts b/source/server/routes/scenes/scene/files/put/document.ts index 99c9b235..335325f1 100644 --- a/source/server/routes/scenes/scene/files/put/document.ts +++ b/source/server/routes/scenes/scene/files/put/document.ts @@ -35,13 +35,17 @@ export default async function handlePutDocument(req :Request, res :Response){ if(!refDocString) throw new BadRequestError(`Referenced document is not valid`); const refDoc = JSON.parse(refDocString); + console.log("Ref doc :", JSON.stringify(refDoc.setups![0].tours, null, 2)); + console.log("New doc :", JSON.stringify(newDoc.setups![0].tours, null, 2)); const docDiff = merge.diffDoc(refDoc, newDoc); if(Object.keys(docDiff).length == 0){ //Nothing to do return res.status(204).send(); } + console.log("Diff :", JSON.stringify(docDiff, (key, value)=> value === merge.DELETE_KEY? "*DELETED*":value, 2)); const mergedDoc = merge.applyDoc(currentDoc, docDiff); + console.log("Merged doc :", JSON.stringify(mergedDoc.setups![0].tours, null, 2)); let s = JSON.stringify(mergedDoc); await tr.writeDoc(s, {scene: scene, user_id: uid, name: "scene.svx.json", mime: "application/si-dpo-3d.document+json"}); res.status(204).send(); diff --git a/source/server/utils/merge/apply.test.ts b/source/server/utils/merge/apply.test.ts index 3f4f47c7..da11c536 100644 --- a/source/server/utils/merge/apply.test.ts +++ b/source/server/utils/merge/apply.test.ts @@ -1,5 +1,5 @@ -import {DELETE_KEY} from "./pointers/types.js"; +import {DELETE_KEY, SOURCE_INDEX} from "./pointers/types.js"; import apply from "./apply.js"; describe("merge.apply()", function(){ @@ -38,4 +38,8 @@ describe("merge.apply()", function(){ {name: "C"} // C gets pushed-back to index 2 ]}); }); + + it("merges special SOURCE_INDEX symbol", function(){ + expect(apply({a:{[SOURCE_INDEX]:0}}, {a:{[SOURCE_INDEX]:1}})).to.deep.equal({a:{[SOURCE_INDEX]:1}}); + }); }); \ No newline at end of file diff --git a/source/server/utils/merge/apply.ts b/source/server/utils/merge/apply.ts index 2f189c8d..196a8396 100644 --- a/source/server/utils/merge/apply.ts +++ b/source/server/utils/merge/apply.ts @@ -1,5 +1,5 @@ 'use strict'; -import {Diff, DELETE_KEY} from "./pointers/types.js"; +import {Diff, DELETE_KEY, SOURCE_INDEX, withIndex} from "./pointers/types.js"; /** * Deep assign two or more objects @@ -10,6 +10,7 @@ import {Diff, DELETE_KEY} from "./pointers/types.js"; */ export default function apply>(into :T, ...diffs :Diff[]):T{ for(const diff of diffs){ + if(SOURCE_INDEX in diff) into = withIndex(into, diff[SOURCE_INDEX] as number); for(const key in diff){ const value = diff[key] as T[Extract]; diff --git a/source/server/utils/merge/diff.test.ts b/source/server/utils/merge/diff.test.ts index 234a5f34..be1765c1 100644 --- a/source/server/utils/merge/diff.test.ts +++ b/source/server/utils/merge/diff.test.ts @@ -1,5 +1,5 @@ -import {DELETE_KEY} from "./pointers/types.js"; +import {DELETE_KEY, SOURCE_INDEX} from "./pointers/types.js"; import diff from "./diff.js"; @@ -96,6 +96,16 @@ describe("merge.diff()", function(){ it("handles object creation", function(){ expect(diff({name:"A", articles: null}, {name:"A", articles: {id:"1"}})).to.deep.equal({articles: {id:"1"}}); + }); + + it("handles the special SOURCE_INDEX symbol", function(){ + // SOURCE_INDEX is used internally to reference an array's ordering + // We need this to restore the expected array order in the end + let res = diff( + {v: {a: 1, [SOURCE_INDEX]: 1 }}, + {v: {a: 1, [SOURCE_INDEX]: 2 }} + ); + expect(res).to.deep.equal({v:{[SOURCE_INDEX]: 2}}); }) }); }); diff --git a/source/server/utils/merge/diff.ts b/source/server/utils/merge/diff.ts index c06f5a90..c746fe23 100644 --- a/source/server/utils/merge/diff.ts +++ b/source/server/utils/merge/diff.ts @@ -1,19 +1,25 @@ -import {Diff, DELETE_KEY} from "./pointers/types.js"; +import {Diff, DELETE_KEY, SOURCE_INDEX, withIndex} from "./pointers/types.js"; /** * Computes a diff between two objects. * Applying deepMerge(a, diff(a,b)) should yield b * deleted keys are represented using the DELETE_KEY symbol - * It treats arrays as primitives (ie. will overwrite blindly any array) + * It treats arrays as primitives (ie. will overwrite blindly any array) so they should be pre-transformed into dictionaries where possible + * @warning it messes with properties iteration order by making a `Set` * @param from origin document * @param to target document * @returns */ -export default function diff>(from :T, to :T) :Diff{ +export default function diff&{[SOURCE_INDEX]?:number}>(from :T, to :T) :Diff{ const is_array = Array.isArray(from); if(is_array && !Array.isArray(to)) throw new Error("Can't diff an array with an object"); let r :Diff = {} as any; + + if(SOURCE_INDEX in to && to[SOURCE_INDEX] !== from[SOURCE_INDEX]){ + r = withIndex(r, to[SOURCE_INDEX] as any); + } + const keys :Set= new Set([...Object.keys(from), ...Object.keys(to)]); for(const key of keys.values()){ @@ -21,7 +27,6 @@ export default function diff>(from :T, to :T) :Diff if(typeof from[key] != "undefined") r[key] = DELETE_KEY; continue; } - if(typeof from[key] != "object"){ if(from[key] === to[key]) continue; //Simple case with primitive values @@ -44,13 +49,11 @@ export default function diff>(from :T, to :T) :Diff continue; } - const d = diff(from[key] as any, to[key] as any); - if(Object.keys(d).length){ + const d = diff(from[key], to[key]); + if(Object.keys(d).length || typeof (d as any)[SOURCE_INDEX] !== "undefined"){ //console.log("Diffing", key, from[key], to[key]); r[key] = d; } - - } return r; } diff --git a/source/server/utils/merge/index.ts b/source/server/utils/merge/index.ts index 79f07d32..cc5818e1 100644 --- a/source/server/utils/merge/index.ts +++ b/source/server/utils/merge/index.ts @@ -14,6 +14,8 @@ export { /** * like `diff()` but dereferences a document's pointers first to make a cleaner diff + * + * `diff()` holds the generalized diff logic while `toPointers` is in charge of the implementation details regarding the document's schema * @see diff */ export function diffDoc(from:IDocument,to:IDocument){ @@ -22,6 +24,8 @@ export function diffDoc(from:IDocument,to:IDocument){ /** * like `apply()` but dereferences a document's pointers first to make a cleaner diff, then re-references the result + * + * `apply()` holds the generalized merge logic while `toPointers` and `fromPointers` are in charge of the implementation details regarding the document's schema * @see apply * @see fromPointers * @see toPointers diff --git a/source/server/utils/merge/merge.test.ts b/source/server/utils/merge/merge.test.ts index 0d75af98..32500291 100644 --- a/source/server/utils/merge/merge.test.ts +++ b/source/server/utils/merge/merge.test.ts @@ -6,9 +6,9 @@ import { fixturesDir } from "../../__test_fixtures/fixtures.js"; import { IDocument, INode } from "../schema/document.js"; -import {DELETE_KEY, apply, applyDoc, diff, diffDoc} from "./index.js"; +import { apply, applyDoc, diff, diffDoc } from "./index.js"; import { ISetup } from "../schema/setup.js"; -import { DerefScene, DerefSnapshots } from "./pointers/types.js"; +import { DerefScene, fromMap, SOURCE_INDEX, toIdMap } from "./pointers/types.js"; @@ -21,7 +21,21 @@ describe("fast-forward", function(){ const next = {nodes:{bar:{name:"bar"}}}; expect(apply(ref, diff(ref, next))).to.deep.equal({nodes:{bar:{name:"bar"}}}); }); -}) + + it("keys reordering", function(){ + //ECMA2015 and up guarantees iteration over string-keyed properties to respect insertion order. + // https://tc39.es/ecma262/#sec-ordinaryownpropertykeys + const ref = {keys: toIdMap([{id: "a"}, {id:"b"}, {id: "c"}])}; + const next = {keys: toIdMap([{id: "c"}, {id:"a"}, {id: "b"}])}; + console.log("next: ", next, Object.values(next.keys).map(o=>`${o.id} ${(o as any)[SOURCE_INDEX]}`)); + const d = diff(ref, next); + console.log("Diff : ", d); + const result = apply(ref, d); + console.log("Res : ", result, Object.values(result.keys).map(o=>`${(o as any).id} ${(o as any)[SOURCE_INDEX]}`)); + expect(result).to.have.property("keys").an("object"); + expect(fromMap(result.keys), `Array order should have been kept`).to.deep.equal([{id: "c"}, {id:"a"}, {id: "b"}]); + }); +}); describe("three-way merge", function(){ /* @@ -56,137 +70,193 @@ describe("merge documents", function(){ this.beforeEach(function(){ doc = JSON.parse(docString); }); - it("merge simple document changes", function(){ - const current = JSON.parse(docString); - current.lights.push({type:"ambiant"}); - - current.nodes.push({ - "id": "QE4H7dSQw9sY", - "name": "Ambiant Light", - "light": 1, + + describe("fast-forward", function(){ + it("reorders tour steps", function(){ + const ref = JSON.parse(docString); + ref.setups[0].tours = [{ + "id": "fxQkZ9rUwNAU", + "steps": [ + {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, + {"id": "hdh7ob", "titles": {"EN": "New Step #1"}}, + ] + }]; + + const next = JSON.parse(docString); + next.setups[0].tours = [{ + "id": "fxQkZ9rUwNAU", + "steps": [ + {"id": "hdh7ob", "titles": {"EN": "New Step #1"}}, + {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, + ] + }]; + + const exp = JSON.parse(JSON.stringify(next)); + const d = diffDoc(doc, next); + console.log("Diff :", JSON.stringify(d, null, 2)); + + const result = applyDoc(doc, d); + + console.log("Merged doc :", JSON.stringify(result.setups![0].tours, null, 2)); + expect(result).to.deep.equal(exp); }); - current.nodes.find( (n:INode) =>n.name=="Lights").children.push(current.nodes.length-1); + it("reorders tour steps (bis)", async function(){ + //When reordering tour steps, their snapshots are not reordered + docString = await fs.readFile(path.resolve(fixturesDir, "documents/04_tours.svx.json"), "utf8"); + const ref = JSON.parse(docString); + + const next = JSON.parse(docString); + next.setups[0].tours = [{ + "id": "fxQkZ9rUwNAU", + "steps": [ + ref.setups[0].tours[0].steps[1], + ref.setups[0].tours[0].steps[0], + ] + }]; - const next = JSON.parse(docString); - next.lights.push({type:"directional"}); + const exp = JSON.parse(JSON.stringify(next)); + const d = diffDoc(doc, next); - next.nodes.push({ - "id": "bCZEzSXPERGa", - "name": "Directional Light", - "light": 1, + const result = applyDoc(doc, d); + expect(result).to.deep.equal(exp); }); - next.nodes.find( (n:INode) =>n.name=="Lights").children.push(next.nodes.length-1); - const d = diffDoc(doc, next); - - const result = applyDoc(current, d); - - expect(result.nodes, "merged nodes").to.deep.equal([ - (doc.nodes as any)[0], //The camera - {id: "QE4H7dSQw9sY", name: "Lights", children: [2, 3, 4]}, - (doc.nodes as any)[2], //The base light (index 2) - { //index 3 + }); + + describe("three-way", function(){ + it("merge simple document changes", function(){ + const current = JSON.parse(docString); + current.lights.push({type:"ambiant"}); + + current.nodes.push({ "id": "QE4H7dSQw9sY", "name": "Ambiant Light", "light": 1, - }, - { //index 4 - "name": "Directional Light", + }); + current.nodes.find( (n:INode) =>n.name=="Lights").children.push(current.nodes.length-1); + + + const next = JSON.parse(docString); + next.lights.push({type:"directional"}); + + next.nodes.push({ "id": "bCZEzSXPERGa", - "light": 2, - }, - (doc.nodes as any)[3], //The model - ]); - expect(result.lights).to.deep.equal([ - ...(doc.lights as any), - {type: "ambiant"}, - {type:"directional"} - ]) - }); - - it("merge updated tours", async function(){ - const [ - doc, - current, - next, - ] = await Promise.all([ - "02_tours.svx.json", - "03_tours.svx.json", - "04_tours.svx.json", - ].map( async (file) => { - const str = await fs.readFile(path.resolve(fixturesDir, "documents/", file), {encoding:"utf8"}); - return JSON.parse(str); - })); - - const d = diffDoc(doc, next); - const result = applyDoc(current, d); - expect(result.setups).to.have.length(1); - const {snapshots} = (result.setups as Required[])[0]; - expect(snapshots).to.have.property("targets").to.deep.equal([ - "model/0/visible", - "node/0/position", - "node/0/scale" - ]); - expect(snapshots).to.have.property("states").to.have.length(3); - const values = []; - for(let idx = 0; idx < 3; idx++){ - values.push(snapshots.states.map(s=>s.values[idx])); - } - expect(values[0]).to.deep.equal([true, true, false]); - expect(values[1]).to.deep.equal([[0,0,0], [1,1,0], [0,0,0]]); - expect(values[2]).to.deep.equal([[1,1,1], [2,2,2], [1,1,1]]); - }); - - it("merge added tour steps", function(){ - const doc = JSON.parse(docString); - doc.setups[0].tours = [{ - "id": "fxQkZ9rUwNAU", - "steps": [ - {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, - {"id": "bdh7ob", "titles": {"EN": "New Step #1"}} - ] - }]; - - const current = JSON.parse(docString); - current.setups[0].tours = [{ - "id": "fxQkZ9rUwNAU", - "steps": [ - {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, - {"id": "bdh7ob", "titles": {"EN": "New Step #1"}}, - {"id": "bYMguT", "titles": {"EN": "New Step #2"}} - ] - }]; - - const next = JSON.parse(docString); - next.setups[0].tours = [{ - "id": "fxQkZ9rUwNAU", - "steps": [ - {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, - ] - }]; - const d = diffDoc(doc, next); - - expect((d?.scene as DerefScene)?.setup?.tours).to.have.property("fxQkZ9rUwNAU"); - - const result = applyDoc(current, d); - expect(result.setups).to.have.length(1); - const setup = (result.setups as any)[0] - expect(setup, JSON.stringify(setup.tours, null, 2)).to.have.property("tours").to.deep.equal([{ - "id": "fxQkZ9rUwNAU", - "steps": [ - {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, - {"id": "bYMguT", "titles": {"EN": "New Step #2"}} - ] - }]); - }) + "name": "Directional Light", + "light": 1, + }); + next.nodes.find( (n:INode) =>n.name=="Lights").children.push(next.nodes.length-1); + const d = diffDoc(doc, next); + + const result = applyDoc(current, d); + + expect(result.nodes, "merged nodes").to.deep.equal([ + (doc.nodes as any)[0], //The camera + {id: "QE4H7dSQw9sY", name: "Lights", children: [2, 3, 4]}, + (doc.nodes as any)[2], //The base light (index 2) + { //index 3 + "id": "QE4H7dSQw9sY", + "name": "Ambiant Light", + "light": 1, + }, + { //index 4 + "name": "Directional Light", + "id": "bCZEzSXPERGa", + "light": 2, + }, + (doc.nodes as any)[3], //The model + ]); + expect(result.lights).to.deep.equal([ + ...(doc.lights as any), + {type: "ambiant"}, + {type:"directional"} + ]) + }); - it("detects a no-op", function(){ - const current = JSON.parse(docString); - const next = JSON.parse(docString); - const d = diffDoc(doc, next); - expect(d, JSON.stringify(d)).to.deep.equal({}); + it("merge updated tours", async function(){ + const [ + doc, + current, + next, + ] = await Promise.all([ + "02_tours.svx.json", + "03_tours.svx.json", + "04_tours.svx.json", + ].map( async (file) => { + const str = await fs.readFile(path.resolve(fixturesDir, "documents/", file), {encoding:"utf8"}); + return JSON.parse(str); + })); + + const d = diffDoc(doc, next); + const result = applyDoc(current, d); + expect(result.setups).to.have.length(1); + const {snapshots} = (result.setups as Required[])[0]; + expect(snapshots).to.have.property("targets").to.deep.equal([ + "model/0/visible", + "node/0/position", + "node/0/scale" + ]); + expect(snapshots).to.have.property("states").to.have.length(3); + const values = []; + for(let idx = 0; idx < 3; idx++){ + values.push(snapshots.states.map(s=>s.values[idx])); + } + expect(values[0]).to.deep.equal([true, true, false]); + expect(values[1]).to.deep.equal([[0,0,0], [1,1,0], [0,0,0]]); + expect(values[2]).to.deep.equal([[1,1,1], [2,2,2], [1,1,1]]); + }); + + it("merge added tour steps", function(){ + const doc = JSON.parse(docString); + doc.setups[0].tours = [{ + "id": "fxQkZ9rUwNAU", + "steps": [ + {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, + {"id": "bdh7ob", "titles": {"EN": "New Step #1"}} + ] + }]; + + const current = JSON.parse(docString); + current.setups[0].tours = [{ + "id": "fxQkZ9rUwNAU", + "steps": [ + {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, + {"id": "bdh7ob", "titles": {"EN": "New Step #1"}}, + {"id": "bYMguT", "titles": {"EN": "New Step #2"}} + ] + }]; + + const next = JSON.parse(docString); + next.setups[0].tours = [{ + "id": "fxQkZ9rUwNAU", + "steps": [ + {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, + ] + }]; + const d = diffDoc(doc, next); + + expect((d?.scene as DerefScene)?.setup?.tours).to.have.property("fxQkZ9rUwNAU"); + + const result = applyDoc(current, d); + expect(result.setups).to.have.length(1); + const setup = (result.setups as any)[0]; + expect(setup, JSON.stringify(setup.tours, null, 2)).to.have.property("tours").to.deep.equal([{ + "id": "fxQkZ9rUwNAU", + "steps": [ + {"id": "gLi0xz", "titles": {"EN": "New Step #0"}}, + {"id": "bYMguT", "titles": {"EN": "New Step #2"}} + ] + }]); + }) - const result = applyDoc(current, d); - expect(result, JSON.stringify(result)).to.deep.equal(current); - }); + it("detects a no-op", function(){ + const current = JSON.parse(docString); + const next = JSON.parse(docString); + const d = diffDoc(doc, next); + expect(d).to.deep.equal({}); + + const result = applyDoc(current, d); + + expect(result).to.deep.equal(current); + }); + }); }) \ No newline at end of file diff --git a/source/server/utils/merge/pointers/index.ts b/source/server/utils/merge/pointers/index.ts index 103cea95..b1c6e602 100644 --- a/source/server/utils/merge/pointers/index.ts +++ b/source/server/utils/merge/pointers/index.ts @@ -18,13 +18,13 @@ import { DerefDocument, DerefMeta, DerefNode, DerefScene, DerefSetup, SOURCE_IND * It's akin to the `toDocument()`methods seen in DPO-voyager, in particular in `CVScene.ts`. * @see toPointers for the inverse operation */ -export function fromPointers({asset, scene} :DerefDocument): IDocument{ +export function fromPointers({asset, scene} :Partial): IDocument{ //The output document. // All fields are initially created to later filter unused ones let document :Required = { - asset, - scene: -1, + asset: asset as any, + scene: undefined as any, "scenes": [] as IScene[], "nodes": [] as INode[], "cameras": [] as ICamera[], @@ -36,7 +36,7 @@ export function fromPointers({asset, scene} :DerefDocument): IDocument{ - document.scene = appendScene(document, scene); + if(scene) document.scene = appendScene(document, scene); return cleanDocument(document); } diff --git a/source/server/utils/merge/pointers/meta.ts b/source/server/utils/merge/pointers/meta.ts index cc96b360..80ad65ce 100644 --- a/source/server/utils/merge/pointers/meta.ts +++ b/source/server/utils/merge/pointers/meta.ts @@ -1,6 +1,6 @@ import { IDocument } from "../../schema/document.js"; import { IMeta } from "../../schema/meta.js"; -import { DerefMeta, toIdMap, toUriMap } from "./types.js"; +import { DerefMeta, fromMap, toIdMap, toUriMap } from "./types.js"; export function appendMeta(document :Required, meta :DerefMeta) :number{ let iMeta :IMeta = {}; @@ -9,17 +9,17 @@ export function appendMeta(document :Required, meta :DerefMeta) :numb if(meta.process) iMeta.process = meta.process; - let images = Object.values(meta.images ?? {}); + let images = fromMap(meta.images ?? {}); if(images.length){ iMeta.images = images; } - let articles = Object.values(meta.articles ?? {}); + let articles = fromMap(meta.articles ?? {}); if(articles.length){ iMeta.articles = articles; } - let audio = Object.values(meta.audio ?? {}); + let audio = fromMap(meta.audio ?? {}); if(audio.length){ iMeta.audio = audio; } diff --git a/source/server/utils/merge/pointers/model.ts b/source/server/utils/merge/pointers/model.ts index 83dbe852..ddd4a717 100644 --- a/source/server/utils/merge/pointers/model.ts +++ b/source/server/utils/merge/pointers/model.ts @@ -2,22 +2,19 @@ import { IDocument } from "../../schema/document.js"; import { IModel } from "../../schema/model.js"; -import { DerefModel, SOURCE_INDEX, toIdMap, toUriMap } from "./types.js"; +import { DerefDerivative, DerefModel, fromMap, SOURCE_INDEX, toIdMap, toUriMap } from "./types.js"; export function appendModel(document :Required, {derivatives, annotations, [SOURCE_INDEX]: src_index, ...model} :DerefModel) :number{ let iModel :IModel = { ...model, - derivatives: [], + derivatives: fromMap(derivatives).map(d=>({ + ...d, + assets: fromMap(d.assets), + })), }; - - for (let derivative in derivatives){ - iModel.derivatives.push({ - ...derivatives[derivative], - assets: Object.values(derivatives[derivative].assets), - }); - } + if(annotations){ - iModel.annotations = Object.values(annotations); + iModel.annotations = fromMap(annotations); } const idx = document.models.push(iModel) - 1; @@ -32,7 +29,7 @@ export function mapModel({annotations, derivatives, ...iModel} :IModel, index: n annotations: annotations?toIdMap(annotations): undefined, [SOURCE_INDEX]: index, } - for(let derivative of derivatives){ + for(let [index, derivative] of derivatives.entries()){ const id = `${derivative.usage}/${derivative.quality}`; if(!Array.isArray(derivative.assets)){ @@ -40,7 +37,8 @@ export function mapModel({annotations, derivatives, ...iModel} :IModel, index: n } model.derivatives[id] = { ...derivative, - assets: toUriMap(derivative.assets) + assets: toUriMap(derivative.assets), + [SOURCE_INDEX]: index, }; } diff --git a/source/server/utils/merge/pointers/pointers.test.ts b/source/server/utils/merge/pointers/pointers.test.ts index a1b4e308..a3ffbc41 100644 --- a/source/server/utils/merge/pointers/pointers.test.ts +++ b/source/server/utils/merge/pointers/pointers.test.ts @@ -58,22 +58,23 @@ describe("(de)reference pointers", function(){ scene: { units: "m", nodes:{ - "Th8JYtrkNCV6": {id: "Th8JYtrkNCV6", name: "Camera", camera: {type: "perspective"}}, - "aubbHqyLuye2": {id: "aubbHqyLuye2", name: "Lights", children: [{id: "Xm20ZazxwRbP", name:"L1",light:{"type":"ambient"}}]}, - "PeIZ72MDwAGH": {id: "PeIZ72MDwAGH", name: "Model", model: {uri: "model.gltf"}}, + "Th8JYtrkNCV6": {[SOURCE_INDEX]:0, id: "Th8JYtrkNCV6", name: "Camera", camera: {type: "perspective"}}, + "aubbHqyLuye2": {[SOURCE_INDEX]:1, id: "aubbHqyLuye2", name: "Lights", children: [{[SOURCE_INDEX]:0, id: "Xm20ZazxwRbP", name:"L1",light:{"type":"ambient"}}]}, + "PeIZ72MDwAGH": {[SOURCE_INDEX]:2, id: "PeIZ72MDwAGH", name: "Model", model: {derivatives:{"High/Web3D":{assets:{ "model.gltf":{uri: "model.gltf"}}}}}}, }, } }; const doc:IDocument = fromPointers(deref as any); //use JSON.parse(JSON.stringify()) to remove undefined values + expect(doc).to.deep.equal({ ...baseDoc, nodes: [ {id: "Th8JYtrkNCV6", name: "Camera", camera: 0}, - {id: "aubbHqyLuye2", name: "Lights", children: [2]}, + { id: "aubbHqyLuye2", name: "Lights", children: [2]}, {id: "Xm20ZazxwRbP", name: "L1", light: 0}, - {id: "PeIZ72MDwAGH", name: "Model", model: 0}, + { id: "PeIZ72MDwAGH", name: "Model", model: 0}, ], scenes:[{ nodes: [0, 1, 3], @@ -81,7 +82,7 @@ describe("(de)reference pointers", function(){ }], cameras: [{type: "perspective"}], lights: [{type: "ambient"}], - models: [{uri: "model.gltf", derivatives: []}], + models: [{derivatives: [{assets:[{uri: "model.gltf"}]}]}], }); }); @@ -93,7 +94,7 @@ describe("(de)reference pointers", function(){ [SOURCE_INDEX]: 0, units: "cm", derivatives:{ - "High/Web3D":{quality: "High", usage: "Web3D", assets:{"model.gltf": {uri: "model.gltf", type:"Model"}}} + "High/Web3D": {[SOURCE_INDEX]: 0, quality: "High", usage: "Web3D", assets:{ "model.gltf": {[SOURCE_INDEX]: 0, uri: "model.gltf", type:"Model"}}} } }, matrix: [0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,0], @@ -378,6 +379,18 @@ describe("(de)reference pointers", function(){ expect(deref.scene.nodes["ot9vj20DZ6Y5"]).to.have.property("meta").to.deep.equal({collection: {titles:{EN: "Meta Title"}}}); }); + it("SOURCE_INDEX is removed", async function(){ + function walk(obj :any, path:string){ + if( (SOURCE_INDEX in obj)) throw new Error(`${path}: Symbol("SOURCE_INDEX")`); + for(let key in obj){ + if(typeof obj[key] === "object") walk(obj[key], `${path}.${key}`); + } + } + const deref = toPointers(doc); + const redoc = fromPointers(deref); + expect(()=>walk(redoc, "doc")).not.to.throw(); + }); + it("dereferencing is indempotent", async function(){ const deref = toPointers(doc); const redoc = fromPointers(deref); diff --git a/source/server/utils/merge/pointers/scene.ts b/source/server/utils/merge/pointers/scene.ts index d16eb7ff..f03b5a14 100644 --- a/source/server/utils/merge/pointers/scene.ts +++ b/source/server/utils/merge/pointers/scene.ts @@ -3,7 +3,7 @@ import { appendMeta } from "./meta.js"; import { appendNode } from "./node.js"; import { appendSetup } from "./setup.js"; -import { DerefScene } from "./types.js"; +import { DerefScene, fromMap } from "./types.js"; @@ -18,10 +18,10 @@ export function appendScene(document :Required, scene :DerefScene){ if(scene.meta){ iScene.meta = appendMeta(document, scene.meta); } - const nodes = Object.values(scene.nodes ?? {}); + const nodes = fromMap(scene.nodes ?? {}); if(nodes.length){ iScene.nodes = nodes.map((node)=>appendNode(document, node)); - } + } //Order is important: appendSetup relies on nodes being properly inserted. if(scene.setup){ diff --git a/source/server/utils/merge/pointers/setup.test.ts b/source/server/utils/merge/pointers/setup.test.ts index fe75c11e..8e820535 100644 --- a/source/server/utils/merge/pointers/setup.test.ts +++ b/source/server/utils/merge/pointers/setup.test.ts @@ -1,6 +1,6 @@ import { IDocument } from "../../schema/document.js"; import { appendSetup } from "./setup.js"; -import { DerefSetup } from "./types.js"; +import { DerefSetup, SOURCE_INDEX } from "./types.js"; describe("appendSetup()", function(){ @@ -25,7 +25,7 @@ describe("appendSetup()", function(){ // (As defined in ITour schema) const setup :DerefSetup = { tours: { - "tour1": {id: "tour1", title: "Tour Title", steps: {}} + "tour1": {[SOURCE_INDEX]: 0, id: "tour1", title: "Tour Title", steps: {}} }, snapshots: { features: [], diff --git a/source/server/utils/merge/pointers/setup.ts b/source/server/utils/merge/pointers/setup.ts index e51c5fea..308bb6fc 100644 --- a/source/server/utils/merge/pointers/setup.ts +++ b/source/server/utils/merge/pointers/setup.ts @@ -2,18 +2,17 @@ import { IDocument } from "../../schema/document.js"; import { ISetup, ITour, ITourStep } from "../../schema/setup.js"; import uid from "../../uid.js"; import { mapTarget, unmapTarget } from "./snapshot.js"; -import { DerefNode, DerefSetup, DerefSnapshots, DerefTour, IdMap } from "./types.js"; +import { DerefNode, DerefSetup, DerefSnapshots, DerefState, DerefTour, fromMap, IdMap, toIdMap } from "./types.js"; export function appendSetup(document :Required, {tours: toursMap, snapshots, ...setup} :DerefSetup) :number{ let iSetup :ISetup = {...setup}; - const tours = Object.values(toursMap ?? {}); + const tours = fromMap(toursMap ?? {}); if(tours.length){ - iSetup.tours = tours.map(({steps, ...t})=>{ - const tour = t as ITour; - tour.steps = Object.values(steps); - return tour; - }); + iSetup.tours = tours.map(({steps, ...t})=>({ + ...t, + steps: fromMap(steps), + })); } if(snapshots){ @@ -24,7 +23,7 @@ export function appendSetup(document :Required, {tours: toursMap, sna iSetup.snapshots = { ...snapshots, targets, - states: Object.values(snapshots.states ?? {}).map(s=>({ + states: fromMap(snapshots.states ?? {}).map(s=>({ ...s, values: targetStrings.map(t=>s.values[t]) })), @@ -39,18 +38,14 @@ export function appendSetup(document :Required, {tours: toursMap, sna export function mapSetup({tours, snapshots, ...iSetup} :ISetup, nodes :DerefNode[]) :DerefSetup { const setup = { ...iSetup, - tours: (tours?.length ? {} : undefined) as IdMap, + tours: (tours?.length ? toIdMap(tours.map(t=>({ + ...t, + id:t.id??uid(), + steps: toIdMap(t.steps) + }))) : undefined) as IdMap, snapshots: undefined as any as DerefSnapshots, } - - for(let iTour of tours??[]){ - const tour :DerefTour = {...iTour, steps: {}}; - tour.id ??= uid(); - for(let step of iTour.steps ?? []){ - tour.steps[step.id] = step; - } - setup.tours[tour.id] = tour as DerefTour & {id: string}; - } + if(snapshots){ //Nest targets into objects to ease deep merge by node ID const targetNames = snapshots.targets.map(t=>mapTarget(t, nodes)) @@ -62,13 +57,7 @@ export function mapSetup({tours, snapshots, ...iSetup} :ISetup, nodes :DerefNode }, {} as Record>); - setup.snapshots = { - ...snapshots, - targets , - states: {} as DerefSnapshots["states"], - } as DerefSnapshots; - - + const states :DerefState[] = [] for(let state of snapshots?.states??[]){ if(state.values.length != targetNames.length){ throw new Error(`Invalid snapshot states length ${state.values.length} != ${targets.length}`); @@ -79,10 +68,15 @@ export function mapSetup({tours, snapshots, ...iSetup} :ISetup, nodes :DerefNode const key = targetNames[idx]; values[key] = state.values[idx]; } - - setup.snapshots.states[state.id] = {...state, values}; + states.push({...state, values}) } + setup.snapshots = { + ...snapshots, + targets , + states: toIdMap(states), + }; + } return setup; } diff --git a/source/server/utils/merge/pointers/types.test.ts b/source/server/utils/merge/pointers/types.test.ts index 2f5fc280..7fb1ba2e 100644 --- a/source/server/utils/merge/pointers/types.test.ts +++ b/source/server/utils/merge/pointers/types.test.ts @@ -1,18 +1,31 @@ -import { toIdMap } from "./types.js" +import { fromMap, SOURCE_INDEX, toIdMap } from "./types.js" -describe("idMap", function(){ +describe("toIdMap() / fromMap()", function(){ it("uses ID if available", function(){ const map = toIdMap([{id: "foo", name: "bar"}]) - expect(map).to.deep.equal({foo:{id: "foo", name: "bar"}}) + expect(map).to.deep.equal({foo:{id: "foo", name: "bar", [SOURCE_INDEX]: 0}}) }); it("falls back to uri", function(){ const map = toIdMap([{uri: "baz", name: "bar"}]) - expect(map).to.deep.equal({baz:{uri: "baz", name: "bar"}}) + expect(map).to.deep.equal({baz:{uri: "baz", name: "bar", [SOURCE_INDEX]: 0}}) }); it("falls back to name", function(){ const map = toIdMap([{name: "bar"}]) - expect(map).to.deep.equal({bar:{name: "bar"}}) + expect(map).to.deep.equal({bar:{name: "bar", [SOURCE_INDEX]: 0}}) }); -}); \ No newline at end of file + + it("keeps arrays ordering", function(){ + let init = [{id: "c"}, {id: "a"}, {id: "b"}]; + const m = toIdMap(init); + //Try to mess it up + expect(m).to.have.property("c"); + let ref = m["c"]; + delete m.c; + m["c"] = ref; + + const arr = fromMap(m); + expect(arr).to.deep.equal([{id: "c"}, {id: "a"}, {id: "b"}]); + }); +}); diff --git a/source/server/utils/merge/pointers/types.ts b/source/server/utils/merge/pointers/types.ts index 66cf8df9..a790e50a 100644 --- a/source/server/utils/merge/pointers/types.ts +++ b/source/server/utils/merge/pointers/types.ts @@ -9,20 +9,44 @@ import uid from "../../uid.js"; /** * Special symbol to track original index within an array + * We lie about its type to simplify usage as an indexed value */ -export const SOURCE_INDEX = Symbol("SOURCE_INDEX"); +export const SOURCE_INDEX :"_SOURCE_INDEX" = Symbol("_SOURCE_INDEX") as any; export interface Indexed{ [SOURCE_INDEX]: number; } +/** + * Inserts an index as a hidden Symbol property in an object + * Since it's a symbol it will never appear in `Object.keys()` or `for .. in` loops. + * It's enumerable so it can be obtained through `Object.assign` or Object spread. + * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties#traversing_object_properties + * @param obj obj to insert an index into + * @param index + * @returns + */ +export function withIndex(obj :T, index: number):Indexed&T{ + return Object.defineProperty(obj, SOURCE_INDEX, { + configurable: true, //May be deleted + enumerable: true, + value: index + }) as Indexed&T; +} + /** * Special symbol to mark a field for deletion in a diff object */ export const DELETE_KEY = Symbol("_DELETE_KEY"); -export type Diff = { - [K in keyof T]?: typeof DELETE_KEY|Diff|T[K]|Record; -} + +export type Diff> = { + [K in keyof T]?: + | typeof DELETE_KEY + | Diff + | T[K] + | Record + | (K extends keyof T ? number : never); +}; /** * IScene where all indexed references were replaced by pointers to the actual objects. @@ -69,6 +93,13 @@ export interface DerefTour{ taglist?: Dictionary; } +export interface DerefState{ + id: string; + curve: string; + duration: number; + threshold: number; + values: Record; +} /** * A snapshot is a set of states that can be restored to the scene. @@ -78,13 +109,7 @@ export interface DerefTour{ export interface DerefSnapshots{ features: string[]; targets :Record>; - states: IdMap<{ - id: string; - curve: string; - duration: number; - threshold: number; - values: Record; - }>; + states: IdMap; } export interface DerefMeta extends Omit { @@ -121,51 +146,58 @@ export interface DerefDerivative extends Omit{ // Maps are replacing arrays of identified nodes with a map-by-id export type AbstractMap = { - [id: string]: T; + [id: string]: Indexed & T; } type MappableType = {id:string} | {uri:string} | {name?:string}; -export type IdMap = { - [id: string]: T; -} +export type IdMap = AbstractMap; export function toIdMap(arr :T[]) :IdMap{ const map = {} as IdMap; - for(let item of arr){ + for(let [index, item] of arr.entries()){ const id :string = (item as any).id || (item as any).uri || (item as any).name || uid(); - map[id] = item; + map[id] = withIndex(item, index); } return map; } -export type NameMap = { - [name: string]: T; -} +export type NameMap = IdMap; export function toNameMap(arr :T[]) :NameMap{ const map = {} as NameMap; - for(let item of arr){ - map[item.name] = item; + for(let [index, item] of arr.entries()){ + map[item.name] = withIndex(item, index); } return map; } - -export type UriMap = { - [uri: string]: T; -} +export type UriMap = IdMap; export function toUriMap(arr :T[]) :UriMap{ const map = {} as UriMap; - for(let item of arr){ - map[item.uri] = item; + for(let [index, item] of arr.entries()){ + map[item.uri] = withIndex(item, index); } return map; } +export function fromMap(map :IdMap):T[] +export function fromMap(map :AbstractMap):T[] +export function fromMap(map :AbstractMap):T[]{ + return restoreIndex(Object.values(map)); +} + +/** Sort an array of symbol-indexed items to restore their initial order */ +export function restoreIndex(src: Array):Array{ + return src.sort((a, b)=>a[SOURCE_INDEX] -b[SOURCE_INDEX]).map((item: T & Indexed)=>{ + delete (item as T & Partial)[SOURCE_INDEX]; + return item as T; + }); +} + /** * A document where all indexed references were replaced by pointers to the actual objects. * @see DerefNode for nodes