type._map points to the last element instead to enable merging of deletes in Map

src/structs/AbstractItem.js

@@ -140,6 +140,9 @@ export class AbstractItem extends AbstractStruct {
       o = this.left.right
     } else if (parentSub !== null) {
       o = parent._map.get(parentSub) || null
+      while (o !== null && o.left !== null) {
+        o = o.left
+      }
     } else {
       o = parent._start
     }
@@ -180,39 +183,39 @@ export class AbstractItem extends AbstractStruct {
       const right = this.left.right
       this.right = right
       this.left.right = this
-      if (right !== null) {
-        right.left = this
-      }
     } else {
       let r
       if (parentSub !== null) {
-        const pmap = parent._map
+        r = parent._map.get(parentSub) || null
-        r = pmap.get(parentSub) || null
+        while (r !== null && r.left !== null) {
-        pmap.set(parentSub, this)
+          r = r.left
+        }
       } else {
         r = parent._start
         parent._start = this
       }
       this.right = r
-      if (r !== null) {
+    }
-        r.left = this
+    if (this.right !== null) {
+      this.right.left = this
+    } else if (parentSub !== null) {
+      // set as current parent value if right === null and this is parentSub
+      parent._map.set(parentSub, this)
+      if (this.left !== null) {
+        // this is the current attribute value of parent. delete right
+        this.left.delete(transaction)
       }
     }
-    // adjust the length of parent
+    // adjust length of parent
     if (parentSub === null && this.countable && !this.deleted) {
       parent._length += length
     }
     addStruct(store, this)
-    if (parent !== null) {
     maplib.setIfUndefined(transaction.changed, parent, set.create).add(parentSub)
-    }
     // @ts-ignore
-    if ((parent._item !== null && parent._item.deleted) || (this.left !== null && parentSub !== null)) {
+    if ((parent._item !== null && parent._item.deleted) || (this.right !== null && parentSub !== null)) {
       // delete if parent is deleted or if this is not the current attribute value of parent
       this.delete(transaction)
-    } else if (parentSub !== null && this.left === null && this.right !== null) {
-      // this is the current attribute value of parent. delete right
-      this.right.delete(transaction)
     }
   }
 
@@ -281,9 +284,9 @@ export class AbstractItem extends AbstractStruct {
       left = this.left
       right = this
     } else {
-      // Is a map item. Insert at the start
+      // Is a map item. Insert as current value
-      left = null
+      left = parent.type._map.get(this.parentSub)
-      right = parent.type._map.get(this.parentSub)
+      right = null
     }
     // make sure that parent is redone
     if (parent._deleted === true && parent.redone === null) {
@@ -308,7 +311,7 @@ export class AbstractItem extends AbstractStruct {
         if (right.redone !== null && right.redone.parent === parent) {
           right = right.redone
         }
-        right = right._right
+        right = right.right
       }
     }
     this.redone = this.copy(nextID(transaction), left, left === null ? null : left.lastId, right, right === null ? null : right.id, parent, this.parentSub)
@@ -411,18 +414,15 @@ export class AbstractItem extends AbstractStruct {
       r = new GC(this.id, this.length)
     } else {
       r = new ItemDeleted(this.id, this.left, this.origin, this.right, this.rightOrigin, this.parent, this.parentSub, this.length)
-      if (r.left !== null) {
-        r.left.right = r
-      }
       if (r.right !== null) {
         r.right.left = r
-      }
+      } else if (r.parentSub !== null) {
-      if (r.left === null) {
-        if (r.parentSub === null) {
-          r.parent._start = r
-        } else {
         r.parent._map.set(r.parentSub, r)
       }
+      if (r.left !== null) {
+        r.left.right = r
+      } else if (r.parentSub === null) {
+        r.parent._start = r
       }
     }
     replaceStruct(store, this, r)

src/structs/ItemType.js

@@ -21,18 +21,6 @@ import {
 import * as encoding from 'lib0/encoding.js' // eslint-disable-line
 import * as decoding from 'lib0/decoding.js'
 
-/**
- * @param {Transaction} transaction
- * @param {StructStore} store
- * @param {AbstractItem | null} item
- */
-const gcChildren = (transaction, store, item) => {
-  while (item !== null) {
-    item.gc(transaction, store)
-    item = item.right
-  }
-}
-
 export const structTypeRefNumber = 7
 
 /**
@@ -113,34 +101,26 @@ export class ItemType extends AbstractItem {
     super.delete(transaction)
     transaction.changed.delete(this.type)
     transaction.changedParentTypes.delete(this.type)
-    // delete map types
-    for (let value of this.type._map.values()) {
-      if (!value.deleted) {
-        value.delete(transaction)
-      }
-    }
-    // delete array types
-    let t = this.type._start
-    while (t !== null) {
-      if (!t.deleted) {
-        t.delete(transaction)
-      }
-      t = t.right
-    }
-    if (gcChildren) {
     this.gcChildren(transaction, transaction.y.store)
   }
-  }
 
   /**
    * @param {Transaction} transaction
    * @param {StructStore} store
    */
   gcChildren (transaction, store) {
-    gcChildren(transaction, store, this.type._start)
+    let item = this.type._start
+    while (item !== null) {
+      item.gc(transaction, store)
+      item = item.right
+    }
     this.type._start = null
     this.type._map.forEach(item => {
-      gcChildren(transaction, store, item)
+      while (item !== null) {
+        item.gc(transaction, store)
+        // @ts-ignore
+        item = item.left
+      }
     })
     this._map = new Map()
   }

src/types/AbstractType.js

@@ -447,9 +447,9 @@ export const typeMapDelete = (transaction, parent, key) => {
  * @param {Object|number|Array<any>|string|ArrayBuffer|AbstractType<any>} value
  */
 export const typeMapSet = (transaction, parent, key, value) => {
-  const right = parent._map.get(key) || null
+  const left = parent._map.get(key) || null
   if (value == null) {
-    new ItemJSON(nextID(transaction), null, null, right, right === null ? null : right.id, parent, key, [value]).integrate(transaction)
+    new ItemJSON(nextID(transaction), left, left === null ? null : left.lastId, null, null, parent, key, [value]).integrate(transaction)
     return
   }
   switch (value.constructor) {
@@ -457,14 +457,14 @@ export const typeMapSet = (transaction, parent, key, value) => {
     case Object:
     case Array:
     case String:
-      new ItemJSON(nextID(transaction), null, null, right, right === null ? null : right.id, parent, key, [value]).integrate(transaction)
+      new ItemJSON(nextID(transaction), left, left === null ? null : left.lastId, null, null, parent, key, [value]).integrate(transaction)
       break
     case ArrayBuffer:
-      new ItemBinary(nextID(transaction), null, null, right, right === null ? null : right.id, parent, key, value).integrate(transaction)
+      new ItemBinary(nextID(transaction), left, left === null ? null : left.lastId, null, null, parent, key, value).integrate(transaction)
       break
     default:
       if (value instanceof AbstractType) {
-        new ItemType(nextID(transaction), null, null, right, right === null ? null : right.id, parent, key, value).integrate(transaction)
+        new ItemType(nextID(transaction), left, left === null ? null : left.lastId, null, null, parent, key, value).integrate(transaction)
       } else {
         throw new Error('Unexpected content type')
       }
@@ -492,7 +492,7 @@ export const typeMapGetAll = (parent) => {
   let res = {}
   for (const [key, value] of parent._map) {
     if (!value.deleted) {
-      res[key] = value.getContent()[0]
+      res[key] = value.getContent()[value.length - 1]
     }
   }
   return res
@@ -517,9 +517,9 @@ export const typeMapHas = (parent, key) => {
 export const typeMapGetSnapshot = (parent, key, snapshot) => {
   let v = parent._map.get(key) || null
   while (v !== null && (!snapshot.sm.has(v.id.client) || v.id.clock >= (snapshot.sm.get(v.id.client) || 0))) {
-    v = v.right
+    v = v.left
   }
-  return v !== null && isVisible(v, snapshot) ? v.getContent()[0] : undefined
+  return v !== null && isVisible(v, snapshot) ? v.getContent()[v.length - 1] : undefined
 }
 
 /**

src/utils/Transaction.js

@@ -190,6 +190,10 @@ export const transact = (y, f) => {
         if (left.deleted === right.deleted && left.constructor === right.constructor) {
           if (left.mergeWith(right)) {
             structs.splice(pos, 1)
+            if (right instanceof AbstractItem && right.parentSub !== null && right.parent._map.get(right.parentSub) === right) {
+              // @ts-ignore we already did a constructor check above
+              right.parent._map.set(right.parentSub, left)
+            }
           }
         }
       }
@@ -210,6 +214,7 @@ export const transact = (y, f) => {
         }
       }
       // try to merge replacedItems
+      // TODO: replacedItems should hold ids
       for (const replacedItem of transaction._replacedItems) {
         const id = replacedItem.id
         const client = id.client

tests/y-map.tests.js

@@ -117,7 +117,7 @@ export const testGetAndSetOfMapPropertyWithConflict = tc => {
   testConnector.flushAllMessages()
   for (let user of users) {
     var u = user.getMap('map')
-    t.compare(u.get('stuff'), 'c0')
+    t.compare(u.get('stuff'), 'c1')
   }
   compare(users)
 }
@@ -128,8 +128,8 @@ export const testGetAndSetOfMapPropertyWithConflict = tc => {
 export const testGetAndSetAndDeleteOfMapProperty = tc => {
   const { testConnector, users, map0, map1 } = init(tc, { users: 3 })
   map0.set('stuff', 'c0')
-  map0.delete('stuff')
   map1.set('stuff', 'c1')
+  map1.delete('stuff')
   testConnector.flushAllMessages()
   for (let user of users) {
     var u = user.getMap('map')
@@ -150,7 +150,7 @@ export const testGetAndSetOfMapPropertyWithThreeConflicts = tc => {
   testConnector.flushAllMessages()
   for (let user of users) {
     var u = user.getMap('map')
-    t.compare(u.get('stuff'), 'c0')
+    t.compare(u.get('stuff'), 'c3')
   }
   compare(users)
 }
@@ -166,10 +166,10 @@ export const testGetAndSetAndDeleteOfMapPropertyWithThreeConflicts = tc => {
   map2.set('stuff', 'c3')
   testConnector.flushAllMessages()
   map0.set('stuff', 'deleteme')
-  map0.delete('stuff')
   map1.set('stuff', 'c1')
   map2.set('stuff', 'c2')
   map3.set('stuff', 'c3')
+  map3.delete('stuff')
   testConnector.flushAllMessages()
   for (let user of users) {
     var u = user.getMap('map')
@@ -348,7 +348,7 @@ const mapTransactions = [
  * @param {t.TestCase} tc
  */
 export const testRepeatGeneratingYmapTests10 = tc => {
-  applyRandomTests(tc, mapTransactions, 10)
+  applyRandomTests(tc, mapTransactions, 4)
 }
 
 /**