Generate identifiers for unhydrated nodes

.changeset/public-snakes-fetch.md

@@ -0,0 +1,19 @@
+---
+"@fluidframework/tree": minor
+"fluid-framework": minor
+"__section": tree
+---
+TreeNodes now implicitly generate identifiers on access instead of throwing
+
+Accessing a defaulted [identifier](https://fluidframework.com/docs/api/fluid-framework/schemafactory-class#identifier-property) on an [Unhydrated](https://fluidframework.com/docs/api/fluid-framework/unhydrated-typealias) `TreeNode` no longer throws a usage error.
+Instead a new UUID is allocated for the identifier and returned.
+These UUIDs will be more compressible than random ones, since they all come from a single sequence (starting with a random UUID).
+They will not be fully compressed like the identifiers generated after hydration that leverage the document's [IIdCompressor](https://fluidframework.com/docs/api/id-compressor/iidcompressor-interface).
+
+```typescript
+const factory = new SchemaFactory("test");
+class HasIdentifier extends schema.object("A", { id: factory.identifier }) {}
+// This used to throw an error:
+const id = new HasIdentifier({}).id;
+
+```

.vale/config/vocabularies/fluid/accept.txt

@@ -6,3 +6,5 @@ JavaScript
 SharedTree
 TypeScript
 namespace
+UUID
+UUIDs

packages/dds/tree/src/simple-tree/api/schemaFactory.ts

@@ -1072,10 +1072,12 @@ export class SchemaFactory<
 	 * - It is a UUID which will not collide with other generated UUIDs.
 	 *
 	 * - It is compressed to a space-efficient representation when stored in the document.
+	 * Reading the identifier before inserting the node into a tree prevents the identifier from being stored in its compressed form,
+	 * resulting in a larger storage footprint.
 	 *
 	 * - A compressed form of the identifier can be accessed at runtime via the {@link TreeNodeApi.shortId|Tree.shortId()} API.
 	 *
-	 * - It will error if read (and will not be present in the object's iterable properties) before the node has been inserted into a tree.
+	 * - It will not be present in the object's iterable properties until explicitly read or until having been inserted into a tree.
 	 *
 	 * However, a user may alternatively supply their own string as the identifier if desired (for example, if importing identifiers from another system).
 	 * In that case, if the user requires it to be unique, it is up to them to ensure uniqueness.

packages/dds/tree/src/simple-tree/api/treeNodeApi.ts

@@ -40,6 +40,7 @@ import {
 } from "../core/index.js";
 import { isObjectNodeSchema } from "../objectNodeTypes.js";
 import type { TreeChangeEvents } from "./treeChangeEvents.js";
+import { lazilyAllocateIdentifier } from "../objectNode.js";
 
 /**
  * Provides various functions for analyzing {@link TreeNode}s.
@@ -302,12 +303,11 @@ export function getIdentifierFromNode(
 		case 0:
 			return undefined;
 		case 1: {
-			const identifier = flexNode.tryGetField(identifierFieldKeys[0] ?? oob())?.boxedAt(0);
+			const key = identifierFieldKeys[0] ?? oob();
+			const identifier = flexNode.tryGetField(key)?.boxedAt(0);
 			if (flexNode instanceof UnhydratedFlexTreeNode) {
 				if (identifier === undefined) {
-					throw new UsageError(
-						"Tree.shortId cannot access default identifiers on unhydrated nodes",
-					);
+					return lazilyAllocateIdentifier(flexNode, key);
 				}
 				return identifier.value as string;
 			}

packages/dds/tree/src/simple-tree/objectNode.ts

@@ -5,6 +5,8 @@
 
 import { assert, Lazy, fail, debugAssert } from "@fluidframework/core-utils/internal";
 import { UsageError } from "@fluidframework/telemetry-utils/internal";
+import type { IIdCompressor } from "@fluidframework/id-compressor";
+import { createIdCompressor } from "@fluidframework/id-compressor/internal";
 
 import type { FieldKey, SchemaPolicy } from "../core/index.js";
 import {
@@ -14,7 +16,7 @@ import {
 	type FlexTreeOptionalField,
 	type FlexTreeRequiredField,
 } from "../feature-libraries/index.js";
-import type { RestrictiveStringRecord, FlattenKeys } from "../util/index.js";
+import { type RestrictiveStringRecord, type FlattenKeys, brand } from "../util/index.js";
 
 import {
 	type TreeNodeSchema,
@@ -57,6 +59,7 @@ import {
 import type { SimpleObjectFieldSchema } from "./simpleSchema.js";
 import { mapTreeFromNodeData, type InsertableContent } from "./toMapTree.js";
 import { TreeNodeValid, type MostDerivedData } from "./treeNodeValid.js";
+import { stringSchema } from "./leafNodeSchema.js";
 
 /**
  * Generates the properties for an ObjectNode from its field schema object.
@@ -198,6 +201,37 @@ function createFlexKeyMapping(fields: Record<string, ImplicitFieldSchema>): Simp
 	return keyMap;
 }
 
+const globalIdentifierAllocator: IIdCompressor = createIdCompressor();
+
+/**
+ * Modify `flexNode` to add a newly generated identifier under the given `storedKey`.
+ * @remarks
+ * This is used after checking if the user is trying to read an identifier field of an unhydrated node, but the identifier is not present.
+ * This means the identifier is an "auto-generated identifier", because otherwise it would have been supplied by the user at construction time and would have been successfully read just above.
+ * In this case, it is categorically impossible to provide an identifier (auto-generated identifiers can't be created until hydration/insertion time), so we emit an error.
+ * @privateRemarks
+ * TODO: this special case logic should move to the inner node (who's schema claims it has an identifier), rather than here, after we already read undefined out of a required field.
+ * TODO: unify this with a more general defaults mechanism.
+ */
+export function lazilyAllocateIdentifier(
+	flexNode: UnhydratedFlexTreeNode,
+	storedKey: FieldKey,
+): string {
+	debugAssert(() => !flexNode.mapTree.fields.has(storedKey) || "Identifier field already set");
+	const value = globalIdentifierAllocator.decompress(
+		globalIdentifierAllocator.generateCompressedId(),
+	);
+	flexNode.mapTree.fields.set(storedKey, [
+		{
+			type: brand(stringSchema.identifier),
+			value,
+			fields: new Map(),
+		},
+	]);
+
+	return value;
+}
+
 /**
  * Creates a proxy handler for the given schema.
  *
@@ -231,17 +265,11 @@ function createProxyHandler(
 					return getTreeNodeForField(field);
 				}
 
-				// TODO: this special case logic should move to the inner node (who's schema claims it has an identifier), rather than here, after we already read undefined out of a required field.
-				// Check if the user is trying to read an identifier field of an unhydrated node, but the identifier is not present.
-				// This means the identifier is an "auto-generated identifier", because otherwise it would have been supplied by the user at construction time and would have been successfully read just above.
-				// In this case, it is categorically impossible to provide an identifier (auto-generated identifiers can't be created until hydration/insertion time), so we emit an error.
 				if (
 					fieldInfo.schema.kind === FieldKind.Identifier &&
 					flexNode instanceof UnhydratedFlexTreeNode
 				) {
-					throw new UsageError(
-						"An automatically generated node identifier may not be queried until the node is inserted into the tree",
-					);
+					return lazilyAllocateIdentifier(flexNode, fieldInfo.storedKey);
 				}
 
 				return undefined;

packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts

@@ -331,14 +331,9 @@ describe("treeNodeApi", () => {
 				const node = new HasIdentifier({ identifier: "x" });
 				assert.equal(Tree.shortId(node), "x");
 			});
-			it("errors accessing defaulted", () => {
+			it("accessing defaulted", () => {
 				const node = new HasIdentifier({});
-				assert.throws(
-					() => {
-						Tree.shortId(node);
-					},
-					validateUsageError(/default/),
-				);
+				assert(typeof Tree.shortId(node) === "string");
 			});
 
 			// TODO: this policy seems questionable, but its whats implemented, and is documented in TreeStatus.new
@@ -440,17 +435,12 @@ describe("treeNodeApi", () => {
 		});
 
 		describe("unhydrated", () => {
-			it("errors accessing defaulted", () => {
+			it("accessing defaulted", () => {
 				class HasIdentifier extends schema.object("HasIdentifier", {
 					identifier: schema.identifier,
 				}) {}
 				const node = new HasIdentifier({});
-				assert.throws(
-					() => {
-						TreeAlpha.identifier(node);
-					},
-					validateUsageError(/default/),
-				);
+				assert(typeof TreeAlpha.identifier(node) === "string");
 			});
 		});
 
@@ -553,14 +543,9 @@ describe("treeNodeApi", () => {
 					const node = new HasIdentifier({ identifier: "x" });
 					assert.equal(TreeAlpha.identifier.getShort(node), undefined);
 				});
-				it("errors accessing defaulted", () => {
+				it("returns undefined accessing defaulted for unhydrated nodes", () => {
 					const node = new HasIdentifier({});
-					assert.throws(
-						() => {
-							TreeAlpha.identifier.getShort(node);
-						},
-						validateUsageError(/default/),
-					);
+					assert.equal(TreeAlpha.identifier.getShort(node), undefined);
 				});
 
 				// TODO: this policy seems questionable, but its whats implemented, and is documented in TreeStatus.new
@@ -1615,20 +1600,32 @@ describe("treeNodeApi", () => {
 			assert.deepEqual(a, { x: 1 });
 		});
 
-		it("object with defaulted identifier field", () => {
+		it("unhydrated object with defaulted read identifier field", () => {
 			const A = schema.object("A", { x: schema.identifier });
 			const node = TreeAlpha.create(A, { x: undefined });
 
 			// TODO: make this work instead of error:
-			// assert(isStableId(node.x));
-			// // Since no id compressor is associated with the node, Tree.shortId should give back a UUID string.
-			// assert.equal(Tree.shortId(node), node.x)
+			const id = node.x;
+			// Check allocated id is saved on node, and thus not regenerated on second access.
+			assert.equal(id, node.x);
+			// Id should be a valid UUID.
+			assert(isStableId(id));
+			// Since no id compressor is associated with the node, Tree.shortId should give back a UUID string.
+			assert.equal(Tree.shortId(node), node.x);
 
-			// For now validate the error is the correct one:
-			assert.throws(
-				() => node.x,
-				validateUsageError(/identifier may not be queried until the node is inserted/),
-			);
+			hydrate(A, node);
+
+			assert.equal(Tree.shortId(node), node.x);
+		});
+
+		it("hydrated object with defaulted unread identifier field", () => {
+			const A = schema.object("A", { x: schema.identifier });
+			const node = TreeAlpha.create(A, { x: undefined });
+
+			hydrate(A, node);
+			assert(isStableId(node.x));
+			const short = Tree.shortId(node);
+			assert.equal(typeof short, "number");
 		});
 
 		it("object with explicit identifier field", () => {

packages/dds/tree/src/test/simple-tree/objectNode.spec.ts

@@ -6,6 +6,7 @@
 import { strict as assert } from "node:assert";
 
 import { validateAssertionError } from "@fluidframework/test-runtime-utils/internal";
+import { isStableId } from "@fluidframework/id-compressor/internal";
 
 import {
 	SchemaFactory,
@@ -815,28 +816,20 @@ describeHydration(
 			});
 		});
 
-		it("unhydrated default identifier access errors", () => {
+		it("unhydrated default identifier access works", () => {
 			class HasId extends schemaFactory.object("hasID", { id: schemaFactory.identifier }) {}
 			const newNode = new HasId({});
-			assert.throws(
-				() => {
-					const id = newNode.id;
-				},
-				validateUsageError(/identifier/),
-			);
+			const id = newNode.id;
+			const id2 = new HasId({}).id;
+			assert.notEqual(id, id2);
 		});
 
-		it("unhydrated default identifier access via shortId errors", () => {
+		it("unhydrated default identifier access via shortId returns UUID", () => {
 			class HasId extends schemaFactory.object("hasID", { id: schemaFactory.identifier }) {}
 			const newNode = new HasId({});
-			assert.throws(
-				() => {
-					const id = Tree.shortId(newNode);
-				},
-				validateUsageError(
-					/Tree.shortId cannot access default identifiers on unhydrated nodes/,
-				),
-			);
+			const id = Tree.shortId(newNode);
+			assert(typeof id === "string");
+			assert(isStableId(id));
 		});
 
 		it("unhydrated custom identifier access works", () => {

packages/dds/tree/src/test/simple-tree/unhydratedNode.spec.ts

@@ -5,6 +5,8 @@
 
 import { strict as assert } from "node:assert";
 
+import { isStableId } from "@fluidframework/id-compressor/internal";
+
 import { Tree } from "../../shared-tree/index.js";
 import { rootFieldKey } from "../../core/index.js";
 import {
@@ -20,7 +22,6 @@ import type {
 	FieldProvider,
 	// eslint-disable-next-line import/no-internal-modules
 } from "../../simple-tree/schemaTypes.js";
-import { validateAssertionError } from "@fluidframework/test-runtime-utils/internal";
 import { hydrate } from "./utils.js";
 import { TreeStatus } from "../../feature-libraries/index.js";
 import { validateUsageError } from "../utils.js";
@@ -320,20 +321,13 @@ describe("Unhydrated nodes", () => {
 		assert.equal(object.id, id);
 	});
 
-	it("fail to read automatically generated identifiers", () => {
+	it("read automatically generated identifiers", () => {
 		class TestObjectWithId extends schemaFactory.object("HasId", {
 			id: schemaFactory.identifier,
 		}) {}
 
 		const object = new TestObjectWithId({ id: undefined });
-		assert.throws(
-			() => object.id,
-			(error: Error) =>
-				validateAssertionError(
-					error,
-					/An automatically generated node identifier may not be queried until the node is inserted into the tree/,
-				),
-		);
+		assert(isStableId(object.id));
 	});
 
 	it("correctly iterate identifiers", () => {