From 1b17b5e40098403c419869c904263b2d4c893812 Mon Sep 17 00:00:00 2001
From: Kevin Jahns <kevin.jahns@protonmail.com>
Date: Sat, 6 Apr 2019 13:00:32 +0200
Subject: [PATCH] fixed 10 tests

---
 src/structs/AbstractItem.js |  4 ++-
 src/structs/ItemDeleted.js  | 21 ++++++++++++--
 src/structs/ItemJSON.js     |  4 +--
 src/structs/ItemString.js   |  2 +-
 src/structs/ItemType.js     |  2 +-
 src/types/AbstractType.js   | 23 +++++++++------
 src/types/YText.js          | 30 +++++++++++---------
 src/types/YXmlElement.js    | 55 +++++++++++++++++++-----------------
 src/utils/StructStore.js    | 21 ++++++++++----
 src/utils/YEvent.js         | 22 ++++++---------
 tests/y-array.tests.js      | 56 ++++++++++---------------------------
 tests/y-map.tests.js        | 13 ++++++---
 12 files changed, 133 insertions(+), 120 deletions(-)

diff --git a/src/structs/AbstractItem.js b/src/structs/AbstractItem.js
index 1cd1cc45..a229446f 100644
--- a/src/structs/AbstractItem.js
+++ b/src/structs/AbstractItem.js
@@ -28,6 +28,8 @@ import * as binary from 'lib0/binary.js'
  * @param {AbstractItem} leftItem
  * @param {number} diff
  * @return {AbstractItem}
+ *
+ * @todo remove store param0
  */
 export const splitItem = (store, leftItem, diff) => {
   const id = leftItem.id
@@ -35,7 +37,7 @@ export const splitItem = (store, leftItem, diff) => {
   const rightItem = leftItem.copy(
     createID(id.client, id.clock + diff),
     leftItem,
-    leftItem.lastId,
+    createID(id.client, id.clock + diff - 1),
     leftItem.right,
     leftItem.rightOrigin,
     leftItem.parent,
diff --git a/src/structs/ItemDeleted.js b/src/structs/ItemDeleted.js
index b4d32603..262205ac 100644
--- a/src/structs/ItemDeleted.js
+++ b/src/structs/ItemDeleted.js
@@ -12,8 +12,9 @@ import {
   getItemType,
   changeItemRefOffset,
   GC,
+  splitItem,
   compareIDs,
-  Transaction, ID, AbstractType // eslint-disable-line
+  StructStore, Transaction, ID, AbstractType // eslint-disable-line
 } from '../internals.js'
 
 import * as encoding from 'lib0/encoding.js'
@@ -52,13 +53,27 @@ 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 {StructStore} store
+   * @param {number} diff
+   */
+  splitAt (store, diff) {
+    /**
+     * @type {ItemDeleted}
+     */
+    // @ts-ignore
+    const right = splitItem(store, this, diff)
+    right._len -= diff
+    this._len = diff
+    return right
+  }
   /**
    * @param {ItemDeleted} right
    * @return {boolean}
    */
   mergeWith (right) {
-    if (compareIDs(right.origin, this.lastId) && this.right === right) {
-      this._len += right.length
+    if (compareIDs(right.origin, this.lastId) && this.right === right && compareIDs(this.rightOrigin, right.rightOrigin)) {
+      this._len += right._len
       return true
     }
     return false
diff --git a/src/structs/ItemJSON.js b/src/structs/ItemJSON.js
index b927b4b8..a0118f4e 100644
--- a/src/structs/ItemJSON.js
+++ b/src/structs/ItemJSON.js
@@ -66,7 +66,7 @@ export class ItemJSON extends AbstractItem {
      * @type {ItemJSON}
      */
     // @ts-ignore
-    const right = splitItem(this, diff)
+    const right = splitItem(store, this, diff)
     right.content = this.content.splice(diff)
     return right
   }
@@ -75,7 +75,7 @@ export class ItemJSON extends AbstractItem {
    * @return {boolean}
    */
   mergeWith (right) {
-    if (compareIDs(right.origin, this.lastId) && this.right === right) {
+    if (compareIDs(right.origin, this.lastId) && this.right === right && compareIDs(this.rightOrigin, right.rightOrigin)) {
       this.content = this.content.concat(right.content)
       return true
     }
diff --git a/src/structs/ItemString.js b/src/structs/ItemString.js
index 532e243d..db852532 100644
--- a/src/structs/ItemString.js
+++ b/src/structs/ItemString.js
@@ -76,7 +76,7 @@ export class ItemString extends AbstractItem {
    * @return {boolean}
    */
   mergeWith (right) {
-    if (compareIDs(right.origin, this.lastId) && this.right === right) {
+    if (compareIDs(right.origin, this.lastId) && this.right === right && compareIDs(this.rightOrigin, right.rightOrigin)) {
       this.string += right.string
       return true
     }
diff --git a/src/structs/ItemType.js b/src/structs/ItemType.js
index 69bb3fa3..07435c9c 100644
--- a/src/structs/ItemType.js
+++ b/src/structs/ItemType.js
@@ -93,8 +93,8 @@ export class ItemType extends AbstractItem {
    * @param {Transaction} transaction
    */
   integrate (transaction) {
-    this.type._integrate(transaction.y, this)
     super.integrate(transaction)
+    this.type._integrate(transaction.y, this)
   }
   /**
    * @param {encoding.Encoder} encoder
diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js
index 63770a59..20fee63f 100644
--- a/src/types/AbstractType.js
+++ b/src/types/AbstractType.js
@@ -14,7 +14,6 @@ import {
   ItemBinary,
   createID,
   getItemCleanStart,
-  getItemCleanEnd,
   Y, Snapshot, Transaction, EventHandler, YEvent, AbstractItem, // eslint-disable-line
 } from '../internals.js'
 
@@ -327,7 +326,7 @@ export const typeArrayGet = (type, index) => {
  * @param {Array<Object<string,any>|Array<any>|number|string|ArrayBuffer>} content
  */
 export const typeArrayInsertGenericsAfter = (transaction, parent, referenceItem, content) => {
-  const left = referenceItem
+  let left = referenceItem
   const right = referenceItem === null ? parent._start : referenceItem.right
   /**
    * @type {Array<Object|Array|number>}
@@ -335,8 +334,8 @@ export const typeArrayInsertGenericsAfter = (transaction, parent, referenceItem,
   let jsonContent = []
   const packJsonContent = () => {
     if (jsonContent.length > 0) {
-      const item = new ItemJSON(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, null, jsonContent)
-      item.integrate(transaction)
+      left = new ItemJSON(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, null, jsonContent)
+      left.integrate(transaction)
       jsonContent = []
     }
   }
@@ -353,11 +352,14 @@ export const typeArrayInsertGenericsAfter = (transaction, parent, referenceItem,
         switch (c.constructor) {
           case ArrayBuffer:
             // @ts-ignore c is definitely an ArrayBuffer
-            new ItemBinary(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, null, c).integrate(transaction)
+            left = new ItemBinary(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, null, c)
+            // @ts-ignore
+            left.integrate(transaction)
             break
           default:
             if (c instanceof AbstractType) {
-              new ItemType(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, null, c).integrate(transaction)
+              left = new ItemType(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, null, c)
+              left.integrate(transaction)
             } else {
               throw new Error('Unexpected content type in insert operation')
             }
@@ -401,10 +403,11 @@ export const typeArrayInsertGenerics = (transaction, parent, index, content) =>
  */
 export const typeArrayDelete = (transaction, parent, index, length) => {
   let n = parent._start
+  // compute the first item to be deleted
   for (; n !== null; n = n.right) {
     if (!n.deleted && n.countable) {
       if (index <= n.length) {
-        if (index < n.length) {
+        if (index < n.length && index > 0) {
           n = getItemCleanStart(transaction.y.store, createID(n.id.client, n.id.clock + index))
         }
         break
@@ -412,16 +415,20 @@ export const typeArrayDelete = (transaction, parent, index, length) => {
       index -= n.length
     }
   }
+  // delete all items until done
   while (length > 0 && n !== null) {
     if (!n.deleted) {
       if (length < n.length) {
-        getItemCleanEnd(transaction.y.store, createID(n.id.client, n.id.clock + length))
+        getItemCleanStart(transaction.y.store, createID(n.id.client, n.id.clock + length))
       }
       n.delete(transaction)
       length -= n.length
     }
     n = n.right
   }
+  if (length > 0) {
+    throw error.create('array length exceeded')
+  }
 }
 
 /**
diff --git a/src/types/YText.js b/src/types/YText.js
index 4482c9ca..8896da74 100644
--- a/src/types/YText.js
+++ b/src/types/YText.js
@@ -601,14 +601,15 @@ export class YText extends AbstractType {
   toString () {
     let str = ''
     /**
-     * @type {any}
+     * @type {AbstractItem|null}
      */
     let n = this._start
     while (n !== null) {
-      if (!n._deleted && n._countable) {
-        str += n._content
+      if (!n.deleted && n.countable && n.constructor === ItemString) {
+        // @ts-ignore
+        str += n.string
       }
-      n = n._right
+      n = n.right
     }
     return str
   }
@@ -693,8 +694,9 @@ export class YText extends AbstractType {
     const currentAttributes = new Map()
     let str = ''
     /**
-     * @type {any}
+     * @type {AbstractItem|null}
      */
+    // @ts-ignore
     let n = this._start
     function packStr () {
       if (str.length > 0) {
@@ -702,7 +704,7 @@ export class YText extends AbstractType {
         /**
          * @type {Object<string,any>}
          */
-        let attributes = {}
+        const attributes = {}
         let addAttributes = false
         for (let [key, value] of currentAttributes) {
           addAttributes = true
@@ -711,7 +713,7 @@ export class YText extends AbstractType {
         /**
          * @type {Object<string,any>}
          */
-        let op = { insert: str }
+        const op = { insert: str }
         if (addAttributes) {
           op.attributes = attributes
         }
@@ -725,28 +727,30 @@ export class YText extends AbstractType {
           case ItemString:
             const cur = currentAttributes.get('ychange')
             if (snapshot !== undefined && !isVisible(n, snapshot)) {
-              if (cur === undefined || cur.user !== n._id.user || cur.state !== 'removed') {
+              if (cur === undefined || cur.user !== n.id.client || cur.state !== 'removed') {
                 packStr()
-                currentAttributes.set('ychange', { user: n._id.user, state: 'removed' })
+                currentAttributes.set('ychange', { user: n.id.client, state: 'removed' })
               }
             } else if (prevSnapshot !== undefined && !isVisible(n, prevSnapshot)) {
-              if (cur === undefined || cur.user !== n._id.user || cur.state !== 'added') {
+              if (cur === undefined || cur.user !== n.id.client || cur.state !== 'added') {
                 packStr()
-                currentAttributes.set('ychange', { user: n._id.user, state: 'added' })
+                currentAttributes.set('ychange', { user: n.id.client, state: 'added' })
               }
             } else if (cur !== undefined) {
               packStr()
               currentAttributes.delete('ychange')
             }
-            str += n._content
+            // @ts-ignore
+            str += n.string
             break
           case ItemFormat:
             packStr()
+            // @ts-ignore
             updateCurrentAttributes(currentAttributes, n)
             break
         }
       }
-      n = n._right
+      n = n.right
     }
     packStr()
     return ops
diff --git a/src/types/YXmlElement.js b/src/types/YXmlElement.js
index b0bfd51e..45f2eedd 100644
--- a/src/types/YXmlElement.js
+++ b/src/types/YXmlElement.js
@@ -52,16 +52,17 @@ import * as decoding from 'lib0/decoding.js'
 export class YXmlTreeWalker {
   /**
    * @param {YXmlFragment | YXmlElement} root
-   * @param {function(AbstractType<any>):boolean} f
+   * @param {function(AbstractType<any>):boolean} [f]
    */
-  constructor (root, f) {
-    this._filter = f || (() => true)
+  constructor (root, f = () => true) {
+    this._filter = f
     this._root = root
     /**
      * @type {ItemType | null}
      */
     // @ts-ignore
     this._currentNode = root._start
+    this._firstCall = true
   }
   [Symbol.iterator] () {
     return this
@@ -75,34 +76,36 @@ export class YXmlTreeWalker {
    */
   next () {
     let n = this._currentNode
+    if (n !== null && (!this._firstCall || n.deleted || !this._filter(n.type))) { // if first call, we check if we can use the first item
+      do {
+        if (!n.deleted && (n.type.constructor === YXmlElement || n.type.constructor === YXmlFragment) && n.type._start !== null) {
+          // walk down in the tree
+          // @ts-ignore
+          n = n.type._start
+        } else {
+          // walk right or up in the tree
+          while (n !== null) {
+            if (n.right !== null) {
+              // @ts-ignore
+              n = n.right
+              break
+            } else if (n.parent === this._root) {
+              n = null
+            } else {
+              n = n.parent._item
+            }
+          }
+        }
+      } while (n !== null && (n.deleted || !this._filter(n.type)))
+    }
+    this._firstCall = false
+    this._currentNode = n
     if (n === null) {
       // @ts-ignore return undefined if done=true (the expected result)
       return { value: undefined, done: true }
     }
-    const nextValue = n
-    do {
-      if (!n.deleted && (n.constructor === YXmlElement || n.constructor === YXmlFragment) && n.type._start !== null) {
-        // walk down in the tree
-        // @ts-ignore
-        n = n.type._start
-      } else {
-        // walk right or up in the tree
-        while (n !== null) {
-          if (n.right !== null) {
-            // @ts-ignore
-            n = n.right
-            break
-          } else if (n.parent === this._root) {
-            n = null
-          } else {
-            n = n.parent._item
-          }
-        }
-      }
-    } while (n !== null && (n.deleted || !this._filter(n.type)))
-    this._currentNode = n
     // @ts-ignore
-    return { value: nextValue.type, done: false }
+    return { value: n.type, done: false }
   }
 }
 
diff --git a/src/utils/StructStore.js b/src/utils/StructStore.js
index b07c417d..79fdd9fa 100644
--- a/src/utils/StructStore.js
+++ b/src/utils/StructStore.js
@@ -3,7 +3,6 @@ import {
   Transaction, ID, ItemType, AbstractItem, AbstractStruct // eslint-disable-line
 } from '../internals.js'
 
-import * as map from 'lib0/map.js'
 import * as math from 'lib0/math.js'
 import * as error from 'lib0/error.js'
 import * as encoding from 'lib0/encoding.js'
@@ -67,7 +66,17 @@ export const integretyCheck = store => {
  * @param {AbstractStruct} struct
  */
 export const addStruct = (store, struct) => {
-  map.setIfUndefined(store.clients, struct.id.client, () => []).push(struct)
+  let structs = store.clients.get(struct.id.client)
+  if (structs === undefined) {
+    structs = []
+    store.clients.set(struct.id.client, structs)
+  } else {
+    const lastStruct = structs[structs.length - 1]
+    if (lastStruct.id.clock + lastStruct.length !== struct.id.clock) {
+      throw error.unexpectedCase()
+    }
+  }
+  structs.push(struct)
 }
 
 /**
@@ -147,7 +156,7 @@ export const getItemCleanStart = (store, id) => {
   let struct = structs[index]
   if (struct.id.clock < id.clock) {
     struct = struct.splitAt(store, id.clock - struct.id.clock)
-    structs.splice(index, 0, struct)
+    structs.splice(index + 1, 0, struct)
   }
   return struct
 }
@@ -169,7 +178,7 @@ export const getItemCleanEnd = (store, id) => {
   const index = findIndexSS(structs, id.clock)
   const struct = structs[index]
   if (id.clock !== struct.id.clock + struct.length - 1) {
-    structs.splice(index, 0, struct.splitAt(store, id.clock - struct.id.clock + 1))
+    structs.splice(index + 1, 0, struct.splitAt(store, id.clock - struct.id.clock + 1))
   }
   return struct
 }
@@ -196,7 +205,7 @@ export const getItemRange = (store, client, clock, len) => {
   if (struct.id.clock <= clock) {
     if (struct.id.clock < clock) {
       struct = struct.splitAt(store, clock - struct.id.clock)
-      structs.splice(index, 0, struct)
+      structs.splice(index + 1, 0, struct)
     }
     range.push(struct)
   }
@@ -210,7 +219,7 @@ export const getItemRange = (store, client, clock, len) => {
     }
   }
   if (struct.id.clock < clock + len && struct.id.clock + struct.length > clock + len) {
-    structs.splice(index, 0, struct.splitAt(store, clock + len - struct.id.clock))
+    structs.splice(index + 1, 0, struct.splitAt(store, clock + len - struct.id.clock))
   }
   return range
 }
diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js
index e6700956..ee0e19d8 100644
--- a/src/utils/YEvent.js
+++ b/src/utils/YEvent.js
@@ -47,7 +47,7 @@ export class YEvent {
    */
   get path () {
     // @ts-ignore _item is defined because target is integrated
-    return getPathTo(this.currentTarget, this.target._item)
+    return getPathTo(this.currentTarget, this.target)
   }
 
   /**
@@ -82,21 +82,20 @@ export class YEvent {
  *   child === type.get(path[0]).get(path[1])
  *
  * @param {AbstractType<any>} parent
- * @param {AbstractItem} child target
+ * @param {AbstractType<any>} child target
  * @return {Array<string|number>} Path to the target
  */
 const getPathTo = (parent, child) => {
   const path = []
-  while (true) {
-    const cparent = child.parent
-    if (child.parentSub !== null) {
+  while (child._item !== null && child !== parent) {
+    if (child._item.parentSub !== null) {
       // parent is map-ish
-      path.unshift(child.parentSub)
+      path.unshift(child._item.parentSub)
     } else {
       // parent is array-ish
       let i = 0
-      let c = cparent._start
-      while (c !== child && c !== null) {
+      let c = child._item.parent._start
+      while (c !== child._item && c !== null) {
         if (!c.deleted) {
           i++
         }
@@ -104,10 +103,7 @@ const getPathTo = (parent, child) => {
       }
       path.unshift(i)
     }
-    if (parent === cparent) {
-      return path
-    }
-    // @ts-ignore parent._item cannot be null, because it is integrated
-    child = parent._item
+    child = child._item.parent
   }
+  return path
 }
diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js
index bd94cd2c..ae0f9162 100644
--- a/tests/y-array.tests.js
+++ b/tests/y-array.tests.js
@@ -120,44 +120,27 @@ export const testInsertThenMergeDeleteOnSync = tc => {
   compare(users)
 }
 
-/**
- * @param {Object<string,any>} is
- * @param {Object<string,any>} should
- */
-const compareEvent = (is, should) => {
-  for (var key in should) {
-    t.assert(
-      should[key] === is[key] ||
-      JSON.stringify(should[key]) === JSON.stringify(is[key])
-      , 'event works as expected'
-    )
-  }
-}
-
 /**
  * @param {t.TestCase} tc
  */
 export const testInsertAndDeleteEvents = tc => {
   const { array0, users } = init(tc, { users: 2 })
   /**
-   * @type {Object<string,any>}
+   * @type {Object<string,any>?}
    */
-  let event = {}
+  let event = null
   array0.observe(e => {
     event = e
   })
   array0.insert(0, [0, 1, 2])
-  compareEvent(event, {
-    remote: false
-  })
+  t.assert(event !== null)
+  event = null
   array0.delete(0)
-  compareEvent(event, {
-    remote: false
-  })
+  t.assert(event !== null)
+  event = null
   array0.delete(0, 2)
-  compareEvent(event, {
-    remote: false
-  })
+  t.assert(event !== null)
+  event = null
   compare(users)
 }
 
@@ -167,20 +150,18 @@ export const testInsertAndDeleteEvents = tc => {
 export const testInsertAndDeleteEventsForTypes = tc => {
   const { array0, users } = init(tc, { users: 2 })
   /**
-   * @type {Object<string,any>}
+   * @type {Object<string,any>|null}
    */
-  let event = {}
+  let event = null
   array0.observe(e => {
     event = e
   })
   array0.insert(0, [new Y.Array()])
-  compareEvent(event, {
-    remote: false
-  })
+  t.assert(event !== null)
+  event = null
   array0.delete(0)
-  compareEvent(event, {
-    remote: false
-  })
+  t.assert(event !== null)
+  event = null
   compare(users)
 }
 
@@ -197,14 +178,8 @@ export const testInsertAndDeleteEventsForTypes2 = tc => {
     events.push(e)
   })
   array0.insert(0, ['hi', new Y.Map()])
-  compareEvent(events[0], {
-    remote: false
-  })
   t.assert(events.length === 1, 'Event is triggered exactly once for insertion of two elements')
   array0.delete(1)
-  compareEvent(events[1], {
-    remote: false
-  })
   t.assert(events.length === 2, 'Event is triggered exactly once for deletion')
   compare(users)
 }
@@ -254,9 +229,6 @@ export const testEventTargetIsSetCorrectlyOnRemote = tc => {
   })
   array1.insert(0, ['stuff'])
   testConnector.flushAllMessages()
-  compareEvent(event, {
-    remote: true
-  })
   t.assert(event.target === array0, '"target" property is set correctly')
   compare(users)
 }
diff --git a/tests/y-map.tests.js b/tests/y-map.tests.js
index 359743df..c50fbd5a 100644
--- a/tests/y-map.tests.js
+++ b/tests/y-map.tests.js
@@ -1,5 +1,9 @@
 import { init, compare, applyRandomTests, TestYInstance } from './testHelper.js' // eslint-disable-line
 
+import {
+  compareIDs
+} from '../src/internals.js'
+
 import * as Y from '../src/index.js'
 import * as t from 'lib0/testing.js'
 import * as prng from 'lib0/prng.js'
@@ -190,7 +194,7 @@ export const testObserveDeepProperties = tc => {
       t.assert(event.path.length === 1)
       t.assert(event.path[0] === 'map')
       // @ts-ignore
-      dmapid = event.target.get('deepmap')._id
+      dmapid = event.target.get('deepmap')._item.id
     })
   })
   testConnector.flushAllMessages()
@@ -204,9 +208,10 @@ export const testObserveDeepProperties = tc => {
   const dmap2 = _map2.get('deepmap')
   const dmap3 = _map3.get('deepmap')
   t.assert(calls > 0)
-  t.assert(dmap1._id.equals(dmap2._id))
-  t.assert(dmap1._id.equals(dmap3._id))
-  t.assert(dmap1._id.equals(dmapid))
+  t.assert(compareIDs(dmap1._item.id, dmap2._item.id))
+  t.assert(compareIDs(dmap1._item.id, dmap3._item.id))
+  // @ts-ignore we want the possibility of dmapid being undefined
+  t.assert(compareIDs(dmap1._item.id, dmapid))
   compare(users)
 }