Skip to content

Commit

Permalink
Compute indexes for cursors
Browse files Browse the repository at this point in the history
  • Loading branch information
ept committed Feb 3, 2021
1 parent 64d10e6 commit 37d7311
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 63 deletions.
2 changes: 1 addition & 1 deletion backend/columnar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
132 changes: 83 additions & 49 deletions backend/op_set.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand All @@ -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})
}
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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))
}
Expand All @@ -204,20 +217,17 @@ 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)
}
return opSet
}

/**
* 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
Expand All @@ -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
}

/**
Expand All @@ -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')) {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion frontend/apply_patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 6 additions & 1 deletion frontend/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion frontend/cursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/backend_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
}}
})
})
Expand Down Expand Up @@ -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'}}
}}
})
})
Expand Down
71 changes: 63 additions & 8 deletions test/cursor_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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)
})
})

0 comments on commit 37d7311

Please sign in to comment.