Skip to content

Commit

Permalink
fix a bug where null elements in an array would cause structured scen…
Browse files Browse the repository at this point in the history
…e merge to fail
  • Loading branch information
sdumetz committed Jan 20, 2025
1 parent dada2a9 commit 87ad5f8
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 6 deletions.
3 changes: 3 additions & 0 deletions source/server/utils/merge/apply.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ describe("merge.apply()", function(){
it("merges updated arrays", function(){
expect(apply({a:[1,2]}, {a:{0:1, 1:3}})).to.deep.equal({a:[1,3]});
});
it("merges array of null", function(){
expect(apply({a:[null,null]}, {a:{0:null, 1:3}})).to.deep.equal({a:[null,3]});
});

it("merges created arrays", function(){
expect(apply({a:"A"}, { b:[{name:"A1"}]} as any)).to.deep.equal({a:"A", b:[{name:"A1"}]});
Expand Down
2 changes: 2 additions & 0 deletions source/server/utils/merge/apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export default function apply<T extends Record<string, any>>(into :T, ...diffs :

}else if(typeof value !== "object" /*primitive*/){
into[key] = value;
}else if(value == null /* null is typeof object*/){
into[key] = value;
}else if(Array.isArray(value)){
//Replace arrays without looking.
//This will generally not happen unles there is an exception in diff() that says so.
Expand Down
9 changes: 9 additions & 0 deletions source/server/utils/merge/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ describe("merge.diff()", function(){
{v: {a: 1, [SOURCE_INDEX]: 2 }}
);
expect(res).to.deep.equal({v:{[SOURCE_INDEX]: 2}});
});

it("handles key renames", function(){
let res = diff<any>(
{a: {name: "foo"}},
{b: {name: "foo"}}
);
expect(res).to.have.property("a", DELETE_KEY);
expect(res).to.have.deep.property("b", {name: "foo"});
})
});
});
5 changes: 0 additions & 5 deletions source/server/utils/merge/merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,10 @@ describe("fast-forward", function(){
});

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<any>(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"}]);
});
Expand Down
2 changes: 1 addition & 1 deletion source/server/utils/merge/pointers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function cleanDocument(document :Required<IDocument>) :IDocument{
* @fixme should be simplified by exporting most of its code into separate functions, like in `fromPointers()`
*/
export function toPointers(src :IDocument) :DerefDocument {

if(!src) throw new Error(`Can't dereference invalid document ${src}`);
// Dereference self-contained collections
const metas = src.metas?.map(mapMeta) ?? [];
const models = src.models?.map(mapModel)?? [];
Expand Down
6 changes: 6 additions & 0 deletions source/server/utils/merge/pointers/pointers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,12 @@ describe("(de)reference pointers", function(){
});

describe("throws", function(){
it("if no doc is provided", function(){
expect(()=> toPointers(null as any)).to.throw("Can't dereference invalid document null");
expect(()=> toPointers(undefined as any)).to.throw("Can't dereference invalid document undefined");
expect(()=> toPointers("" as any)).to.throw("Can't dereference invalid document ");
});

it("if doc has no scene", function(){
const doc = {...baseDoc, scenes: []};
expect(()=> toPointers(doc)).to.throw("Document has no valid scene");
Expand Down

0 comments on commit 87ad5f8

Please sign in to comment.