splitting an item must always happen inside a transaction, because we always need to check if we can merge it back

This commit is contained in:
Kevin Jahns 2019-04-11 00:23:08 +02:00
parent 9fe47e98d5
commit 2ef11a5344
18 changed files with 72 additions and 63 deletions

View File

@ -29,11 +29,12 @@ import * as binary from 'lib0/binary.js'
/** /**
* Split leftItem into two items * Split leftItem into two items
* @param {Transaction} transaction
* @param {AbstractItem} leftItem * @param {AbstractItem} leftItem
* @param {number} diff * @param {number} diff
* @return {AbstractItem} * @return {AbstractItem}
*/ */
export const splitItem = (leftItem, diff) => { export const splitItem = (transaction, leftItem, diff) => {
const id = leftItem.id const id = leftItem.id
// create rightItem // create rightItem
const rightItem = leftItem.copy( const rightItem = leftItem.copy(
@ -54,6 +55,8 @@ export const splitItem = (leftItem, diff) => {
if (rightItem.right !== null) { if (rightItem.right !== null) {
rightItem.right.left = rightItem rightItem.right.left = rightItem
} }
// right is more specific.
transaction._replacedItems.add(rightItem)
return rightItem return rightItem
} }
@ -357,10 +360,11 @@ export class AbstractItem extends AbstractStruct {
* *
* This method should only be cally by StructStore. * This method should only be cally by StructStore.
* *
* @param {Transaction} transaction
* @param {number} diff * @param {number} diff
* @return {AbstractItem} * @return {AbstractItem}
*/ */
splitAt (diff) { splitAt (transaction, diff) {
throw new Error('unimplemented') throw new Error('unimplemented')
} }
@ -549,7 +553,7 @@ export const changeItemRefOffset = (item, offset) => {
* Outsourcing some of the logic of computing the item params from a received struct. * Outsourcing some of the logic of computing the item params from a received struct.
* If parent === null, it is expected to gc the read struct. Otherwise apply it. * If parent === null, it is expected to gc the read struct. Otherwise apply it.
* *
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {ID|null} leftid * @param {ID|null} leftid
* @param {ID|null} rightid * @param {ID|null} rightid
@ -558,13 +562,9 @@ export const changeItemRefOffset = (item, offset) => {
* @param {string|null} parentYKey * @param {string|null} parentYKey
* @return {{left:AbstractItem?,right:AbstractItem?,parent:AbstractType<YEvent>?,parentSub:string?}} * @return {{left:AbstractItem?,right:AbstractItem?,parent:AbstractType<YEvent>?,parentSub:string?}}
*/ */
export const computeItemParams = (y, store, leftid, rightid, parentid, parentSub, parentYKey) => { export const computeItemParams = (transaction, store, leftid, rightid, parentid, parentSub, parentYKey) => {
const left = leftid === null ? null : getItemCleanEnd(store, leftid) const left = leftid === null ? null : getItemCleanEnd(transaction, store, leftid)
if (left !== null && left.constructor !== GC && left.right !== null && left.right.id.client === left.id.client && left.right.id.clock === left.id.clock + left.length) { const right = rightid === null ? null : getItemCleanStart(transaction, store, rightid)
// we split a merged op, we may need to merge it again after the transaction
y.transaction._replacedItems.add(left)
}
const right = rightid === null ? null : getItemCleanStart(store, rightid)
let parent = null let parent = null
if (parentid !== null) { if (parentid !== null) {
const parentItem = getItemType(store, parentid) const parentItem = getItemType(store, parentid)
@ -576,7 +576,7 @@ export const computeItemParams = (y, store, leftid, rightid, parentid, parentSub
parent = parentItem.type parent = parentItem.type
} }
} else if (parentYKey !== null) { } else if (parentYKey !== null) {
parent = y.get(parentYKey) parent = transaction.y.get(parentYKey)
} else if (left !== null) { } else if (left !== null) {
if (left.constructor !== GC) { if (left.constructor !== GC) {
parent = left.parent parent = left.parent

View File

@ -76,12 +76,12 @@ export class AbstractRef {
return this._missing return this._missing
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {AbstractStruct} * @return {AbstractStruct}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
throw error.methodUnimplemented() throw error.methodUnimplemented()
} }
/** /**

View File

@ -82,12 +82,12 @@ export class GCRef extends AbstractRef {
] ]
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {GC} * @return {GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
if (offset > 0) { if (offset > 0) {
// @ts-ignore // @ts-ignore
this.id = createID(this.id.client, this.id.clock + offset) this.id = createID(this.id.client, this.id.clock + offset)

View File

@ -7,7 +7,7 @@ import {
AbstractItemRef, AbstractItemRef,
computeItemParams, computeItemParams,
GC, GC,
StructStore, Y, AbstractType, ID, YEvent // eslint-disable-line StructStore, Transaction, AbstractType, ID // eslint-disable-line
} from '../internals.js' } from '../internals.js'
import * as encoding from 'lib0/encoding.js' import * as encoding from 'lib0/encoding.js'
@ -69,13 +69,13 @@ export class ItemBinaryRef extends AbstractItemRef {
this.content = decoding.readPayload(decoder) this.content = decoding.readPayload(decoder)
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemBinary|GC} * @return {ItemBinary|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemBinary( : new ItemBinary(

View File

@ -57,14 +57,15 @@ export class ItemDeleted extends AbstractItem {
addToDeleteSet(transaction.deleteSet, this.id, this.length) addToDeleteSet(transaction.deleteSet, this.id, this.length)
} }
/** /**
* @param {Transaction} transaction
* @param {number} diff * @param {number} diff
*/ */
splitAt (diff) { splitAt (transaction, diff) {
/** /**
* @type {ItemDeleted} * @type {ItemDeleted}
*/ */
// @ts-ignore // @ts-ignore
const right = splitItem(this, diff) const right = splitItem(transaction, this, diff)
right._len -= diff right._len -= diff
this._len = diff this._len = diff
return right return right
@ -107,18 +108,18 @@ export class ItemDeletedRef extends AbstractItemRef {
return this.len return this.len
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemDeleted|GC} * @return {ItemDeleted|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
if (offset > 0) { if (offset > 0) {
changeItemRefOffset(this, offset) changeItemRefOffset(this, offset)
this.len = this.len - offset this.len = this.len - offset
} }
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemDeleted( : new ItemDeleted(

View File

@ -7,7 +7,7 @@ import {
AbstractItemRef, AbstractItemRef,
computeItemParams, computeItemParams,
GC, GC,
Y, StructStore, ID, AbstractType // eslint-disable-line Transaction, StructStore, ID, AbstractType // eslint-disable-line
} from '../internals.js' } from '../internals.js'
import * as encoding from 'lib0/encoding.js' import * as encoding from 'lib0/encoding.js'
@ -66,13 +66,13 @@ export class ItemEmbedRef extends AbstractItemRef {
this.embed = JSON.parse(decoding.readVarString(decoder)) this.embed = JSON.parse(decoding.readVarString(decoder))
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemEmbed|GC} * @return {ItemEmbed|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemEmbed( : new ItemEmbed(

View File

@ -7,7 +7,7 @@ import {
AbstractItemRef, AbstractItemRef,
computeItemParams, computeItemParams,
GC, GC,
Y, StructStore, ID, AbstractType // eslint-disable-line Transaction, StructStore, ID, AbstractType // eslint-disable-line
} from '../internals.js' } from '../internals.js'
import * as encoding from 'lib0/encoding.js' import * as encoding from 'lib0/encoding.js'
@ -73,13 +73,13 @@ export class ItemFormatRef extends AbstractItemRef {
this.value = JSON.parse(decoding.readVarString(decoder)) this.value = JSON.parse(decoding.readVarString(decoder))
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemFormat|GC} * @return {ItemFormat|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemFormat( : new ItemFormat(

View File

@ -9,7 +9,7 @@ import {
splitItem, splitItem,
changeItemRefOffset, changeItemRefOffset,
GC, GC,
StructStore, Y, ID, AbstractType // eslint-disable-line Transaction, StructStore, Y, ID, AbstractType // eslint-disable-line
} from '../internals.js' } from '../internals.js'
import * as encoding from 'lib0/encoding.js' import * as encoding from 'lib0/encoding.js'
@ -54,14 +54,15 @@ export class ItemJSON extends AbstractItem {
return this.content return this.content
} }
/** /**
* @param {Transaction} transaction
* @param {number} diff * @param {number} diff
*/ */
splitAt (diff) { splitAt (transaction, diff) {
/** /**
* @type {ItemJSON} * @type {ItemJSON}
*/ */
// @ts-ignore // @ts-ignore
const right = splitItem(this, diff) const right = splitItem(transaction, this, diff)
right.content = this.content.splice(diff) right.content = this.content.splice(diff)
return right return right
} }
@ -118,17 +119,17 @@ export class ItemJSONRef extends AbstractItemRef {
return this.content.length return this.content.length
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemJSON|GC} * @return {ItemJSON|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
if (offset > 0) { if (offset > 0) {
changeItemRefOffset(this, offset) changeItemRefOffset(this, offset)
this.content = this.content.slice(offset) this.content = this.content.slice(offset)
} }
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemJSON( : new ItemJSON(

View File

@ -8,7 +8,7 @@ import {
splitItem, splitItem,
changeItemRefOffset, changeItemRefOffset,
GC, GC,
StructStore, Y, ID, AbstractType // eslint-disable-line Transaction, StructStore, Y, ID, AbstractType // eslint-disable-line
} from '../internals.js' } from '../internals.js'
import * as encoding from 'lib0/encoding.js' import * as encoding from 'lib0/encoding.js'
@ -53,15 +53,16 @@ export class ItemString extends AbstractItem {
return this.string.length return this.string.length
} }
/** /**
* @param {Transaction} transaction
* @param {number} diff * @param {number} diff
* @return {ItemString} * @return {ItemString}
*/ */
splitAt (diff) { splitAt (transaction, diff) {
/** /**
* @type {ItemString} * @type {ItemString}
*/ */
// @ts-ignore // @ts-ignore
const right = splitItem(this, diff) const right = splitItem(transaction, this, diff)
right.string = this.string.slice(diff) right.string = this.string.slice(diff)
this.string = this.string.slice(0, diff) this.string = this.string.slice(0, diff)
return right return right
@ -104,18 +105,18 @@ export class ItemStringRef extends AbstractItemRef {
return this.string.length return this.string.length
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemString|GC} * @return {ItemString|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
if (offset > 0) { if (offset > 0) {
changeItemRefOffset(this, offset) changeItemRefOffset(this, offset)
this.string = this.string.slice(offset) this.string = this.string.slice(offset)
} }
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemString( : new ItemString(

View File

@ -150,13 +150,13 @@ export class ItemTypeRef extends AbstractItemRef {
this.type = typeRefs[typeRef](decoder) this.type = typeRefs[typeRef](decoder)
} }
/** /**
* @param {Y} y * @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {number} offset * @param {number} offset
* @return {ItemType|GC} * @return {ItemType|GC}
*/ */
toStruct (y, store, offset) { toStruct (transaction, store, offset) {
const { left, right, parent, parentSub } = computeItemParams(y, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey) const { left, right, parent, parentSub } = computeItemParams(transaction, store, this.left, this.right, this.parent, this.parentSub, this.parentYKey)
return parent === null return parent === null
? new GC(this.id, this.length) ? new GC(this.id, this.length)
: new ItemType( : new ItemType(

View File

@ -381,7 +381,7 @@ export const typeArrayInsertGenerics = (transaction, parent, index, content) =>
if (index <= n.length) { if (index <= n.length) {
if (index < n.length) { if (index < n.length) {
// insert in-between // insert in-between
getItemCleanStart(transaction.y.store, createID(n.id.client, n.id.clock + index)) getItemCleanStart(transaction, transaction.y.store, createID(n.id.client, n.id.clock + index))
} }
break break
} }
@ -405,7 +405,7 @@ export const typeArrayDelete = (transaction, parent, index, length) => {
if (!n.deleted && n.countable) { if (!n.deleted && n.countable) {
if (index <= n.length) { if (index <= n.length) {
if (index < n.length && index > 0) { if (index < n.length && index > 0) {
n = getItemCleanStart(transaction.y.store, createID(n.id.client, n.id.clock + index)) n = getItemCleanStart(transaction, transaction.y.store, createID(n.id.client, n.id.clock + index))
} }
break break
} }
@ -416,7 +416,7 @@ export const typeArrayDelete = (transaction, parent, index, length) => {
while (length > 0 && n !== null) { while (length > 0 && n !== null) {
if (!n.deleted) { if (!n.deleted) {
if (length < n.length) { if (length < n.length) {
getItemCleanStart(transaction.y.store, createID(n.id.client, n.id.clock + length)) getItemCleanStart(transaction, transaction.y.store, createID(n.id.client, n.id.clock + length))
} }
n.delete(transaction) n.delete(transaction)
length -= n.length length -= n.length

View File

@ -39,7 +39,7 @@ const findNextPosition = (transaction, store, currentAttributes, left, right, co
if (!right.deleted) { if (!right.deleted) {
if (count < right.length) { if (count < right.length) {
// split right // split right
getItemCleanStart(store, createID(right.id.client, right.id.clock + count)) getItemCleanStart(transaction, store, createID(right.id.client, right.id.clock + count))
} }
count -= right.length count -= right.length
} }
@ -251,7 +251,7 @@ const formatText = (transaction, parent, left, right, currentAttributes, length,
case ItemEmbed: case ItemEmbed:
case ItemString: case ItemString:
if (length < right.length) { if (length < right.length) {
getItemCleanStart(transaction.y.store, createID(right.id.client, right.id.clock + length)) getItemCleanStart(transaction, transaction.y.store, createID(right.id.client, right.id.clock + length))
} }
length -= right.length length -= right.length
break break
@ -284,7 +284,7 @@ const deleteText = (transaction, parent, left, right, currentAttributes, length)
case ItemEmbed: case ItemEmbed:
case ItemString: case ItemString:
if (length < right.length) { if (length < right.length) {
getItemCleanStart(transaction.y.store, createID(right.id.client, right.id.clock + length)) getItemCleanStart(transaction, transaction.y.store, createID(right.id.client, right.id.clock + length))
} }
length -= right.length length -= right.length
right.delete(transaction) right.delete(transaction)

View File

@ -193,7 +193,7 @@ export const readDeleteSet = (decoder, transaction, store) => {
let struct = structs[index] let struct = structs[index]
// split the first item if necessary // split the first item if necessary
if (!struct.deleted && struct.id.clock < clock) { if (!struct.deleted && struct.id.clock < clock) {
structs.splice(index + 1, 0, struct.splitAt(clock - struct.id.clock)) structs.splice(index + 1, 0, struct.splitAt(transaction, clock - struct.id.clock))
index++ // increase we now want to use the next struct index++ // increase we now want to use the next struct
} }
while (index < structs.length) { while (index < structs.length) {
@ -202,7 +202,7 @@ export const readDeleteSet = (decoder, transaction, store) => {
if (struct.id.clock < clock + len) { if (struct.id.clock < clock + len) {
if (!struct.deleted) { if (!struct.deleted) {
if (clock + len < struct.id.clock + struct.length) { if (clock + len < struct.id.clock + struct.length) {
structs.splice(index, 0, struct.splitAt(clock + len - struct.id.clock)) structs.splice(index, 0, struct.splitAt(transaction, clock + len - struct.id.clock))
} }
struct.delete(transaction) struct.delete(transaction)
} }

View File

@ -2,7 +2,7 @@
import { import {
GC, GC,
AbstractRef, ID, ItemType, AbstractItem, AbstractStruct // eslint-disable-line Transaction, AbstractRef, ID, ItemType, AbstractItem, AbstractStruct // eslint-disable-line
} from '../internals.js' } from '../internals.js'
import * as math from 'lib0/math.js' import * as math from 'lib0/math.js'
@ -167,13 +167,15 @@ export const getItemType = (store, id) => find(store, id)
/** /**
* Expects that id is actually in store. This function throws or is an infinite loop otherwise. * Expects that id is actually in store. This function throws or is an infinite loop otherwise.
*
* @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {ID} id * @param {ID} id
* @return {AbstractItem} * @return {AbstractItem}
* *
* @private * @private
*/ */
export const getItemCleanStart = (store, id) => { export const getItemCleanStart = (transaction, store, id) => {
/** /**
* @type {Array<AbstractItem>} * @type {Array<AbstractItem>}
*/ */
@ -185,7 +187,7 @@ export const getItemCleanStart = (store, id) => {
*/ */
let struct = structs[index] let struct = structs[index]
if (struct.id.clock < id.clock && struct.constructor !== GC) { if (struct.id.clock < id.clock && struct.constructor !== GC) {
struct = struct.splitAt(id.clock - struct.id.clock) struct = struct.splitAt(transaction, id.clock - struct.id.clock)
structs.splice(index + 1, 0, struct) structs.splice(index + 1, 0, struct)
} }
return struct return struct
@ -193,13 +195,15 @@ export const getItemCleanStart = (store, id) => {
/** /**
* Expects that id is actually in store. This function throws or is an infinite loop otherwise. * Expects that id is actually in store. This function throws or is an infinite loop otherwise.
*
* @param {Transaction} transaction
* @param {StructStore} store * @param {StructStore} store
* @param {ID} id * @param {ID} id
* @return {AbstractItem} * @return {AbstractItem}
* *
* @private * @private
*/ */
export const getItemCleanEnd = (store, id) => { export const getItemCleanEnd = (transaction, store, id) => {
/** /**
* @type {Array<AbstractItem>} * @type {Array<AbstractItem>}
*/ */
@ -208,7 +212,7 @@ export const getItemCleanEnd = (store, id) => {
const index = findIndexSS(structs, id.clock) const index = findIndexSS(structs, id.clock)
const struct = structs[index] const struct = structs[index]
if (id.clock !== struct.id.clock + struct.length - 1 && struct.constructor !== GC) { if (id.clock !== struct.id.clock + struct.length - 1 && struct.constructor !== GC) {
structs.splice(index + 1, 0, struct.splitAt(id.clock - struct.id.clock + 1)) structs.splice(index + 1, 0, struct.splitAt(transaction, id.clock - struct.id.clock + 1))
} }
return struct return struct
} }

View File

@ -170,7 +170,7 @@ export const transact = (y, f) => {
const deleteItem = deleteItems[di] const deleteItem = deleteItems[di]
for (let si = findIndexSS(structs, deleteItem.clock); si < structs.length; si++) { for (let si = findIndexSS(structs, deleteItem.clock); si < structs.length; si++) {
const struct = structs[si] const struct = structs[si]
if (deleteItem.clock + deleteItem.len < struct.id.clock) { if (deleteItem.clock + deleteItem.len <= struct.id.clock) {
break break
} }
if (struct.deleted && struct instanceof AbstractItem && (struct.constructor !== ItemDeleted || (struct.parent._item !== null && struct.parent._item.deleted))) { if (struct.deleted && struct instanceof AbstractItem && (struct.constructor !== ItemDeleted || (struct.parent._item !== null && struct.parent._item.deleted))) {

View File

@ -207,7 +207,7 @@ const resumeStructIntegration = (transaction, store) => {
} }
if (m.length === 0) { if (m.length === 0) {
if (offset < ref.length) { if (offset < ref.length) {
ref.toStruct(transaction.y, store, offset).integrate(transaction) ref.toStruct(transaction, store, offset).integrate(transaction)
} }
stack.pop() stack.pop()
} }

View File

@ -306,6 +306,8 @@ const arrayTransactions = [
} }
] ]
// TODO: http://127.0.0.1:3443/?filter=\[22/&seed=1943600076
/** /**
* @param {t.TestCase} tc * @param {t.TestCase} tc
*/ */

View File

@ -348,7 +348,7 @@ const mapTransactions = [
* @param {t.TestCase} tc * @param {t.TestCase} tc
*/ */
export const testRepeatGeneratingYmapTests10 = tc => { export const testRepeatGeneratingYmapTests10 = tc => {
applyRandomTests(tc, mapTransactions, 4) applyRandomTests(tc, mapTransactions, 10)
} }
/** /**