Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OpId to remove actions in Patch #453

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add OpId to remove actions in Patch #453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion @types/automerge/index.d.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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