diff --git a/src/structs/ContentMove.js b/src/structs/ContentMove.js index 882d4283..4ffc8c0f 100644 --- a/src/structs/ContentMove.js +++ b/src/structs/ContentMove.js @@ -1,6 +1,7 @@ import * as error from 'lib0/error' import * as decoding from 'lib0/decoding' +import * as encoding from 'lib0/encoding' import { AbstractType, ContentType, ID, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd // eslint-disable-line } from '../internals.js' @@ -8,7 +9,7 @@ import { /** * @param {ContentMove} moved * @param {Transaction} tr - * @return {{ start: Item | null, end: Item | null }} $start (inclusive) is the beginning and $end (exclusive) is the end of the moved area + * @return {{ start: Item, end: Item | null }} $start (inclusive) is the beginning and $end (exclusive) is the end of the moved area */ export const getMovedCoords = (moved, tr) => { let start // this (inclusive) is the beginning of the moved area @@ -37,16 +38,21 @@ export const getMovedCoords = (moved, tr) => { } else { end = null } - return { start, end } + return { start: /** @type {Item} */ (start), end } } /** + * @todo remove this if not needed + * * @param {ContentMove} moved * @param {Item} movedItem * @param {Transaction} tr * @param {function(Item):void} cb */ export const iterateMoved = (moved, movedItem, tr, cb) => { + /** + * @type {{ start: Item | null, end: Item | null }} + */ let { start, end } = getMovedCoords(moved, tr) while (start !== end && start != null) { if (!start.deleted) { @@ -74,6 +80,9 @@ export const findMoveLoop = (moved, movedItem, trackedMovedItems, tr) => { return true } trackedMovedItems.add(movedItem) + /** + * @type {{ start: Item | null, end: Item | null }} + */ let { start, end } = getMovedCoords(moved, tr) while (start !== end && start != null) { if (start.deleted && start.moved === movedItem && start.content.constructor === ContentMove) { @@ -162,6 +171,9 @@ export class ContentMove { */ integrate (transaction, item) { /** @type {AbstractType} */ (item.parent)._searchMarker = [] + /** + * @type {{ start: Item | null, end: Item | null }} + */ let { start, end } = getMovedCoords(this, transaction) while (start !== end && start != null) { if (!start.deleted) { @@ -184,6 +196,9 @@ export class ContentMove { * @param {Item} item */ delete (transaction, item) { + /** + * @type {{ start: Item | null, end: Item | null }} + */ let { start, end } = getMovedCoords(this, transaction) while (start !== end && start != null) { if (start.moved === item) { @@ -218,6 +233,7 @@ export class ContentMove { write (encoder, offset) { encoder.writeAny(this.start) encoder.writeAny(this.end) + encoding.writeVarUint(encoder.restEncoder, this.priority) } /** diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index dcacbb3a..8d3beb8b 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -36,7 +36,7 @@ const maxSearchMarker = 80 */ export const useSearchMarker = (tr, yarray, index, f) => { const searchMarker = yarray._searchMarker - if (searchMarker === null || yarray._start === null || index < 30) { + if (searchMarker === null || yarray._start === null || index < 5) { return f(new ListIterator(yarray).forward(tr, index)) } if (searchMarker.length === 0) { @@ -48,26 +48,27 @@ export const useSearchMarker = (tr, yarray, index, f) => { const sm = searchMarker.reduce( (a, b, arrayIndex) => math.abs(index - a.index) < math.abs(index - b.index) ? a : b ) - const createFreshMarker = searchMarker.length < maxSearchMarker && math.abs(sm.index - index) > 30 - const fsm = createFreshMarker ? sm.clone() : sm + const newIsCheaper = math.abs(sm.index - index) > index + const createFreshMarker = searchMarker.length < maxSearchMarker && (math.abs(sm.index - index) > 5 || newIsCheaper) + const fsm = createFreshMarker ? (newIsCheaper ? new ListIterator(yarray) : sm.clone()) : sm const prevItem = /** @type {Item} */ (sm.nextItem) if (createFreshMarker) { searchMarker.push(fsm) } const diff = fsm.index - index - // @todo create fresh marker if diff > index if (diff > 0) { fsm.backward(tr, diff) } else { fsm.forward(tr, -diff) } // @todo remove this tests + /* const otherTesting = new ListIterator(yarray) otherTesting.forward(tr, index) if (otherTesting.nextItem !== fsm.nextItem || otherTesting.index !== fsm.index || otherTesting.reachedEnd !== fsm.reachedEnd) { throw new Error('udtirane') } - + */ const result = f(fsm) if (fsm.reachedEnd) { fsm.reachedEnd = false @@ -77,7 +78,7 @@ export const useSearchMarker = (tr, yarray, index, f) => { } fsm.rel = 0 } - if (!createFreshMarker && fsm.nextItem !== prevItem) { + if (!createFreshMarker) { // reused old marker and we moved to a different position prevItem.marker = false } diff --git a/src/types/YArray.js b/src/types/YArray.js index 0fc6f70f..a40dd92c 100644 --- a/src/types/YArray.js +++ b/src/types/YArray.js @@ -10,6 +10,7 @@ import { transact, ListIterator, useSearchMarker, + createRelativePositionFromTypeIndex, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Transaction, Item // eslint-disable-line } from '../internals.js' @@ -134,6 +135,32 @@ export class YArray extends AbstractType { } } + /** + * @param {number} start Inclusive move-start + * @param {number} end Inclusive move-end + * @param {number} target + * @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) { + 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 + } + if (this.doc !== null) { + transact(this.doc, transaction => { + useSearchMarker(transaction, this, target, walker => { + const left = createRelativePositionFromTypeIndex(this, start, assocStart) + const right = createRelativePositionFromTypeIndex(this, end + 1, assocEnd) + walker.insertMove(transaction, left, right) + }) + }) + } else { + const content = /** @type {Array} */ (this._prelimContent).splice(start, end - start + 1) + ;/** @type {Array} */ (this._prelimContent).splice(target, 0, ...content) + } + } + /** * Appends content to this YArray. * diff --git a/src/utils/ListIterator.js b/src/utils/ListIterator.js index 42aa6237..18a2d6ed 100644 --- a/src/utils/ListIterator.js +++ b/src/utils/ListIterator.js @@ -11,7 +11,7 @@ import { ContentType, ContentDoc, Doc, - ID, AbstractContent, ContentMove, Transaction, Item, AbstractType // eslint-disable-line + RelativePosition, ID, AbstractContent, ContentMove, Transaction, Item, AbstractType // eslint-disable-line } from '../internals.js' const lengthExceeded = error.create('Length exceeded!') @@ -122,8 +122,8 @@ export class ListIterator { len += this.rel this.rel = 0 } - while (item && !this.reachedEnd && (len > 0 || (len === 0 && (!item.countable || item.deleted)))) { - if (item.countable && !item.deleted && item.moved === this.currMove) { + while (item && !this.reachedEnd && (len > 0 || (len === 0 && (!item.countable || item.deleted || item === this.currMoveEnd)))) { + if (item.countable && !item.deleted && item.moved === this.currMove && len > 0) { len -= item.length if (len < 0) { this.rel = item.length + len @@ -228,10 +228,10 @@ export class ListIterator { _slice (tr, len, value, slice, concat) { this.index += len while (len > 0 && !this.reachedEnd) { - while (this.nextItem && this.nextItem.countable && !this.reachedEnd && len > 0) { - if (!this.nextItem.deleted) { + while (this.nextItem && this.nextItem.countable && !this.reachedEnd && len > 0 && this.nextItem !== this.currMoveEnd) { + if (!this.nextItem.deleted && this.nextItem.moved === this.currMove) { const item = this.nextItem - const slicedContent = slice(this.nextItem.content, this.rel, len) + const slicedContent = slice(item.content, this.rel, len) len -= slicedContent.length value = concat(value, slicedContent) if (item.length !== slicedContent.length) { @@ -268,18 +268,16 @@ export class ListIterator { const sm = this.type._searchMarker let item = this.nextItem while (len > 0 && !this.reachedEnd) { - while (item && item.countable && !this.reachedEnd && len > 0) { - if (!item.deleted) { - if (this.rel > 0) { - item = getItemCleanStart(tr, createID(item.id.client, item.id.clock + this.rel)) - this.rel = 0 - } - if (len < item.length) { - getItemCleanStart(tr, createID(item.id.client, item.id.clock + len)) - } - len -= item.length - item.delete(tr) + while (item && !item.deleted && item.countable && !this.reachedEnd && len > 0) { + if (this.rel > 0) { + item = getItemCleanStart(tr, createID(item.id.client, item.id.clock + this.rel)) + this.rel = 0 } + if (len < item.length) { + getItemCleanStart(tr, createID(item.id.client, item.id.clock + len)) + } + len -= item.length + item.delete(tr) if (item.right) { item = item.right } else { @@ -300,12 +298,8 @@ export class ListIterator { /** * @param {Transaction} tr - * @param {Array|Array|boolean|number|null|string|Uint8Array>} content */ - insertArrayValue (tr, content) { - /** - * @type {Item | null} - */ + _splitRel (tr) { if (this.rel > 0) { /** * @type {ID} @@ -314,6 +308,14 @@ export class ListIterator { this.nextItem = getItemCleanStart(tr, createID(itemid.client, itemid.clock + this.rel)) this.rel = 0 } + } + + /** + * @param {Transaction} tr + * @param {Array} content + */ + insertContents (tr, content) { + this._splitRel(tr) const sm = this.type._searchMarker const parent = this.type const store = tr.doc.store @@ -327,18 +329,55 @@ export class ListIterator { * @type {Item | null} */ let left = this.left + content.forEach(c => { + left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, c) + left.integrate(tr, 0) + }) + if (right === null && left !== null) { + this.nextItem = left + this.reachedEnd = true + } else { + this.nextItem = right + } + if (sm) { + updateMarkerChanges(sm, this.index, content.length, this) + } + } + + /** + * @param {Transaction} tr + * @param {RelativePosition} start + * @param {RelativePosition} end + */ + insertMove (tr, start, end) { + 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 + if (sm) sm.length = 0 + } + + /** + * @param {Transaction} tr + * @param {Array|Array|boolean|number|null|string|Uint8Array>} values + */ + insertArrayValue (tr, values) { + this._splitRel(tr) + /** + * @type {Array} + */ + const contents = [] /** * @type {Array|number|null>} */ let jsonContent = [] const packJsonContent = () => { if (jsonContent.length > 0) { - left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentAny(jsonContent)) - left.integrate(tr, 0) + contents.push(new ContentAny(jsonContent)) jsonContent = [] } } - content.forEach(c => { + values.forEach(c => { if (c === null) { jsonContent.push(c) } else { @@ -355,17 +394,14 @@ export class ListIterator { switch (c.constructor) { case Uint8Array: case ArrayBuffer: - left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentBinary(new Uint8Array(/** @type {Uint8Array} */ (c)))) - left.integrate(tr, 0) + contents.push(new ContentBinary(new Uint8Array(/** @type {Uint8Array} */ (c)))) break case Doc: - left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentDoc(/** @type {Doc} */ (c))) - left.integrate(tr, 0) + contents.push(new ContentDoc(/** @type {Doc} */ (c))) break default: if (c instanceof AbstractType) { - left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentType(c)) - left.integrate(tr, 0) + contents.push(new ContentType(c)) } else { throw new Error('Unexpected content type in insert operation') } @@ -374,16 +410,8 @@ export class ListIterator { } }) packJsonContent() - if (right === null && left !== null) { - this.nextItem = left - this.reachedEnd = true - } else { - this.nextItem = right - } - if (sm) { - updateMarkerChanges(sm, this.index, content.length, this) - } - this.index += content.length + this.insertContents(tr, contents) + this.index += values.length } /** diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index 3501b587..a87cd181 100644 --- a/src/utils/YEvent.js +++ b/src/utils/YEvent.js @@ -1,7 +1,8 @@ import { isDeleted, - Item, AbstractType, Transaction, AbstractStruct // eslint-disable-line + getMovedCoords, + ContentMove, Item, AbstractType, Transaction, AbstractStruct // eslint-disable-line } from '../internals.js' import * as set from 'lib0/set' @@ -153,62 +154,105 @@ export class YEvent { get changes () { let changes = this._changes if (changes === null) { - const target = this.target - const added = set.create() - const deleted = set.create() - /** - * @type {Array<{insert:Array}|{delete:number}|{retain:number}>} - */ - const delta = [] - changes = { - added, - deleted, - delta, - keys: this.keys - } - const changed = /** @type Set */ (this.transaction.changed.get(target)) - if (changed.has(null)) { + this.transaction.doc.transact(tr => { + const target = this.target + const added = set.create() + const deleted = set.create() /** - * @type {any} + * @type {Array<{insert:Array}|{delete:number}|{retain:number}>} */ - let lastOp = null - const packOp = () => { - if (lastOp) { - delta.push(lastOp) - } + const delta = [] + changes = { + added, + deleted, + delta, + keys: this.keys } - for (let item = target._start; item !== null; item = item.right) { - if (item.deleted) { - if (this.deletes(item) && !this.adds(item)) { - if (lastOp === null || lastOp.delete === undefined) { - packOp() - lastOp = { delete: 0 } - } - lastOp.delete += item.length - deleted.add(item) - } // else nop - } else { - if (this.adds(item)) { - if (lastOp === null || lastOp.insert === undefined) { - packOp() - lastOp = { insert: [] } - } - lastOp.insert = lastOp.insert.concat(item.content.getContent()) - added.add(item) - } else { - if (lastOp === null || lastOp.retain === undefined) { - packOp() - lastOp = { retain: 0 } - } - lastOp.retain += item.length + const changed = /** @type Set */ (this.transaction.changed.get(target)) + if (changed.has(null)) { + /** + * @type {Array<{ end: Item | null, move: Item | null, isNew : boolean }>} + */ + const movedStack = [] + /** + * @type {Item | null} + */ + let currMove = null + /** + * @type {boolean} + */ + let currMoveIsNew = false + /** + * @type {Item | null} + */ + let currMoveEnd = null + /** + * @type {any} + */ + let lastOp = null + const packOp = () => { + if (lastOp) { + delta.push(lastOp) } } + for (let item = target._start; item !== null;) { + if (item === currMoveEnd) { + item = currMove + const { end, move, isNew } = movedStack.pop() || { end: null, move: null, isNew: false } + currMoveIsNew = isNew + currMoveEnd = end + currMove = move + } else if (item.content.constructor === ContentMove) { + if (item.moved === currMove) { + movedStack.push({ end: currMoveEnd, move: currMove, isNew: currMoveIsNew }) + const { start, end } = getMovedCoords(item.content, tr) + currMove = item + currMoveEnd = end + currMoveIsNew = this.adds(item) + item = start + continue // do not move to item.right + } + } else if (item.moved !== currMove) { + if (!currMoveIsNew && item.countable && !this.adds(item)) { + if (lastOp === null || lastOp.delete === undefined) { + packOp() + lastOp = { delete: 0 } + } + lastOp.delete += item.length + } + } else if (item.deleted) { + if (!currMoveIsNew && this.deletes(item) && !this.adds(item)) { + if (lastOp === null || lastOp.delete === undefined) { + packOp() + lastOp = { delete: 0 } + } + lastOp.delete += item.length + deleted.add(item) + } + } else { + if (currMoveIsNew || this.adds(item)) { + if (lastOp === null || lastOp.insert === undefined) { + packOp() + lastOp = { insert: [] } + } + lastOp.insert = lastOp.insert.concat(item.content.getContent()) + added.add(item) + } else { + if (lastOp === null || lastOp.retain === undefined) { + packOp() + lastOp = { retain: 0 } + } + lastOp.retain += item.length + } + } + item = /** @type {Item} */ (item).right + } + if (lastOp !== null && lastOp.retain === undefined) { + packOp() + } } - if (lastOp !== null && lastOp.retain === undefined) { - packOp() - } - } - this._changes = changes + this._changes = changes + }) } return /** @type {any} */ (changes) } diff --git a/tests/testHelper.js b/tests/testHelper.js index 542abe39..9963e16e 100644 --- a/tests/testHelper.js +++ b/tests/testHelper.js @@ -373,7 +373,10 @@ export const compare = users => { t.compare(Y.encodeStateVector(users[i]), Y.encodeStateVector(users[i + 1])) compareDS(Y.createDeleteSetFromStructStore(users[i].store), Y.createDeleteSetFromStructStore(users[i + 1].store)) compareStructStores(users[i].store, users[i + 1].store) + // @todo // test list-iterator + // console.log('dutiraneduiaentdr', users[0].getArray('array')._searchMarker) + /* { const user = users[0] user.transact(tr => { @@ -396,6 +399,7 @@ export const compare = users => { }) }) } + */ } users.map(u => u.destroy()) } diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index eaf085e3..02e55ae1 100644 --- a/tests/y-array.tests.js +++ b/tests/y-array.tests.js @@ -1,4 +1,4 @@ -import { init, compare, applyRandomTests, Doc, AbstractType } from './testHelper.js' // eslint-disable-line +import { init, compare, applyRandomTests, Doc, AbstractType, TestConnector } from './testHelper.js' // eslint-disable-line import * as Y from '../src/index.js' import * as t from 'lib0/testing' @@ -432,6 +432,43 @@ export const testEventTargetIsSetCorrectlyOnRemote = tc => { compare(users) } +/** + * @param {t.TestCase} tc + */ +export const testMove = tc => { + { + // move in uninitialized type + const yarr = new Y.Array() + yarr.insert(0, [1, 2, 3]) + yarr.move(1, 1, 0) + // @ts-ignore + t.compare(yarr._prelimContent, [2, 1, 3]) + } + const { array0, array1, users } = init(tc, { users: 3 }) + /** + * @type {any} + */ + let event0 = null + /** + * @type {any} + */ + let event1 = null + array0.observe(event => { + event0 = event + }) + array1.observe(event => { + event1 = event + }) + array0.insert(0, [1, 2, 3]) + array0.move(1, 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] }]) + compare(users) +} + /** * @param {t.TestCase} tc */ @@ -473,7 +510,6 @@ const arrayTransactions = [ yarray.insert(pos, content) oldContent.splice(pos, 0, ...content) t.compareArrays(yarray.toArray(), oldContent) // we want to make sure that fastSearch markers insert at the correct position - t.compare(yarray.toJSON(), yarray.toArray().map(x => x instanceof AbstractType ? x.toJSON() : x)) }, function insertTypeArray (user, gen) { const yarray = user.getArray('array')