From 5c294ccf7e11c6fad4f67b0374e02892f0ca49ae Mon Sep 17 00:00:00 2001 From: streamich Date: Sat, 25 Nov 2023 15:05:54 +0100 Subject: [PATCH 1/2] =?UTF-8?q?fix(json-crdt):=20=F0=9F=90=9B=20make=20sur?= =?UTF-8?q?e=20obj,=20vec,=20arr,=20and=20val=20nodes=20don't=20throw=20wh?= =?UTF-8?q?en=20deleted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../__tests__/Model.node-deletion.spec.ts | 76 +++++++++++++++++++ src/json-crdt/nodes/arr/ArrNode.ts | 11 ++- src/json-crdt/nodes/obj/ObjNode.ts | 9 ++- src/json-crdt/nodes/val/ValNode.ts | 4 +- src/json-crdt/nodes/vec/VecNode.ts | 3 +- 5 files changed, 95 insertions(+), 8 deletions(-) diff --git a/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts b/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts index 45285a71fa..78f19b35e3 100644 --- a/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts +++ b/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts @@ -1,3 +1,4 @@ +import {s} from '../../../json-crdt-patch'; import {ValNode} from '../../nodes'; import {Model} from '../Model'; @@ -160,3 +161,78 @@ test('removes from index recursively after LWW register write', () => { expect(!!doc.index.get(val3)).toBe(false); expect(!!doc.index.get(val4)).toBe(false); }); + +test('calling .view() on dangling "obj" when it was deleted, should not throw', () => { + const doc = Model.withLogicalClock() + .setSchema(s.obj({ + foo: s.obj({ + bar: s.obj({ + baz: s.con(123), + qux: s.str('asdf'), + }), + }), + })); + + const bar = doc.root.child()!.get('foo')!.get('bar')!; + const baz = bar.get('baz')!; + const qux = bar.get('qux')!; + expect(bar.view()).toStrictEqual({ + baz: 123, + qux: 'asdf', + }); + doc.api.obj(['foo']).del(['bar']); + expect(bar.view()).toStrictEqual({}); + expect((bar + '').includes(bar.id.time + '')).toBe(true); + expect(baz.view()).toBe(123); + expect(qux.view()).toBe('asdf'); +}); + +test('calling .view() on dangling "arr" when it was deleted, should not throw', () => { + const doc = Model.withLogicalClock() + .setSchema(s.obj({ + foo: s.obj({ + bar: s.arr([ + s.con(123), + s.str('asdf'), + ]), + }), + })); + const bar = doc.root.child()!.get('foo')!.get('bar')!; + expect(bar.view()).toStrictEqual([123, 'asdf']); + doc.api.obj(['foo']).del(['bar']); + expect(bar.view()).toStrictEqual([]); + expect((bar + '').includes(bar.id.time + '')).toBe(true); +}); + +test('calling .view() on dangling "vec" when it was deleted, should not throw', () => { + const doc = Model.withLogicalClock() + .setSchema(s.obj({ + foo: s.obj({ + bar: s.vec( + s.con(123), + s.str('asdf'), + ), + }), + })); + const bar = doc.root.child()!.get('foo')!.get('bar')!; + expect(bar.view()).toStrictEqual([123, 'asdf']); + doc.api.obj(['foo']).del(['bar']); + expect(bar.view()).toStrictEqual([undefined, undefined]); + expect((bar + '').includes(bar.id.time + '')).toBe(true); +}); + +test('calling .view() on dangling "val" when it was deleted, should not throw', () => { + const doc = Model.withLogicalClock() + .setSchema(s.obj({ + foo: s.obj({ + bar: s.val( + s.str('asdf'), + ), + }), + })); + const bar = doc.root.child()!.get('foo')!.get('bar')!; + expect(bar.view()).toStrictEqual('asdf'); + doc.api.obj(['foo']).del(['bar']); + expect(bar.view()).toStrictEqual(undefined); + expect((bar + '').includes(bar.id.time + '')).toBe(true); +}); diff --git a/src/json-crdt/nodes/arr/ArrNode.ts b/src/json-crdt/nodes/arr/ArrNode.ts index 3ab18ac511..1766c87c02 100644 --- a/src/json-crdt/nodes/arr/ArrNode.ts +++ b/src/json-crdt/nodes/arr/ArrNode.ts @@ -155,7 +155,12 @@ export class ArrNode for (let chunk = this.first(); chunk; chunk = this.next(chunk)) { if (chunk.del) continue; for (const node of chunk.data!) { - const element = index.get(node)!.view() as JsonNodeView; + const elementNode = index.get(node); + if (!elementNode) { + useCache = false; + continue; + } + const element = elementNode.view() as JsonNodeView; if (_view[view.length] !== element) useCache = false; view.push(element); } @@ -188,8 +193,8 @@ export class ArrNode const index = this.doc.index; valueTree = printTree( tab, - chunk.data!.map( - (id, i) => (tab) => `[${pos + i}]: ${index.get(id)!.toString(tab + ' ' + ' '.repeat(String(i).length))}`, + chunk.data!.map((id) => index.get(id)).filter((node) => !!node).map( + (node, i) => (tab) => `[${pos + i}]: ${node!.toString(tab + ' ' + ' '.repeat(String(i).length))}`, ), ); } diff --git a/src/json-crdt/nodes/obj/ObjNode.ts b/src/json-crdt/nodes/obj/ObjNode.ts index db0e089366..5c5496c292 100644 --- a/src/json-crdt/nodes/obj/ObjNode.ts +++ b/src/json-crdt/nodes/obj/ObjNode.ts @@ -111,7 +111,12 @@ export class ObjNode = Record { - const value = index.get(id)!.view(); + const valueNode = index.get(id); + if (!valueNode) { + useCache = false; + return; + } + const value = valueNode.view(); if (value !== undefined) { if (_view[key] !== value) useCache = false; (view)[key] = value; @@ -137,7 +142,7 @@ export class ObjNode = Record !!this.doc.index.get(id)).map( ([key, id]) => (tab) => JSON.stringify(key) + printTree(tab + ' ', [(tab) => this.doc.index.get(id)!.toString(tab)]), diff --git a/src/json-crdt/nodes/val/ValNode.ts b/src/json-crdt/nodes/val/ValNode.ts index 7534f9735b..84122a87b1 100644 --- a/src/json-crdt/nodes/val/ValNode.ts +++ b/src/json-crdt/nodes/val/ValNode.ts @@ -45,13 +45,13 @@ export class ValNode implements JsonNodeUNDEFINED : this.child()!; + return this.val.sid === SESSION.SYSTEM ? UNDEFINED : this.child(); } // ----------------------------------------------------------------- JsonNode public view(): Readonly> { - return this.node().view() as Readonly>; + return this.node()?.view() as Readonly>; } /** diff --git a/src/json-crdt/nodes/vec/VecNode.ts b/src/json-crdt/nodes/vec/VecNode.ts index 88bc764bfb..a04997d760 100644 --- a/src/json-crdt/nodes/vec/VecNode.ts +++ b/src/json-crdt/nodes/vec/VecNode.ts @@ -183,12 +183,13 @@ export class VecNode if (extNode) { return this.child()!.toString(tab); } + const index = this.doc.index; return ( header + printTree(tab, [ ...this.elements.map( (id, i) => (tab: string) => - `${i}: ${!id ? 'nil' : this.doc.index.get(id)!.toString(tab + ' ' + ' '.repeat(('' + i).length))}`, + `${i}: ${!id ? 'nil' : index.get(id) ? index.get(id)!.toString(tab + ' ' + ' '.repeat(('' + i).length)) : 'nil'}`, ), ...(extNode ? [(tab: string) => `${this.child()!.toString(tab)}`] : []), ]) From 397468d54f6dd1d67a4c2185fa4d6c987bff6e8c Mon Sep 17 00:00:00 2001 From: streamich Date: Sat, 25 Nov 2023 15:06:47 +0100 Subject: [PATCH 2/2] =?UTF-8?q?style(json-crdt):=20=F0=9F=92=84=20run=20Pr?= =?UTF-8?q?ettier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../__tests__/Model.node-deletion.spec.ts | 44 +++++++++---------- src/json-crdt/nodes/arr/ArrNode.ts | 7 +-- src/json-crdt/nodes/obj/ObjNode.ts | 12 ++--- src/json-crdt/nodes/vec/VecNode.ts | 4 +- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts b/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts index 78f19b35e3..292c750d1d 100644 --- a/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts +++ b/src/json-crdt/model/__tests__/Model.node-deletion.spec.ts @@ -163,16 +163,17 @@ test('removes from index recursively after LWW register write', () => { }); test('calling .view() on dangling "obj" when it was deleted, should not throw', () => { - const doc = Model.withLogicalClock() - .setSchema(s.obj({ + const doc = Model.withLogicalClock().setSchema( + s.obj({ foo: s.obj({ bar: s.obj({ baz: s.con(123), qux: s.str('asdf'), }), }), - })); - + }), + ); + const bar = doc.root.child()!.get('foo')!.get('bar')!; const baz = bar.get('baz')!; const qux = bar.get('qux')!; @@ -188,15 +189,13 @@ test('calling .view() on dangling "obj" when it was deleted, should not throw', }); test('calling .view() on dangling "arr" when it was deleted, should not throw', () => { - const doc = Model.withLogicalClock() - .setSchema(s.obj({ + const doc = Model.withLogicalClock().setSchema( + s.obj({ foo: s.obj({ - bar: s.arr([ - s.con(123), - s.str('asdf'), - ]), + bar: s.arr([s.con(123), s.str('asdf')]), }), - })); + }), + ); const bar = doc.root.child()!.get('foo')!.get('bar')!; expect(bar.view()).toStrictEqual([123, 'asdf']); doc.api.obj(['foo']).del(['bar']); @@ -205,15 +204,13 @@ test('calling .view() on dangling "arr" when it was deleted, should not throw', }); test('calling .view() on dangling "vec" when it was deleted, should not throw', () => { - const doc = Model.withLogicalClock() - .setSchema(s.obj({ + const doc = Model.withLogicalClock().setSchema( + s.obj({ foo: s.obj({ - bar: s.vec( - s.con(123), - s.str('asdf'), - ), + bar: s.vec(s.con(123), s.str('asdf')), }), - })); + }), + ); const bar = doc.root.child()!.get('foo')!.get('bar')!; expect(bar.view()).toStrictEqual([123, 'asdf']); doc.api.obj(['foo']).del(['bar']); @@ -222,14 +219,13 @@ test('calling .view() on dangling "vec" when it was deleted, should not throw', }); test('calling .view() on dangling "val" when it was deleted, should not throw', () => { - const doc = Model.withLogicalClock() - .setSchema(s.obj({ + const doc = Model.withLogicalClock().setSchema( + s.obj({ foo: s.obj({ - bar: s.val( - s.str('asdf'), - ), + bar: s.val(s.str('asdf')), }), - })); + }), + ); const bar = doc.root.child()!.get('foo')!.get('bar')!; expect(bar.view()).toStrictEqual('asdf'); doc.api.obj(['foo']).del(['bar']); diff --git a/src/json-crdt/nodes/arr/ArrNode.ts b/src/json-crdt/nodes/arr/ArrNode.ts index 1766c87c02..6b42013a0f 100644 --- a/src/json-crdt/nodes/arr/ArrNode.ts +++ b/src/json-crdt/nodes/arr/ArrNode.ts @@ -193,9 +193,10 @@ export class ArrNode const index = this.doc.index; valueTree = printTree( tab, - chunk.data!.map((id) => index.get(id)).filter((node) => !!node).map( - (node, i) => (tab) => `[${pos + i}]: ${node!.toString(tab + ' ' + ' '.repeat(String(i).length))}`, - ), + chunk + .data!.map((id) => index.get(id)) + .filter((node) => !!node) + .map((node, i) => (tab) => `[${pos + i}]: ${node!.toString(tab + ' ' + ' '.repeat(String(i).length))}`), ); } return ( diff --git a/src/json-crdt/nodes/obj/ObjNode.ts b/src/json-crdt/nodes/obj/ObjNode.ts index 5c5496c292..88121979d2 100644 --- a/src/json-crdt/nodes/obj/ObjNode.ts +++ b/src/json-crdt/nodes/obj/ObjNode.ts @@ -142,11 +142,13 @@ export class ObjNode = Record !!this.doc.index.get(id)).map( - ([key, id]) => - (tab) => - JSON.stringify(key) + printTree(tab + ' ', [(tab) => this.doc.index.get(id)!.toString(tab)]), - ), + [...this.keys.entries()] + .filter(([, id]) => !!this.doc.index.get(id)) + .map( + ([key, id]) => + (tab) => + JSON.stringify(key) + printTree(tab + ' ', [(tab) => this.doc.index.get(id)!.toString(tab)]), + ), ) ); } diff --git a/src/json-crdt/nodes/vec/VecNode.ts b/src/json-crdt/nodes/vec/VecNode.ts index a04997d760..dab0721bef 100644 --- a/src/json-crdt/nodes/vec/VecNode.ts +++ b/src/json-crdt/nodes/vec/VecNode.ts @@ -189,7 +189,9 @@ export class VecNode printTree(tab, [ ...this.elements.map( (id, i) => (tab: string) => - `${i}: ${!id ? 'nil' : index.get(id) ? index.get(id)!.toString(tab + ' ' + ' '.repeat(('' + i).length)) : 'nil'}`, + `${i}: ${ + !id ? 'nil' : index.get(id) ? index.get(id)!.toString(tab + ' ' + ' '.repeat(('' + i).length)) : 'nil' + }`, ), ...(extNode ? [(tab: string) => `${this.child()!.toString(tab)}`] : []), ])