adding more sanity checkss to yarray.tests

src/structs/ContentMove.js

@@ -6,6 +6,7 @@ import * as math from 'lib0/math'
 import {
   AbstractType, ContentType, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd // eslint-disable-line
 } from '../internals.js'
+import { decodeRelativePosition, encodeRelativePosition } from 'yjs'
 
 /**
  * @param {ContentMove} moved
@@ -246,8 +247,12 @@ export class ContentMove {
    * @param {number} offset
    */
   write (encoder, offset) {
-    encoder.writeAny(this.start)
-    encoder.writeAny(this.end)
+    const isCollapsed = this.isCollapsed()
+    encoding.writeUint8(encoder.restEncoder, isCollapsed ? 1 : 0)
+    encoder.writeBuf(encodeRelativePosition(this.start))
+    if (!isCollapsed) {
+      encoder.writeBuf(encodeRelativePosition(this.end))
+    }
     encoding.writeVarUint(encoder.restEncoder, this.priority)
   }
 
@@ -257,6 +262,10 @@ export class ContentMove {
   getRef () {
     return 11
   }
+
+  isCollapsed () {
+    return this.start.item === this.end.item && this.start.item !== null
+  }
 }
 
 /**
@@ -266,4 +275,12 @@ export class ContentMove {
  * @param {UpdateDecoderV1 | UpdateDecoderV2} decoder
  * @return {ContentMove}
  */
-export const readContentMove = decoder => new ContentMove(decoder.readAny(), decoder.readAny(), decoding.readVarUint(decoder.restDecoder))
+export const readContentMove = decoder => {
+  const isCollapsed = decoding.readUint8(decoder.restDecoder) === 1
+  const start = decodeRelativePosition(decoder.readBuf())
+  const end = isCollapsed ? start.clone() : decodeRelativePosition(decoder.readBuf())
+  if (isCollapsed) {
+    end.assoc = -1
+  }
+  return new ContentMove(start, end, decoding.readVarUint(decoder.restDecoder))
+}

src/types/YArray.js

@@ -137,6 +137,34 @@ export class YArray extends AbstractType {
     }
   }
 
+  /**
+   * Move a single item from $index to $target.
+   *
+   * @todo make sure that collapsed moves are removed (i.e. when moving the same item twice)
+   *
+   * @param {number} index
+   * @param {number} target
+   */
+  move (index, target) {
+    if (index === target || index + 1 === target || index >= this.length) {
+      // It doesn't make sense to move a range into the same range (it's basically a no-op).
+      return
+    }
+    if (this.doc !== null) {
+      transact(this.doc, transaction => {
+        const left = createRelativePositionFromTypeIndex(this, index, 1)
+        const right = left.clone()
+        right.assoc = -1
+        useSearchMarker(transaction, this, target, walker => {
+          walker.insertMove(transaction, left, right)
+        })
+      })
+    } else {
+      const content = /** @type {Array<any>} */ (this._prelimContent).splice(index, 1)
+      ;/** @type {Array<any>} */ (this._prelimContent).splice(target, 0, ...content)
+    }
+  }
+
   /**
    * @param {number} start Inclusive move-start
    * @param {number} end Inclusive move-end
@@ -144,7 +172,7 @@ export class YArray extends AbstractType {
    * @param {number} assocStart >=0 if start should be associated with the right character. See relative-position assoc parameter.
    * @param {number} assocEnd >= 0 if end should be associated with the right character.
    */
-  move (start, end, target, assocStart = 1, assocEnd = -1) {
+  moveRange (start, end, target, assocStart = 1, assocEnd = -1) {
     if (start <= target && target <= end) {
       // It doesn't make sense to move a range into the same range (it's basically a no-op).
       return

src/utils/ListIterator.js

@@ -122,8 +122,17 @@ export class ListIterator {
       len += this.rel
       this.rel = 0
     }
-    while (item && !this.reachedEnd && (len > 0 || (len === 0 && (!item.countable || item.deleted || item === this.currMoveEnd || item.moved !== this.currMove)))) {
-      if (item.countable && !item.deleted && item.moved === this.currMove && len > 0) {
+    while ((!this.reachedEnd || this.currMove !== null) && (len > 0 || (len === 0 && item && (!item.countable || item.deleted || item === this.currMoveEnd || (this.reachedEnd && this.currMoveEnd === null) || item.moved !== this.currMove)))) {
+      if (item === this.currMoveEnd || (this.currMoveEnd === null && this.reachedEnd && this.currMove)) {
+        item = /** @type {Item} */ (this.currMove) // we iterate to the right after the current condition
+        const { start, end, move } = this.movedStack.pop() || { start: null, end: null, move: null }
+        this.currMove = move
+        this.currMoveStart = start
+        this.currMoveEnd = end
+        this.reachedEnd = false
+      } else if (item === null) {
+        break
+      } else if (item.countable && !item.deleted && item.moved === this.currMove && len > 0) {
         len -= item.length
         if (len < 0) {
           this.rel = item.length + len
@@ -141,13 +150,6 @@ export class ListIterator {
         item = start
         continue
       }
-      if (item === this.currMoveEnd) {
-        item = /** @type {Item} */ (this.currMove) // we iterate to the right after the current condition
-        const { start, end, move } = this.movedStack.pop() || { start: null, end: null, move: null }
-        this.currMove = move
-        this.currMoveStart = start
-        this.currMoveEnd = end
-      }
       if (item.right) {
         item = item.right
       } else {
@@ -159,6 +161,23 @@ export class ListIterator {
     return this
   }
 
+  /**
+   * @param {Transaction} tr
+   */
+  reduceMoves (tr) {
+    let item = this.nextItem
+    if (item !== null) {
+      while (item === this.currMoveStart) {
+        item = /** @type {Item} */ (this.currMove) // we iterate to the left after the current condition
+        const { start, end, move } = this.movedStack.pop() || { start: null, end: null, move: null }
+        this.currMove = move
+        this.currMoveStart = start
+        this.currMoveEnd = end
+      }
+      this.nextItem = item
+    }
+  }
+
   /**
    * @param {Transaction} tr
    * @param {number} len
@@ -249,7 +268,7 @@ export class ListIterator {
           this.reachedEnd = true
         }
       }
-      if (this.nextItem && !this.reachedEnd && len > 0) {
+      if (this.nextItem && (!this.reachedEnd || this.currMove !== null) && len > 0) {
         this.forward(tr, 0)
       }
     }
@@ -317,6 +336,7 @@ export class ListIterator {
    * @param {Array<AbstractContent>} content
    */
   insertContents (tr, content) {
+    this.reduceMoves(tr)
     this._splitRel(tr)
     const parent = this.type
     const store = tr.doc.store

src/utils/RelativePosition.js

@@ -75,6 +75,10 @@ export class RelativePosition {
      */
     this.assoc = assoc
   }
+
+  clone () {
+    return new RelativePosition(this.type, this.tname, this.item, this.assoc)
+  }
 }
 
 /**

src/utils/YEvent.js

@@ -213,7 +213,7 @@ export class YEvent {
                 continue // do not move to item.right
               }
             } else if (item.moved !== currMove) {
-              if (!currMoveIsNew && item.countable && !this.adds(item) && (this.transaction.prevMoved.get(item) || null) === currMove) {
+              if (!currMoveIsNew && item.countable && item.moved && !this.adds(item) && !this.adds(item.moved) && (this.transaction.prevMoved.get(item) || null) === currMove) {
                 if (lastOp === null || lastOp.delete === undefined) {
                   packOp()
                   lastOp = { delete: 0 }

src/utils/updates.js

@@ -308,6 +308,7 @@ export const mergeUpdatesV2 = (updates, YDecoder = UpdateDecoderV2, YEncoder = U
   // Note: Should handle that some operations cannot be applied yet ()
 
   while (true) {
+    // @todo this incurs an exponential overhead. We could instead only sort the item that changed.
     // Write higher clients first ⇒ sort by clientID & clock and remove decoders without content
     lazyStructDecoders = lazyStructDecoders.filter(dec => dec.curr !== null)
     lazyStructDecoders.sort(

tests/y-array.tests.js

@@ -440,7 +440,7 @@ export const testMove = tc => {
     // move in uninitialized type
     const yarr = new Y.Array()
     yarr.insert(0, [1, 2, 3])
-    yarr.move(1, 1, 0)
+    yarr.move(1, 0)
     // @ts-ignore
     t.compare(yarr._prelimContent, [2, 1, 3])
   }
@@ -460,13 +460,13 @@ export const testMove = tc => {
     event1 = event
   })
   array0.insert(0, [1, 2, 3])
-  array0.move(1, 1, 0)
+  array0.move(1, 0)
   t.compare(array0.toArray(), [2, 1, 3])
   t.compare(event0.delta, [{ insert: [2] }, { retain: 1 }, { delete: 1 }])
   Y.applyUpdate(users[1], Y.encodeStateAsUpdate(users[0]))
   t.compare(array1.toArray(), [2, 1, 3])
   t.compare(event1.delta, [{ insert: [2, 1, 3] }])
-  array0.move(0, 0, 2)
+  array0.move(0, 2)
   t.compare(array0.toArray(), [1, 2, 3])
   t.compare(event0.delta, [{ delete: 1 }, { retain: 1 }, { insert: [2] }])
   compare(users)
@@ -500,6 +500,19 @@ const getUniqueNumber = () => _uniqueNumber++
  * @todo to replace content to a separate data structure so we know that insert & returns work as expected!!!
  */
 const arrayTransactions = [
+  function move (user, gen) {
+    const yarray = user.getArray('array')
+    if (yarray.length === 0) {
+      return
+    }
+    const pos = prng.int32(gen, 0, yarray.length - 1)
+    const newPos = prng.int32(gen, 0, yarray.length)
+    const oldContent = yarray.toArray()
+    yarray.move(pos, newPos)
+    const [x] = oldContent.splice(pos, 1)
+    oldContent.splice(pos < newPos ? newPos - 1 : newPos, 0, x)
+    t.compareArrays(yarray.toArray(), oldContent) // we want to make sure that fastSearch markers insert at the correct position
+  },
   function insert (user, gen) {
     const yarray = user.getArray('array')
     const uniqueNumber = getUniqueNumber()
@@ -558,11 +571,49 @@ const arrayTransactions = [
   }
 ]
 
+/**
+ * @param {Y.Doc} user
+ */
+const monitorArrayTestObject = user => {
+  /**
+   * @type {Array<any>}
+   */
+  const arr = []
+  const yarr = user.getArray('array')
+  yarr.observe(event => {
+    let currpos = 0
+    const delta = event.delta
+    for (let i = 0; i < delta.length; i++) {
+      const d = delta[i]
+      if (d.insert != null) {
+        arr.splice(currpos, 0, ...(/** @type {Array<any>} */ (d.insert)))
+        currpos += /** @type {Array<any>} */ (d.insert).length
+      } else if (d.retain != null) {
+        currpos += d.retain
+      } else {
+        arr.splice(currpos, d.delete)
+      }
+    }
+  })
+  return arr
+}
+
+/**
+ * @param {{ testObjects: Array<Array<any>>, users: Array<Y.Doc> }} cmp
+ */
+const compareTestobjects = cmp => {
+  const arrs = cmp.testObjects
+  for (let i = 0; i < arrs.length; i++) {
+    const type = cmp.users[i].getArray('array')
+    t.compareArrays(arrs[i], type.toArray())
+  }
+}
+
 /**
  * @param {t.TestCase} tc
  */
 export const testRepeatGeneratingYarrayTests6 = tc => {
-  applyRandomTests(tc, arrayTransactions, 10)
+  compareTestobjects(applyRandomTests(tc, arrayTransactions, 3, monitorArrayTestObject))
 }
 
 /**