Remove the findTile API (microsoft#18908)

Finalize the removal of the findTile API, in favor of the new searchForMarker API. AB#6199
rahulpatil04 · Dec 20, 2023 · 29b093e · 29b093e
1 parent 27bea85
commit 29b093e
Show file tree

Hide file tree

Showing 15 changed files with 80 additions and 507 deletions.
diff --git a/.changeset/many-pugs-exist.md b/.changeset/many-pugs-exist.md
@@ -0,0 +1,8 @@
+---
+"@fluidframework/merge-tree": minor
+"@fluidframework/sequence": minor
+---
+
+Remove the findTile API from mergeTree, Client, and SharedString
+
+The findTile API that was previously deprecated is now being removed. The new searchForMarker function provides similar functionality, and can be called with the start position, the client ID, the desired marker label to find, and the search direction, where a value of true indicates a forward search.
diff --git a/examples/data-objects/webflow/src/document/index.ts b/examples/data-objects/webflow/src/document/index.ts
@@ -417,20 +417,20 @@ export class FlowDocument extends LazyLoadedDataObject<ISharedDirectory, IFlowDo
 		this.sharedString.annotateRange(start, end, { attr });
 	}
 
-	public findTile(
-		position: number,
-		tileType: DocTile,
-		preceding: boolean,
-	): { tile: ReferencePosition; pos: number } {
-		return this.sharedString.findTile(position, tileType as unknown as string, preceding);
+	public searchForMarker(startPos: number, markerLabel: string, forwards: boolean) {
+		return this.sharedString.searchForMarker(startPos, markerLabel, forwards);
 	}
 
 	public findParagraph(position: number) {
-		const maybeStart = this.findTile(position, DocTile.paragraph, /* preceding: */ true);
-		const start = maybeStart ? maybeStart.pos : 0;
-
-		const maybeEnd = this.findTile(position, DocTile.paragraph, /* preceding: */ false);
-		const end = maybeEnd ? maybeEnd.pos + 1 : this.length;
+		const maybeStart = this.searchForMarker(position, DocTile.paragraph, /* forwards: */ true);
+		const start = maybeStart
+			? this.sharedString.localReferencePositionToPosition(maybeStart)
+			: 0;
+
+		const maybeEnd = this.searchForMarker(position, DocTile.paragraph, /* forwards: */ false);
+		const end = maybeEnd
+			? this.sharedString.localReferencePositionToPosition(maybeEnd) + 1
+			: this.length;
 
 		return { start, end };
 	}

diff --git a/packages/dds/merge-tree/api-report/merge-tree.api.md b/packages/dds/merge-tree/api-report/merge-tree.api.md
@@ -114,11 +114,6 @@ export class Client extends TypedEventEmitter<IClientEvents> {
     // (undocumented)
     createTextHelper(): IMergeTreeTextHelper;
     findReconnectionPosition(segment: ISegment, localSeq: number): number;
-    // @deprecated (undocumented)
-    findTile(startPos: number, tileLabel: string, preceding?: boolean): {
-        tile: ReferencePosition;
-        pos: number;
-    } | undefined;
     // (undocumented)
     getClientId(): number;
     // (undocumented)

diff --git a/packages/dds/merge-tree/package.json b/packages/dds/merge-tree/package.json
@@ -187,6 +187,10 @@
 		}
 	},
 	"typeValidation": {
-		"broken": {}
+		"broken": {
+			"ClassDeclaration_Client": {
+				"backCompat": false
+			}
+		}
 	}
 }
diff --git a/packages/dds/merge-tree/src/client.ts b/packages/dds/merge-tree/src/client.ts
@@ -1233,14 +1233,6 @@ export class Client extends TypedEventEmitter<IClientEvents> {
 		}
 	}
 
-	/**
-	 * @deprecated Use searchForMarker instead.
-	 */
-	findTile(startPos: number, tileLabel: string, preceding = true) {
-		const clientId = this.getClientId();
-		return this._mergeTree.findTile(startPos, clientId, tileLabel, preceding);
-	}
-
 	/**
 	 * Searches a string for the nearest marker in either direction to a given start position.
 	 * The search will include the start position, so markers at the start position are valid

diff --git a/packages/dds/merge-tree/src/mergeTree.ts b/packages/dds/merge-tree/src/mergeTree.ts
@@ -41,7 +41,6 @@ import {
 	MaxNodesInBlock,
 	MergeBlock,
 	reservedMarkerIdKey,
-	SegmentActions,
 	// eslint-disable-next-line import/no-deprecated
 	SegmentGroup,
 	toMoveInfo,
@@ -137,62 +136,6 @@ const LRUSegmentComparer: IComparer<LRUSegment> = {
 	compare: (a, b) => a.maxSeq - b.maxSeq,
 };
 
-interface IReferenceSearchInfo {
-	mergeTree: MergeTree;
-	tileLabel: string;
-	tilePrecedesPos?: boolean;
-	tile?: ReferencePosition;
-}
-
-function recordTileStart(
-	segment: ISegment,
-	segpos: number,
-	refSeq: number,
-	clientId: number,
-	start: number,
-	end: number,
-	searchInfo: IReferenceSearchInfo,
-) {
-	if (Marker.is(segment)) {
-		if (refHasTileLabel(segment, searchInfo.tileLabel)) {
-			searchInfo.tile = segment;
-		}
-	}
-	return false;
-}
-
-function tileShift(
-	node: IMergeNode,
-	segpos: number,
-	refSeq: number,
-	clientId: number,
-	offset: number | undefined,
-	end: number | undefined,
-	searchInfo: IReferenceSearchInfo,
-) {
-	if (node.isLeaf()) {
-		const seg = node;
-		if ((searchInfo.mergeTree.localNetLength(seg) ?? 0) > 0 && Marker.is(seg)) {
-			if (refHasTileLabel(seg, searchInfo.tileLabel)) {
-				searchInfo.tile = seg;
-			}
-		}
-	} else {
-		const block = node as IHierBlock;
-		const marker = searchInfo.tilePrecedesPos
-			? block.rightmostTiles[searchInfo.tileLabel]
-			: block.leftmostTiles[searchInfo.tileLabel];
-		if (marker !== undefined) {
-			assert(
-				marker.isLeaf() && Marker.is(marker),
-				0x868 /* Object returned is not a valid marker */,
-			);
-			searchInfo.tile = marker;
-		}
-	}
-	return true;
-}
-
 function addTile(tile: ReferencePosition, tiles: MapLike<ReferencePosition>) {
 	const tileLabels = refGetTileLabels(tile);
 	if (tileLabels) {
@@ -1135,60 +1078,6 @@ export class MergeTree {
 		return DetachedReferencePosition;
 	}
 
-	// TODO: filter function
-	/**
-	 * Finds the nearest reference with ReferenceType.Tile to `startPos` in the direction dictated by `tilePrecedesPos`.
-	 * @deprecated Use searchForMarker instead.
-	 *
-	 * @param startPos - Position at which to start the search
-	 * @param clientId - clientId dictating the perspective to search from
-	 * @param tileLabel - Label of the tile to search for
-	 * @param tilePrecedesPos - Whether the desired tile comes before (true) or after (false) `startPos`
-	 */
-	public findTile(startPos: number, clientId: number, tileLabel: string, tilePrecedesPos = true) {
-		const searchInfo: IReferenceSearchInfo = {
-			mergeTree: this,
-			tilePrecedesPos,
-			tileLabel,
-		};
-
-		if (tilePrecedesPos) {
-			this.search(
-				startPos,
-				UniversalSequenceNumber,
-				clientId,
-				{ leaf: recordTileStart, shift: tileShift },
-				searchInfo,
-			);
-		} else {
-			this.backwardSearch(
-				startPos,
-				UniversalSequenceNumber,
-				clientId,
-				{ leaf: recordTileStart, shift: tileShift },
-				searchInfo,
-			);
-		}
-
-		if (searchInfo.tile) {
-			let pos: number;
-			if (searchInfo.tile.isLeaf()) {
-				const marker = searchInfo.tile as Marker;
-				pos = this.getPosition(marker, UniversalSequenceNumber, clientId);
-			} else {
-				const localRef = searchInfo.tile;
-				pos = this.referencePositionToLocalPosition(
-					localRef,
-					UniversalSequenceNumber,
-					clientId,
-				);
-			}
-			return { tile: searchInfo.tile, pos };
-		}
-
-		return undefined;
-	}
-
 	/**
 	 * Finds the nearest reference with ReferenceType.Tile to `startPos` in the direction dictated by `forwards`.
 	 * Uses depthFirstNodeWalk in addition to block-accelerated functionality. The search position will be included in
@@ -1246,121 +1135,6 @@ export class MergeTree {
 		return foundMarker;
 	}
 
-	private search<TClientData>(
-		pos: number,
-		refSeq: number,
-		clientId: number,
-		actions: SegmentActions<TClientData> | undefined,
-		clientData: TClientData,
-	): ISegment | undefined {
-		return this.searchBlock(this.root, pos, 0, refSeq, clientId, actions, clientData);
-	}
-
-	private searchBlock<TClientData>(
-		block: IMergeBlock,
-		pos: number,
-		segpos: number,
-		refSeq: number,
-		clientId: number,
-		actions: SegmentActions<TClientData> | undefined,
-		clientData: TClientData,
-		localSeq?: number,
-	): ISegment | undefined {
-		const { pre, post, contains, leaf, shift } = actions ?? {};
-		let _pos = pos;
-		let _segpos = segpos;
-		const children = block.children;
-		pre?.(block, _segpos, refSeq, clientId, undefined, undefined, clientData);
-		for (let childIndex = 0; childIndex < block.childCount; childIndex++) {
-			const child = children[childIndex];
-			const len = this.nodeLength(child, refSeq, clientId, localSeq) ?? 0;
-			if (
-				(!contains && _pos < len) ||
-				contains?.(child, _pos, refSeq, clientId, undefined, undefined, clientData)
-			) {
-				// Found entry containing pos
-				if (!child.isLeaf()) {
-					return this.searchBlock(
-						child,
-						_pos,
-						_segpos,
-						refSeq,
-						clientId,
-						actions,
-						clientData,
-						localSeq,
-					);
-				} else {
-					leaf?.(child, _segpos, refSeq, clientId, _pos, -1, clientData);
-					return child;
-				}
-			} else {
-				shift?.(child, _segpos, refSeq, clientId, _pos, undefined, clientData);
-				_pos -= len;
-				_segpos += len;
-			}
-		}
-		post?.(block, _segpos, refSeq, clientId, undefined, undefined, clientData);
-	}
-
-	private backwardSearch<TClientData>(
-		pos: number,
-		refSeq: number,
-		clientId: number,
-		actions: SegmentActions<TClientData> | undefined,
-		clientData: TClientData,
-	): ISegment | undefined {
-		const len = this.getLength(refSeq, clientId);
-		if (pos > len) {
-			return undefined;
-		}
-		return this.backwardSearchBlock(this.root, pos, len, refSeq, clientId, actions, clientData);
-	}
-
-	private backwardSearchBlock<TClientData>(
-		block: IMergeBlock,
-		pos: number,
-		segEnd: number,
-		refSeq: number,
-		clientId: number,
-		actions: SegmentActions<TClientData> | undefined,
-		clientData: TClientData,
-	): ISegment | undefined {
-		const { pre, post, contains, leaf, shift } = actions ?? {};
-		let _segEnd = segEnd;
-		const children = block.children;
-		pre?.(block, _segEnd, refSeq, clientId, undefined, undefined, clientData);
-		for (let childIndex = block.childCount - 1; childIndex >= 0; childIndex--) {
-			const child = children[childIndex];
-			const len = this.nodeLength(child, refSeq, clientId) ?? 0;
-			const segpos = _segEnd - len;
-			if (
-				(!contains && pos >= segpos) ||
-				contains?.(child, pos, refSeq, clientId, undefined, undefined, clientData)
-			) {
-				// Found entry containing pos
-				if (!child.isLeaf()) {
-					return this.backwardSearchBlock(
-						child,
-						pos,
-						_segEnd,
-						refSeq,
-						clientId,
-						actions,
-						clientData,
-					);
-				} else {
-					leaf?.(child, segpos, refSeq, clientId, pos, -1, clientData);
-					return child;
-				}
-			} else {
-				shift?.(child, segpos, refSeq, clientId, pos, undefined, clientData);
-				_segEnd = segpos;
-			}
-		}
-		post?.(block, _segEnd, refSeq, clientId, undefined, undefined, clientData);
-	}
-
 	private updateRoot(splitNode: IMergeBlock | undefined) {
 		if (splitNode !== undefined) {
 			const newRoot = this.makeBlock(2);

diff --git a/packages/dds/merge-tree/src/ops.ts b/packages/dds/merge-tree/src/ops.ts
@@ -10,7 +10,7 @@
 export enum ReferenceType {
 	Simple = 0x0,
 	/**
-	 * Allows this reference to be located using the `findTile` API on merge-tree.
+	 * Allows this reference to be located using the `searchForMarker` API on merge-tree.
 	 */
 	Tile = 0x1,
 

diff --git a/packages/dds/merge-tree/src/test/client.annotateMarker.spec.ts b/packages/dds/merge-tree/src/test/client.annotateMarker.spec.ts
@@ -0,0 +1,46 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { strict as assert } from "assert";
+import { UniversalSequenceNumber } from "../constants";
+import { Marker, reservedMarkerIdKey } from "../mergeTreeNodes";
+import { ReferenceType } from "../ops";
+import { TextSegment } from "../textSegment";
+import { TestClient } from "./testClient";
+import { insertSegments } from "./testUtils";
+
+describe("TestClient", () => {
+	const localUserLongId = "localUser";
+	let client: TestClient;
+
+	beforeEach(() => {
+		client = new TestClient();
+		insertSegments({
+			mergeTree: client.mergeTree,
+			pos: 0,
+			segments: [TextSegment.make("")],
+			refSeq: UniversalSequenceNumber,
+			clientId: client.getClientId(),
+			seq: UniversalSequenceNumber,
+			opArgs: undefined,
+		});
+		client.startOrUpdateCollaboration(localUserLongId);
+	});
+
+	describe(".annotateMarker", () => {
+		it("annotate valid marker", () => {
+			const insertOp = client.insertMarkerLocal(0, ReferenceType.Tile, {
+				[reservedMarkerIdKey]: "123",
+			});
+			assert(insertOp);
+			const markerInfo = client.getContainingSegment(0);
+			const marker = markerInfo.segment as Marker;
+			const annotateOp = client.annotateMarker(marker, { foo: "bar" });
+			assert(annotateOp);
+			assert(marker.properties);
+			assert(marker.properties.foo, "bar");
+		});
+	});
+});