diff --git a/@types/automerge/index.d.ts b/@types/automerge/index.d.ts index c0a40e16d..8fca06540 100644 --- a/@types/automerge/index.d.ts +++ b/@types/automerge/index.d.ts @@ -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 @@ -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 } diff --git a/backend/new.js b/backend/new.js index 6e19d2227..02b027f7a 100644 --- a/backend/new.js +++ b/backend/new.js @@ -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') @@ -1021,7 +1021,8 @@ 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 } @@ -1029,7 +1030,21 @@ function updatePatchProperty(patches, newBlock, objectId, op, docState, propStat } 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 } } diff --git a/frontend/apply_patch.js b/frontend/apply_patch.js index 2b4dd3f9d..a156ef782 100644 --- a/frontend/apply_patch.js +++ b/frontend/apply_patch.js @@ -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') @@ -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 @@ -59,8 +61,13 @@ 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 { @@ -68,11 +75,11 @@ function applyProperties(props, object, conflicts, 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 } } diff --git a/frontend/context.js b/frontend/context.js index 926c5713a..20627fdff 100644 --- a/frontend/context.js +++ b/frontend/context.js @@ -167,7 +167,6 @@ class Context { subpatch = values[nextOpId] object = this.getPropertyValue(object, pathElem.key, nextOpId) } - return subpatch } diff --git a/src/common.js b/src/common.js index 02e91392d..9586d2b8a 100644 --- a/src/common.js +++ b/src/common.js @@ -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 } diff --git a/test/backend_test.js b/test/backend_test.js index 56d37eb4a..c7e4734cf 100644 --- a/test/backend_test.js +++ b/test/backend_test.js @@ -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 @@ -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}}} }) }) @@ -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}}} }) }) @@ -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, @@ -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}`} ] }}}} }) @@ -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}`} ] }}}} }) @@ -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}`} ] }}}} }) @@ -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}`} ] }}}} }) @@ -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'} ]}} }} }) @@ -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}`} ] }}}} }) diff --git a/test/new_backend_test.js b/test/new_backend_test.js index a3d3b63ea..3529e0856 100644 --- a/test/new_backend_test.js +++ b/test/new_backend_test.js @@ -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) { @@ -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], { @@ -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], { @@ -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) @@ -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'}} ] }}}} @@ -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}`} ] }}}} }) @@ -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