From 6cd1693b777865308ee063ffba9bcfd7dc9da17a Mon Sep 17 00:00:00 2001 From: Endel Dreyer Date: Mon, 13 Jul 2020 21:03:28 -0300 Subject: [PATCH] fixes decoder refId garbage collection when calling .clear(). #75 --- src/Schema.ts | 16 +++++----------- src/changes/ChangeTree.ts | 2 +- src/types/ArraySchema.ts | 9 ++++++++- src/types/CollectionSchema.ts | 9 ++++++++- src/types/MapSchema.ts | 9 ++++++++- src/types/SetSchema.ts | 9 ++++++++- test/InstanceSharingTest.ts | 33 +++++++++++++++++++++++++++++++++ 7 files changed, 71 insertions(+), 16 deletions(-) diff --git a/src/Schema.ts b/src/Schema.ts index c02ee5b1..b612ccc8 100644 --- a/src/Schema.ts +++ b/src/Schema.ts @@ -25,7 +25,7 @@ export interface SchemaDecoderCallbacks { onAdd?: (item: any, key: any) => void; onRemove?: (item: any, key: any) => void; onChange?: (item: any, key: any) => void; - clear(); + clear(decoding?: boolean); decode?(byte, it: decode.Iterator); } @@ -166,9 +166,6 @@ export abstract class Schema { public assign( props: { [prop in NonFunctionPropNames]?: this[prop] } ) { - // - // TODO: recursivelly assign child Schema structures. - // Object.assign(this, props); return this; } @@ -247,14 +244,11 @@ export abstract class Schema { if (operation === OPERATION.CLEAR) { // - // TODO: - // - // flag all children refId's for garbage collection. - // (if not a collection of primitive type) - // - // $root.removeRef(refId); + // TODO: refactor me! + // The `.clear()` method is calling `$root.removeRef(refId)` is + // being called for each item inside this collection // - (ref as SchemaDecoderCallbacks).clear(); + (ref as SchemaDecoderCallbacks).clear(true); continue; } diff --git a/src/changes/ChangeTree.ts b/src/changes/ChangeTree.ts index 74b05627..dbee52a5 100644 --- a/src/changes/ChangeTree.ts +++ b/src/changes/ChangeTree.ts @@ -221,7 +221,7 @@ export class ChangeTree { } } - getType(index: number) { + getType(index?: number) { if (this.ref['_definition']) { const definition = (this.ref as Schema)['_definition']; return definition.schema[ definition.fieldsByIndex[index] ]; diff --git a/src/types/ArraySchema.ts b/src/types/ArraySchema.ts index 691cc3d1..5a344867 100644 --- a/src/types/ArraySchema.ts +++ b/src/types/ArraySchema.ts @@ -166,13 +166,20 @@ export class ArraySchema implements Array, SchemaDecoderCallbacks { return this.$items.delete(index); } - clear() { + clear(isDecoding?: boolean) { // discard previous operations. this.$changes.discard(true); // clear previous indexes this.$indexes.clear(); + // flag child items for garbage collection. + if (isDecoding && typeof (this.$changes.getType()) !== "string") { + this.$items.forEach((item: V) => { + this.$changes.root.removeRef(item['$changes'].refId); + }); + } + // clear items this.$items.clear(); diff --git a/src/types/CollectionSchema.ts b/src/types/CollectionSchema.ts index 2458669c..cffc01f8 100644 --- a/src/types/CollectionSchema.ts +++ b/src/types/CollectionSchema.ts @@ -77,13 +77,20 @@ export class CollectionSchema implements SchemaDecoderCallbacks { return this.$items.delete(index); } - clear() { + clear(isDecoding?: boolean) { // discard previous operations. this.$changes.discard(true); // clear previous indexes this.$indexes.clear(); + // flag child items for garbage collection. + if (isDecoding && typeof (this.$changes.getType()) !== "string") { + this.$items.forEach((item: V) => { + this.$changes.root.removeRef(item['$changes'].refId); + }); + } + // clear items this.$items.clear(); diff --git a/src/types/MapSchema.ts b/src/types/MapSchema.ts index c81ca4dd..bc78fc47 100644 --- a/src/types/MapSchema.ts +++ b/src/types/MapSchema.ts @@ -133,13 +133,20 @@ export class MapSchema implements Map, SchemaDecoderCallbacks return this.$items.delete(key); } - clear() { + clear(isDecoding?: boolean) { // discard previous operations. this.$changes.discard(true); // clear previous indexes this.$indexes.clear(); + // flag child items for garbage collection. + if (isDecoding && typeof (this.$changes.getType()) !== "string") { + this.$items.forEach((item: V) => { + this.$changes.root.removeRef(item['$changes'].refId); + }); + } + // clear items this.$items.clear(); diff --git a/src/types/SetSchema.ts b/src/types/SetSchema.ts index ab2ea435..73d24098 100644 --- a/src/types/SetSchema.ts +++ b/src/types/SetSchema.ts @@ -76,13 +76,20 @@ export class SetSchema implements SchemaDecoderCallbacks { return this.$items.delete(index); } - clear() { + clear(isDecoding?: boolean) { // discard previous operations. this.$changes.discard(true); // clear previous indexes this.$indexes.clear(); + // flag child items for garbage collection. + if (isDecoding && typeof (this.$changes.getType()) !== "string") { + this.$items.forEach((item: V) => { + this.$changes.root.removeRef(item['$changes'].refId); + }); + } + // clear items this.$items.clear(); diff --git a/test/InstanceSharingTest.ts b/test/InstanceSharingTest.ts index f55fabcf..0b92e09d 100644 --- a/test/InstanceSharingTest.ts +++ b/test/InstanceSharingTest.ts @@ -132,4 +132,37 @@ describe("Instance sharing", () => { const newRefCount = decodedState['$changes'].root.refs.size; assert.equal(refCount - 4, newRefCount); }); + + it("clearing ArraySchema", () => { + const state = new State(); + + const player1 = new Player().assign({ + position: new Position().assign({ + x: 10, y: 10 + }) + }); + state.arrayOfPlayers.push(player1); + state.arrayOfPlayers.push(player1); + state.arrayOfPlayers.push(player1); + + const player2 = new Player().assign({ + position: new Position().assign({ + x: 10, y: 10 + }) + }); + state.arrayOfPlayers.push(player2); + + const decodedState = new State(); + decodedState.decode(state.encode()); + + const refCount = decodedState['$changes'].root.refs.size; + assert.equal(7, refCount); + + state.arrayOfPlayers.clear(); + + decodedState.decode(state.encode()); + + const newRefCount = decodedState['$changes'].root.refs.size; + assert.equal(refCount - 4, newRefCount); + }); }); \ No newline at end of file