Merge branch 'main' of https://github.com/microsoft/FluidFramework in…

…to merge-tree-ack-event

common/lib/common-utils/src/bufferBrowser.ts

@@ -48,6 +48,11 @@ export const bufferToString = (blob: ArrayBufferLike, encoding: string): string
  * Minimal implementation of Buffer for our usages in the browser environment.
  */
 export class IsoBuffer extends Uint8Array {
+    // Need to have ctor for it to be in proto chain for instanceof check in from() method to work
+    public constructor(buffer: ArrayBufferLike, byteOffset?: number, length?: number) {
+        super(buffer, byteOffset, length);
+    }
+
     /**
      * Convert the buffer to a string.
      * Only supports encoding the whole string (unlike the Node Buffer equivalent)
@@ -66,6 +71,8 @@ export class IsoBuffer extends Uint8Array {
     static from(value, encodingOrOffset?, length?): IsoBuffer {
         if (typeof value === "string") {
             return IsoBuffer.fromString(value, encodingOrOffset as string | undefined);
+        } else if (value instanceof IsoBuffer) {
+            return value;
         } else if (value instanceof ArrayBuffer) {
             return IsoBuffer.fromArrayBuffer(value, encodingOrOffset as number | undefined, length);
         } else {

common/lib/common-utils/src/test/jest/buffer.spec.ts

@@ -171,4 +171,16 @@ describe("Buffer isomorphism", () => {
         expect(nodeStringUtf8).toEqual("hellothere");
         expect(browserStringUtf8).toEqual("hellothere");
     });
+
+    test("bufferToString working with IsoBuffer",() => {
+        const test = "aGVsbG90aGVyZQ==";
+
+        const buffer = BufferBrowser.IsoBuffer.from(test, "base64");
+        expect(BufferBrowser.bufferToString(buffer, "base64")).toEqual(test);
+        expect(BufferBrowser.bufferToString(buffer, "utf-8")).toEqual("hellothere");
+
+        const buffer2 = BufferNode.IsoBuffer.from(test, "base64");
+        expect(BufferNode.bufferToString(buffer2, "base64")).toEqual(test);
+        expect(BufferNode.bufferToString(buffer2, "utf-8")).toEqual("hellothere");
+    });
 });

experimental/dds/tree/README.md

@@ -18,15 +18,6 @@ The order of the edits is:
 1. All acknowledged edits, in the order agreed upon by Fluid's consensus.
 2. All local edits (not acknowledged by Fluid yet), in the order they were created.
 
-## Maintenance
-
-This package is currently being maintained in an internal Microsoft repository as well as in the Fluid Framework repository.
-As a result, there are some inconsistencies in development tooling configuration between this package and a typical Fluid package.
-The main difference is that code is formatted using `prettier`.
-Correct formatting can still be produced using the `lint:fix` script and is enforced with the `lint` script.
-For the most part, this means contributers can ignore these style differences.
-However, this dual maintenance should be kept in mind when considering changes to tooling configuration.
-
 # Getting Started
 
 ## Tree Abstraction

experimental/dds/tree/api-extractor.json

@@ -1,4 +1,15 @@
+/**
+ * Config file for API Extractor. For more info, please visit: https://api-extractor.com
+ */
 {
 	"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
-	"extends": "@fluidframework/build-common/api-extractor-common.json"
+	"extends": "@fluidframework/build-common/api-extractor-common.json",
+	/**
+	 * For now, retain explicit report generation in the package folder.
+	 */
+	"apiReport": {
+		"enabled": true,
+		"reportFolder": "<projectFolder>",
+		"reportTempFolder": "<projectFolder>/_api-extractor-temp"
+	}
 }

experimental/dds/tree/src/Common.ts

@@ -25,7 +25,6 @@ class SharedTreeAssertionError extends Error {
 	public constructor(message: string) {
 		super(message);
 		this.name = 'Assertion error';
-
 		Error.captureStackTrace?.(this);
 	}
 }
@@ -81,6 +80,32 @@ export function assertArrayOfOne<T>(array: readonly T[], message = 'array value
 	return array[0];
 }
 
+/**
+ * Redefine a property to have the given value. This is simply a type-safe wrapper around
+ * `Object.defineProperty`, but it is useful for caching getters on first read.
+ * @example
+ * ```
+ * // `randomOnce()` will return a random number, but always the same random number.
+ * {
+ *   get randomOnce(): number {
+ *     return memoizeGetter(this, 'randomOnce', random(100))
+ *   }
+ * }
+ * ```
+ * @param object - the object containing the property
+ * @param propName - the name of the property on the object
+ * @param value - the value of the property
+ */
+export function memoizeGetter<T, K extends keyof T>(object: T, propName: K, value: T[K]): T[K] {
+	Object.defineProperty(object, propName, {
+		value,
+		enumerable: true,
+		configurable: true,
+	});
+
+	return value;
+}
+
 /**
  * Iterate through two iterables and return true if they yield equivalent elements in the same order.
  * @param iterableA - the first iterable to compare

experimental/dds/tree/src/EditLog.ts

@@ -52,6 +52,14 @@ interface LocalOrderedEdit {
 
 type OrderedEdit = SequencedOrderedEdit | LocalOrderedEdit;
 
+/**
+ * Event fired when an edit is added to an `EditLog`.
+ * @param edit - The edit that was added to the log
+ * @param isLocal - true iff this edit was generated locally
+ * @internal
+ */
+export type EditAddedHandler = (edit: Edit, isLocal: boolean) => void;
+
 /**
  * The edit history log for SharedTree.
  * Contains only completed edits (no in-progress edits).
@@ -61,10 +69,10 @@ type OrderedEdit = SequencedOrderedEdit | LocalOrderedEdit;
  */
 export class EditLog implements OrderedEditSet {
 	private localEditSequence = 0;
-	private version = 0;
 	private readonly sequencedEdits: Edit[] = [];
 	private readonly localEdits: Edit[] = [];
 	private readonly allEdits: Map<EditId, OrderedEdit> = new Map();
+	private readonly editAddedHandlers: EditAddedHandler[] = [];
 
 	/**
 	 * Construct an `EditLog` with the given sequenced `Edits`
@@ -77,10 +85,10 @@ export class EditLog implements OrderedEditSet {
 	}
 
 	/**
-	 * Get a value which can be compared with === to determine if a log has not changed.
+	 * Registers a handler for when an edit is added to this `EditLog`.
 	 */
-	public versionIdentifier(): unknown {
-		return this.version;
+	public registerEditAddedHandler(handler: EditAddedHandler): void {
+		this.editAddedHandlers.push(handler);
 	}
 
 	/**
@@ -153,7 +161,6 @@ export class EditLog implements OrderedEditSet {
 	 * If the id of the supplied edit matches a local edit already present in the log, the local edit will be replaced.
 	 */
 	public addSequencedEdit(edit: Edit): void {
-		this.version++;
 		const sequencedEdit: SequencedOrderedEdit = { edit, index: this.sequencedEdits.length, isLocal: false };
 		this.sequencedEdits.push(edit);
 		const existingEdit = this.allEdits.get(edit.id);
@@ -166,18 +173,25 @@ export class EditLog implements OrderedEditSet {
 		}
 
 		this.allEdits.set(edit.id, sequencedEdit);
+		this.emitAdd(edit, false);
 	}
 
 	/**
 	 * Adds a non-sequenced (local) edit to the edit log.
 	 * Duplicate edits are ignored.
 	 */
 	public addLocalEdit(edit: Edit): void {
-		this.version++;
 		assert(!this.allEdits.has(edit.id));
 		const localEdit: LocalOrderedEdit = { edit, localSequence: this.localEditSequence++, isLocal: true };
 		this.localEdits.push(edit);
 		this.allEdits.set(edit.id, localEdit);
+		this.emitAdd(edit, true);
+	}
+
+	private emitAdd(editAdded: Edit, isLocal: boolean): void {
+		for (const handler of this.editAddedHandlers) {
+			handler(editAdded, isLocal);
+		}
 	}
 
 	/**

experimental/dds/tree/src/LogViewer.ts

@@ -4,10 +4,11 @@
  */
 
 import BTree from 'sorted-btree';
+import { ITelemetryBaseLogger } from '@fluidframework/common-definitions';
 import { assert, fail, noop } from './Common';
 import { EditLog } from './EditLog';
 import { Snapshot } from './Snapshot';
-import { EditResult } from './PersistedTypes';
+import { Edit, EditResult } from './PersistedTypes';
 import { EditId } from './Identifiers';
 import { Transaction } from './Transaction';
 import { initialTree } from './InitialTree';
@@ -17,7 +18,6 @@ import { initialTree } from './InitialTree';
  * Note that edits may be applied multiple times (and with different results due to concurrent edits),
  * and might not be applied when added.
  * This callback cannot be used to simply log each edit as it comes it to see its status.
- * @internal
  */
 export type EditResultCallback = (editResult: EditResult, editId: EditId) => void;
 
@@ -64,16 +64,11 @@ export class CachingLogViewer implements LogViewer {
 	private readonly sequencedSnapshotCache = new BTree<number, Snapshot>();
 
 	/**
-	 * The value of log.versionIdentifier when lastHeadSnapshot was cached.
-	 */
-	private lastVersionIdentifier: unknown = undefined;
-
-	/**
-	 * A cached Snapshot for Head (the newest revision) of log when it was at lastVersionIdentifier.
+	 * A cached Snapshot for Head (the newest revision) of `log`. It is undefined when not computed or invalidated by a change in the log.
 	 * This cache is important as the Head revision is frequently viewed, and just using the sequencedSnapshotCache
 	 * would not cache processing of local edits in this case.
 	 */
-	private lastHeadSnapshot: Snapshot;
+	private lastHeadSnapshot?: Snapshot;
 
 	/**
 	 * Called whenever an edit is processed.
@@ -87,6 +82,18 @@ export class CachingLogViewer implements LogViewer {
 	 */
 	private readonly expensiveValidation: boolean;
 
+	/**
+	 * Telemetry logger, used to log events such as edit application rejection.
+	 */
+	private readonly logger: ITelemetryBaseLogger;
+
+	/**
+	 * The ordered list of edits that originated from this client that have never been applied (by this log viewer) in a sequenced state.
+	 * This means these edits may be local or sequenced, and may have been applied (possibly multiple times) while still local.
+	 * Used to log telemetry about the result of edit application. Edits are removed when first applied after being sequenced.
+	 */
+	private readonly unappliedSelfEdits: EditId[] = [];
+
 	/**
 	 * Create a new LogViewer
 	 * @param log - the edit log which snapshots will be based on.
@@ -97,22 +104,29 @@ export class CachingLogViewer implements LogViewer {
 		log: EditLog,
 		baseTree = initialTree,
 		expensiveValidation = false,
-		processEditResult: EditResultCallback = noop
+		processEditResult: EditResultCallback = noop,
+		logger: ITelemetryBaseLogger
 	) {
 		this.log = log;
 		const initialSnapshot = Snapshot.fromTree(baseTree);
-		this.lastHeadSnapshot = initialSnapshot;
 		this.sequencedSnapshotCache.set(0, initialSnapshot);
 		this.processEditResult = processEditResult ?? noop;
 		this.expensiveValidation = expensiveValidation;
+		this.logger = logger;
+		this.log.registerEditAddedHandler(this.handleEditAdded.bind(this));
+	}
+
+	private handleEditAdded(edit: Edit, isLocal: boolean): void {
+		this.lastHeadSnapshot = undefined; // Invalidate HEAD snapshot cache.
+		if (isLocal) {
+			this.unappliedSelfEdits.push(edit.id);
+		}
 	}
 
 	public getSnapshot(revision: number): Snapshot {
 		// Per the documentation for this method, the returned snapshot should be the output of the edit at the largest index <= `revision`.
-		if (revision >= this.log.length) {
-			if (this.lastVersionIdentifier === this.log.versionIdentifier()) {
-				return this.lastHeadSnapshot;
-			}
+		if (revision >= this.log.length && this.lastHeadSnapshot) {
+			return this.lastHeadSnapshot;
 		}
 
 		const [startRevision, startSnapshot] =
@@ -130,15 +144,33 @@ export class CachingLogViewer implements LogViewer {
 			// This avoids having to invalidate cache entries when concurrent edits cause local revision
 			// numbers to change when acknowledged.
 			if (i < this.log.numberOfSequencedEdits) {
-				// Revision is the result of the edit being applied.
-				this.sequencedSnapshotCache.set(/* revision: */ i + 1, currentSnapshot);
+				const revision = i + 1; // Revision is the result of the edit being applied.
+				this.sequencedSnapshotCache.set(revision, currentSnapshot);
+
+				// This is the first time this sequenced edit has been processed by this LogViewer. If it was a local edit, log telemetry
+				// in the event that it was invalid or malformed.
+				if (this.unappliedSelfEdits.length > 0) {
+					if (edit.id === this.unappliedSelfEdits[0]) {
+						if (editingResult.result !== EditResult.Applied) {
+							this.logger.send({
+								category: 'generic',
+								eventName:
+									editingResult.result === EditResult.Malformed
+										? 'MalformedSharedTreeEdit'
+										: 'InvalidSharedTreeEdit',
+							});
+						}
+						this.unappliedSelfEdits.shift();
+					} else if (this.expensiveValidation) {
+						assert(this.unappliedSelfEdits.indexOf(edit.id) < 0, 'Local edits processed out of order.');
+					}
+				}
 			}
 
 			this.processEditResult(editingResult.result, edit.id);
 		}
 
 		if (revision >= this.log.length) {
-			this.lastVersionIdentifier = this.log.versionIdentifier();
 			this.lastHeadSnapshot = currentSnapshot;
 		}
 

experimental/dds/tree/src/PersistedTypes.ts

@@ -224,15 +224,15 @@ export enum ConstraintEffect {
  * Values are the content of the trait specified by the key.
  * @public
  */
-export interface TraitMap<TChild = ChangeNode> {
+export interface TraitMap<TChild> {
 	readonly [key: string]: TreeNodeSequence<TChild>;
 }
 
 /**
  * A sequence of Nodes that make up a trait under a Node
  * @public
  */
-export type TreeNodeSequence<TChild = ChangeNode> = readonly TChild[];
+export type TreeNodeSequence<TChild> = readonly TChild[];
 
 /**
  * Valid if (transitively) all DetachedSequenceId are used according to their rules (use here counts as a destination),

experimental/dds/tree/src/SharedTree.ts

@@ -9,6 +9,8 @@ import { IFluidDataStoreRuntime, IChannelStorageService } from '@fluidframework/
 import { AttachState } from '@fluidframework/container-definitions';
 import { SharedObject } from '@fluidframework/shared-object-base';
 import { IFluidSerializer } from '@fluidframework/core-interfaces';
+import { ITelemetryLogger } from '@fluidframework/common-definitions';
+import { ChildLogger } from '@fluidframework/telemetry-utils';
 import { assert, fail } from './Common';
 import { EditLog, OrderedEditSet } from './EditLog';
 import {
@@ -207,6 +209,8 @@ export class SharedTree extends SharedObject {
 	 */
 	public payloadCache: Map<BlobId, Payload> = new Map();
 
+	protected readonly logger: ITelemetryLogger;
+
 	/**
 	 * Iff true, the snapshots passed to setKnownRevision will be asserted to be correct.
 	 */
@@ -221,7 +225,8 @@ export class SharedTree extends SharedObject {
 	public constructor(runtime: IFluidDataStoreRuntime, id: string, expensiveValidation = false) {
 		super(id, runtime, SharedTreeFactory.Attributes);
 		this.expensiveValidation = expensiveValidation;
-		const { editLog, logViewer } = loadSummary(initialSummary, this.expensiveValidation);
+		this.logger = ChildLogger.create(super.logger, 'SharedTree');
+		const { editLog, logViewer } = loadSummary(initialSummary, this.expensiveValidation, this.logger);
 		this.editLog = editLog;
 		this.logViewer = logViewer;
 	}
@@ -330,7 +335,7 @@ export class SharedTree extends SharedObject {
 	 * @internal
 	 */
 	public loadSummary(summary: SharedTreeSummary): void {
-		const { editLog, logViewer } = loadSummary(summary, this.expensiveValidation);
+		const { editLog, logViewer } = loadSummary(summary, this.expensiveValidation, this.logger);
 		this.editLog = editLog;
 		this.logViewer = logViewer;
 	}
@@ -412,13 +417,14 @@ export class SharedTree extends SharedObject {
 
 function loadSummary(
 	summary: SharedTreeSummary,
-	expensiveValidation: boolean
+	expensiveValidation: boolean,
+	logger: ITelemetryLogger
 ): { editLog: EditLog; logViewer: LogViewer } {
 	const { version, sequencedEdits, currentTree } = summary;
 	assert(version === formatVersion);
 	const currentView = Snapshot.fromTree(currentTree);
 	const editLog = new EditLog(sequencedEdits);
-	const logViewer = new CachingLogViewer(editLog, initialTree, expensiveValidation);
+	const logViewer = new CachingLogViewer(editLog, initialTree, expensiveValidation, undefined, logger);
 
 	// TODO:#47830: Store the associated revision on the snapshot.
 	// The current view should only be stored in the cache if the revision it's associated with is known.