Remove previous element when setting new value (#216)

src/api/converter.ts

@@ -472,6 +472,7 @@ function toJSONObject(obj: JSONObject): PbJSONElement {
   const pbJSONObject = new PbJSONElement.JSONObject();
   pbJSONObject.setNodesList(toRHTNodes(obj.getRHT()));
   pbJSONObject.setCreatedAt(toTimeTicket(obj.getCreatedAt()));
+  pbJSONObject.setMovedAt(toTimeTicket(obj.getMovedAt()));
   pbJSONObject.setRemovedAt(toTimeTicket(obj.getRemovedAt()));
 
   const pbJSONElement = new PbJSONElement();
@@ -486,6 +487,7 @@ function toJSONArray(arr: JSONArray): PbJSONElement {
   const pbJSONArray = new PbJSONElement.JSONArray();
   pbJSONArray.setNodesList(toRGANodes(arr.getElements()));
   pbJSONArray.setCreatedAt(toTimeTicket(arr.getCreatedAt()));
+  pbJSONArray.setMovedAt(toTimeTicket(arr.getMovedAt()));
   pbJSONArray.setRemovedAt(toTimeTicket(arr.getRemovedAt()));
 
   const pbJSONElement = new PbJSONElement();
@@ -501,6 +503,7 @@ function toJSONPrimitive(primitive: JSONPrimitive): PbJSONElement {
   pbJSONPrimitive.setType(toValueType(primitive.getType()));
   pbJSONPrimitive.setValue(primitive.toBytes());
   pbJSONPrimitive.setCreatedAt(toTimeTicket(primitive.getCreatedAt()));
+  pbJSONPrimitive.setMovedAt(toTimeTicket(primitive.getMovedAt()));
   pbJSONPrimitive.setRemovedAt(toTimeTicket(primitive.getRemovedAt()));
 
   const pbJSONElement = new PbJSONElement();
@@ -515,6 +518,7 @@ function toPlainText(text: PlainText): PbJSONElement {
   const pbText = new PbJSONElement.Text();
   pbText.setNodesList(toTextNodes(text.getRGATreeSplit()));
   pbText.setCreatedAt(toTimeTicket(text.getCreatedAt()));
+  pbText.setMovedAt(toTimeTicket(text.getMovedAt()));
   pbText.setRemovedAt(toTimeTicket(text.getRemovedAt()));
 
   const pbJSONElement = new PbJSONElement();
@@ -530,6 +534,7 @@ function toCounter(counter: Counter): PbJSONElement {
   pbJSONCounter.setType(toCounterType(counter.getType()));
   pbJSONCounter.setValue(counter.toBytes());
   pbJSONCounter.setCreatedAt(toTimeTicket(counter.getCreatedAt()));
+  pbJSONCounter.setMovedAt(toTimeTicket(counter.getMovedAt()));
   pbJSONCounter.setRemovedAt(toTimeTicket(counter.getRemovedAt()));
 
   const pbJSONElement = new PbJSONElement();
@@ -929,7 +934,8 @@ function fromJSONObject(pbObject: PbJSONElement.JSONObject): JSONObject {
   }
 
   const obj = new JSONObject(fromTimeTicket(pbObject.getCreatedAt())!, rht);
-  obj.remove(fromTimeTicket(pbObject.getRemovedAt()));
+  obj.setMovedAt(fromTimeTicket(pbObject.getMovedAt()));
+  obj.setRemovedAt(fromTimeTicket(pbObject.getRemovedAt()));
   return obj;
 }
 
@@ -947,7 +953,8 @@ function fromJSONArray(pbArray: PbJSONElement.JSONArray): JSONArray {
     fromTimeTicket(pbArray.getCreatedAt())!,
     rgaTreeList,
   );
-  arr.remove(fromTimeTicket(pbArray.getRemovedAt()));
+  arr.setMovedAt(fromTimeTicket(pbArray.getMovedAt()));
+  arr.setRemovedAt(fromTimeTicket(pbArray.getRemovedAt()));
   return arr;
 }
 
@@ -964,7 +971,8 @@ function fromJSONPrimitive(
     ),
     fromTimeTicket(pbPrimitive.getCreatedAt())!,
   );
-  primitive.remove(fromTimeTicket(pbPrimitive.getRemovedAt()));
+  primitive.setMovedAt(fromTimeTicket(pbPrimitive.getMovedAt()));
+  primitive.setRemovedAt(fromTimeTicket(pbPrimitive.getRemovedAt()));
   return primitive;
 }
 
@@ -989,7 +997,8 @@ function fromJSONText(pbText: PbJSONElement.Text): PlainText {
     rgaTreeSplit,
     fromTimeTicket(pbText.getCreatedAt())!,
   );
-  text.remove(fromTimeTicket(pbText.getRemovedAt()));
+  text.setMovedAt(fromTimeTicket(pbText.getMovedAt()));
+  text.setRemovedAt(fromTimeTicket(pbText.getRemovedAt()));
   return text;
 }
 
@@ -1014,7 +1023,8 @@ function fromJSONRichText(pbText: PbJSONElement.RichText): RichText {
     rgaTreeSplit,
     fromTimeTicket(pbText.getCreatedAt())!,
   );
-  text.remove(fromTimeTicket(pbText.getRemovedAt()));
+  text.setMovedAt(fromTimeTicket(pbText.getMovedAt()));
+  text.setRemovedAt(fromTimeTicket(pbText.getRemovedAt()));
   return text;
 }
 
@@ -1029,7 +1039,8 @@ function fromCounter(pbCounter: PbJSONElement.Counter): Counter {
     ),
     fromTimeTicket(pbCounter.getCreatedAt())!,
   );
-  counter.remove(fromTimeTicket(pbCounter.getRemovedAt()));
+  counter.setMovedAt(fromTimeTicket(pbCounter.getMovedAt()));
+  counter.setRemovedAt(fromTimeTicket(pbCounter.getRemovedAt()));
   return counter;
 }
 

src/document/json/element.ts

@@ -70,11 +70,22 @@ export abstract class JSONElement {
     return false;
   }
 
+  /**
+   * `setRemovedAt` sets the remove time of this element.
+   */
+  public setRemovedAt(removedAt?: TimeTicket): void {
+    this.removedAt = removedAt;
+  }
+
   /**
    * `remove` removes this element.
    */
   public remove(removedAt?: TimeTicket): boolean {
-    if (!this.removedAt || (removedAt && removedAt.after(this.removedAt))) {
+    if (
+      removedAt &&
+      removedAt.after(this.createdAt) &&
+      (!this.removedAt || removedAt.after(this.removedAt))
+    ) {
       this.removedAt = removedAt;
       return true;
     }

src/document/json/object.ts

@@ -98,8 +98,8 @@ export class JSONObject extends JSONContainer {
   /**
    * `set` sets the given element of the given key.
    */
-  public set(key: string, value: JSONElement): void {
-    this.memberNodes.set(key, value);
+  public set(key: string, value: JSONElement): JSONElement | undefined {
+    return this.memberNodes.set(key, value);
   }
 
   /**

src/document/json/rga_tree_list.ts

@@ -139,7 +139,7 @@ export class RGATreeList {
 
   constructor() {
     const dummyValue = JSONPrimitive.of(0, InitialTimeTicket);
-    dummyValue.remove(InitialTimeTicket);
+    dummyValue.setRemovedAt(InitialTimeTicket);
     this.dummyHead = new RGATreeListNode(dummyValue);
     this.last = this.dummyHead;
     this.size = 0;

src/document/json/rht_pq_map.ts

@@ -81,7 +81,24 @@ export class RHTPQMap {
   /**
    * `set` sets the value of the given key.
    */
-  public set(key: string, value: JSONElement): void {
+  public set(key: string, value: JSONElement): JSONElement | undefined {
+    let removed;
+    const queue = this.elementQueueMapByKey.get(key);
+    if (queue && queue.len()) {
+      const node = queue.peek() as RHTPQMapNode;
+      if (!node.isRemoved() && node.remove(value.getCreatedAt())) {
+        removed = node.getValue();
+      }
+    }
+
+    this.setInternal(key, value);
+    return removed;
+  }
+
+  /**
+   * `setInternal` sets the value of the given key.
+   */
+  public setInternal(key: string, value: JSONElement): void {
     if (!this.elementQueueMapByKey.has(key)) {
       this.elementQueueMapByKey.set(key, new Heap(TicketComparator));
     }
@@ -125,18 +142,19 @@ export class RHTPQMap {
     );
     if (!node) {
       logger.fatal(`fail to find ${element.getCreatedAt().toIDString()}`);
+      return;
     }
 
-    const queue = this.elementQueueMapByKey.get(node!.getStrKey());
+    const queue = this.elementQueueMapByKey.get(node.getStrKey());
     if (!queue) {
       logger.fatal(
         `fail to find queue of ${element.getCreatedAt().toIDString()}`,
       );
+      return;
     }
 
-    queue!.release(node!);
-
-    this.nodeMapByCreatedAt.delete(element.getCreatedAt().toIDString());
+    queue.release(node);
+    this.nodeMapByCreatedAt.delete(node.getValue().getCreatedAt().toIDString());
   }
 
   /**

src/document/proxy/object_proxy.ts

@@ -19,6 +19,7 @@ import { TimeTicket } from '../time/ticket';
 import { SetOperation } from '../operation/set_operation';
 import { RemoveOperation } from '../operation/remove_operation';
 import { ChangeContext } from '../change/context';
+import { JSONElement } from '../json/element';
 import { JSONObject } from '../json/object';
 import { JSONArray } from '../json/array';
 import { JSONPrimitive, PrimitiveValue } from '../json/primitive';
@@ -131,17 +132,23 @@ export class ObjectProxy {
   ): void {
     const ticket = context.issueTimeTicket();
 
+    const setAndRegister = function (elem: JSONElement) {
+      const removed = target.set(key, elem);
+      context.registerElement(elem, target);
+      if (removed) {
+        context.registerRemovedElement(removed);
+      }
+    };
+
     if (JSONPrimitive.isSupport(value)) {
       const primitive = JSONPrimitive.of(value as PrimitiveValue, ticket);
-      target.set(key, primitive);
-      context.registerElement(primitive, target);
+      setAndRegister(primitive);
       context.push(
         SetOperation.create(key, primitive, target.getCreatedAt(), ticket),
       );
     } else if (Array.isArray(value)) {
       const array = JSONArray.create(ticket);
-      target.set(key, array);
-      context.registerElement(array, target);
+      setAndRegister(array);
       context.push(
         SetOperation.create(
           key,
@@ -155,8 +162,7 @@ export class ObjectProxy {
       }
     } else if (typeof value === 'object') {
       if (value instanceof PlainText) {
-        target.set(key, value);
-        context.registerElement(value, target);
+        setAndRegister(value);
         context.push(
           SetOperation.create(
             key,
@@ -167,8 +173,7 @@ export class ObjectProxy {
         );
       } else {
         const obj = JSONObject.create(ticket);
-        target.set(key, obj);
-        context.registerElement(obj, target);
+        setAndRegister(obj);
         context.push(
           SetOperation.create(
             key,

test/unit/document/document_test.ts

@@ -15,6 +15,7 @@
  */
 
 import { assert } from 'chai';
+import { MaxTimeTicket } from '../../../src/document/time/ticket';
 import { DocumentReplica, DocEventType } from '../../../src/document/document';
 
 describe('DocumentReplica', function () {
@@ -389,4 +390,19 @@ describe('DocumentReplica', function () {
     assert.equal('{"data":[3,4,2]}', doc.toSortedJSON());
     assert.equal(3, doc.getRoot().data.length);
   });
+
+  it('should remove previously inserted elements in heap when running GC', function () {
+    const doc = DocumentReplica.create('test-col', 'test-doc');
+    doc.update((root) => {
+      root.a = 1;
+      root.a = 2;
+      delete root.a;
+    });
+    assert.equal('{}', doc.toSortedJSON());
+    assert.equal(1, doc.getGarbageLen());
+
+    doc.garbageCollect(MaxTimeTicket);
+    assert.equal('{}', doc.toSortedJSON());
+    assert.equal(0, doc.getGarbageLen());
+  });
 });