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|Array|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} @@ -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} */ - 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} */ - 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):boolean} f + * @param {function(AbstractType):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} parent - * @param {AbstractItem} child target + * @param {AbstractType} child target * @return {Array} 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} is - * @param {Object} 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} + * @type {Object?} */ - 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} + * @type {Object|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) }