ArraySchema: exceeding deletes fix

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,

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;
     }

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<number>;
+            }
+
+            const state = new State();
+            state.numbers = new ArraySchema<number>();
+
+            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<Number>()