From 170c069e5c1f6310dad760e9ff7b417d49388cf4 Mon Sep 17 00:00:00 2001 From: Martin Kleppmann Date: Tue, 2 Feb 2021 13:51:45 +0000 Subject: [PATCH 1/5] Rename chld columns to ref; remove link operation Link operation is currently unused (it was previously there for undo, but we have removed the undo feature for now). The chldActor and chldCtr columns were only used by link operations, so they can be removed too. However, for cursors we also need an (actor, counter) pair to reference the list element in question, so I'm renaming these columns to refActor and refCtr and will use them for cursor storage. --- backend/columnar.js | 48 ++++++++++++++++++++------------------------- backend/op_set.js | 10 +++------- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/backend/columnar.js b/backend/columnar.js index e016612c3..0572921e7 100644 --- a/backend/columnar.js +++ b/backend/columnar.js @@ -33,7 +33,7 @@ const VALUE_TYPE = { } // make* actions must be at even-numbered indexes in this list -const ACTIONS = ['makeMap', 'set', 'makeList', 'del', 'makeText', 'inc', 'makeTable', 'link'] +const ACTIONS = ['makeMap', 'set', 'makeList', 'del', 'makeText', 'inc', 'makeTable'] const OBJECT_TYPE = {makeMap: 'map', makeList: 'list', makeText: 'text', makeTable: 'table'} @@ -49,8 +49,8 @@ const COMMON_COLUMNS = { action: 4 << 3 | COLUMN_TYPE.INT_RLE, valLen: 5 << 3 | COLUMN_TYPE.VALUE_LEN, valRaw: 5 << 3 | COLUMN_TYPE.VALUE_RAW, - chldActor: 6 << 3 | COLUMN_TYPE.ACTOR_ID, - chldCtr: 6 << 3 | COLUMN_TYPE.INT_DELTA + refActor: 6 << 3 | COLUMN_TYPE.ACTOR_ID, + refCtr: 6 << 3 | COLUMN_TYPE.INT_DELTA } const CHANGE_COLUMNS = Object.assign({ @@ -129,11 +129,11 @@ function parseAllOpIds(changes, single) { op = copyObject(op) if (op.obj !== '_root') op.obj = parseOpId(op.obj) if (op.elemId && op.elemId !== '_head') op.elemId = parseOpId(op.elemId) - if (op.child) op.child = parseOpId(op.child) + if (op.ref) op.ref = parseOpId(op.ref) if (op.pred) op.pred = op.pred.map(parseOpId) if (op.obj.actorId) actors[op.obj.actorId] = true if (op.elemId && op.elemId.actorId) actors[op.elemId.actorId] = true - if (op.child && op.child.actorId) actors[op.child.actorId] = true + if (op.ref && op.ref.actorId) actors[op.ref.actorId] = true for (let pred of op.pred) actors[pred.actorId] = true return op }) @@ -151,7 +151,7 @@ function parseAllOpIds(changes, single) { op.id = {counter: change.startOp + i, actorNum: change.actorNum, actorId: change.actor} op.obj = actorIdToActorNum(op.obj, actorIds) op.elemId = actorIdToActorNum(op.elemId, actorIds) - op.child = actorIdToActorNum(op.child, actorIds) + op.ref = actorIdToActorNum(op.ref, actorIds) op.pred = op.pred.map(pred => actorIdToActorNum(pred, actorIds)) } } @@ -366,8 +366,8 @@ function encodeOps(ops, forDocument) { action : new RLEEncoder('uint'), valLen : new RLEEncoder('uint'), valRaw : new Encoder(), - chldActor : new RLEEncoder('uint'), - chldCtr : new DeltaEncoder() + refActor : new RLEEncoder('uint'), + refCtr : new DeltaEncoder() } if (forDocument) { @@ -389,12 +389,12 @@ function encodeOps(ops, forDocument) { encodeOperationAction(op, columns) encodeValue(op, columns) - if (op.child && op.child.counter) { - columns.chldActor.appendValue(op.child.actorNum) - columns.chldCtr.appendValue(op.child.counter) + if (op.ref && op.ref.counter) { + columns.refActor.appendValue(op.ref.actorNum) + columns.refCtr.appendValue(op.ref.counter) } else { - columns.chldActor.appendValue(null) - columns.chldCtr.appendValue(null) + columns.refActor.appendValue(null) + columns.refCtr.appendValue(null) } if (forDocument) { @@ -439,10 +439,10 @@ function decodeOps(ops, forDocument) { newOp.value = op.valLen if (op.valLen_datatype) newOp.datatype = op.valLen_datatype } - if (!!op.chldCtr !== !!op.chldActor) { - throw new RangeError(`Mismatched child columns: ${op.chldCtr} and ${op.chldActor}`) + if (!!op.refCtr !== !!op.refActor) { + throw new RangeError(`Mismatched ref columns: ${op.refCtr} and ${op.refActor}`) } - if (op.chldCtr !== null) newOp.child = `${op.chldCtr}@${op.chldActor}` + if (op.refCtr !== null) newOp.ref = `${op.refCtr}@${op.refActor}` if (forDocument) { newOp.id = `${op.idCtr}@${op.idActor}` newOp.succ = op.succNum.map(succ => `${succ.succCtr}@${succ.succActor}`) @@ -1079,12 +1079,6 @@ function addPatchProperty(objects, property) { values[op.opId] = objects[op.opId] } else if (op.actionName === 'set') { values[op.opId] = op.value - } else if (op.actionName === 'link') { - // NB. This assumes that the ID of the child object is greater than the ID of the current - // object. This is true as long as link operations are only used to redo undone make* - // operations, but it will cease to be true once subtree moves are allowed. - if (!op.childId) throw new RangeError(`link operation ${op.opId} without a childId`) - values[op.opId] = objects[op.childId] } else { throw new RangeError(`Unexpected action type: ${op.actionName}`) } @@ -1137,8 +1131,8 @@ function constructPatch(documentBuffer) { const keyActor = col.keyActor.readValue(), keyCtr = col.keyCtr.readValue() const keyStr = col.keyStr.readValue(), insert = !!col.insert.readValue() - const chldActor = col.chldActor.readValue(), chldCtr = col.chldCtr.readValue() - const childId = chldActor === null ? null : `${chldCtr}@${actorIds[chldActor]}` + const refActor = col.refActor.readValue(), refCtr = col.refCtr.readValue() + const ref = refActor === null ? null : `${refCtr}@${actorIds[refActor]}` const sizeTag = col.valLen.readValue() const rawValue = col.valRaw.readRawBytes(sizeTag >> 4) const value = decodeValue(sizeTag, rawValue) @@ -1167,7 +1161,7 @@ function constructPatch(documentBuffer) { if (property) addPatchProperty(objects, property) property = {objId, key, ops: []} } - property.ops.push({opId, actionName, value, childId, succ}) + property.ops.push({opId, actionName, value, ref, succ}) } if (property) addPatchProperty(objects, property) @@ -1185,7 +1179,7 @@ function constructPatch(documentBuffer) { function seekToOp(ops, docCols, actorIds) { const { objActor, objCtr, keyActor, keyCtr, keyStr, idActor, idCtr, insert, action, consecutiveOps } = ops const [objActorD, objCtrD, keyActorD, keyCtrD, keyStrD, idActorD, idCtrD, insertD, actionD, - valLenD, valRawD, chldActorD, chldCtrD, succNumD] = docCols.map(col => col.decoder) + valLenD, valRawD, refActorD, refCtrD, succNumD] = docCols.map(col => col.decoder) let skipCount = 0, visibleCount = 0, elemVisible = false, nextObjActor = null, nextObjCtr = null let nextIdActor = null, nextIdCtr = null, nextKeyStr = null, nextInsert = null, nextSuccNum = 0 @@ -2020,7 +2014,7 @@ class BackendDoc { getAllColumns(changeCols) { const expectedCols = [ 'objActor', 'objCtr', 'keyActor', 'keyCtr', 'keyStr', 'idActor', 'idCtr', 'insert', - 'action', 'valLen', 'valRaw', 'chldActor', 'chldCtr', 'predNum', 'predActor', 'predCtr' + 'action', 'valLen', 'valRaw', 'refActor', 'refCtr', 'predNum', 'predActor', 'predCtr' ] let allCols = {} for (let i = 0; i < expectedCols.length; i++) { diff --git a/backend/op_set.js b/backend/op_set.js index 320078f47..84e9bcbab 100644 --- a/backend/op_set.js +++ b/backend/op_set.js @@ -112,8 +112,7 @@ function updateListElement(opSet, objectId, elemId, patch) { * Returns true if the operation `op` introduces a child object. */ function isChildOp(op) { - const action = op.get('action') - return action.startsWith('make') || action === 'link' + return op.get('action').startsWith('make') } /** @@ -136,7 +135,7 @@ function getOperationKey(op) { } /** - * Processes a 'set', 'del', 'make*', 'link', or 'inc' operation. Mutates `patch` + * Processes a 'set', 'del', 'make*', or 'inc' operation. Mutates `patch` * to describe the change and returns an updated `opSet`. */ function applyAssign(opSet, op, patch) { @@ -170,9 +169,6 @@ function applyAssign(opSet, op, patch) { opSet = applyMake(opSet, op) } } - if (action === 'link' && patch) { - patch.props[key][op.get('opId')] = constructObject(opSet, getChildId(op)) - } const ops = getFieldOps(opSet, objectId, key) let overwritten, remaining @@ -320,7 +316,7 @@ function applyOps(opSet, change, patch) { let newObjects = Set() change.get('ops').forEach((op, index) => { const action = op.get('action'), obj = op.get('obj'), insert = op.get('insert') - if (!['set', 'del', 'inc', 'link', 'makeMap', 'makeList', 'makeText', 'makeTable'].includes(action)) { + if (!['set', 'del', 'inc', 'makeMap', 'makeList', 'makeText', 'makeTable'].includes(action)) { throw new RangeError(`Unknown operation action: ${action}`) } if (!op.get('pred')) { From 08fd595670bfca45944bc185ba5f8164c7a4274a Mon Sep 17 00:00:00 2001 From: Martin Kleppmann Date: Tue, 2 Feb 2021 15:54:44 +0000 Subject: [PATCH 2/5] Creating and storing Cursor objects For now it just stores the elemId referenced by the cursor in the backend, updates the frontend-backend protocol to handle cursors, and instantiates Cursor objects in the frontend. Does not actually perform the conversion from elemId to index yet; that's next. --- backend/columnar.js | 19 +++++++++++++------ backend/op_set.js | 10 +++++++--- frontend/apply_patch.js | 3 +++ frontend/context.js | 28 ++++++++++++++++++++------- frontend/cursor.js | 40 +++++++++++++++++++++++++++++++++++++++ frontend/index.js | 3 ++- frontend/proxies.js | 3 ++- frontend/text.js | 5 +++++ src/automerge.js | 3 ++- test/backend_test.js | 42 +++++++++++++++++++++++++++++++++++++++++ test/cursor_test.js | 42 +++++++++++++++++++++++++++++++++++++++++ 11 files changed, 179 insertions(+), 19 deletions(-) create mode 100644 frontend/cursor.js create mode 100644 test/cursor_test.js diff --git a/backend/columnar.js b/backend/columnar.js index 0572921e7..272996ffc 100644 --- a/backend/columnar.js +++ b/backend/columnar.js @@ -29,7 +29,8 @@ const COLUMN_TYPE = { const VALUE_TYPE = { NULL: 0, FALSE: 1, TRUE: 2, LEB128_UINT: 3, LEB128_INT: 4, IEEE754: 5, - UTF8: 6, BYTES: 7, COUNTER: 8, TIMESTAMP: 9, MIN_UNKNOWN: 10, MAX_UNKNOWN: 15 + UTF8: 6, BYTES: 7, COUNTER: 8, TIMESTAMP: 9, CURSOR: 10, + MIN_UNKNOWN: 11, MAX_UNKNOWN: 15 } // make* actions must be at even-numbered indexes in this list @@ -50,7 +51,7 @@ const COMMON_COLUMNS = { valLen: 5 << 3 | COLUMN_TYPE.VALUE_LEN, valRaw: 5 << 3 | COLUMN_TYPE.VALUE_RAW, refActor: 6 << 3 | COLUMN_TYPE.ACTOR_ID, - refCtr: 6 << 3 | COLUMN_TYPE.INT_DELTA + refCtr: 6 << 3 | COLUMN_TYPE.INT_RLE } const CHANGE_COLUMNS = Object.assign({ @@ -239,9 +240,6 @@ function encodeValue(op, columns) { columns.valLen.appendValue(VALUE_TYPE.FALSE) } else if (op.value === true) { columns.valLen.appendValue(VALUE_TYPE.TRUE) - } else if (typeof op.value === 'string') { - const numBytes = columns.valRaw.appendRawString(op.value) - columns.valLen.appendValue(numBytes << 4 | VALUE_TYPE.UTF8) } else if (ArrayBuffer.isView(op.value)) { const numBytes = columns.valRaw.appendRawBytes(new Uint8Array(op.value.buffer)) columns.valLen.appendValue(numBytes << 4 | VALUE_TYPE.BYTES) @@ -249,6 +247,11 @@ function encodeValue(op, columns) { encodeInteger(op.value, VALUE_TYPE.COUNTER, columns) } else if (op.datatype === 'timestamp' && typeof op.value === 'number') { encodeInteger(op.value, VALUE_TYPE.TIMESTAMP, columns) + } else if (op.datatype === 'cursor') { + columns.valLen.appendValue(VALUE_TYPE.CURSOR) // cursor's elemId is stored in the ref columns + } else if (typeof op.value === 'string') { + const numBytes = columns.valRaw.appendRawString(op.value) + columns.valLen.appendValue(numBytes << 4 | VALUE_TYPE.UTF8) } else if (typeof op.datatype === 'number' && op.datatype >= VALUE_TYPE.MIN_UNKNOWN && op.datatype <= VALUE_TYPE.MAX_UNKNOWN && op.value instanceof Uint8Array) { const numBytes = columns.valRaw.appendRawBytes(op.value) @@ -310,6 +313,8 @@ function decodeValue(sizeTag, bytes) { return {value: new Decoder(bytes).readInt53(), datatype: 'counter'} } else if (sizeTag % 16 === VALUE_TYPE.TIMESTAMP) { return {value: new Decoder(bytes).readInt53(), datatype: 'timestamp'} + } else if (sizeTag === VALUE_TYPE.CURSOR) { + return {value: '', datatype: 'cursor'} } else { return {value: bytes, datatype: sizeTag % 16} } @@ -1075,7 +1080,9 @@ function addPatchProperty(objects, property) { for (let succId of op.succ) counter.succ[succId] = true } else if (op.succ.length === 0) { // Ignore any ops that have been overwritten - if (op.actionName.startsWith('make')) { + if (op.actionName === 'set' && op.value.datatype === 'cursor') { + values[op.opId] = {elemId: op.ref, datatype: 'cursor'} + } else if (op.actionName.startsWith('make')) { values[op.opId] = objects[op.opId] } else if (op.actionName === 'set') { values[op.opId] = op.value diff --git a/backend/op_set.js b/backend/op_set.js index 84e9bcbab..12eb4693f 100644 --- a/backend/op_set.js +++ b/backend/op_set.js @@ -255,9 +255,13 @@ function setPatchProps(opSet, objectId, key, patch) { ops[opId] = true if (op.get('action') === 'set') { - patch.props[key][opId] = {value: op.get('value')} - if (op.get('datatype')) { - patch.props[key][opId].datatype = op.get('datatype') + if (op.get('datatype') === 'cursor') { + patch.props[key][opId] = {elemId: op.get('ref'), datatype: 'cursor'} + } else { + patch.props[key][opId] = {value: op.get('value')} + if (op.get('datatype')) { + patch.props[key][opId].datatype = op.get('datatype') + } } } else if (isChildOp(op)) { if (!patch.props[key][opId]) { diff --git a/frontend/apply_patch.js b/frontend/apply_patch.js index 366d9b1c0..091f46cb9 100644 --- a/frontend/apply_patch.js +++ b/frontend/apply_patch.js @@ -3,6 +3,7 @@ const { OPTIONS, OBJECT_ID, CONFLICTS, ELEM_IDS } = require('./constants') const { Text, instantiateText } = require('./text') const { Table, instantiateTable } = require('./table') const { Counter } = require('./counter') +const { Cursor } = require('./cursor') /** * Reconstructs the value from the patch object `patch`. @@ -20,6 +21,8 @@ function getValue(patch, object, updated) { return new Date(patch.value) } else if (patch.datatype === 'counter') { return new Counter(patch.value) + } else if (patch.datatype === 'cursor') { + return new Cursor('', patch.index, patch.elemId) } else if (patch.datatype !== undefined) { throw new TypeError(`Unknown datatype: ${patch.datatype}`) } else { diff --git a/frontend/context.js b/frontend/context.js index 777be166a..cc78cfe24 100644 --- a/frontend/context.js +++ b/frontend/context.js @@ -3,6 +3,7 @@ const { interpretPatch } = require('./apply_patch') const { Text } = require('./text') const { Table } = require('./table') const { Counter, getWriteableCounter } = require('./counter') +const { Cursor } = require('./cursor') const { isObject, copyObject } = require('../src/common') const uuid = require('../src/uuid') @@ -50,9 +51,11 @@ class Context { return {value: value.getTime(), datatype: 'timestamp'} } else if (value instanceof Counter) { - // Counter object return {value: value.value, datatype: 'counter'} + } else if (value instanceof Cursor) { + return {elemId: value.elemId, datatype: 'cursor'} + } else { // Nested object (map, list, text, or table) const objectId = value[OBJECT_ID] @@ -175,6 +178,9 @@ class Context { if (object[key] instanceof Counter) { return getWriteableCounter(object[key].value, this, path, objectId, key) + } else if (object[key] instanceof Cursor) { + return object[key].getWriteable(context, path) + } else if (isObject(object[key])) { const childId = object[key][OBJECT_ID] const subpath = path.concat([{key, objectId: childId}]) @@ -264,16 +270,24 @@ class Context { throw new RangeError('The key of a map entry must not be an empty string') } - if (isObject(value) && !(value instanceof Date) && !(value instanceof Counter)) { - // Nested object (map, list, text, or table) - return this.createNestedObjects(objectId, key, value, insert, pred, elemId) - } else { - // Date or counter object, or primitive value (number, string, boolean, or null) + if (!isObject(value) || value instanceof Date || value instanceof Counter || value instanceof Cursor) { + // Date/counter/cursor object, or primitive value (number, string, boolean, or null) const description = this.getValueDescription(value) const op = elemId ? {action: 'set', obj: objectId, elemId, insert, pred} : {action: 'set', obj: objectId, key, insert, pred} - this.addOp(Object.assign(op, description)) + + if (description.datatype === 'cursor') { + op.ref = description.elemId + op.datatype = 'cursor' + } else { + op.value = description.value + if (description.datatype) op.datatype = description.datatype + } + this.addOp(op) return description + } else { + // Nested object (map, list, text, or table) + return this.createNestedObjects(objectId, key, value, insert, pred, elemId) } } diff --git a/frontend/cursor.js b/frontend/cursor.js new file mode 100644 index 000000000..cda161132 --- /dev/null +++ b/frontend/cursor.js @@ -0,0 +1,40 @@ +const { OBJECT_ID, ELEM_IDS } = require('./constants') +const { isObject } = require('../src/common') + +/** + * A cursor references a particular element in a list, or a character in an + * Automerge.Text object. As list elements get inserted/deleted ahead of the + * cursor position, the index of the cursor is automatically recomputed. + */ +class Cursor { + constructor(object, index, elemId = undefined) { + if (Array.isArray(object) && object[ELEM_IDS] && typeof index === 'number') { + this.objectId = object[OBJECT_ID] + this.elemId = object[ELEM_IDS][index] + this.index = index + } else if (isObject(object) && object.getElemId && typeof index === 'number') { + this.objectId = object[OBJECT_ID] + this.elemId = object.getElemId(index) + this.index = index + } else if (typeof object == 'string' && /*typeof index === 'number' &&*/ typeof elemId === 'string') { + this.objectId = object + this.elemId = elemId + this.index = index + } else { + throw new TypeError('Construct a cursor using a list/text object and index') + } + } + + /** + * Called when a cursor is accessed within a change callback. `context` is the + * proxy context that keeps track of any mutations. + */ + getWriteable(context, path) { + const instance = new Cursor(this.objectId, this.index, this.elemId) + instance.context = context + instance.path = path + return instance + } +} + +module.exports = { Cursor } diff --git a/frontend/index.js b/frontend/index.js index 27ba3cedb..f52dc8441 100644 --- a/frontend/index.js +++ b/frontend/index.js @@ -7,6 +7,7 @@ const { Context } = require('./context') const { Text } = require('./text') const { Table } = require('./table') const { Counter } = require('./counter') +const { Cursor } = require('./cursor') /** * Actor IDs must consist only of hexadecimal digits so that they can be encoded @@ -370,5 +371,5 @@ module.exports = { init, from, change, emptyChange, applyPatch, getObjectId, getObjectById, getActorId, setActorId, getConflicts, getLastLocalChange, getBackendState, getElementIds, - Text, Table, Counter + Text, Table, Counter, Cursor } diff --git a/frontend/proxies.js b/frontend/proxies.js index 779fc14c4..816f978b1 100644 --- a/frontend/proxies.js +++ b/frontend/proxies.js @@ -1,4 +1,4 @@ -const { OBJECT_ID, CHANGE, STATE } = require('./constants') +const { OBJECT_ID, CHANGE, STATE, ELEM_IDS } = require('./constants') const { Counter } = require('./counter') const { Text } = require('./text') const { Table } = require('./table') @@ -148,6 +148,7 @@ const ListHandler = { if (key === Symbol.iterator) return context.getObject(objectId)[Symbol.iterator] if (key === OBJECT_ID) return objectId if (key === CHANGE) return context + if (key === ELEM_IDS) return context.getObject(objectId)[ELEM_IDS] if (key === 'length') return context.getObject(objectId).length if (typeof key === 'string' && /^[0-9]+$/.test(key)) { return context.getObjectField(path, objectId, parseListIndex(key)) diff --git a/frontend/text.js b/frontend/text.js index 97493d689..a1e724f0a 100644 --- a/frontend/text.js +++ b/frontend/text.js @@ -1,5 +1,6 @@ const { OBJECT_ID } = require('./constants') const { isObject } = require('../src/common') +const { Cursor } = require('./cursor') class Text { constructor (text) { @@ -35,6 +36,10 @@ class Text { return this.elems[index].elemId } + getCursorAt (index) { + return new Cursor(this, index) + } + /** * Iterates over the text elements character by character, including any * inline objects. diff --git a/src/automerge.js b/src/automerge.js index 49f17e1eb..32d6f5529 100644 --- a/src/automerge.js +++ b/src/automerge.js @@ -132,6 +132,7 @@ module.exports = { } for (let name of ['getObjectId', 'getObjectById', 'getActorId', - 'setActorId', 'getConflicts', 'getLastLocalChange', 'Text', 'Table', 'Counter']) { + 'setActorId', 'getConflicts', 'getLastLocalChange', + 'Text', 'Table', 'Counter', 'Cursor']) { module.exports[name] = Frontend[name] } diff --git a/test/backend_test.js b/test/backend_test.js index e20832afe..02ddd582b 100644 --- a/test/backend_test.js +++ b/test/backend_test.js @@ -254,6 +254,27 @@ describe('Automerge.Backend', () => { }}}} }) }) + + it('should support Cursor objects', () => { + const actor = uuid() + const change = {actor, seq: 1, startOp: 1, time: 0, deps: [], ops: [ + {action: 'makeList', obj: '_root', key: 'list', pred: []}, + {action: 'set', obj: `1@${actor}`, elemId: '_head', insert: true, value: 'fish', pred: []}, + {action: 'set', obj: '_root', key: 'cursor', ref: `2@${actor}`, datatype: 'cursor', pred: []} + ]} + const [s1, patch] = Backend.applyChanges(Backend.init(), [encodeChange(change)]) + assert.deepStrictEqual(patch, { + clock: {[actor]: 1}, deps: [hash(change)], maxOp: 3, + diffs: {objectId: '_root', type: 'map', props: { + list: {[`1@${actor}`]: { + objectId: `1@${actor}`, type: 'list', + edits: [{action: 'insert', index: 0, elemId: `2@${actor}`}], + props: {0: {[`2@${actor}`]: {value: 'fish'}}} + }}, + cursor: {[`3@${actor}`]: {elemId: `2@${actor}`, datatype: 'cursor'}} + }} + }) + }) }) describe('applyLocalChange()', () => { @@ -630,5 +651,26 @@ describe('Automerge.Backend', () => { }}}} }) }) + + it('should include Cursor objects', () => { + const actor = uuid() + const change = {actor, seq: 1, startOp: 1, time: 0, deps: [], ops: [ + {action: 'makeList', obj: '_root', key: 'list', pred: []}, + {action: 'set', obj: `1@${actor}`, elemId: '_head', insert: true, value: 'fish', pred: []}, + {action: 'set', obj: '_root', key: 'cursor', ref: `2@${actor}`, datatype: 'cursor', pred: []} + ]} + const [s1, patch] = Backend.applyChanges(Backend.init(), [encodeChange(change)]) + assert.deepStrictEqual(patch, { + clock: {[actor]: 1}, deps: [hash(change)], maxOp: 3, + diffs: {objectId: '_root', type: 'map', props: { + list: {[`1@${actor}`]: { + objectId: `1@${actor}`, type: 'list', + edits: [{action: 'insert', index: 0, elemId: `2@${actor}`}], + props: {0: {[`2@${actor}`]: {value: 'fish'}}} + }}, + cursor: {[`3@${actor}`]: {elemId: `2@${actor}`, datatype: 'cursor'}} + }} + }) + }) }) }) diff --git a/test/cursor_test.js b/test/cursor_test.js new file mode 100644 index 000000000..f214bc70b --- /dev/null +++ b/test/cursor_test.js @@ -0,0 +1,42 @@ +const assert = require('assert') +const Automerge = process.env.TEST_DIST === '1' ? require('../dist/automerge') : require('../src/automerge') + +describe('Automerge.Cursor', () => { + it('should allow a cursor on a list element', () => { + let s1 = Automerge.change(Automerge.init(), doc => { + doc.list = [1,2,3] + doc.cursor = new Automerge.Cursor(doc.list, 2) + assert.ok(doc.cursor instanceof Automerge.Cursor) + assert.strictEqual(doc.cursor.elemId, `4@${Automerge.getActorId(doc)}`) + }) + assert.ok(s1.cursor instanceof Automerge.Cursor) + assert.strictEqual(s1.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + + let s2 = Automerge.applyChanges(Automerge.init(), Automerge.getAllChanges(s1)) + assert.ok(s2.cursor instanceof Automerge.Cursor) + assert.strictEqual(s2.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + + let s3 = Automerge.load(Automerge.save(s1)) + assert.ok(s3.cursor instanceof Automerge.Cursor) + assert.strictEqual(s3.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + }) + + it('should allow a cursor on a text character', () => { + let s1 = Automerge.change(Automerge.init(), doc => { + doc.text = new Automerge.Text(['a', 'b', 'c']) + doc.cursor = doc.text.getCursorAt(2) + assert.ok(doc.cursor instanceof Automerge.Cursor) + assert.strictEqual(doc.cursor.elemId, `4@${Automerge.getActorId(doc)}`) + }) + assert.ok(s1.cursor instanceof Automerge.Cursor) + assert.strictEqual(s1.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + + let s2 = Automerge.applyChanges(Automerge.init(), Automerge.getAllChanges(s1)) + assert.ok(s2.cursor instanceof Automerge.Cursor) + assert.strictEqual(s2.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + + let s3 = Automerge.load(Automerge.save(s1)) + assert.ok(s3.cursor instanceof Automerge.Cursor) + assert.strictEqual(s3.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + }) +}) From 64d10e6e9c37e13547eb1b1ff4b5ef762150988e Mon Sep 17 00:00:00 2001 From: Martin Kleppmann Date: Wed, 3 Feb 2021 10:54:07 +0000 Subject: [PATCH 3/5] Bounds checking when creating cursor --- frontend/cursor.js | 1 + frontend/text.js | 1 + test/cursor_test.js | 15 +++++++++++++++ 3 files changed, 17 insertions(+) diff --git a/frontend/cursor.js b/frontend/cursor.js index cda161132..9e15f9fc3 100644 --- a/frontend/cursor.js +++ b/frontend/cursor.js @@ -9,6 +9,7 @@ const { isObject } = require('../src/common') class Cursor { constructor(object, index, elemId = undefined) { if (Array.isArray(object) && object[ELEM_IDS] && typeof index === 'number') { + if (index < 0 || index >= object[ELEM_IDS].length) throw new RangeError('list index out of bounds') this.objectId = object[OBJECT_ID] this.elemId = object[ELEM_IDS][index] this.index = index diff --git a/frontend/text.js b/frontend/text.js index a1e724f0a..899934d03 100644 --- a/frontend/text.js +++ b/frontend/text.js @@ -33,6 +33,7 @@ class Text { } getElemId (index) { + if (index < 0 || index >= this.elems.length) throw new RangeError('text index out of bounds') return this.elems[index].elemId } diff --git a/test/cursor_test.js b/test/cursor_test.js index f214bc70b..dcff52de5 100644 --- a/test/cursor_test.js +++ b/test/cursor_test.js @@ -39,4 +39,19 @@ describe('Automerge.Cursor', () => { assert.ok(s3.cursor instanceof Automerge.Cursor) assert.strictEqual(s3.cursor.elemId, `4@${Automerge.getActorId(s1)}`) }) + + it('should not allow an index beyond the length of the list', () => { + assert.throws(() => { + Automerge.change(Automerge.init(), doc => { + doc.list = [1] + doc.cursor = new Automerge.Cursor(doc.list, 1) + }) + }, /index out of bounds/) + assert.throws(() => { + Automerge.change(Automerge.init(), doc => { + doc.text = new Automerge.Text('a') + doc.cursor = doc.text.getCursorAt(1) + }) + }, /index out of bounds/) + }) }) From 37d7311aa9c35e11c0dd6e28a07f147d7dc8486e Mon Sep 17 00:00:00 2001 From: Martin Kleppmann Date: Wed, 3 Feb 2021 16:08:27 +0000 Subject: [PATCH 4/5] Compute indexes for cursors --- backend/columnar.js | 2 +- backend/op_set.js | 132 +++++++++++++++++++++++++--------------- frontend/apply_patch.js | 2 +- frontend/context.js | 7 ++- frontend/cursor.js | 2 +- test/backend_test.js | 4 +- test/cursor_test.js | 71 ++++++++++++++++++--- 7 files changed, 157 insertions(+), 63 deletions(-) diff --git a/backend/columnar.js b/backend/columnar.js index 272996ffc..f218b1849 100644 --- a/backend/columnar.js +++ b/backend/columnar.js @@ -372,7 +372,7 @@ function encodeOps(ops, forDocument) { valLen : new RLEEncoder('uint'), valRaw : new Encoder(), refActor : new RLEEncoder('uint'), - refCtr : new DeltaEncoder() + refCtr : new RLEEncoder('uint') } if (forDocument) { diff --git a/backend/op_set.js b/backend/op_set.js index 12eb4693f..b4465c376 100644 --- a/backend/op_set.js +++ b/backend/op_set.js @@ -71,6 +71,32 @@ function applyInsert(opSet, op) { .setIn(['byObject', objectId, '_insertion', opId], op) } +// Find the index of the closest visible list element that precedes the given element ID. +// Returns -1 if there is no such element. +function getPrecedingListIndex(opSet, objectId, elemId) { + const elemIds = opSet.getIn(['byObject', objectId, '_elemIds']) + + let prevId = elemId, index + while (true) { + index = -1 + prevId = getPrevious(opSet, objectId, prevId) + if (!prevId) break + index = elemIds.indexOf(prevId) + if (index >= 0) break + } + + return index +} + +// Finds the index of the list element with a given ID. If that list element is not visible +// (typically because it has been deleted), returns the index of the closest visible preceding +// element. Returns -1 if there is no such preceding element. +function getListIndex(opSet, objectId, elemId) { + const index = opSet.getIn(['byObject', objectId, '_elemIds']).indexOf(elemId) + if (index >= 0) return index + return getPrecedingListIndex(opSet, objectId, elemId) +} + function updateListElement(opSet, objectId, elemId, patch) { const ops = getFieldOps(opSet, objectId, elemId) let elemIds = opSet.getIn(['byObject', objectId, '_elemIds']) @@ -87,21 +113,9 @@ function updateListElement(opSet, objectId, elemId, patch) { } else { elemIds = elemIds.setValue(elemId, ops.first().get('value')) } - } else { if (ops.isEmpty()) return opSet // deleting a non-existent element = no-op - - // find the index of the closest preceding list element - let prevId = elemId - while (true) { - index = -1 - prevId = getPrevious(opSet, objectId, prevId) - if (!prevId) break - index = elemIds.indexOf(prevId) - if (index >= 0) break - } - - index += 1 + index = getPrecedingListIndex(opSet, objectId, elemId) + 1 elemIds = elemIds.insertIndex(index, elemId, ops.first().get('value')) if (patch) patch.edits.push({action: 'insert', index, elemId}) } @@ -141,24 +155,8 @@ function getOperationKey(op) { function applyAssign(opSet, op, patch) { const objectId = op.get('obj'), action = op.get('action'), key = getOperationKey(op) if (!opSet.get('byObject').has(objectId)) throw new RangeError(`Modification of unknown object ${objectId}`) - const type = getObjectType(opSet, objectId) - - if (patch) { - patch.objectId = patch.objectId || objectId - if (patch.objectId !== objectId) { - throw new RangeError(`objectId mismatch in patch: ${patch.objectId} != ${objectId}`) - } - if (patch.props === undefined) { - patch.props = {} - } - if (patch.props[key] === undefined) { - patch.props[key] = {} - } - - patch.type = patch.type || type - if (patch.type !== type) { - throw new RangeError(`object type mismatch in patch: ${patch.type} != ${type}`) - } + if (patch && patch.props[key] === undefined) { + patch.props[key] = {} } if (action.startsWith('make')) { @@ -194,6 +192,21 @@ function applyAssign(opSet, op, patch) { opSet = opSet.updateIn(['byObject', getChildId(old), '_inbound'], ops => ops.remove(old)) } + // Add any new cursors, and remove any overwritten cursors + for (let oldCursor of overwritten.filter(op => op.get('datatype') === 'cursor')) { + opSet = opSet.update('cursors', cursors => cursors.remove(oldCursor.get('opId'))) + } + if (op.get('datatype') === 'cursor') { + // Find the list/text object that the cursor points to by iterating over all objects + let refObjectId + for (let [listId, listObj] of opSet.get('byObject').entries()) { + if (listObj.hasIn(['_insertion', op.get('ref')])) refObjectId = listId + } + if (!refObjectId) throw new RangeError(`Cursor points at unknown list element ${op.get('ref')}`) + op = op.set('refObjectId', refObjectId) + opSet = opSet.setIn(['cursors', op.get('opId')], op) + } + if (isChildOp(op)) { opSet = opSet.updateIn(['byObject', getChildId(op), '_inbound'], Set(), ops => ops.add(op)) } @@ -204,6 +217,7 @@ function applyAssign(opSet, op, patch) { opSet = opSet.setIn(['byObject', objectId, '_keys', key], remaining) setPatchProps(opSet, objectId, key, patch) + const type = getObjectType(opSet, objectId) if (type === 'list' || type === 'text') { opSet = updateListElement(opSet, objectId, key, patch) } @@ -211,13 +225,9 @@ function applyAssign(opSet, op, patch) { } /** - * Updates `patch` with the fields required in a patch. `pathOp` is an operation - * along the path from the root to the object being modified, as returned by - * `getPath()`. Returns the sub-object representing the child identified by this - * operation. + * Mutates `patch` to contain updates for the object with ID `objectId`. */ -function initializePatch(opSet, pathOp, patch) { - const objectId = pathOp.get('obj'), opId = pathOp.get('opId'), key = getOperationKey(pathOp) +function initializePatchObject(opSet, objectId, patch) { const type = getObjectType(opSet, objectId) patch.objectId = patch.objectId || objectId patch.type = patch.type || type @@ -228,12 +238,26 @@ function initializePatch(opSet, pathOp, patch) { if (patch.type !== type) { throw new RangeError(`object type mismatch in path: ${patch.type} != ${type}`) } - setPatchProps(opSet, objectId, key, patch) +} - if (patch.props[key][opId] === undefined) { - throw new RangeError(`field ops for ${key} did not contain opId ${opId}`) +/** + * Mutates `patch` to contain the object with ID `updatedObjectId`, and the path + * from the root to that object. Returns the subpatch for that object. + */ +function initializePatch(opSet, updatedObjectId, patch) { + for (let pathOp of getPath(opSet, updatedObjectId)) { + const objectId = pathOp.get('obj'), opId = pathOp.get('opId'), key = getOperationKey(pathOp) + initializePatchObject(opSet, objectId, patch) + setPatchProps(opSet, objectId, key, patch) + if (patch.props[key][opId] === undefined) { + throw new RangeError(`field ops for ${key} did not contain opId ${opId}`) + } + patch = patch.props[key][opId] } - return patch.props[key][opId] + + initializePatchObject(opSet, updatedObjectId, patch) + if (patch.props === undefined) patch.props = {} + return patch } /** @@ -256,7 +280,9 @@ function setPatchProps(opSet, objectId, key, patch) { if (op.get('action') === 'set') { if (op.get('datatype') === 'cursor') { - patch.props[key][opId] = {elemId: op.get('ref'), datatype: 'cursor'} + const refObjectId = op.get('refObjectId'), elemId = op.get('ref') + const index = getListIndex(opSet, refObjectId, elemId) + patch.props[key][opId] = {refObjectId, elemId, index, datatype: 'cursor'} } else { patch.props[key][opId] = {value: op.get('value')} if (op.get('datatype')) { @@ -327,13 +353,7 @@ function applyOps(opSet, change, patch) { throw new RangeError(`Missing 'pred' field in operation ${op}`) } - let localPatch = patch - if (patch) { - for (let pathOp of getPath(opSet, obj)) { - localPatch = initializePatch(opSet, pathOp, localPatch) - } - } - + const localPatch = patch && initializePatch(opSet, obj, patch) const opWithId = op.merge({opId: `${startOp + index}@${actor}`}) if (insert) { opSet = applyInsert(opSet, opWithId) @@ -343,6 +363,19 @@ function applyOps(opSet, change, patch) { } opSet = applyAssign(opSet, opWithId, localPatch) }) + + // Recompute cursor indexes. This could perhaps be made more efficient by only recomputing if we + // updated the list that the cursor references; for now we just recompute them all. + if (patch) { + for (let cursor of opSet.get('cursors').values()) { + const index = getListIndex(opSet, cursor.get('refObjectId'), cursor.get('ref')) + if (index !== cursor.get('index')) { + opSet = opSet.setIn(['cursors', cursor.get('opId'), 'index'], index) + const localPatch = initializePatch(opSet, cursor.get('obj'), patch) + setPatchProps(opSet, cursor.get('obj'), getOperationKey(cursor), localPatch) + } + } + } return opSet } @@ -428,6 +461,7 @@ function init() { .set('states', Map()) .set('history', List()) .set('byObject', Map().set('_root', Map().set('_keys', Map()))) + .set('cursors', Map()) .set('hashes', Map()) .set('deps', Set()) .set('maxOp', 0) diff --git a/frontend/apply_patch.js b/frontend/apply_patch.js index 091f46cb9..63386eb81 100644 --- a/frontend/apply_patch.js +++ b/frontend/apply_patch.js @@ -22,7 +22,7 @@ function getValue(patch, object, updated) { } else if (patch.datatype === 'counter') { return new Counter(patch.value) } else if (patch.datatype === 'cursor') { - return new Cursor('', patch.index, patch.elemId) + return new Cursor(patch.refObjectId, patch.index, patch.elemId) } else if (patch.datatype !== undefined) { throw new TypeError(`Unknown datatype: ${patch.datatype}`) } else { diff --git a/frontend/context.js b/frontend/context.js index cc78cfe24..713002734 100644 --- a/frontend/context.js +++ b/frontend/context.js @@ -54,7 +54,12 @@ class Context { return {value: value.value, datatype: 'counter'} } else if (value instanceof Cursor) { - return {elemId: value.elemId, datatype: 'cursor'} + return { + refObjectId: value.objectId, + elemId: value.elemId, + index: value.index, + datatype: 'cursor' + } } else { // Nested object (map, list, text, or table) diff --git a/frontend/cursor.js b/frontend/cursor.js index 9e15f9fc3..0e943a3ea 100644 --- a/frontend/cursor.js +++ b/frontend/cursor.js @@ -17,7 +17,7 @@ class Cursor { this.objectId = object[OBJECT_ID] this.elemId = object.getElemId(index) this.index = index - } else if (typeof object == 'string' && /*typeof index === 'number' &&*/ typeof elemId === 'string') { + } else if (typeof object == 'string' && typeof index === 'number' && typeof elemId === 'string') { this.objectId = object this.elemId = elemId this.index = index diff --git a/test/backend_test.js b/test/backend_test.js index 02ddd582b..fd70710b4 100644 --- a/test/backend_test.js +++ b/test/backend_test.js @@ -271,7 +271,7 @@ describe('Automerge.Backend', () => { edits: [{action: 'insert', index: 0, elemId: `2@${actor}`}], props: {0: {[`2@${actor}`]: {value: 'fish'}}} }}, - cursor: {[`3@${actor}`]: {elemId: `2@${actor}`, datatype: 'cursor'}} + cursor: {[`3@${actor}`]: {refObjectId: `1@${actor}`, elemId: `2@${actor}`, index: 0, datatype: 'cursor'}} }} }) }) @@ -668,7 +668,7 @@ describe('Automerge.Backend', () => { edits: [{action: 'insert', index: 0, elemId: `2@${actor}`}], props: {0: {[`2@${actor}`]: {value: 'fish'}}} }}, - cursor: {[`3@${actor}`]: {elemId: `2@${actor}`, datatype: 'cursor'}} + cursor: {[`3@${actor}`]: {refObjectId: `1@${actor}`, elemId: `2@${actor}`, index: 0, datatype: 'cursor'}} }} }) }) diff --git a/test/cursor_test.js b/test/cursor_test.js index dcff52de5..59a149daa 100644 --- a/test/cursor_test.js +++ b/test/cursor_test.js @@ -8,17 +8,16 @@ describe('Automerge.Cursor', () => { doc.cursor = new Automerge.Cursor(doc.list, 2) assert.ok(doc.cursor instanceof Automerge.Cursor) assert.strictEqual(doc.cursor.elemId, `4@${Automerge.getActorId(doc)}`) + assert.strictEqual(doc.cursor.index, 2) }) assert.ok(s1.cursor instanceof Automerge.Cursor) assert.strictEqual(s1.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + assert.strictEqual(s1.cursor.index, 2) let s2 = Automerge.applyChanges(Automerge.init(), Automerge.getAllChanges(s1)) assert.ok(s2.cursor instanceof Automerge.Cursor) assert.strictEqual(s2.cursor.elemId, `4@${Automerge.getActorId(s1)}`) - - let s3 = Automerge.load(Automerge.save(s1)) - assert.ok(s3.cursor instanceof Automerge.Cursor) - assert.strictEqual(s3.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + assert.strictEqual(s2.cursor.index, 2) }) it('should allow a cursor on a text character', () => { @@ -27,17 +26,16 @@ describe('Automerge.Cursor', () => { doc.cursor = doc.text.getCursorAt(2) assert.ok(doc.cursor instanceof Automerge.Cursor) assert.strictEqual(doc.cursor.elemId, `4@${Automerge.getActorId(doc)}`) + assert.strictEqual(doc.cursor.index, 2) }) assert.ok(s1.cursor instanceof Automerge.Cursor) assert.strictEqual(s1.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + assert.strictEqual(s1.cursor.index, 2) let s2 = Automerge.applyChanges(Automerge.init(), Automerge.getAllChanges(s1)) assert.ok(s2.cursor instanceof Automerge.Cursor) assert.strictEqual(s2.cursor.elemId, `4@${Automerge.getActorId(s1)}`) - - let s3 = Automerge.load(Automerge.save(s1)) - assert.ok(s3.cursor instanceof Automerge.Cursor) - assert.strictEqual(s3.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + assert.strictEqual(s2.cursor.index, 2) }) it('should not allow an index beyond the length of the list', () => { @@ -54,4 +52,61 @@ describe('Automerge.Cursor', () => { }) }, /index out of bounds/) }) + + it('should allow a cursor to be updated', () => { + const s1 = Automerge.change(Automerge.init(), doc => { + doc.text = new Automerge.Text(['a', 'b', 'c']) + doc.cursor = doc.text.getCursorAt(1) + }) + assert.strictEqual(s1.cursor.elemId, `3@${Automerge.getActorId(s1)}`) + assert.strictEqual(s1.cursor.index, 1) + const s2 = Automerge.change(s1, doc => { + doc.cursor = doc.text.getCursorAt(2) + }) + assert.strictEqual(s2.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + assert.strictEqual(s2.cursor.index, 2) + }) + + it('should update a cursor when its index changes', () => { + const s1 = Automerge.change(Automerge.init(), doc => { + doc.text = new Automerge.Text(['b', 'c']) + doc.cursor = doc.text.getCursorAt(1) + }) + assert.strictEqual(s1.cursor.elemId, `3@${Automerge.getActorId(s1)}`) + assert.strictEqual(s1.cursor.index, 1) + const s2 = Automerge.change(s1, doc => { + doc.text.insertAt(0, 'a') + }) + assert.strictEqual(s2.cursor.elemId, `3@${Automerge.getActorId(s1)}`) + assert.strictEqual(s2.cursor.index, 2) + }) + + it('should support cursors in deeply nested objects', () => { + const s1 = Automerge.change(Automerge.init(), doc => { + doc.paragraphs = [{text: new Automerge.Text(['b', 'c']), style: []}] + doc.paragraphs[0].style.push({ + format: 'bold', + from: doc.paragraphs[0].text.getCursorAt(0), + to: doc.paragraphs[0].text.getCursorAt(1) + }) + }) + assert.strictEqual(s1.paragraphs[0].style[0].from.elemId, `5@${Automerge.getActorId(s1)}`) + assert.strictEqual(s1.paragraphs[0].style[0].from.index, 0) + const s2 = Automerge.change(s1, doc => { + doc.paragraphs[0].text.insertAt(0, 'a') + }) + assert.strictEqual(s2.paragraphs[0].style[0].from.elemId, `5@${Automerge.getActorId(s1)}`) + assert.strictEqual(s2.paragraphs[0].style[0].from.index, 1) + }) + + it.skip('should restore cursors on load', () => { + let s1 = Automerge.change(Automerge.init(), doc => { + doc.text = new Automerge.Text(['a', 'b', 'c']) + doc.cursor = doc.text.getCursorAt(1) + }) + let s2 = Automerge.load(Automerge.save(s1)) + assert.ok(s2.cursor instanceof Automerge.Cursor) + assert.strictEqual(s2.cursor.elemId, `4@${Automerge.getActorId(s1)}`) + assert.strictEqual(s2.cursor.index, 1) + }) }) From 00293ab21ee1f1237a32b56a2919cb2e1b23d67d Mon Sep 17 00:00:00 2001 From: Martin Kleppmann Date: Wed, 3 Feb 2021 16:37:55 +0000 Subject: [PATCH 5/5] Nicer error message --- frontend/cursor.js | 12 ++++++++++-- test/cursor_test.js | 13 +++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/frontend/cursor.js b/frontend/cursor.js index 0e943a3ea..2a18381e6 100644 --- a/frontend/cursor.js +++ b/frontend/cursor.js @@ -8,12 +8,20 @@ const { isObject } = require('../src/common') */ class Cursor { constructor(object, index, elemId = undefined) { - if (Array.isArray(object) && object[ELEM_IDS] && typeof index === 'number') { - if (index < 0 || index >= object[ELEM_IDS].length) throw new RangeError('list index out of bounds') + if (Array.isArray(object) && typeof index === 'number') { + if (!object[OBJECT_ID] || !object[ELEM_IDS]) { + throw new RangeError('The object referenced by a cursor must be part of a document') + } + if (index < 0 || index >= object[ELEM_IDS].length) { + throw new RangeError('list index out of bounds') + } this.objectId = object[OBJECT_ID] this.elemId = object[ELEM_IDS][index] this.index = index } else if (isObject(object) && object.getElemId && typeof index === 'number') { + if (!object[OBJECT_ID]) { + throw new RangeError('The object referenced by a cursor must be part of a document') + } this.objectId = object[OBJECT_ID] this.elemId = object.getElemId(index) this.index = index diff --git a/test/cursor_test.js b/test/cursor_test.js index 59a149daa..36f949a6a 100644 --- a/test/cursor_test.js +++ b/test/cursor_test.js @@ -38,6 +38,19 @@ describe('Automerge.Cursor', () => { assert.strictEqual(s2.cursor.index, 2) }) + it('should ensure that the referenced object is part of the document', () => { + assert.throws(() => { + Automerge.change(Automerge.init(), doc => { + doc.cursor = new Automerge.Text(['a', 'b', 'c']).getCursorAt(2) + }) + }, /must be part of a document/) + assert.throws(() => { + Automerge.change(Automerge.init(), doc => { + doc.cursor = new Automerge.Cursor([1, 2, 3], 2) + }) + }, /must be part of a document/) + }) + it('should not allow an index beyond the length of the list', () => { assert.throws(() => { Automerge.change(Automerge.init(), doc => {