Merge pull request #370 from streamich/recursion

streamich · web-flow · commit 950ffc6ea537 · 2023-11-05T14:33:01.000+01:00
JSON CRDT Patch prevent `new_val` recursion
diff --git a/package.json b/package.json
@@ -61,7 +61,8 @@
     "coverage": "yarn test --collectCoverage",
     "typedoc": "typedoc",
     "build:pages": "rimraf gh-pages && mkdir -p gh-pages && cp -r typedocs/* gh-pages && cp -r coverage gh-pages/coverage",
-    "deploy:pages": "gh-pages -d gh-pages"
+    "deploy:pages": "gh-pages -d gh-pages",
+    "publish-coverage-and-typedocs": "yarn typedoc && yarn coverage && yarn build:pages && yarn deploy:pages"
   },
   "keywords": [],
   "peerDependencies": {
diff --git a/src/json-crdt-patch/Patch.ts b/src/json-crdt-patch/Patch.ts
@@ -1,20 +1,4 @@
-import {
-  NewConOp,
-  NewObjOp,
-  NewValOp,
-  NewVecOp,
-  NewStrOp,
-  NewBinOp,
-  NewArrOp,
-  InsValOp,
-  InsObjOp,
-  InsVecOp,
-  InsStrOp,
-  InsBinOp,
-  InsArrOp,
-  DelOp,
-  NopOp,
-} from './operations';
+import * as operations from './operations';
 import {ITimestampStruct, ts, toDisplayString} from './clock';
 import {SESSION} from './constants';
 import {encode, decode} from './codec/binary';
@@ -24,21 +8,21 @@ import type {Printable} from '../util/print/types';
  * A union type of all possible JSON CRDT patch operations.
  */
 export type JsonCrdtPatchOperation =
-  | NewConOp
-  | NewValOp
-  | NewVecOp
-  | NewObjOp
-  | NewStrOp
-  | NewBinOp
-  | NewArrOp
-  | InsValOp
-  | InsObjOp
-  | InsVecOp
-  | InsStrOp
-  | InsBinOp
-  | InsArrOp
-  | DelOp
-  | NopOp;
+  | operations.NewConOp
+  | operations.NewValOp
+  | operations.NewVecOp
+  | operations.NewObjOp
+  | operations.NewStrOp
+  | operations.NewBinOp
+  | operations.NewArrOp
+  | operations.InsValOp
+  | operations.InsObjOp
+  | operations.InsVecOp
+  | operations.InsStrOp
+  | operations.InsBinOp
+  | operations.InsArrOp
+  | operations.DelOp
+  | operations.NopOp;
 
 /**
  * Represents a JSON CRDT patch.
@@ -135,27 +119,31 @@ export class Patch implements Printable {
     const patchOps = patch.ops;
     for (let i = 0; i < length; i++) {
       const op = ops[i];
-      if (op instanceof DelOp) patchOps.push(new DelOp(ts(op.id), ts(op.obj), op.what));
-      else if (op instanceof NewConOp) patchOps.push(new NewConOp(ts(op.id), op.val));
-      else if (op instanceof NewVecOp) patchOps.push(new NewVecOp(ts(op.id)));
-      else if (op instanceof NewValOp) patchOps.push(new NewValOp(ts(op.id), ts(op.val)));
-      else if (op instanceof NewObjOp) patchOps.push(new NewObjOp(ts(op.id)));
-      else if (op instanceof NewStrOp) patchOps.push(new NewStrOp(ts(op.id)));
-      else if (op instanceof NewBinOp) patchOps.push(new NewBinOp(ts(op.id)));
-      else if (op instanceof NewArrOp) patchOps.push(new NewArrOp(ts(op.id)));
-      else if (op instanceof InsArrOp) patchOps.push(new InsArrOp(ts(op.id), ts(op.obj), ts(op.ref), op.data.map(ts)));
-      else if (op instanceof InsStrOp) patchOps.push(new InsStrOp(ts(op.id), ts(op.obj), ts(op.ref), op.data));
-      else if (op instanceof InsBinOp) patchOps.push(new InsBinOp(ts(op.id), ts(op.obj), ts(op.ref), op.data));
-      else if (op instanceof InsValOp) patchOps.push(new InsValOp(ts(op.id), ts(op.obj), ts(op.val)));
-      else if (op instanceof InsObjOp)
+      if (op instanceof operations.DelOp) patchOps.push(new operations.DelOp(ts(op.id), ts(op.obj), op.what));
+      else if (op instanceof operations.NewConOp) patchOps.push(new operations.NewConOp(ts(op.id), op.val));
+      else if (op instanceof operations.NewVecOp) patchOps.push(new operations.NewVecOp(ts(op.id)));
+      else if (op instanceof operations.NewValOp) patchOps.push(new operations.NewValOp(ts(op.id)));
+      else if (op instanceof operations.NewObjOp) patchOps.push(new operations.NewObjOp(ts(op.id)));
+      else if (op instanceof operations.NewStrOp) patchOps.push(new operations.NewStrOp(ts(op.id)));
+      else if (op instanceof operations.NewBinOp) patchOps.push(new operations.NewBinOp(ts(op.id)));
+      else if (op instanceof operations.NewArrOp) patchOps.push(new operations.NewArrOp(ts(op.id)));
+      else if (op instanceof operations.InsArrOp)
+        patchOps.push(new operations.InsArrOp(ts(op.id), ts(op.obj), ts(op.ref), op.data.map(ts)));
+      else if (op instanceof operations.InsStrOp)
+        patchOps.push(new operations.InsStrOp(ts(op.id), ts(op.obj), ts(op.ref), op.data));
+      else if (op instanceof operations.InsBinOp)
+        patchOps.push(new operations.InsBinOp(ts(op.id), ts(op.obj), ts(op.ref), op.data));
+      else if (op instanceof operations.InsValOp)
+        patchOps.push(new operations.InsValOp(ts(op.id), ts(op.obj), ts(op.val)));
+      else if (op instanceof operations.InsObjOp)
         patchOps.push(
-          new InsObjOp(
+          new operations.InsObjOp(
             ts(op.id),
             ts(op.obj),
             op.data.map(([key, value]) => [key, ts(value)]),
           ),
         );
-      else if (op instanceof NopOp) patchOps.push(new NopOp(ts(op.id), op.len));
+      else if (op instanceof operations.NopOp) patchOps.push(new operations.NopOp(ts(op.id), op.len));
     }
     return patch;
   }
diff --git a/src/json-crdt-patch/PatchBuilder.ts b/src/json-crdt-patch/PatchBuilder.ts
@@ -154,11 +154,12 @@ export class PatchBuilder {
    *
    * @param val Reference to another object.
    * @returns ID of the new operation.
+   * @todo Rename to `newVal`.
    */
-  public val(val: ITimestampStruct): ITimestampStruct {
+  public val(): ITimestampStruct {
     this.pad();
     const id = this.clock.tick(1);
-    this.patch.ops.push(new NewValOp(id, val));
+    this.patch.ops.push(new NewValOp(id));
     return id;
   }
 
@@ -210,6 +211,7 @@ export class PatchBuilder {
    * Set value of a "val" object.
    *
    * @returns ID of the new operation.
+   * @todo Rename to "insVal".
    */
   public setVal(obj: ITimestampStruct, val: ITimestampStruct): ITimestampStruct {
     this.pad();
@@ -349,8 +351,10 @@ export class PatchBuilder {
    * Run builder commands to create a JSON value.
    */
   public jsonVal(value: unknown): ITimestampStruct {
+    const valId = this.val();
     const id = this.const(value);
-    return this.val(id);
+    this.setVal(valId, id);
+    return valId;
   }
 
   /**
diff --git a/src/json-crdt-patch/builder/schema.ts b/src/json-crdt-patch/builder/schema.ts
@@ -32,8 +32,10 @@ export namespace nodes {
 
     constructor(public readonly value: T) {
       super((builder) => {
+        const valId = builder.val();
         const valueId = value.build(builder);
-        return builder.val(valueId);
+        builder.setVal(valId, valueId);
+        return valId;
       });
     }
   }
diff --git a/src/json-crdt-patch/codec/__tests__/PatchFuzzer.ts b/src/json-crdt-patch/codec/__tests__/PatchFuzzer.ts
@@ -28,7 +28,7 @@ export class PatchFuzzer extends Fuzzer {
         () => builder.arr(),
         () => builder.str(),
         () => builder.bin(),
-        () => builder.val(ts()),
+        () => builder.val(),
         () => builder.const(RandomJson.generate()),
         () => builder.root(ts()),
         () =>
diff --git a/src/json-crdt-patch/codec/binary/Decoder.ts b/src/json-crdt-patch/codec/binary/Decoder.ts
@@ -73,7 +73,7 @@ export class Decoder extends CborDecoder<CrdtReader> {
         break;
       }
       case JsonCrdtPatchOpcode.new_val: {
-        builder.val(this.decodeId());
+        builder.val();
         break;
       }
       case JsonCrdtPatchOpcode.new_obj: {
diff --git a/src/json-crdt-patch/codec/binary/Encoder.ts b/src/json-crdt-patch/codec/binary/Encoder.ts
@@ -94,10 +94,7 @@ export class Encoder extends CborEncoder<CrdtWriter> {
         break;
       }
       case operations.NewValOp: {
-        const operation = <operations.NewValOp>op;
-        const val = operation.val;
         writer.u8(JsonCrdtPatchOpcode.new_val);
-        this.encodeId(val);
         break;
       }
       case operations.NewObjOp: {
diff --git a/src/json-crdt-patch/codec/compact/decode.ts b/src/json-crdt-patch/codec/compact/decode.ts
@@ -38,7 +38,7 @@ export const decode = (data: types.CompactCodecPatch): Patch => {
         break;
       }
       case JsonCrdtPatchOpcode.new_val: {
-        builder.val(timestamp(sid, op[1]));
+        builder.val();
         break;
       }
       case JsonCrdtPatchOpcode.new_obj: {
diff --git a/src/json-crdt-patch/codec/compact/encode.ts b/src/json-crdt-patch/codec/compact/encode.ts
@@ -47,7 +47,7 @@ export const encode = (patch: Patch): types.CompactCodecPatch => {
         res.push([JsonCrdtPatchOpcode.new_con, val]);
       }
     } else if (op instanceof operations.NewValOp) {
-      res.push([JsonCrdtPatchOpcode.new_val, timestamp(sid, op.val)]);
+      res.push([JsonCrdtPatchOpcode.new_val]);
     } else if (op instanceof operations.NewObjOp) {
       res.push([JsonCrdtPatchOpcode.new_obj]);
     } else if (op instanceof operations.NewVecOp) {
diff --git a/src/json-crdt-patch/codec/verbose/__tests__/bug-1.spec.ts b/src/json-crdt-patch/codec/verbose/__tests__/bug-1.spec.ts
@@ -9,7 +9,6 @@ const encoded1 = {
     },
     {
       op: 'new_val',
-      value: 1,
     },
     {
       op: 'new_bin',
@@ -22,7 +21,6 @@ const encoded1 = {
     },
     {
       op: 'new_val',
-      value: 2,
     },
     {
       op: 'ins_obj',
diff --git a/src/json-crdt-patch/codec/verbose/decode.ts b/src/json-crdt-patch/codec/verbose/decode.ts
@@ -30,7 +30,7 @@ export const decode = (data: types.JsonCodecPatch): Patch => {
         break;
       }
       case 'new_val': {
-        builder.val(decodeId(op.value));
+        builder.val();
         break;
       }
       case 'new_obj': {
diff --git a/src/json-crdt-patch/codec/verbose/encode.ts b/src/json-crdt-patch/codec/verbose/encode.ts
@@ -45,7 +45,7 @@ export const encode = (patch: Patch): types.JsonCodecPatch => {
     } else if (op instanceof operations.NewBinOp) {
       ops.push({op: 'new_bin'});
     } else if (op instanceof operations.NewValOp) {
-      ops.push({op: 'new_val', value: encodeTimestamp(op.val)});
+      ops.push({op: 'new_val'});
     } else if (op instanceof operations.InsObjOp) {
       ops.push({
         op: 'ins_obj',
diff --git a/src/json-crdt-patch/codec/verbose/types.ts b/src/json-crdt-patch/codec/verbose/types.ts
@@ -119,12 +119,7 @@ export interface JsonCodecNewConOperation extends JsonCodecOperationBase<'new_co
  * Operation which creates a new "val" CRDT data type, which is a
  * Last-Write-Wins Register of a pointer to another CRDT data type.
  */
-export interface JsonCodecNewValOperation extends JsonCodecOperationBase<'new_val'> {
-  /**
-   * ID of the "val" LWW-Register object latest value.
-   */
-  value: JsonCodecTimestamp;
-}
+export type JsonCodecNewValOperation = JsonCodecOperationBase<'new_val'>;
 
 /**
  * Operation which creates a new "object" CRDT data type, which is a map of
diff --git a/src/json-crdt-patch/operations.ts b/src/json-crdt-patch/operations.ts
@@ -37,7 +37,7 @@ export class NewConOp implements IJsonCrdtPatchOperation {
  * @category Operations
  */
 export class NewValOp implements IJsonCrdtPatchOperation {
-  constructor(public readonly id: ITimestampStruct, public readonly val: ITimestampStruct) {}
+  constructor(public readonly id: ITimestampStruct) {}
 
   public span(): number {
     return 1;
@@ -48,7 +48,7 @@ export class NewValOp implements IJsonCrdtPatchOperation {
   }
 
   public toString(): string {
-    return `"${this.name()}" ${toDisplayString(this.id)} { ${toDisplayString(this.val)} }`;
+    return `"${this.name()}" ${toDisplayString(this.id)}`;
   }
 }
 
@@ -165,6 +165,7 @@ export class NewArrOp implements IJsonCrdtPatchOperation {
 export class InsValOp implements IJsonCrdtPatchEditOperation {
   constructor(
     public readonly id: ITimestampStruct,
+    /** @todo Rename to `node`. */
     public readonly obj: ITimestampStruct,
     public readonly val: ITimestampStruct,
   ) {}
diff --git a/src/json-crdt/__tests__/recursion.spec.ts b/src/json-crdt/__tests__/recursion.spec.ts
diff --git a/src/json-crdt/model/Model.ts b/src/json-crdt/model/Model.ts
diff --git a/src/json-crdt/model/__tests__/Model.server-clock.spec.ts b/src/json-crdt/model/__tests__/Model.server-clock.spec.ts
diff --git a/src/json-crdt/model/__tests__/Model.value.spec.ts b/src/json-crdt/model/__tests__/Model.value.spec.ts
diff --git a/src/json-crdt/nodes/root/RootNode.ts b/src/json-crdt/nodes/root/RootNode.ts
diff --git a/src/json-crdt/nodes/val/ValNode.ts b/src/json-crdt/nodes/val/ValNode.ts

Original file line number	Diff line number	Diff line change
`@@ -32,8 +32,10 @@ export namespace nodes {`
`32`	`32`
`33`	`33`	`constructor(public readonly value: T) {`
`34`	`34`	`super((builder) => {`
	`35`	`+ const valId = builder.val();`
`35`	`36`	`const valueId = value.build(builder);`
`36`		`- return builder.val(valueId);`
	`37`	`+ builder.setVal(valId, valueId);`
	`38`	`+ return valId;`
`37`	`39`	`});`
`38`	`40`	`}`
`39`	`41`	`}`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ export class Decoder extends CborDecoder<CrdtReader> {`
`73`	`73`	`break;`
`74`	`74`	`}`
`75`	`75`	`case JsonCrdtPatchOpcode.new_val: {`
`76`		`- builder.val(this.decodeId());`
	`76`	`+ builder.val();`
`77`	`77`	`break;`
`78`	`78`	`}`
`79`	`79`	`case JsonCrdtPatchOpcode.new_obj: {`
Original file line number	Diff line number	Diff line change
`@@ -94,10 +94,7 @@ export class Encoder extends CborEncoder<CrdtWriter> {`
`94`	`94`	`break;`
`95`	`95`	`}`
`96`	`96`	`case operations.NewValOp: {`
`97`		`- const operation = <operations.NewValOp>op;`
`98`		`- const val = operation.val;`
`99`	`97`	`writer.u8(JsonCrdtPatchOpcode.new_val);`
`100`		`- this.encodeId(val);`
`101`	`98`	`break;`
`102`	`99`	`}`
`103`	`100`	`case operations.NewObjOp: {`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ export const decode = (data: types.CompactCodecPatch): Patch => {`
`38`	`38`	`break;`
`39`	`39`	`}`
`40`	`40`	`case JsonCrdtPatchOpcode.new_val: {`
`41`		`- builder.val(timestamp(sid, op[1]));`
	`41`	`+ builder.val();`
`42`	`42`	`break;`
`43`	`43`	`}`
`44`	`44`	`case JsonCrdtPatchOpcode.new_obj: {`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ export const encode = (patch: Patch): types.CompactCodecPatch => {`
`47`	`47`	`res.push([JsonCrdtPatchOpcode.new_con, val]);`
`48`	`48`	`}`
`49`	`49`	`} else if (op instanceof operations.NewValOp) {`
`50`		`- res.push([JsonCrdtPatchOpcode.new_val, timestamp(sid, op.val)]);`
	`50`	`+ res.push([JsonCrdtPatchOpcode.new_val]);`
`51`	`51`	`} else if (op instanceof operations.NewObjOp) {`
`52`	`52`	`res.push([JsonCrdtPatchOpcode.new_obj]);`
`53`	`53`	`} else if (op instanceof operations.NewVecOp) {`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,6 @@ const encoded1 = {`
`9`	`9`	`},`
`10`	`10`	`{`
`11`	`11`	`op: 'new_val',`
`12`		`- value: 1,`
`13`	`12`	`},`
`14`	`13`	`{`
`15`	`14`	`op: 'new_bin',`
`@@ -22,7 +21,6 @@ const encoded1 = {`
`22`	`21`	`},`
`23`	`22`	`{`
`24`	`23`	`op: 'new_val',`
`25`		`- value: 2,`
`26`	`24`	`},`
`27`	`25`	`{`
`28`	`26`	`op: 'ins_obj',`