Add Clone to SharedTree Revertible (#23044)

#### Description [13864](https://dev.azure.com/fluidframework/internal/_workitems/edit/13864/) This PR adds forkable revertible feature to the `Revertible` object of SharedTree. - Removed `DisposableRevertible` and replaced by `RevertibleAlpha`. - Added `clone()` method to the new interface. - Uses `TreeBranch` (which is subset of `TreeCheckout`) to access data necessary for revert operation. --------- Co-authored-by: Noah Encke <[email protected]> Co-authored-by: Joshua Smithrud <[email protected]> Co-authored-by: Jenn <[email protected]>
microsoft · Nov 20, 2024 · 5abfa01 · 5abfa01
1 parent d3bf90c
commit 5abfa01
Show file tree

Hide file tree

Showing 11 changed files with 398 additions and 68 deletions.
diff --git a/.changeset/dull-words-work.md b/.changeset/dull-words-work.md
@@ -0,0 +1,11 @@
+---
+"fluid-framework": minor
+"@fluidframework/tree": minor
+---
+---
+"section": tree
+---
+
+Enables Revertible objects to be cloned using `RevertibleAlpha.clone()`.
+
+Replaced `DisposableRevertible` with `RevertibleAlpha`. The new `RevertibleAlpha` interface extends `Revertible` and includes a `clone(branch: TreeBranch)` method to facilitate cloning a Revertible to a specified target branch. The source branch where the `RevertibleAlpha` was created must share revision logs with the target branch where the `RevertibleAlpha` is being cloned. If this condition is not met, the operation will throw an error.
diff --git a/packages/dds/tree/api-report/tree.alpha.api.md b/packages/dds/tree/api-report/tree.alpha.api.md
@@ -539,6 +539,14 @@ export interface Revertible {
     readonly status: RevertibleStatus;
 }
 
+// @alpha @sealed
+export interface RevertibleAlpha extends Revertible {
+    clone: (branch: TreeBranch) => RevertibleAlpha;
+}
+
+// @alpha @sealed
+export type RevertibleAlphaFactory = (onRevertibleDisposed?: (revertible: RevertibleAlpha) => void) => RevertibleAlpha;
+
 // @public @sealed
 export type RevertibleFactory = (onRevertibleDisposed?: (revertible: Revertible) => void) => Revertible;
 
@@ -723,8 +731,8 @@ export interface TreeBranch extends IDisposable {
 
 // @alpha @sealed
 export interface TreeBranchEvents {
-    changed(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
-    commitApplied(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
+    changed(data: CommitMetadata, getRevertible?: RevertibleAlphaFactory): void;
+    commitApplied(data: CommitMetadata, getRevertible?: RevertibleAlphaFactory): void;
     schemaChanged(): void;
 }
 

diff --git a/packages/dds/tree/src/core/index.ts b/packages/dds/tree/src/core/index.ts
@@ -206,4 +206,10 @@ export {
 	AllowedUpdateType,
 } from "./schema-view/index.js";
 
-export { type Revertible, RevertibleStatus, type RevertibleFactory } from "./revertible.js";
+export {
+	type Revertible,
+	RevertibleStatus,
+	type RevertibleFactory,
+	type RevertibleAlphaFactory,
+	type RevertibleAlpha,
+} from "./revertible.js";
diff --git a/packages/dds/tree/src/core/revertible.ts b/packages/dds/tree/src/core/revertible.ts
@@ -3,6 +3,8 @@
  * Licensed under the MIT License.
  */
 
+import type { TreeBranch } from "../simple-tree/index.js";
+
 /**
  * Allows reversion of a change made to SharedTree.
  *
@@ -36,6 +38,23 @@ export interface Revertible {
 	dispose(): void;
 }
 
+/**
+ * A {@link Revertible} with features that are not yet stable.
+ *
+ * @sealed @alpha
+ */
+export interface RevertibleAlpha extends Revertible {
+	/**
+	 * Clones the {@link Revertible} to a target branch.
+	 *
+	 * @param branch - A target branch to apply the revertible to.
+	 * The target branch must contain the same commit that this revertible is meant to revert, otherwise will throw an error.
+	 * @returns A cloned revertible is independent of the original, meaning disposing of one will not affect the other,
+	 * provided they do not belong to the same branch. Both revertibles can be reverted independently.
+	 */
+	clone: (branch: TreeBranch) => RevertibleAlpha;
+}
+
 /**
  * The status of a {@link Revertible}.
  *
@@ -50,7 +69,7 @@ export enum RevertibleStatus {
 
 /**
  * Factory for creating a {@link Revertible}.
- * Will error if invoked outside the scope of the `changed` event that provides it, or if invoked multiple times.
+ * @throws error if invoked outside the scope of the `changed` event that provides it, or if invoked multiple times.
  *
  * @param onRevertibleDisposed - A callback that will be invoked when the `Revertible` generated by this factory is disposed.
  * This happens when the `Revertible` is disposed manually, or when the `TreeView` that the `Revertible` belongs to is disposed,
@@ -62,3 +81,18 @@ export enum RevertibleStatus {
 export type RevertibleFactory = (
 	onRevertibleDisposed?: (revertible: Revertible) => void,
 ) => Revertible;
+
+/**
+ * Factory for creating a {@link RevertibleAlpha}.
+ * @throws error if invoked outside the scope of the `changed` event that provides it, or if invoked multiple times.
+ *
+ * @param onRevertibleDisposed - A callback that will be invoked when the {@link RevertibleAlpha | Revertible} generated by this factory is disposed.
+ * This happens when the `Revertible` is disposed manually, or when the `TreeView` that the `Revertible` belongs to is disposed,
+ * whichever happens first.
+ * This is typically used to clean up any resources associated with the `Revertible` in the host application.
+ *
+ * @sealed @alpha
+ */
+export type RevertibleAlphaFactory = (
+	onRevertibleDisposed?: (revertible: RevertibleAlpha) => void,
+) => RevertibleAlpha;
diff --git a/packages/dds/tree/src/index.ts b/packages/dds/tree/src/index.ts
@@ -29,6 +29,8 @@ export {
 	MapNodeStoredSchema,
 	LeafNodeStoredSchema,
 	type RevertibleFactory,
+	type RevertibleAlphaFactory,
+	type RevertibleAlpha,
 } from "./core/index.js";
 export { type Brand } from "./util/index.js";
 

diff --git a/packages/dds/tree/src/shared-tree/schematizingTreeView.ts b/packages/dds/tree/src/shared-tree/schematizingTreeView.ts
@@ -409,7 +409,7 @@ export class SchematizingSimpleTreeView<
  * @remarks Currently, all contexts are also {@link SchematizingSimpleTreeView}s.
  * Other checkout implementations (e.g. not associated with a view) may be supported in the future.
  */
-function getCheckout(context: TreeBranch): TreeCheckout {
+export function getCheckout(context: TreeBranch): TreeCheckout {
 	if (context instanceof SchematizingSimpleTreeView) {
 		return context.checkout;
 	}

diff --git a/packages/dds/tree/src/shared-tree/treeCheckout.ts b/packages/dds/tree/src/shared-tree/treeCheckout.ts
@@ -24,7 +24,6 @@ import {
 	type IEditableForest,
 	type IForestSubscription,
 	type JsonableTree,
-	type Revertible,
 	RevertibleStatus,
 	type RevisionTag,
 	type RevisionTagCodec,
@@ -37,7 +36,8 @@ import {
 	rootFieldKey,
 	tagChange,
 	visitDelta,
-	type RevertibleFactory,
+	type RevertibleAlphaFactory,
+	type RevertibleAlpha,
 } from "../core/index.js";
 import {
 	type HasListeners,
@@ -80,8 +80,9 @@ import type {
 	TreeViewConfiguration,
 	UnsafeUnknownSchema,
 	ViewableTree,
+	TreeBranch,
 } from "../simple-tree/index.js";
-import { SchematizingSimpleTreeView } from "./schematizingTreeView.js";
+import { getCheckout, SchematizingSimpleTreeView } from "./schematizingTreeView.js";
 
 /**
  * Events for {@link ITreeCheckout}.
@@ -104,7 +105,7 @@ export interface CheckoutEvents {
 	 * @param getRevertible - a function provided that allows users to get a revertible for the change. If not provided,
 	 * this change is not revertible.
 	 */
-	changed(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
+	changed(data: CommitMetadata, getRevertible?: RevertibleAlphaFactory): void;
 }
 
 /**
@@ -409,7 +410,7 @@ export class TreeCheckout implements ITreeCheckoutFork {
 	/**
 	 * Set of revertibles maintained for automatic disposal
 	 */
-	private readonly revertibles = new Set<DisposableRevertible>();
+	private readonly revertibles = new Set<RevertibleAlpha>();
 
 	/**
 	 * Each branch's head commit corresponds to a revertible commit.
@@ -542,7 +543,7 @@ export class TreeCheckout implements ITreeCheckoutFork {
 
 						const getRevertible = hasSchemaChange(change)
 							? undefined
-							: (onRevertibleDisposed?: (revertible: Revertible) => void) => {
+							: (onRevertibleDisposed?: (revertible: RevertibleAlpha) => void) => {
 									if (!withinEventContext) {
 										throw new UsageError(
 											"Cannot get a revertible outside of the context of a changed event.",
@@ -553,42 +554,12 @@ export class TreeCheckout implements ITreeCheckoutFork {
 											"Cannot generate the same revertible more than once. Note that this can happen when multiple changed event listeners are registered.",
 										);
 									}
-									const revertibleCommits = this.revertibleCommitBranches;
-									const revertible: DisposableRevertible = {
-										get status(): RevertibleStatus {
-											const revertibleCommit = revertibleCommits.get(revision);
-											return revertibleCommit === undefined
-												? RevertibleStatus.Disposed
-												: RevertibleStatus.Valid;
-										},
-										revert: (release: boolean = true) => {
-											if (revertible.status === RevertibleStatus.Disposed) {
-												throw new UsageError(
-													"Unable to revert a revertible that has been disposed.",
-												);
-											}
-
-											const revertMetrics = this.revertRevertible(revision, kind);
-											this.logger?.sendTelemetryEvent({
-												eventName: TreeCheckout.revertTelemetryEventName,
-												...revertMetrics,
-											});
-
-											if (release) {
-												revertible.dispose();
-											}
-										},
-										dispose: () => {
-											if (revertible.status === RevertibleStatus.Disposed) {
-												throw new UsageError(
-													"Unable to dispose a revertible that has already been disposed.",
-												);
-											}
-											this.disposeRevertible(revertible, revision);
-											onRevertibleDisposed?.(revertible);
-										},
-									};
-
+									const revertible = this.createRevertible(
+										revision,
+										kind,
+										this,
+										onRevertibleDisposed,
+									);
 									this.revertibleCommitBranches.set(revision, _branch.fork(commit));
 									this.revertibles.add(revertible);
 									return revertible;
@@ -643,6 +614,76 @@ export class TreeCheckout implements ITreeCheckoutFork {
 		}
 	}
 
+	/**
+	 * Creates a {@link RevertibleAlpha} object that can undo a specific change in the tree's history.
+	 * Revision must exist in the given {@link TreeCheckout}'s branch.
+	 *
+	 * @param revision - The revision tag identifying the change to be made revertible.
+	 * @param kind - The {@link CommitKind} that produced this revertible (e.g., Default, Undo, Redo).
+	 * @param checkout - The {@link TreeCheckout} instance this revertible belongs to.
+	 * @param onRevertibleDisposed - Callback function that will be called when the revertible is disposed.
+	 * @returns - {@link RevertibleAlpha}
+	 */
+	private createRevertible(
+		revision: RevisionTag,
+		kind: CommitKind,
+		checkout: TreeCheckout,
+		onRevertibleDisposed: ((revertible: RevertibleAlpha) => void) | undefined,
+	): RevertibleAlpha {
+		const commitBranches = checkout.revertibleCommitBranches;
+
+		const revertible: RevertibleAlpha = {
+			get status(): RevertibleStatus {
+				const revertibleCommit = commitBranches.get(revision);
+				return revertibleCommit === undefined
+					? RevertibleStatus.Disposed
+					: RevertibleStatus.Valid;
+			},
+			revert: (release: boolean = true) => {
+				if (revertible.status === RevertibleStatus.Disposed) {
+					throw new UsageError("Unable to revert a revertible that has been disposed.");
+				}
+
+				const revertMetrics = checkout.revertRevertible(revision, kind);
+				checkout.logger?.sendTelemetryEvent({
+					eventName: TreeCheckout.revertTelemetryEventName,
+					...revertMetrics,
+				});
+
+				if (release) {
+					revertible.dispose();
+				}
+			},
+			clone: (forkedBranch: TreeBranch) => {
+				if (forkedBranch === undefined) {
+					return this.createRevertible(revision, kind, checkout, onRevertibleDisposed);
+				}
+
+				// TODO:#23442: When a revertible is cloned for a forked branch, optimize to create a fork of a revertible branch once per revision NOT once per revision per checkout.
+				const forkedCheckout = getCheckout(forkedBranch);
+				const revertibleBranch = this.revertibleCommitBranches.get(revision);
+				assert(
+					revertibleBranch !== undefined,
+					"change to revert does not exist on the given forked branch",
+				);
+				forkedCheckout.revertibleCommitBranches.set(revision, revertibleBranch.fork());
+
+				return this.createRevertible(revision, kind, forkedCheckout, onRevertibleDisposed);
+			},
+			dispose: () => {
+				if (revertible.status === RevertibleStatus.Disposed) {
+					throw new UsageError(
+						"Unable to dispose a revertible that has already been disposed.",
+					);
+				}
+				checkout.disposeRevertible(revertible, revision);
+				onRevertibleDisposed?.(revertible);
+			},
+		};
+
+		return revertible;
+	}
+
 	// For the new TreeViewAlpha API
 	public viewWith<TRoot extends ImplicitFieldSchema | UnsafeUnknownSchema>(
 		config: TreeViewConfiguration<ReadSchema<TRoot>>,
@@ -809,7 +850,7 @@ export class TreeCheckout implements ITreeCheckoutFork {
 		}
 	}
 
-	private disposeRevertible(revertible: DisposableRevertible, revision: RevisionTag): void {
+	private disposeRevertible(revertible: RevertibleAlpha, revision: RevisionTag): void {
 		this.revertibleCommitBranches.get(revision)?.dispose();
 		this.revertibleCommitBranches.delete(revision);
 		this.revertibles.delete(revertible);
@@ -922,7 +963,3 @@ export function runSynchronous(
 		? view.transaction.abort()
 		: view.transaction.commit();
 }
-
-interface DisposableRevertible extends Revertible {
-	dispose: () => void;
-}
diff --git a/packages/dds/tree/src/simple-tree/api/tree.ts b/packages/dds/tree/src/simple-tree/api/tree.ts
@@ -6,7 +6,11 @@
 import type { IFluidLoadable, IDisposable } from "@fluidframework/core-interfaces";
 import { UsageError } from "@fluidframework/telemetry-utils/internal";
 
-import type { CommitMetadata, RevertibleFactory } from "../../core/index.js";
+import type {
+	CommitMetadata,
+	RevertibleAlphaFactory,
+	RevertibleFactory,
+} from "../../core/index.js";
 import type { Listenable } from "../../events/index.js";
 
 import {
@@ -629,7 +633,7 @@ export interface TreeBranchEvents {
 	 * @param getRevertible - a function that allows users to get a revertible for the change. If not provided,
 	 * this change is not revertible.
 	 */
-	changed(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
+	changed(data: CommitMetadata, getRevertible?: RevertibleAlphaFactory): void;
 
 	/**
 	 * Fired when:
@@ -644,7 +648,7 @@ export interface TreeBranchEvents {
 	 * @param getRevertible - a function provided that allows users to get a revertible for the commit that was applied. If not provided,
 	 * this commit is not revertible.
 	 */
-	commitApplied(data: CommitMetadata, getRevertible?: RevertibleFactory): void;
+	commitApplied(data: CommitMetadata, getRevertible?: RevertibleAlphaFactory): void;
 }
 
 /**