From 09482294224cac397b06950712110b725e86971d Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Mon, 6 Dec 2021 15:01:37 +0100 Subject: [PATCH] handle nested moves --- src/structs/ContentMove.js | 20 ++++++++++++++++++-- src/structs/Item.js | 6 ++++++ src/types/YArray.js | 4 ++-- src/utils/ListIterator.js | 4 ++-- src/utils/RelativePosition.js | 27 ++++++++++++--------------- src/utils/Transaction.js | 8 ++++++++ src/utils/YEvent.js | 2 +- tests/y-array.tests.js | 3 +++ 8 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/structs/ContentMove.js b/src/structs/ContentMove.js index 4ffc8c0f..1366b2d5 100644 --- a/src/structs/ContentMove.js +++ b/src/structs/ContentMove.js @@ -2,8 +2,9 @@ import * as error from 'lib0/error' import * as decoding from 'lib0/decoding' import * as encoding from 'lib0/encoding' +import * as math from 'lib0/math' import { - AbstractType, ContentType, ID, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd // eslint-disable-line + AbstractType, ContentType, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd // eslint-disable-line } from '../internals.js' /** @@ -175,13 +176,24 @@ export class ContentMove { * @type {{ start: Item | null, end: Item | null }} */ let { start, end } = getMovedCoords(this, transaction) + let maxPriority = 0 + // If this ContentMove was created locally, we set prio = -1. This indicates + // that we want to set prio to the current prio-maximum of the moved range. + const adaptPriority = this.priority < 0 while (start !== end && start != null) { if (!start.deleted) { const currMoved = start.moved - if (currMoved === null || /** @type {ContentMove} */ (currMoved.content).priority < this.priority || currMoved.id.client < item.id.client || (currMoved.id.client === item.id.client && currMoved.id.clock < item.id.clock)) { + const nextPrio = currMoved ? /** @type {ContentMove} */ (currMoved.content).priority : -1 + if (currMoved === null || adaptPriority || nextPrio < this.priority || currMoved.id.client < item.id.client || (currMoved.id.client === item.id.client && currMoved.id.clock < item.id.clock)) { if (currMoved !== null) { this.overrides.add(currMoved) } + maxPriority = math.max(maxPriority, nextPrio) + // was already moved + if (start.moved && !transaction.prevMoved.has(start)) { + // we need to know which item previously moved an item + transaction.prevMoved.set(start, start.moved) + } start.moved = item } else { /** @type {ContentMove} */ (currMoved.content).overrides.add(item) @@ -189,6 +201,9 @@ export class ContentMove { } start = start.right } + if (adaptPriority) { + this.priority = maxPriority + 1 + } } /** @@ -246,6 +261,7 @@ export class ContentMove { /** * @private + * @todo use binary encoding option for start & end relpos's * * @param {UpdateDecoderV1 | UpdateDecoderV2} decoder * @return {ContentMove} diff --git a/src/structs/Item.js b/src/structs/Item.js index 2fdff261..44538faf 100644 --- a/src/structs/Item.js +++ b/src/structs/Item.js @@ -117,6 +117,12 @@ export const splitItem = (transaction, leftItem, diff) => { /** @type {AbstractType} */ (rightItem.parent)._map.set(rightItem.parentSub, rightItem) } leftItem.length = diff + if (leftItem.moved) { + const m = transaction.prevMoved.get(leftItem) + if (m) { + transaction.prevMoved.set(rightItem, m) + } + } return rightItem } diff --git a/src/types/YArray.js b/src/types/YArray.js index a40dd92c..2f425a58 100644 --- a/src/types/YArray.js +++ b/src/types/YArray.js @@ -149,9 +149,9 @@ export class YArray extends AbstractType { } if (this.doc !== null) { transact(this.doc, transaction => { + const left = createRelativePositionFromTypeIndex(this, start, assocStart) + const right = createRelativePositionFromTypeIndex(this, end + 1, assocEnd) useSearchMarker(transaction, this, target, walker => { - const left = createRelativePositionFromTypeIndex(this, start, assocStart) - const right = createRelativePositionFromTypeIndex(this, end + 1, assocEnd) walker.insertMove(transaction, left, right) }) }) diff --git a/src/utils/ListIterator.js b/src/utils/ListIterator.js index 18a2d6ed..1e7385dc 100644 --- a/src/utils/ListIterator.js +++ b/src/utils/ListIterator.js @@ -122,7 +122,7 @@ export class ListIterator { len += this.rel this.rel = 0 } - while (item && !this.reachedEnd && (len > 0 || (len === 0 && (!item.countable || item.deleted || item === this.currMoveEnd)))) { + 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) { len -= item.length if (len < 0) { @@ -350,7 +350,7 @@ export class ListIterator { * @param {RelativePosition} end */ insertMove (tr, start, end) { - this.insertContents(tr, [new ContentMove(start, end, 1)]) // @todo adjust priority + this.insertContents(tr, [new ContentMove(start, end, -1)]) // @todo adjust priority // @todo is there a better alrogirthm to update searchmarkers? We could simply remove the markers that are in the updated range. // Also note that searchmarkers are updated in insertContents as well. const sm = this.type._searchMarker diff --git a/src/utils/RelativePosition.js b/src/utils/RelativePosition.js index 614c0bc5..719d7ba6 100644 --- a/src/utils/RelativePosition.js +++ b/src/utils/RelativePosition.js @@ -9,6 +9,8 @@ import { createID, ContentType, followRedone, + transact, + useSearchMarker, ID, Doc, AbstractType // eslint-disable-line } from '../internals.js' @@ -161,7 +163,6 @@ export const createRelativePosition = (type, item, assoc) => { * @function */ export const createRelativePositionFromTypeIndex = (type, index, assoc = 0) => { - let t = type._start if (assoc < 0) { // associated to the left character or the beginning of a type, increment index if possible. if (index === 0) { @@ -169,21 +170,17 @@ export const createRelativePositionFromTypeIndex = (type, index, assoc = 0) => { } index-- } - while (t !== null) { - if (!t.deleted && t.countable) { - if (t.length > index) { - // case 1: found position somewhere in the linked list - return createRelativePosition(type, createID(t.id.client, t.id.clock + index), assoc) + return transact(/** @type {Doc} */ (type.doc), tr => + useSearchMarker(tr, type, index, walker => { + if (walker.reachedEnd) { + const item = assoc < 0 ? /** @type {Item} */ (walker.nextItem).lastId : null + return createRelativePosition(type, item, assoc) + } else { + const id = /** @type {Item} */ (walker.nextItem).id + return createRelativePosition(type, createID(id.client, id.clock + walker.rel), assoc) } - index -= t.length - } - if (t.right === null && assoc < 0) { - // left-associated position, return last available id - return createRelativePosition(type, t.lastId, assoc) - } - t = t.right - } - return createRelativePosition(type, null, assoc) + }) + ) } /** diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index 7fb507f1..6476fa30 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -114,6 +114,14 @@ export class Transaction { * @type {Set} */ this.subdocsLoaded = new Set() + /** + * We store the reference that last moved an item. + * This is needed to compute the delta when multiple ContentMove move + * the same item. + * + * @type {Map} + */ + this.prevMoved = new Map() } } diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index a87cd181..10c757de 100644 --- a/src/utils/YEvent.js +++ b/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)) { + if (!currMoveIsNew && item.countable && !this.adds(item) && (this.transaction.prevMoved.get(item) || null) === currMove) { if (lastOp === null || lastOp.delete === undefined) { packOp() lastOp = { delete: 0 } diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index 02e55ae1..6cc65c8e 100644 --- a/tests/y-array.tests.js +++ b/tests/y-array.tests.js @@ -466,6 +466,9 @@ export const testMove = tc => { 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) + t.compare(array0.toArray(), [1, 2, 3]) + t.compare(event0.delta, [{ delete: 1 }, { retain: 1 }, { insert: [2] }]) compare(users) }