diff --git a/src/decoder/DecodeOperation.ts b/src/decoder/DecodeOperation.ts index 883fd05..f3af32e 100644 --- a/src/decoder/DecodeOperation.ts +++ b/src/decoder/DecodeOperation.ts @@ -302,11 +302,22 @@ export const decodeArray: DecodeOperation = function ( return; } else if (operation === OPERATION.DELETE_BY_REFID) { - // TODO: refactor here, try to follow same flow as below const refId = decode.number(bytes, it); const previousValue = decoder.root.refs.get(refId); + + // Skip stale DELETE operations (item not known to this decoder) + if (!previousValue) { return; } + index = ref.findIndex((value) => value === previousValue); + + // Skip if item is not in the array + if (index === -1) { return; } + ref[$deleteByIndex](index); + + // Flag refId for garbage collection + decoder.root.removeRef(refId); + allChanges.push({ ref, refId: decoder.currentRefId, diff --git a/src/encoder/EncodeOperation.ts b/src/encoder/EncodeOperation.ts index d4d07ec..54c1f93 100644 --- a/src/encoder/EncodeOperation.ts +++ b/src/encoder/EncodeOperation.ts @@ -172,7 +172,8 @@ export const encodeArray: EncodeOperation = function ( hasView: boolean, ) { const ref = changeTree.ref; - const useOperationByRefId = hasView && changeTree.isFiltered && (typeof (changeTree.getType(field)) !== "string"); + const isSchemaChild = typeof (changeTree.getType(field)) !== "string"; + const useOperationByRefId = hasView && changeTree.isFiltered && isSchemaChild; let refOrIndex: number; @@ -191,6 +192,18 @@ export const encodeArray: EncodeOperation = function ( operation = OPERATION.ADD_BY_REFID; } + } else if (operation === OPERATION.DELETE && isSchemaChild) { + // + // Use DELETE_BY_REFID for Schema children to make DELETE + // operations idempotent. This prevents stale DELETEs + // (accumulated in `changes` before an `encodeAll()`) from + // corrupting a client that received a full encode. + // + const item = ref['tmpItems'][field]; + if (!item) { return; } + refOrIndex = item[$refId]; + operation = OPERATION.DELETE_BY_REFID; + } else { refOrIndex = field; } diff --git a/test/ArraySchema.test.ts b/test/ArraySchema.test.ts index 4d5003b..f15b294 100644 --- a/test/ArraySchema.test.ts +++ b/test/ArraySchema.test.ts @@ -303,7 +303,7 @@ describe("ArraySchema Tests", () => { assertDeepStrictEqualEncodeAll(state); }); - xit("encodeAll() + with enqueued encode() shifts with Schema children", () => { + it("encodeAll() + with enqueued encode() shifts with Schema children", () => { class Entity extends Schema { @type("number") thing: number; } @@ -360,8 +360,59 @@ describe("ArraySchema Tests", () => { assertDeepStrictEqualEncodeAll(state); }); + + xit("encodeAll() + with enqueued encode() shifts with primitive children", () => { + class State extends Schema { + @type(["number"]) numbers: ArraySchema; + } + + const state = new State(); + state.numbers = new ArraySchema(); + + const repopulateArray = (count: number) => { + for (let i = 0; i < count; i++) { + state.numbers.push(i); + } + }; + repopulateArray(35); + + function mutateAllAndShift(count: number = 6) { + for (let i = 0; i < count; i++) { + for (let j = 0; j < state.numbers.length; j++) { + state.numbers[j]++; + } + state.numbers.shift(); + } + } + + const decoded1 = createInstanceFromReflection(state); + decoded1.decode(state.encodeAll()); + mutateAllAndShift(6); + decoded1.decode(state.encode()); + mutateAllAndShift(6); + decoded1.decode(state.encode()); + + mutateAllAndShift(9); + + const decoded2 = createInstanceFromReflection(state); + decoded2.decode(state.encodeAll()); + + mutateAllAndShift(3); + decoded2.decode(state.encode()); + + assert.deepStrictEqual(state.toJSON(), decoded2.toJSON()); + + mutateAllAndShift(6); + decoded2.decode(state.encode()); + + assert.deepStrictEqual(state.toJSON(), decoded2.toJSON()); + + assertDeepStrictEqualEncodeAll(state); + }); + }); + it("should allow mutating primitive value by index", () => { class State extends Schema { @type(['number']) primativeArr = new ArraySchema()