Skip to content

Commit

Permalink
Add OpId for deletions in Patch
Browse files Browse the repository at this point in the history
  • Loading branch information
@begor
begor committed Dec 17, 2021
1 parent 08a456c commit fc55ebd
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 25 deletions.
3 changes: 2 additions & 1 deletion @types/automerge/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ declare module 'automerge' {
// (properties that are not changing are not listed). The nested object is
// empty if the property is being deleted, contains one opId if it is set to
// a single value, and contains multiple opIds if there is a conflict.
props: {[propName: string]: {[opId: string]: MapDiff | ListDiff | ValueDiff }}
props: {[propName: string]: {[opId: string]: MapDiff | ListDiff | ValueDiff | '___DELETED___' }}
}

// Describes changes to a list or Automerge.Text object, in which each element
Expand Down Expand Up @@ -309,6 +309,7 @@ declare module 'automerge' {
// text object.
interface RemoveEdit {
action: 'remove'
opId: OpId // ID of the operation that removed this value. NOTE: if count > 1 opId is the first operation.
index: number // index of the first list element to remove
count: number // number of list elements to remove
}
Expand Down
21 changes: 18 additions & 3 deletions backend/new.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { parseOpId, copyObject } = require('../src/common')
const { parseOpId, copyObject, DELETED_MARKER } = require('../src/common')
const { COLUMN_TYPE, VALUE_TYPE, ACTIONS, OBJECT_TYPE, DOC_OPS_COLUMNS, CHANGE_COLUMNS, DOCUMENT_COLUMNS,
encoderByColumnId, decoderByColumnId, makeDecoders, decodeValue,
encodeChange, decodeChangeColumns, decodeChangeMeta, decodeChanges, decodeDocumentHeader, encodeDocumentHeader } = require('./columnar')
Expand Down Expand Up @@ -1021,15 +1021,30 @@ function updatePatchProperty(patches, newBlock, objectId, op, docState, propStat
} else if (oldSuccNum === 0 && !propState[elemId].action) {
// If the property used to have a non-overwritten/non-deleted value, but no longer, it's a remove
propState[elemId].action = 'remove'
appendEdit(patch.edits, {action: 'remove', index: listIndex, count: 1})
const removeOpId = `${op[succCtrIdx]}@${docState.actorIds[op[succActorIdx]]}`
appendEdit(patch.edits, {action: 'remove', index: listIndex, count: 1, opId: removeOpId})
if (newBlock && newBlock.lastObjectActor === op[objActorIdx] && newBlock.lastObjectCtr === op[objCtrIdx]) {
newBlock.numVisible -= 1
}
}

} else if (patchValue || !isWholeDoc) {
// Updating a map or table (with string key)
if (firstOp || !patch.props[op[keyStrIdx]]) patch.props[op[keyStrIdx]] = {}
if (firstOp || !patch.props[op[keyStrIdx]]) {
patch.props[op[keyStrIdx]] = {}
// Go over succ (which are successors to this operations) and consider them as deletion operation.
// If a succ operation is really a deletion, it'll stay on patch.
// If a succ operation isn't a deletion, we'll see it in this function and delete from patch (see below).
for (let i = 0; i < op[succNumIdx]; i++) {
const succOp = `${op[succCtrIdx][i]}@${docState.actorIds[op[succActorIdx][i]]}`
patch.props[op[keyStrIdx]][succOp] = DELETED_MARKER
}
}
// If we look at operation, we know it's not a deletion (we don't store deletions in history).
// That means we can delete it from props if earlier we added it when considering it as a delete.
if (patch.props[op[keyStrIdx]] !== null && patch.props[op[keyStrIdx]][opId] === DELETED_MARKER) {
delete patch.props[op[keyStrIdx]][opId]
}
if (patchValue) patch.props[op[keyStrIdx]][patchKey] = patchValue
}
}
Expand Down
13 changes: 10 additions & 3 deletions frontend/apply_patch.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { isObject, copyObject, parseOpId } = require('../src/common')
const { isObject, copyObject, parseOpId, DELETED_MARKER } = require('../src/common')
const { OBJECT_ID, CONFLICTS, ELEM_IDS } = require('./constants')
const { instantiateText } = require('./text')
const { instantiateTable } = require('./table')
Expand All @@ -20,6 +20,8 @@ function getValue(patch, object, updated) {
return new Date(patch.value)
} else if (patch.datatype === 'counter') {
return new Counter(patch.value)
} else if (patch === DELETED_MARKER) {
return DELETED_MARKER;
} else {
// Primitive value (int, uint, float64, string, boolean, or null)
return patch.value
Expand Down Expand Up @@ -59,20 +61,25 @@ function applyProperties(props, object, conflicts, updated) {

for (let key of Object.keys(props)) {
const values = {}, opIds = Object.keys(props[key]).sort(lamportCompare).reverse()
const appliedOpIds = []
for (let opId of opIds) {
const subpatch = props[key][opId]
if (subpatch === DELETED_MARKER) {
continue
}
appliedOpIds.push(opId)
if (conflicts[key] && conflicts[key][opId]) {
values[opId] = getValue(subpatch, conflicts[key][opId], updated)
} else {
values[opId] = getValue(subpatch, undefined, updated)
}
}

if (opIds.length === 0) {
if (appliedOpIds.length === 0) {
delete object[key]
delete conflicts[key]
} else {
object[key] = values[opIds[0]]
object[key] = values[appliedOpIds[0]]
conflicts[key] = values
}
}
Expand Down
1 change: 0 additions & 1 deletion frontend/context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ class Context {
subpatch = values[nextOpId]
object = this.getPropertyValue(object, pathElem.key, nextOpId)
}

return subpatch
}

Expand Down
4 changes: 3 additions & 1 deletion src/common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ function createArrayOfNulls(length) {
return array
}

const DELETED_MARKER = '___DELETED___'

module.exports = {
isObject, copyObject, parseOpId, equalBytes, createArrayOfNulls
isObject, copyObject, parseOpId, equalBytes, createArrayOfNulls, DELETED_MARKER
}
19 changes: 10 additions & 9 deletions test/backend_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const Automerge = process.env.TEST_DIST === '1' ? require('../dist/automerge') :
const Backend = Automerge.Backend
const { encodeChange, decodeChange } = require('../backend/columnar')
const uuid = require('../src/uuid')
const { DELETED_MARKER } = require("../src/common")

function hash(change) {
return decodeChange(encodeChange(change)).hash
Expand Down Expand Up @@ -79,7 +80,7 @@ describe('Automerge.Backend', () => {
const [s2, patch2] = Backend.applyChanges(s1, [encodeChange(change2)])
assert.deepStrictEqual(patch2, {
clock: {[actor]: 2}, deps: [hash(change2)], maxOp: 2, pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {bird: {}}}
diffs: {objectId: '_root', type: 'map', props: {bird: {[`2@${actor}`]: DELETED_MARKER}}}
})
})

Expand Down Expand Up @@ -132,7 +133,7 @@ describe('Automerge.Backend', () => {
const [s1, patch1] = Backend.applyChanges(s0, [change1, change2].map(encodeChange))
assert.deepStrictEqual(patch1, {
clock: {[actor]: 2}, deps: [hash(change2)], maxOp: 3, pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {birds: {}}}
diffs: {objectId: '_root', type: 'map', props: {birds: {[`3@${actor}`]: DELETED_MARKER}}}
})
})

Expand Down Expand Up @@ -209,7 +210,7 @@ describe('Automerge.Backend', () => {
const [s2, patch2] = Backend.applyChanges(s1, [encodeChange(change3)])
assert.deepStrictEqual(patch1, {
clock: {[actor1]: 1, [actor2]: 1}, deps: [hash(change2)], maxOp: 3, pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {birds: {}}}
diffs: {objectId: '_root', type: 'map', props: {birds: {[`3@${actor2}`]: DELETED_MARKER}}}
})
assert.deepStrictEqual(patch2, {
clock: {[actor1]: 2, [actor2]: 1}, deps: [hash(change2), hash(change3)].sort(), maxOp: 3, pendingChanges: 0,
Expand Down Expand Up @@ -380,7 +381,7 @@ describe('Automerge.Backend', () => {
clock: {[actor]: 2}, deps: [hash(change2)], maxOp: 3, pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {birds: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'list', edits: [
{action: 'remove', index: 0, count: 1}
{action: 'remove', index: 0, count: 1, opId: `3@${actor}`}
]
}}}}
})
Expand All @@ -403,7 +404,7 @@ describe('Automerge.Backend', () => {
diffs: {objectId: '_root', type: 'map', props: {birds: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'list', edits: [
{action: 'insert', index: 0, elemId: `2@${actor}`, opId: `2@${actor}`, value: {type: 'value', value: 'chaffinch'}},
{action: 'remove', index: 0, count: 1}
{action: 'remove', index: 0, count: 1, opId: `3@${actor}`}
]
}}}}
})
Expand Down Expand Up @@ -518,7 +519,7 @@ describe('Automerge.Backend', () => {
done: {[`4@${actor1}`]: {type: 'value', value: false}}
}
}},
{action: 'remove', index: 0, count: 1}
{action: 'remove', index: 0, count: 1, opId: `5@${actor2}`}
]
}}}}
})
Expand Down Expand Up @@ -710,7 +711,7 @@ describe('Automerge.Backend', () => {
clock: {[actor]: 2}, deps: [hash(change2)], maxOp: 9, pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {todos: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'list', edits: [
{action: 'remove', index: 1, count: 3}
{action: 'remove', index: 1, count: 3, opId: `7@${actor}`}
]
}}}}
})
Expand Down Expand Up @@ -882,7 +883,7 @@ describe('Automerge.Backend', () => {
birds: {'1@111111': {objectId: '1@111111', type: 'list',
edits: [
{action: 'insert', index: 0, elemId: '2@111111', opId: '2@111111', value: {type: 'value', value: 'magpie'}},
{action: 'remove', index: 0, count: 1}
{action: 'remove', index: 0, count: 1, opId: '3@111111'}
]}}
}}
})
Expand Down Expand Up @@ -971,7 +972,7 @@ describe('Automerge.Backend', () => {
clock: {[actor]: 2}, deps: [], maxOp: 9, actor, seq: 2, pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {todos: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'list', edits: [
{action: 'remove', index: 1, count: 3}
{action: 'remove', index: 1, count: 3, opId: `7@${actor}`}
]
}}}}
})
Expand Down
16 changes: 9 additions & 7 deletions test/new_backend_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { checkEncoded } = require('./helpers')
const { DOC_OPS_COLUMNS, encodeChange, decodeChange } = require('../backend/columnar')
const { MAX_BLOCK_SIZE, BackendDoc, bloomFilterContains } = require('../backend/new')
const uuid = require('../src/uuid')
const { DELETED_MARKER } = require("../src/common")

function checkColumns(block, expectedCols) {
for (let actual of block.columns) {
Expand Down Expand Up @@ -625,7 +626,7 @@ describe('BackendDoc applying changes', () => {
assert.deepStrictEqual(backend.applyChanges([encodeChange(change2)]), {
maxOp: 3, clock: {[actor]: 2}, deps: [hash(change2)], pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {text: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'text', edits: [{action: 'remove', index: 0, count: 1}]
objectId: `1@${actor}`, type: 'text', edits: [{action: 'remove', index: 0, count: 1, opId: `3@${actor}`}]
}}}}
})
checkColumns(backend.blocks[0], {
Expand Down Expand Up @@ -678,7 +679,7 @@ describe('BackendDoc applying changes', () => {
assert.deepStrictEqual(backend.applyChanges([encodeChange(change2)]), {
maxOp: 5, clock: {[actor]: 2}, deps: [hash(change2)], pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {text: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'text', edits: [{action: 'remove', index: 1, count: 1}]
objectId: `1@${actor}`, type: 'text', edits: [{action: 'remove', index: 1, count: 1, opId: `5@${actor}`}]
}}}}
})
checkColumns(backend.blocks[0], {
Expand Down Expand Up @@ -1271,7 +1272,7 @@ describe('BackendDoc applying changes', () => {
})
assert.deepStrictEqual(backend.applyChanges([encodeChange(change3)]), {
maxOp: 3, clock: {[actor]: 3}, deps: [hash(change3)], pendingChanges: 0,
diffs: {objectId: '_root', type: 'map', props: {counter: {}}}
diffs: {objectId: '_root', type: 'map', props: {counter: {[`3@${actor}`]: DELETED_MARKER}}}
})
assert.strictEqual(backend.blocks[0].lastKey, 'counter')
assert.strictEqual(backend.blocks[0].numOps, 2)
Expand Down Expand Up @@ -1618,7 +1619,7 @@ describe('BackendDoc applying changes', () => {
diffs: {objectId: '_root', type: 'map', props: {text: {[`1@${actor}`]: {
objectId: `1@${actor}`, type: 'text', edits: [
{action: 'multi-insert', index: 0, elemId: `2@${actor}`, values: ['a', 'b']},
{action: 'remove', index: 0, count: 1},
{action: 'remove', index: 0, count: 1, opId: `4@${actor}`},
{action: 'update', index: 0, opId: `5@${actor}`, value: {type: 'value', value: 'x'}}
]
}}}}
Expand Down Expand Up @@ -1670,7 +1671,7 @@ describe('BackendDoc applying changes', () => {
{action: 'insert', index: 0, elemId: `2@${actor1}`, opId: `2@${actor1}`, value: {
type: 'value', value: 1, datatype: 'uint'
}},
{action: 'remove', index: 0, count: 1}
{action: 'remove', index: 0, count: 1, opId: `3@${actor1}`}
]
}}}}
})
Expand Down Expand Up @@ -2069,14 +2070,15 @@ describe('BackendDoc applying changes', () => {
for (let i = 2; i <= MAX_BLOCK_SIZE; i++) {
change1.ops.push({action: 'set', obj: `1@${actor}`, elemId: `${i}@${actor}`, insert: true, value: 'a', pred: []})
}
const change2 = {actor, seq: 2, startOp: MAX_BLOCK_SIZE + 3, time: 0, deps: [], ops: []}
const delStartOp = MAX_BLOCK_SIZE + 3
const change2 = {actor, seq: 2, startOp: delStartOp, time: 0, deps: [], ops: []}
for (let i = 2; i <= MAX_BLOCK_SIZE + 1; i++) {
change2.ops.push({action: 'del', obj: `1@${actor}`, elemId: `${i}@${actor}`, insert: false, pred: [`${i}@${actor}`]})
}
const backend = new BackendDoc()
backend.applyChanges([encodeChange(change1)])
const patch = backend.applyChanges([encodeChange(change2)])
assert.deepStrictEqual(patch.diffs.props.text[`1@${actor}`].edits, [{action: 'remove', index: 0, count: MAX_BLOCK_SIZE}])
assert.deepStrictEqual(patch.diffs.props.text[`1@${actor}`].edits, [{action: 'remove', index: 0, count: MAX_BLOCK_SIZE, opId: `${delStartOp}@${actor}`}])
assert.strictEqual(backend.blocks.length, 2)
const sizeByte1 = 0x80 | 0x7f & (MAX_BLOCK_SIZE / 2), sizeByte2 = (MAX_BLOCK_SIZE / 2) >>> 7
const firstSucc = MAX_BLOCK_SIZE + 3, secondSucc = MAX_BLOCK_SIZE + 3 + MAX_BLOCK_SIZE / 2
Expand Down

0 comments on commit fc55ebd

Please sign in to comment.