From c635963747db105280edcfc180223fb11a913486 Mon Sep 17 00:00:00 2001
From: Kevin Jahns <kevin.jahns@protonmail.com>
Date: Sat, 6 Apr 2019 15:55:20 +0200
Subject: [PATCH] Compare origin ids in item.integrate

---
 src/structs/AbstractItem.js | 21 +++++++++++++++------
 src/structs/ItemDeleted.js  |  8 ++++++++
 src/utils/StructStore.js    | 10 ++++++++++
 src/utils/Y.js              |  4 ++--
 tests/testHelper.js         |  2 +-
 5 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/src/structs/AbstractItem.js b/src/structs/AbstractItem.js
index a229446f..f9b5ffcd 100644
--- a/src/structs/AbstractItem.js
+++ b/src/structs/AbstractItem.js
@@ -12,6 +12,8 @@ import {
   addToDeleteSet,
   ItemDeleted,
   findRootTypeKey,
+  compareIDs,
+  getItem,
   StructStore, ID, AbstractType, Y, Transaction // eslint-disable-line
 } from '../internals.js'
 
@@ -133,6 +135,7 @@ export class AbstractItem extends AbstractStruct {
    * @param {Transaction} transaction
    */
   integrate (transaction) {
+    const store = transaction.y.store
     const id = this.id
     const parent = this.parent
     const parentSub = this.parentSub
@@ -165,7 +168,13 @@ export class AbstractItem extends AbstractStruct {
     } else {
       o = parent._start
     }
+    /**
+     * @type {Set<AbstractItem>}
+     */
     const conflictingItems = new Set()
+    /**
+     * @type {Set<AbstractItem>}
+     */
     const itemsBeforeOrigin = new Set()
     // Let c in conflictingItems, b in itemsBeforeOrigin
     // ***{origin}bbbb{this}{c,b}{c,b}{o}***
@@ -173,22 +182,22 @@ export class AbstractItem extends AbstractStruct {
     while (o !== null && o !== this.right) {
       itemsBeforeOrigin.add(o)
       conflictingItems.add(o)
-      if (this.origin === o.origin) {
+      if (compareIDs(this.origin, o.origin)) {
         // case 1
         if (o.id.client < id.client) {
           this.left = o
           conflictingItems.clear()
-        }
-      } else if (itemsBeforeOrigin.has(o.origin)) {
+        } // TODO: verify break else?
+      } else if (o.origin !== null && itemsBeforeOrigin.has(getItem(store, o.origin))) {
         // case 2
-        if (!conflictingItems.has(o.origin)) {
+        if (o.origin === null || !conflictingItems.has(getItem(store, o.origin))) {
           this.left = o
           conflictingItems.clear()
         }
       } else {
         break
       }
-      // TODO: try to use right_origin instead.
+      // TODO: experiment with rightOrigin.
       // Then you could basically omit conflictingItems!
       // Note: you probably can't use right_origin in every case.. only when setting _left
       o = o.right
@@ -220,7 +229,7 @@ export class AbstractItem extends AbstractStruct {
     if (parentSub === null && this.countable) {
       parent._length += length
     }
-    addStruct(transaction.y.store, this)
+    addStruct(store, this)
     if (parent !== null) {
       maplib.setIfUndefined(transaction.changed, parent, set.create).add(parentSub)
     }
diff --git a/src/structs/ItemDeleted.js b/src/structs/ItemDeleted.js
index 262205ac..f4efa144 100644
--- a/src/structs/ItemDeleted.js
+++ b/src/structs/ItemDeleted.js
@@ -14,6 +14,7 @@ import {
   GC,
   splitItem,
   compareIDs,
+  addToDeleteSet,
   StructStore, Transaction, ID, AbstractType // eslint-disable-line
 } from '../internals.js'
 
@@ -53,6 +54,13 @@ export class ItemDeleted extends AbstractItem {
   copy (id, left, origin, right, rightOrigin, parent, parentSub) {
     return new ItemDeleted(id, left, origin, right, rightOrigin, parent, parentSub, this.length)
   }
+  /**
+   * @param {Transaction} transaction
+   */
+  integrate (transaction) {
+    super.integrate(transaction)
+    addToDeleteSet(transaction.deleteSet, this.id, this.length)
+  }
   /**
    * @param {StructStore} store
    * @param {number} diff
diff --git a/src/utils/StructStore.js b/src/utils/StructStore.js
index 79fdd9fa..77628ff1 100644
--- a/src/utils/StructStore.js
+++ b/src/utils/StructStore.js
@@ -125,6 +125,16 @@ export const find = (store, id) => {
   return structs[findIndexSS(structs, id.clock)]
 }
 
+/**
+ * Expects that id is actually in store. This function throws or is an infinite loop otherwise.
+ *
+ * @param {StructStore} store
+ * @param {ID} id
+ * @return {AbstractItem}
+ */
+// @ts-ignore
+export const getItem = (store, id) => find(store, id)
+
 /**
  * Expects that id is actually in store. This function throws or is an infinite loop otherwise.
  *
diff --git a/src/utils/Y.js b/src/utils/Y.js
index 6d9432b2..2bad4c12 100644
--- a/src/utils/Y.js
+++ b/src/utils/Y.js
@@ -95,16 +95,16 @@ export class Y extends Observable {
           callEventHandlerListeners(type._dEH, [events, transaction])
         })
         // only call afterTransaction listeners if anything changed
-        const transactionChangedContent = transaction.changedParentTypes.size !== 0
+        const transactionChangedContent = transaction.changedParentTypes.size > 0 || transaction.deleteSet.clients.size > 0
         if (transactionChangedContent) {
           transaction.afterState = getStates(transaction.y.store)
           // when all changes & events are processed, emit afterTransaction event
-          this.emit('afterTransaction', [this, transaction])
           // transaction cleanup
           const store = transaction.y.store
           const ds = transaction.deleteSet
           // replace deleted items with ItemDeleted / GC
           sortAndMergeDeleteSet(ds)
+          this.emit('afterTransaction', [this, transaction])
           /**
            * @type {Set<ItemDeleted|GC>}
            */
diff --git a/tests/testHelper.js b/tests/testHelper.js
index 4baddabe..2bd991ac 100644
--- a/tests/testHelper.js
+++ b/tests/testHelper.js
@@ -63,7 +63,7 @@ export class TestYInstance extends Y.Y {
     this.mMux = createMutex()
     testConnector.allConns.add(this)
     // set up observe on local model
-    this.on('afterTransaction', afterTransaction)
+    this.on('afterTransactionCleanup', afterTransaction)
     this.connect()
   }
   /**