From 09cab3780cad275cd1d8c45cda1f1a52db7e63ec Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Wed, 30 Apr 2025 22:12:09 +0200 Subject: [PATCH] implement idset.delete & idmap.delete --- src/structs/ContentType.js | 4 +-- src/structs/Item.js | 4 +-- src/utils/IdMap.js | 25 +++++++++++-- src/utils/IdSet.js | 64 ++++++++++++++++++++++++++++++++-- src/utils/PermanentUserData.js | 2 +- src/utils/Snapshot.js | 2 +- src/utils/Transaction.js | 2 +- src/utils/UndoManager.js | 2 +- src/utils/YEvent.js | 4 +-- tests/IdMap.tests.js | 56 ++++++++++++++++++++--------- tests/IdSet.tests.js | 26 ++++++++++++-- 11 files changed, 156 insertions(+), 35 deletions(-) diff --git a/src/structs/ContentType.js b/src/structs/ContentType.js index 5f1c71f9..597c8719 100644 --- a/src/structs/ContentType.js +++ b/src/structs/ContentType.js @@ -107,7 +107,7 @@ export class ContentType { while (item !== null) { if (!item.deleted) { item.delete(transaction) - } else if (!transaction.insertSet.has(item.id)) { + } else if (!transaction.insertSet.hasId(item.id)) { // This will be gc'd later and we want to merge it if possible // We try to merge all deleted items after each transaction, // but we have no knowledge about that this needs to be merged @@ -119,7 +119,7 @@ export class ContentType { this.type._map.forEach(item => { if (!item.deleted) { item.delete(transaction) - } else if (!transaction.insertSet.has(item.id)) { + } else if (!transaction.insertSet.hasId(item.id)) { // same as above transaction._mergeStructs.push(item) } diff --git a/src/structs/Item.js b/src/structs/Item.js index 8ca713c3..838d6b11 100644 --- a/src/structs/Item.js +++ b/src/structs/Item.js @@ -124,7 +124,7 @@ export const splitItem = (transaction, leftItem, diff) => { * @param {Array} stack * @param {ID} id */ -const isDeletedByUndoStack = (stack, id) => array.some(stack, /** @param {StackItem} s */ s => s.deletions.has(id)) +const isDeletedByUndoStack = (stack, id) => array.some(stack, /** @param {StackItem} s */ s => s.deletions.hasId(id)) /** * Redoes the effect of this operation. @@ -210,7 +210,7 @@ export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemo left = item // Iterate right while right is in itemsToDelete // If it is intended to delete right while item is redone, we can expect that item should replace right. - while (left !== null && left.right !== null && (left.right.redone || itemsToDelete.has(left.right.id) || isDeletedByUndoStack(um.undoStack, left.right.id) || isDeletedByUndoStack(um.redoStack, left.right.id))) { + while (left !== null && left.right !== null && (left.right.redone || itemsToDelete.hasId(left.right.id) || isDeletedByUndoStack(um.undoStack, left.right.id) || isDeletedByUndoStack(um.redoStack, left.right.id))) { left = left.right // follow redone while (left.redone) left = getItemCleanStart(transaction, left.redone) diff --git a/src/utils/IdMap.js b/src/utils/IdMap.js index 7ec75aec..5f674182 100644 --- a/src/utils/IdMap.js +++ b/src/utils/IdMap.js @@ -2,6 +2,7 @@ import { _diffSet, findIndexInIdRanges, findRangeStartInIdRanges, + _deleteRangeFromIdSet, DSDecoderV1, DSDecoderV2, IdSetEncoderV1, IdSetEncoderV2, IdSet, ID // eslint-disable-line } from '../internals.js' @@ -319,10 +320,19 @@ export class IdMap { * @param {ID} id * @return {boolean} */ - has (id) { - const dr = this.clients.get(id.client) + hasId (id) { + return this.has(id.client, id.clock) + } + + /** + * @param {number} client + * @param {number} clock + * @return {boolean} + */ + has (client, clock) { + const dr = this.clients.get(client) if (dr) { - return findIndexInIdRanges(dr.getIds(), id.clock) !== null + return findIndexInIdRanges(dr.getIds(), clock) !== null } return false } @@ -395,6 +405,15 @@ export class IdMap { ranges.add(clock, len, attrs) } } + + /** + * @param {number} client + * @param {number} clock + * @param {number} len + */ + delete (client, clock, len) { + _deleteRangeFromIdSet(this, client, clock, len) + } } /** diff --git a/src/utils/IdSet.js b/src/utils/IdSet.js index fe52bd86..8d5db4ce 100644 --- a/src/utils/IdSet.js +++ b/src/utils/IdSet.js @@ -113,13 +113,71 @@ export class IdSet { * @param {ID} id * @return {boolean} */ - has (id) { - const dr = this.clients.get(id.client) + hasId (id) { + return this.has(id.client, id.clock) + } + + /** + * @param {number} client + * @param {number} clock + */ + has (client, clock) { + const dr = this.clients.get(client) if (dr) { - return findIndexInIdRanges(dr.getIds(), id.clock) !== null + return findIndexInIdRanges(dr.getIds(), clock) !== null } return false } + + /** + * @param {number} client + * @param {number} clock + * @param {number} len + */ + add (client, clock, len) { + addToIdSet(this, client, clock, len) + } + + /** + * @param {number} client + * @param {number} clock + * @param {number} len + */ + delete (client, clock, len) { + _deleteRangeFromIdSet(this, client, clock, len) + } +} + +/** + * @param {IdSet | IdMap} set + * @param {number} client + * @param {number} clock + * @param {number} len + */ +export const _deleteRangeFromIdSet = (set, client, clock, len) => { + const dr = set.clients.get(client) + if (dr && len > 0) { + const ids = dr.getIds() + let index = findRangeStartInIdRanges(ids, clock) + if (index != null) { + for (let r = ids[index]; index < ids.length && r.clock < clock + len; r = ids[++index]) { + if (r.clock < clock) { + ids[index] = r.copyWith(r.clock, clock-r.clock) + if (clock + len < r.clock + r.len) { + ids.splice(index + 1, 0, r.copyWith(clock + len, r.clock + r.len - clock - len)) + } + } else if (clock + len < r.clock + r.len) { + // need to retain end + ids[index] = r.copyWith(clock + len , r.clock + r.len - clock - len) + } else if (ids.length === 1) { + set.clients.delete(client) + return + } else { + ids.splice(index--, 1) + } + } + } + } } /** diff --git a/src/utils/PermanentUserData.js b/src/utils/PermanentUserData.js index fca1b663..d4067fde 100644 --- a/src/utils/PermanentUserData.js +++ b/src/utils/PermanentUserData.js @@ -131,7 +131,7 @@ export class PermanentUserData { */ getUserByDeletedId (id) { for (const [userDescription, ds] of this.dss.entries()) { - if (ds.has(id)) { + if (ds.hasId(id)) { return userDescription } } diff --git a/src/utils/Snapshot.js b/src/utils/Snapshot.js index fd21a37b..fcf9ea0a 100644 --- a/src/utils/Snapshot.js +++ b/src/utils/Snapshot.js @@ -117,7 +117,7 @@ export const snapshot = doc => createSnapshot(createDeleteSetFromStructStore(doc */ export const isVisible = (item, snapshot) => snapshot === undefined ? !item.deleted - : snapshot.sv.has(item.id.client) && (snapshot.sv.get(item.id.client) || 0) > item.id.clock && !snapshot.ds.has(item.id) + : snapshot.sv.has(item.id.client) && (snapshot.sv.get(item.id.client) || 0) > item.id.clock && !snapshot.ds.hasId(item.id) /** * @param {Transaction} transaction diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index c90f3b76..5f935437 100644 --- a/src/utils/Transaction.js +++ b/src/utils/Transaction.js @@ -199,7 +199,7 @@ export const nextID = transaction => { */ export const addChangedTypeToTransaction = (transaction, type, parentSub) => { const item = type._item - if (item === null || (!item.deleted && !transaction.insertSet.has(item.id))) { + if (item === null || (!item.deleted && !transaction.insertSet.hasId(item.id))) { map.setIfUndefined(transaction.changed, type, set.create).add(parentSub) } } diff --git a/src/utils/UndoManager.js b/src/utils/UndoManager.js index 6fef6aa8..09cdc4b7 100644 --- a/src/utils/UndoManager.js +++ b/src/utils/UndoManager.js @@ -89,7 +89,7 @@ const popStackItem = (undoManager, stack, eventType) => { struct instanceof Item && scope.some(type => type === transaction.doc || isParentOf(/** @type {AbstractType} */ (type), struct)) && // Never redo structs in stackItem.insertions because they were created and deleted in the same capture interval. - !stackItem.insertions.has(struct.id) + !stackItem.insertions.hasId(struct.id) ) { itemsToRedo.add(struct) } diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index c77507c8..6a1aaed5 100644 --- a/src/utils/YEvent.js +++ b/src/utils/YEvent.js @@ -77,7 +77,7 @@ export class YEvent { * @return {boolean} */ deletes (struct) { - return this.transaction.deleteSet.has(struct.id) + return this.transaction.deleteSet.hasId(struct.id) } /** @@ -157,7 +157,7 @@ export class YEvent { * @return {boolean} */ adds (struct) { - return this.transaction.insertSet.has(struct.id) + return this.transaction.insertSet.hasId(struct.id) } /** diff --git a/tests/IdMap.tests.js b/tests/IdMap.tests.js index 82cbcad1..54d2cbcb 100644 --- a/tests/IdMap.tests.js +++ b/tests/IdMap.tests.js @@ -1,6 +1,8 @@ import * as t from 'lib0/testing' import * as idmap from '../src/utils/IdMap.js' -import { compareIdmaps, createIdMap, ID, createRandomIdSet, createRandomIdMap, createAttributionItem } from './testHelper.js' +import * as prng from 'lib0/prng' +import * as math from 'lib0/math' +import { compareIdmaps as compareIdMaps, createIdMap, ID, createRandomIdSet, createRandomIdMap, createAttributionItem } from './testHelper.js' import * as YY from '../src/internals.js' /** @@ -21,49 +23,49 @@ const simpleConstructAttrs = ops => { export const testAmMerge = _tc => { const attrs = [42] t.group('filter out empty items (1))', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 0, attrs]]), simpleConstructAttrs([]) ) }) t.group('filter out empty items (2))', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 0, attrs], [0, 2, 0, attrs]]), simpleConstructAttrs([]) ) }) t.group('filter out empty items (3 - end))', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 1, attrs], [0, 2, 0, attrs]]), simpleConstructAttrs([[0, 1, 1, attrs]]) ) }) t.group('filter out empty items (4 - middle))', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 1, attrs], [0, 2, 0, attrs], [0, 3, 1, attrs]]), simpleConstructAttrs([[0, 1, 1, attrs], [0, 3, 1, attrs]]) ) }) t.group('filter out empty items (5 - beginning))', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 0, attrs], [0, 2, 1, attrs], [0, 3, 1, attrs]]), simpleConstructAttrs([[0, 2, 1, attrs], [0, 3, 1, attrs]]) ) }) t.group('merge of overlapping id ranges', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 2, attrs], [0, 0, 2, attrs]]), simpleConstructAttrs([[0, 0, 3, attrs]]) ) }) t.group('construct without hole', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 2, attrs], [0, 3, 1, attrs]]), simpleConstructAttrs([[0, 1, 3, attrs]]) ) }) t.group('no merge of overlapping id ranges with different attributes', () => { - compareIdmaps( + compareIdMaps( simpleConstructAttrs([[0, 1, 2, [1]], [0, 0, 2, [2]]]), simpleConstructAttrs([[0, 0, 1, [2]], [0, 1, 1, [1, 2]], [0, 2, 1, [1]]]) ) @@ -85,12 +87,12 @@ export const testRepeatMergingMultipleIdMaps = tc => { } const merged = idmap.mergeIdMaps(sets) const mergedReverse = idmap.mergeIdMaps(sets.reverse()) - compareIdmaps(merged, mergedReverse) + compareIdMaps(merged, mergedReverse) const composed = idmap.createIdMap() for (let iclient = 0; iclient < clients; iclient++) { for (let iclock = 0; iclock < clockRange + 42; iclock++) { - const mergedHas = merged.has(new ID(iclient, iclock)) - const oneHas = sets.some(ids => ids.has(new ID(iclient, iclock))) + const mergedHas = merged.hasId(new ID(iclient, iclock)) + const oneHas = sets.some(ids => ids.hasId(new ID(iclient, iclock))) t.assert(mergedHas === oneHas) const mergedAttrs = merged.slice(new ID(iclient, iclock), 1) mergedAttrs.forEach(a => { @@ -100,7 +102,7 @@ export const testRepeatMergingMultipleIdMaps = tc => { }) } } - compareIdmaps(merged, composed) + compareIdMaps(merged, composed) } /** @@ -115,9 +117,9 @@ export const testRepeatRandomDiffing = tc => { const merged = idmap.mergeIdMaps([idset1, idset2]) const e1 = idmap.diffIdMap(idset1, idset2) const e2 = idmap.diffIdMap(merged, idset2) - compareIdmaps(e1, e2) + compareIdMaps(e1, e2) const copy = YY.decodeIdMap(YY.encodeIdMap(e1)) - compareIdmaps(e1, copy) + compareIdMaps(e1, copy) } /** @@ -135,7 +137,27 @@ export const testRepeatRandomDiffing2 = tc => { const e1 = idmap.diffIdMap(idmap1, idsExclude) const e2 = idmap.diffIdMap(idmap2, idsExclude) const excludedMerged = idmap.mergeIdMaps([e1, e2]) - compareIdmaps(mergedExcluded, excludedMerged) + compareIdMaps(mergedExcluded, excludedMerged) const copy = YY.decodeIdMap(YY.encodeIdMap(mergedExcluded)) - compareIdmaps(mergedExcluded, copy) + compareIdMaps(mergedExcluded, copy) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatRandomDeletes = tc => { + const clients = 1 + const clockRange = 100 + const idset = createRandomIdMap(tc.prng, clients, clockRange, []) + const client = Array.from(idset.clients.keys())[0] + const clock = prng.int31(tc.prng, 0, clockRange) + const len = prng.int31(tc.prng, 0, math.round((clockRange - clock) * 1.2)) // allow exceeding range to cover more edge cases + const idsetOfDeletes = idmap.createIdMap() + idsetOfDeletes.add(client, clock, len, []) + const diffed = idmap.diffIdMap(idset, idsetOfDeletes) + idset.delete(client, clock, len) + for (let i = 0; i < len; i++) { + t.assert(!idset.has(client, clock + i)) + } + compareIdMaps(idset, diffed) } diff --git a/tests/IdSet.tests.js b/tests/IdSet.tests.js index 8078afef..d55ba352 100644 --- a/tests/IdSet.tests.js +++ b/tests/IdSet.tests.js @@ -1,5 +1,7 @@ import * as t from 'lib0/testing' import * as d from '../src/utils/IdSet.js' +import * as math from 'lib0/math' +import * as prng from 'lib0/prng' import { compareIdSets, createRandomIdSet, ID } from './testHelper.js' /** @@ -153,6 +155,26 @@ export const testRepeatRandomDiffing = tc => { compareIdSets(e1, e2) } +/** + * @param {t.TestCase} tc + */ +export const testRepeatRandomDeletes = tc => { + const clients = 1 + const clockRange = 100 + const idset = createRandomIdSet(tc.prng, clients, clockRange) + const client = Array.from(idset.clients.keys())[0] + const clock = prng.int31(tc.prng, 0, clockRange) + const len = prng.int31(tc.prng, 0, math.round((clockRange - clock) * 1.2)) // allow exceeding range to cover more edge cases + const idsetOfDeletes = d.createIdSet() + idsetOfDeletes.add(client, clock, len) + const diffed = d.diffIdSet(idset, idsetOfDeletes) + idset.delete(client, clock, len) + for (let i = 0; i < len; i++) { + t.assert(!idset.has(client, clock + i)) + } + compareIdSets(idset, diffed) +} + /** * @param {t.TestCase} tc */ @@ -172,8 +194,8 @@ export const testRepeatMergingMultipleIdsets = tc => { const composed = d.createIdSet() for (let iclient = 0; iclient < clients; iclient++) { for (let iclock = 0; iclock < clockRange + 42; iclock++) { - const mergedHas = merged.has(new ID(iclient, iclock)) - const oneHas = idss.some(ids => ids.has(new ID(iclient, iclock))) + const mergedHas = merged.hasId(new ID(iclient, iclock)) + const oneHas = idss.some(ids => ids.hasId(new ID(iclient, iclock))) t.assert(mergedHas === oneHas) if (oneHas) { d.addToIdSet(composed, iclient, iclock, 1)