diff --git a/packages/dds/sequence/src/intervalCollection.ts b/packages/dds/sequence/src/intervalCollection.ts index c671750a7600..d4c070fa99b2 100644 --- a/packages/dds/sequence/src/intervalCollection.ts +++ b/packages/dds/sequence/src/intervalCollection.ts @@ -739,7 +739,9 @@ function clearEmptyPendingEntry(pendingChanges: PendingChanges, id: string) { } } -function hasEndpointChanges(serialized: SerializedIntervalDelta) { +function hasEndpointChanges( + serialized: SerializedIntervalDelta, +): serialized is ISerializedInterval { return serialized.start !== undefined && serialized.end !== undefined; } @@ -781,6 +783,7 @@ export class IntervalCollection if (md.type === "add" || (md.type === "change" && hasEndpointChanges(op.value))) { const endpointChanges = (pending.endpointChanges ??= new DoublyLinkedList()); md.endpointChangesNode = endpointChanges.push(md).last; + md.rebased = undefined; } submitDelta(op, pending.local.push(md).last); }; @@ -932,7 +935,7 @@ export class IntervalCollection const rebasedValue = localOpMetadata.endpointChangesNode === undefined ? value - : this.rebaseLocalInterval(localOpMetadata); + : this.rebaseLocalInterval(value, localOpMetadata); if (rebasedValue === undefined) { const { id } = getSerializedProperties(value); @@ -974,68 +977,48 @@ export class IntervalCollection } } - private rebasePositionWithSegmentSlide( - pos: number | "start" | "end", - seqNumberFrom: number, + private rebaseReferenceWithSegmentSlide( + ref: LocalReferencePosition, localSeq: number, - ): number | "start" | "end" | undefined { + ): number | "start" | "end" { if (!this.client) { throw new LoggingError("mergeTree client must exist"); } - if (pos === "start" || pos === "end") { - return pos; - } - const { clientId } = this.client.getCollabWindow(); - const { segment, offset } = - this.client.getContainingSegment( - pos, - { - referenceSequenceNumber: seqNumberFrom, - clientId: this.client.getLongClientId(clientId), - }, - localSeq, - ) ?? {}; - - // if segment is undefined, it slid off the string - assert(segment !== undefined && offset !== undefined, 0x54e /* No segment found */); + const segment: ISegmentInternal | undefined = ref.getSegment(); + if (segment?.endpointType) { + return segment.endpointType; + } + const offset = ref.getOffset(); const segoff = getSlideToSegoff( - { segment, offset }, + segment === undefined ? undefined : { segment, offset }, undefined, createLocalReconnectingPerspective(this.client.getCurrentSeq(), clientId, localSeq), this.options.mergeTreeReferencesCanSlideToEndpoint, ); // case happens when rebasing op, but concurrently entire string has been deleted - if (segoff?.segment === undefined || segoff.offset === undefined) { + if (segoff === undefined) { return DetachedReferencePosition; } - - assert( - offset !== undefined && 0 <= offset && offset < segment.cachedLength, - 0x54f /* Invalid offset */, - ); return this.client.findReconnectionPosition(segoff.segment, localSeq) + segoff.offset; } private computeRebasedPositions( localOpMetadata: IntervalAddLocalMetadata | IntervalChangeLocalMetadata, - ): ISerializedInterval | SerializedIntervalDelta { + ): Pick { assert( this.client !== undefined, 0x550 /* Client should be defined when computing rebased position */, ); - const { localSeq, original } = localOpMetadata; - const rebased = { ...original }; - const { start, end, sequenceNumber } = original; - if (start !== undefined) { - rebased.start = this.rebasePositionWithSegmentSlide(start, sequenceNumber, localSeq); - } - if (end !== undefined) { - rebased.end = this.rebasePositionWithSegmentSlide(end, sequenceNumber, localSeq); - } + + const { localSeq, interval } = localOpMetadata; + const rebased = { + start: this.rebaseReferenceWithSegmentSlide(interval.start, localSeq), + end: this.rebaseReferenceWithSegmentSlide(interval.end, localSeq), + }; return rebased; } @@ -1216,7 +1199,7 @@ export class IntervalCollection { type: "add", localSeq, - original: serializedInterval, + interval, }, ); } @@ -1345,7 +1328,7 @@ export class IntervalCollection type: "change", localSeq, previous: interval.serialize(), - original: serializedInterval, + interval: newInterval ?? interval, }; this.submitDelta( @@ -1369,8 +1352,10 @@ export class IntervalCollection } if (newInterval) { this.emitChange(newInterval, interval, true, false); - this.client?.removeLocalReferencePosition(interval.start); - this.client?.removeLocalReferencePosition(interval.end); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + interval.start.properties!.interval = undefined; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + interval.end.properties!.interval = undefined; } return newInterval; } @@ -1477,14 +1462,14 @@ export class IntervalCollection * */ public rebaseLocalInterval( + original: SerializedIntervalDelta, localOpMetadata: IntervalAddLocalMetadata | IntervalChangeLocalMetadata, ): SerializedIntervalDelta | undefined { - const original = localOpMetadata.original; - if (!this.client) { + if (!this.client || !hasEndpointChanges(original)) { // If there's no associated mergeTree client, the originally submitted op is still correct. return original; } - if (!this.attached) { + if (!this.attached || this.localCollection === undefined) { throw new LoggingError("attachSequence must be called"); } @@ -1494,7 +1479,7 @@ export class IntervalCollection const { start: startRebased, end: endRebased } = (localOpMetadata.rebased ??= this.computeRebasedPositions(localOpMetadata)); - const localInterval = this.localCollection?.idIntervalIndex.getIntervalById(id); + const localInterval = this.localCollection.idIntervalIndex.getIntervalById(id); const rebased: SerializedIntervalDelta = { start: startRebased, @@ -1521,7 +1506,7 @@ export class IntervalCollection if (localInterval !== undefined) { // The rebased op may place this interval's endpoints on different segments. Calling `changeInterval` here // updates the local client's state to be consistent with the emitted op. - this.localCollection?.changeInterval( + localOpMetadata.interval = this.localCollection?.changeInterval( localInterval, toOptionalSequencePlace(startRebased, startSide ?? Side.Before), toOptionalSequencePlace(endRebased, endSide ?? Side.Before), diff --git a/packages/dds/sequence/src/intervalCollectionMapInterfaces.ts b/packages/dds/sequence/src/intervalCollectionMapInterfaces.ts index adaea5dca094..4d0680352d25 100644 --- a/packages/dds/sequence/src/intervalCollectionMapInterfaces.ts +++ b/packages/dds/sequence/src/intervalCollectionMapInterfaces.ts @@ -15,28 +15,30 @@ import { ISerializedInterval, IntervalDeltaOpType, SerializedIntervalDelta, + type SequenceIntervalClass, } from "./intervals/index.js"; export interface IntervalAddLocalMetadata { type: typeof IntervalDeltaOpType.ADD; localSeq: number; endpointChangesNode?: ListNode; - rebased?: ISerializedInterval; - original: ISerializedInterval; + rebased?: Pick; + interval: SequenceIntervalClass; } export interface IntervalChangeLocalMetadata { type: typeof IntervalDeltaOpType.CHANGE; localSeq: number; previous: ISerializedInterval; endpointChangesNode?: ListNode; - rebased?: SerializedIntervalDelta; - original: SerializedIntervalDelta; + rebased?: Pick; + interval: SequenceIntervalClass; } export interface IntervalDeleteLocalMetadata { type: typeof IntervalDeltaOpType.DELETE; localSeq: number; previous: ISerializedInterval; endpointChangesNode?: undefined; + interval?: undefined; } export type IntervalMessageLocalMetadata = | IntervalAddLocalMetadata diff --git a/packages/dds/sequence/src/test/fuzz/fuzzUtils.ts b/packages/dds/sequence/src/test/fuzz/fuzzUtils.ts index 988b856541bb..f8e87f67a066 100644 --- a/packages/dds/sequence/src/test/fuzz/fuzzUtils.ts +++ b/packages/dds/sequence/src/test/fuzz/fuzzUtils.ts @@ -483,6 +483,13 @@ export const baseModel: Omit< break; case "removeRange": case "annotateRange": + if (op.start > 0) { + op.start--; + } + if (op.end > 0) { + op.end--; + } + break; case "addInterval": case "changeInterval": { const { startPos, endPos, startSide, endSide } = endpointPosAndSide(