diff --git a/packages/dds/tree/.vscode/settings.json b/packages/dds/tree/.vscode/settings.json index 3cc46a4f4b07..c452102d53ac 100644 --- a/packages/dds/tree/.vscode/settings.json +++ b/packages/dds/tree/.vscode/settings.json @@ -24,6 +24,7 @@ "endregion", "fluidframework", "insertable", + "parameterizing", "reentrantly", "typeparam", "unannotate", diff --git a/packages/dds/tree/src/core/tree/mapTree.ts b/packages/dds/tree/src/core/tree/mapTree.ts index 9e49f1795f6f..4b200a988120 100644 --- a/packages/dds/tree/src/core/tree/mapTree.ts +++ b/packages/dds/tree/src/core/tree/mapTree.ts @@ -3,6 +3,9 @@ * Licensed under the MIT License. */ +import { assert } from "@fluidframework/core-utils/internal"; + +import type { MinimalMapTreeNodeView } from "../../feature-libraries/index.js"; import type { FieldKey } from "../schema-stored/index.js"; import type { NodeData } from "./types.js"; @@ -36,17 +39,32 @@ export interface ExclusiveMapTree extends NodeData, MapTree { * @privateRemarks This is implemented iteratively (rather than recursively, which is much simpler) * to avoid the possibility of a stack overflow for very deep trees. */ -export function deepCopyMapTree(mapTree: MapTree): ExclusiveMapTree { - type Next = [fields: ExclusiveMapTree["fields"], sourceFields: MapTree["fields"]]; +export function deepCopyMapTree(mapTree: MinimalMapTreeNodeView): ExclusiveMapTree { + type Next = [ + fields: ExclusiveMapTree["fields"], + sourceFields: MinimalMapTreeNodeView["fields"], + ]; + function create( + child: MinimalMapTreeNodeView, + fields: ExclusiveMapTree["fields"], + ): ExclusiveMapTree { + return { + type: child.type, + ...(child.value !== undefined ? { value: child.value } : {}), + fields, + }; + } + const rootFields: ExclusiveMapTree["fields"] = new Map(); const nexts: Next[] = [[rootFields, mapTree.fields]]; for (let next = nexts.pop(); next !== undefined; next = nexts.pop()) { const [fields, sourceFields] = next; for (const [key, field] of sourceFields) { + assert(field.length > 0, "invalid map tree: empty field"); if (field.length > 0) { const newField: ExclusiveMapTree[] = []; for (const child of field) { - const childClone: ExclusiveMapTree = { ...child, fields: new Map() }; + const childClone = create(child, new Map()); newField.push(childClone); nexts.push([childClone.fields, child.fields]); } @@ -55,8 +73,5 @@ export function deepCopyMapTree(mapTree: MapTree): ExclusiveMapTree { } } - return { - ...mapTree, - fields: rootFields, - }; + return create(mapTree, rootFields); } diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts index 04e21e41bdea..3a7f96b299ca 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultEditBuilder.ts @@ -434,8 +434,9 @@ export interface OptionalFieldEditBuilder { } /** + * Edit builder for the sequence field kind. */ -export interface SequenceFieldEditBuilder { +export interface SequenceFieldEditBuilder { /** * Issues a change which inserts the `newContent` at the given `index`. * @param index - the index at which to insert the `newContent`. @@ -448,7 +449,7 @@ export interface SequenceFieldEditBuilder { * @param index - The index of the first removed element. * @param count - The number of elements to remove. */ - remove(index: number, count: number): void; + remove(index: number, count: number): TRemoved; } /** diff --git a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts index c62c5911af04..a5f99923d953 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts @@ -7,7 +7,6 @@ import { unreachableCase, fail } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { - type MapTree, type TreeFieldStoredSchema, LeafNodeStoredSchema, ObjectNodeStoredSchema, @@ -16,6 +15,7 @@ import { type SchemaAndPolicy, } from "../../core/index.js"; import { allowsValue } from "../valueUtilities.js"; +import type { MapTreeFieldViewGeneric, MinimalMapTreeNodeView } from "../mapTreeCursor.js"; export enum SchemaValidationError { Field_KindNotInSchemaPolicy, @@ -45,7 +45,7 @@ export function inSchemaOrThrow(maybeError: SchemaValidationError | undefined): * Deeply checks that the provided node complies with the schema based on its identifier. */ export function isNodeInSchema( - node: MapTree, + node: MinimalMapTreeNodeView, schemaAndPolicy: SchemaAndPolicy, ): SchemaValidationError | undefined { // Validate the schema declared by the node exists @@ -86,7 +86,7 @@ export function isNodeInSchema( return SchemaValidationError.ObjectNode_FieldNotInSchema; } } else if (schema instanceof MapNodeStoredSchema) { - for (const field of node.fields.values()) { + for (const [_key, field] of node.fields) { const fieldInSchemaResult = isFieldInSchema(field, schema.mapFields, schemaAndPolicy); if (fieldInSchemaResult !== undefined) { return fieldInSchemaResult; @@ -104,7 +104,7 @@ export function isNodeInSchema( * Deeply checks that the nodes comply with the field schema and included schema. */ export function isFieldInSchema( - childNodes: readonly MapTree[], + childNodes: MapTreeFieldViewGeneric, schema: TreeFieldStoredSchema, schemaAndPolicy: SchemaAndPolicy, ): SchemaValidationError | undefined { diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/context.ts b/packages/dds/tree/src/feature-libraries/flex-tree/context.ts index cc62febfdf46..ebb45465aa7b 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/context.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/context.ts @@ -54,16 +54,12 @@ export interface FlexTreeContext { } /** - * A common context of a "forest" of FlexTrees. - * It handles group operations like transforming cursors into anchors for edits. + * Subset of a hydrated context which can be used in more cases (like before the root and events are set up). */ -export interface FlexTreeHydratedContext extends FlexTreeContext { - readonly events: Listenable; +export interface FlexTreeHydratedContextMinimal { /** - * Gets the root field of the tree. + * The {@link NodeIdentifierManager} responsible for allocating and compressing identifiers for nodes in this context. */ - get root(): FlexTreeField; - readonly nodeKeyManager: NodeIdentifierManager; /** @@ -72,6 +68,20 @@ export interface FlexTreeHydratedContext extends FlexTreeContext { readonly checkout: ITreeCheckout; } +/** + * A common context of a "forest" of FlexTrees. + * It handles group operations like transforming cursors into anchors for edits. + */ +export interface FlexTreeHydratedContext + extends FlexTreeContext, + FlexTreeHydratedContextMinimal { + readonly events: Listenable; + /** + * Gets the root field of the tree. + */ + get root(): FlexTreeField; +} + /** * Creating multiple flex tree contexts for the same branch, and thus with the same underlying AnchorSet does not work due to how TreeNode caching works. * This slot is used to detect if one already exists and error if creating a second. diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts b/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts index 674772001df3..e3988743c703 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/flexTreeTypes.ts @@ -278,7 +278,7 @@ export interface FlexTreeField extends FlexTreeEntity { /** * Typed tree for inserting as the content of a field. */ -export type FlexibleFieldContent = ExclusiveMapTree[]; +export type FlexibleFieldContent = readonly FlexibleNodeContent[]; /** * Tree for inserting as a node. diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/index.ts b/packages/dds/tree/src/feature-libraries/flex-tree/index.ts index 92cae16dde2d..a076c4f903c7 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/index.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/index.ts @@ -17,6 +17,8 @@ export { FlexTreeEntityKind, isFlexTreeNode, flexTreeSlot, + type FlexibleNodeContent, + type FlexibleFieldContent, } from "./flexTreeTypes.js"; export { @@ -32,6 +34,7 @@ export { type FlexTreeHydratedContext, Context, ContextSlot, + type FlexTreeHydratedContextMinimal, } from "./context.js"; export { type FlexTreeNodeEvents } from "./treeEvents.js"; diff --git a/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts b/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts index cfbec7ed14c6..dc4cbf979bcb 100644 --- a/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts +++ b/packages/dds/tree/src/feature-libraries/flex-tree/lazyField.ts @@ -43,6 +43,8 @@ import { type FlexTreeSequenceField, type FlexTreeTypedField, type FlexTreeUnknownUnboxed, + type FlexibleFieldContent, + type FlexibleNodeContent, TreeStatus, flexTreeMarker, flexTreeSlot, @@ -263,7 +265,7 @@ export class LazySequence extends LazyField implements FlexTreeSequenceField { return this.map((x) => x); } - public editor: SequenceFieldEditBuilder = { + public editor: SequenceFieldEditBuilder = { insert: (index, newContent) => { this.sequenceEditor().insert(index, cursorForMapTreeField(newContent)); }, @@ -279,7 +281,7 @@ export class LazySequence extends LazyField implements FlexTreeSequenceField { } export class LazyValueField extends LazyField implements FlexTreeRequiredField { - public editor: ValueFieldEditBuilder = { + public editor: ValueFieldEditBuilder = { set: (newContent) => { this.valueFieldEditor().set(cursorForMapTreeField([newContent])); }, diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index e89e19040f54..93807d9c19f8 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -15,6 +15,9 @@ export { cursorForMapTreeNode, mapTreeFromCursor, mapTreeFieldFromCursor, + type MinimalMapTreeNodeView, + mapTreeFieldsWithField, + mapTreeWithField, } from "./mapTreeCursor.js"; export { buildForest } from "./object-forest/index.js"; export { @@ -174,6 +177,9 @@ export { treeStatusFromAnchorCache, indexForAt, FlexTreeEntityKind, + type FlexibleNodeContent, + type FlexibleFieldContent, + type FlexTreeHydratedContextMinimal, } from "./flex-tree/index.js"; export { TreeCompressionStrategy } from "./treeCompressionUtils.js"; diff --git a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts index f47da900c48d..2ed30eb94219 100644 --- a/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts +++ b/packages/dds/tree/src/feature-libraries/mapTreeCursor.ts @@ -12,49 +12,136 @@ import { type FieldKey, type ITreeCursor, type MapTree, + type NodeData, aboveRootPlaceholder, detachedFieldAsKey, mapCursorField, rootField, + rootFieldKey, } from "../core/index.js"; import { type CursorAdapter, type CursorWithNode, + type Field, stackTreeFieldCursor, stackTreeNodeCursor, } from "./treeCursorUtils.js"; +import type { requireAssignableTo } from "../util/index.js"; + +/** + * A generic variant of {@link MapTree} that can be used to strongly type trees implementing a MapTree-like API. + * @remarks + * Due to how TypeScript handles recursive generic types, explicitly named extension interfaces work best for parameterizing this, and a default type parameter can't be provided. + * @see {@link MinimalMapTreeNodeView} for a minimal configuration of this interface. + */ +export interface MapTreeNodeViewGeneric extends NodeData { + /** + * The non-empty fields on this node. + * @remarks + * This is the subset of map needed to view the tree. + * Theoretically "size" could be removed by measuring the length of keys, but it is included for convenience. + */ + readonly fields: Pick< + ReadonlyMap>, + typeof Symbol.iterator | "get" | "size" | "keys" + >; +} + +/** + * A field in {@link MapTreeNodeViewGeneric}. + */ +export type MapTreeFieldViewGeneric = Pick< + readonly TNode[], + typeof Symbol.iterator | "length" +>; + +/** + * Like {@link MapTree} but with the minimal properties needed for reading. + */ +export interface MinimalMapTreeNodeView + extends MapTreeNodeViewGeneric {} + +{ + // Check that these interfaces are subsets of MapTree as intended: + type _check1 = requireAssignableTo; + type _check2 = requireAssignableTo>; +} /** * Returns an {@link ITreeCursorSynchronous} in nodes mode for a single {@link MapTree}. */ -export function cursorForMapTreeNode(root: MapTree): CursorWithNode { - return stackTreeNodeCursor(adapter, root); +export function cursorForMapTreeNode>( + root: T, +): CursorWithNode { + // There doesn't seem to be a clean way to get TypeScript to type check this without casting + // without declaring the adapter inside this generic function and needlessly recreating it on every call. + const adapterTyped = adapter as CursorAdapter> as CursorAdapter; + return stackTreeNodeCursor(adapterTyped, root); +} + +/** + * Creates an {@link ExclusiveMapTree} with a single field and no value. + * @remarks + * This handles ensuring the field is omitted if empty, and defaults to making the node above the root. + */ +export function mapTreeWithField( + children: ExclusiveMapTree[], + key = rootFieldKey, + type = aboveRootPlaceholder, +): ExclusiveMapTree { + return { + type, + fields: mapTreeFieldsWithField(children, key), + }; +} + +/** + * Creates a Map suitable for use as {@link MapTree.fields} with a single field. + * @remarks + * This handles ensuring the field is omitted if empty. + */ +export function mapTreeFieldsWithField( + children: T, + key: FieldKey, +): Map { + return new Map(children.length === 0 ? [] : [[key, children]]); } /** * Returns an {@link ITreeCursorSynchronous} in fields mode for a MapTree field. */ -export function cursorForMapTreeField( - root: readonly MapTree[], +export function cursorForMapTreeField>( + root: readonly T[], detachedField: DetachedField = rootField, -): CursorWithNode { +): CursorWithNode { const key = detachedFieldAsKey(detachedField); - return stackTreeFieldCursor( - adapter, - { - type: aboveRootPlaceholder, - fields: new Map([[key, root]]), - }, - detachedField, - ); + const adapterTyped = adapter as unknown as CursorAdapter; + const dummyRoot: MapTreeNodeViewGeneric = { + type: aboveRootPlaceholder, + fields: mapTreeFieldsWithField(root, key), + }; + return stackTreeFieldCursor(adapterTyped, dummyRoot as T, detachedField); } -const adapter: CursorAdapter = { +const adapter: CursorAdapter = { value: (node) => node.value, type: (node) => node.type, keysFromNode: (node) => [...node.fields.keys()], // TODO: don't convert this to array here. - getFieldFromNode: (node, key) => node.fields.get(key) ?? [], + getFieldFromNode: (node, key): Field => { + const field = node.fields.get(key) as + | MinimalMapTreeNodeView[] + | Iterable + | undefined; + if (field === undefined) { + return []; + } + if (Array.isArray(field)) { + return field as readonly MinimalMapTreeNodeView[]; + } + // TODO: this copy could be avoided by using Array.at instead of indexing in `Field`, but that requires ES2022. + return [...field] as readonly MinimalMapTreeNodeView[]; + }, }; /** diff --git a/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts b/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts index ff44f721dbd9..6308defcef28 100644 --- a/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts +++ b/packages/dds/tree/src/feature-libraries/treeCursorUtils.ts @@ -72,6 +72,11 @@ export function stackTreeFieldCursor( return cursor; } +/** + * The representation of a field used by {@link CursorAdapter}. + */ +export type Field = Pick; + /** * Provides functionality to allow a {@link stackTreeNodeCursor} and {@link stackTreeFieldCursor} to implement cursors. */ @@ -91,10 +96,10 @@ export interface CursorAdapter { /** * @returns the child nodes for the given node and key. */ - getFieldFromNode(node: TNode, key: FieldKey): readonly TNode[]; + getFieldFromNode(node: TNode, key: FieldKey): Field; } -type SiblingsOrKey = readonly TNode[] | readonly FieldKey[]; +type SiblingsOrKey = Field | readonly FieldKey[]; /** * A class that satisfies part of the ITreeCursorSynchronous implementation. @@ -122,7 +127,7 @@ export abstract class SynchronousCursor { * 2. Support for cursors which are field cursors at the root. */ class StackCursor extends SynchronousCursor implements CursorWithNode { - public readonly [CursorMarker] = true; + public override readonly [CursorMarker] = true; /** * Might start at special root where fields are detached sequences. * @@ -149,14 +154,18 @@ class StackCursor extends SynchronousCursor implements CursorWithNode this.mode === CursorLocationType.Fields ? true : "must be in fields mode", ); - return this.siblings[this.index] as FieldKey; + // index is kept inbounds as an invariant of the class. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return (this.siblings as readonly FieldKey[])[this.index]!; } private getStackedFieldKey(height: number): FieldKey { assert(height % 2 === 1, 0x3b8 /* must field height */); const siblingStack = this.siblingStack[height] ?? oob(); const indexStack = this.indexStack[height] ?? oob(); - return siblingStack[indexStack] as FieldKey; + // index is kept inbounds as an invariant of the class. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return (siblingStack as readonly FieldKey[])[indexStack]!; } private getStackedNodeIndex(height: number): number { @@ -166,9 +175,10 @@ class StackCursor extends SynchronousCursor implements CursorWithNode)[index]!; } public getFieldLength(): number { @@ -358,12 +368,13 @@ class StackCursor extends SynchronousCursor implements CursorWithNode)[this.index]!; } - private getField(): readonly TNode[] { + private getField(): Field { // assert(this.mode === CursorLocationType.Fields, "can only get field when in fields"); const parent = this.getStackedNode(this.indexStack.length - 1); const key: FieldKey = this.getFieldKey(); diff --git a/packages/dds/tree/src/simple-tree/api/customTree.ts b/packages/dds/tree/src/simple-tree/api/customTree.ts index 3d3b44e70fb1..09f5d0dd318b 100644 --- a/packages/dds/tree/src/simple-tree/api/customTree.ts +++ b/packages/dds/tree/src/simple-tree/api/customTree.ts @@ -6,6 +6,7 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces"; import { assert, fail } from "@fluidframework/core-utils/internal"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; +import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { EmptyKey, @@ -231,3 +232,12 @@ export function replaceHandles(tree: unknown, replacer: HandleConverter): } }); } + +/** + * Throws a `UsageError` indicating that a type is unknown in the current context. + */ +export function unknownTypeError(type: string): never { + throw new UsageError( + `Failed to parse tree due to occurrence of type ${JSON.stringify(type)} which is not defined in this context.`, + ); +} diff --git a/packages/dds/tree/src/simple-tree/api/verboseTree.ts b/packages/dds/tree/src/simple-tree/api/verboseTree.ts index 8dd4d4d1fb96..0766da8ba746 100644 --- a/packages/dds/tree/src/simple-tree/api/verboseTree.ts +++ b/packages/dds/tree/src/simple-tree/api/verboseTree.ts @@ -6,6 +6,7 @@ import type { IFluidHandle } from "@fluidframework/core-interfaces"; import { isFluidHandle } from "@fluidframework/runtime-utils/internal"; import { assert, fail } from "@fluidframework/core-utils/internal"; +import { UsageError } from "@fluidframework/telemetry-utils/internal"; import { aboveRootPlaceholder, @@ -14,7 +15,6 @@ import { type FieldKey, type ITreeCursor, type ITreeCursorSynchronous, - type TreeNodeSchemaIdentifier, } from "../../core/index.js"; import { brand } from "../../util/index.js"; import type { @@ -40,6 +40,7 @@ import { isObjectNodeSchema } from "../node-kinds/index.js"; import { customFromCursor, replaceHandles, + unknownTypeError, type CustomTreeNode, type HandleConverter, type SchemalessParseOptions, @@ -153,12 +154,14 @@ export function applySchemaToParserOptions( return key; }, parse: (type, inputKey): FieldKey => { - const simpleNodeSchema = - context.schema.get(brand(type)) ?? fail(0xb3a /* missing schema */); + const simpleNodeSchema = context.schema.get(brand(type)) ?? unknownTypeError(type); if (isObjectNodeSchema(simpleNodeSchema)) { - const info = - simpleNodeSchema.flexKeyMap.get(inputKey) ?? - fail(0xb3b /* missing field info */); + const info = simpleNodeSchema.flexKeyMap.get(inputKey); + if (info === undefined) { + throw new UsageError( + `Failed to parse VerboseTree due to unexpected key ${JSON.stringify(inputKey)} on type ${JSON.stringify(type)}.`, + ); + } return info.storedKey; } return brand(inputKey); @@ -203,19 +206,19 @@ function verboseTreeAdapter(options: SchemalessParseOptions): CursorAdapter { switch (typeof node) { case "number": - return numberSchema.identifier as TreeNodeSchemaIdentifier; + return brand(numberSchema.identifier); case "string": - return stringSchema.identifier as TreeNodeSchemaIdentifier; + return brand(stringSchema.identifier); case "boolean": - return booleanSchema.identifier as TreeNodeSchemaIdentifier; + return brand(booleanSchema.identifier); default: if (node === null) { - return nullSchema.identifier as TreeNodeSchemaIdentifier; + return brand(nullSchema.identifier); } if (isFluidHandle(node)) { - return handleSchema.identifier as TreeNodeSchemaIdentifier; + return brand(handleSchema.identifier); } - return node.type as TreeNodeSchemaIdentifier; + return brand(node.type); } }, keysFromNode: (node: VerboseTree): readonly FieldKey[] => { diff --git a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts index 61d7f5248cd9..dfe12c1c2905 100644 --- a/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts +++ b/packages/dds/tree/src/simple-tree/core/unhydratedFlexTree.ts @@ -42,6 +42,9 @@ import { FieldKinds, type SequenceFieldEditBuilder, cursorForMapTreeNode, + type OptionalFieldEditBuilder, + type ValueFieldEditBuilder, + type FlexibleNodeContent, } from "../../feature-libraries/index.js"; import { brand, getOrCreate, mapIterable } from "../../util/index.js"; @@ -453,7 +456,8 @@ class EagerMapTreeOptionalField } }); }, - }; + } satisfies OptionalFieldEditBuilder & + ValueFieldEditBuilder; public get content(): FlexTreeUnknownUnboxed | undefined { const value = this.mapTrees[0]; diff --git a/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts index cf765466a005..b9148f419169 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts @@ -6,8 +6,9 @@ import { Lazy, oob, fail } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; -import { EmptyKey, type ExclusiveMapTree } from "../../../core/index.js"; +import { EmptyKey } from "../../../core/index.js"; import { + type FlexibleFieldContent, type FlexTreeNode, type FlexTreeSequenceField, isFlexTreeNode, @@ -37,14 +38,12 @@ import { getSimpleNodeSchemaFromInnerNode, getOrCreateInnerNode, type TreeNodeSchemaClass, -} from "../../core/index.js"; -import { type InsertableContent, mapTreeFromNodeData } from "../../toMapTree.js"; -import { prepareArrayContentForInsertion } from "../../prepareForInsertion.js"; -import { getKernel, UnhydratedFlexTreeNode, UnhydratedTreeSequenceField, } from "../../core/index.js"; +import { type InsertableContent, mapTreeFromNodeData } from "../../toMapTree.js"; +import { prepareArrayContentForInsertion } from "../../prepareForInsertion.js"; import { TreeNodeValid, type MostDerivedData } from "../../treeNodeValid.js"; import { getUnhydratedContext } from "../../createContext.js"; import type { System_Unsafe } from "../../api/index.js"; @@ -852,7 +851,7 @@ abstract class CustomArrayNodeBase super(input ?? []); } - #mapTreesFromFieldData(value: Insertable): ExclusiveMapTree[] { + #mapTreesFromFieldData(value: Insertable): FlexibleFieldContent { const sequenceField = getSequenceField(this); const content = value as readonly ( | InsertableContent diff --git a/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts b/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts index 6759ae833b48..4198e7a8bd05 100644 --- a/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts +++ b/packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts @@ -5,6 +5,7 @@ import { Lazy } from "@fluidframework/core-utils/internal"; import type { + FlexibleNodeContent, FlexTreeNode, FlexTreeOptionalField, OptionalFieldEditBuilder, @@ -44,7 +45,6 @@ import { import { prepareForInsertion } from "../../prepareForInsertion.js"; import { brand, count, type RestrictiveStringRecord } from "../../../util/index.js"; import { TreeNodeValid, type MostDerivedData } from "../../treeNodeValid.js"; -import type { ExclusiveMapTree } from "../../../core/index.js"; import { getUnhydratedContext } from "../../createContext.js"; import type { MapNodeCustomizableSchema, MapNodePojoEmulationSchema } from "./mapNodeTypes.js"; @@ -158,7 +158,7 @@ abstract class CustomMapNodeBase extends T return getOrCreateInnerNode(this); } - private editor(key: string): OptionalFieldEditBuilder { + private editor(key: string): OptionalFieldEditBuilder { const field = this.innerNode.getBoxed(brand(key)) as FlexTreeOptionalField; return field.editor; } diff --git a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts index 83e3549a7e76..838d230e2998 100644 --- a/packages/dds/tree/src/simple-tree/prepareForInsertion.ts +++ b/packages/dds/tree/src/simple-tree/prepareForInsertion.ts @@ -17,7 +17,7 @@ import type { import { type FlexTreeContext, getSchemaAndPolicy, - type FlexTreeHydratedContext, + type FlexTreeHydratedContextMinimal, FieldKinds, } from "../feature-libraries/index.js"; import { @@ -110,7 +110,7 @@ export function prepareForInsertionContextless | undefined, + hydratedData: FlexTreeHydratedContextMinimal | undefined, ): TIn extends undefined ? undefined : ExclusiveMapTree { const mapTree = mapTreeFromNodeData(data, schema, hydratedData?.nodeKeyManager); @@ -129,7 +129,7 @@ export function prepareForInsertionContextless | undefined, + hydratedData: FlexTreeHydratedContextMinimal | undefined, fieldSchema: TreeFieldStoredSchema, mapTrees: ExclusiveMapTree[], ): void { diff --git a/packages/dds/tree/src/simple-tree/toMapTree.ts b/packages/dds/tree/src/simple-tree/toMapTree.ts index bfe2253ce68b..442b8569b6fb 100644 --- a/packages/dds/tree/src/simple-tree/toMapTree.ts +++ b/packages/dds/tree/src/simple-tree/toMapTree.ts @@ -20,7 +20,7 @@ import { valueSchemaAllows, type NodeIdentifierManager, } from "../feature-libraries/index.js"; -import { brand, isReadonlyArray, find, hasSome, hasSingle } from "../util/index.js"; +import { brand, isReadonlyArray, find, hasSingle } from "../util/index.js"; import { nullSchema } from "./leafNodeSchema.js"; import { @@ -365,8 +365,8 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): Exclusiv // Loop through field keys without data. // This does NOT apply defaults. for (const [key, fieldInfo] of schema.flexKeyMap) { - if (checkFieldProperty(data, key)) { - const value = (data as Record)[key as string]; + const value = getFieldProperty(data, key); + if (value !== undefined) { setFieldValue(fields, value, fieldInfo.schema, fieldInfo.storedKey); } } @@ -379,20 +379,27 @@ function objectToMapTree(data: FactoryContent, schema: TreeNodeSchema): Exclusiv /** * Check {@link FactoryContentObject} for a property which could be store a field. + * + * @returns If the property exists, return its value. Otherwise, returns undefined. * @remarks * The currently policy is to only consider own properties. * See {@link InsertableObjectFromSchemaRecord} for where this policy is documented in the public API. * - * Explicit undefined members are considered to exist, as long as they are own properties. + * Explicit undefined values are treated the same as missing properties to allow explicit use of undefined with defaulted identifiers. + * + * @privateRemarks + * If we ever want to have an optional field which defaults to something other than undefined, this will need changes. + * It would need to adjusting the handling of explicit undefined in contexts where undefined is allowed, and a default provider also exists. */ -function checkFieldProperty( +function getFieldProperty( data: FactoryContentObject, key: string | symbol, -): data is { - readonly [P in string]: InsertableContent | undefined; -} { +): InsertableContent | undefined { // This policy only allows own properties. - return Object.hasOwnProperty.call(data, key); + if (Object.hasOwnProperty.call(data, key)) { + return (data as Record)[key as string]; + } + return undefined; } function setFieldValue( @@ -421,10 +428,6 @@ function getType( )}.`, ); } - assert( - hasSome(possibleTypes), - 0x84e /* data is incompatible with all types allowed by the schema */, - ); if (!hasSingle(possibleTypes)) { throw new UsageError( `The provided data is compatible with more than one type allowed by the schema. @@ -563,11 +566,7 @@ function shallowCompatibilityTest( // If the schema has a required key which is not present in the input object, reject it. for (const [fieldKey, fieldSchema] of schema.fields) { if (fieldSchema.requiresValue) { - if (checkFieldProperty(data, fieldKey)) { - if (data[fieldKey] === undefined) { - return CompatibilityLevel.None; - } - } else { + if (getFieldProperty(data, fieldKey) === undefined) { return CompatibilityLevel.None; } } diff --git a/packages/dds/tree/src/simple-tree/toStoredSchema.ts b/packages/dds/tree/src/simple-tree/toStoredSchema.ts index d6bc3ac35531..75926929ea01 100644 --- a/packages/dds/tree/src/simple-tree/toStoredSchema.ts +++ b/packages/dds/tree/src/simple-tree/toStoredSchema.ts @@ -93,7 +93,10 @@ export function convertField(schema: SimpleFieldSchema): TreeFieldStoredSchema { return { kind, types }; } -const convertFieldKind = new Map([ +const convertFieldKind: ReadonlyMap = new Map< + FieldKind, + FlexFieldKind +>([ [FieldKind.Optional, FieldKinds.optional], [FieldKind.Required, FieldKinds.required], [FieldKind.Identifier, FieldKinds.identifier], diff --git a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts index cf19711c32f8..b08322680e6f 100644 --- a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts @@ -58,12 +58,10 @@ const configGeneralized2 = new TreeViewConfiguration({ function checkoutWithInitialTree( viewConfig: TreeViewConfiguration, unhydratedInitialTree: InsertableField, - nodeKeyManager = new MockNodeIdentifierManager(), ): TreeCheckout { const initialTree = fieldCursorFromInsertable( viewConfig.schema, unhydratedInitialTree, - nodeKeyManager, ); const treeContent: TreeStoredContent = { schema: toStoredSchema(viewConfig.schema), diff --git a/packages/dds/tree/src/test/simple-tree/api/create.spec.ts b/packages/dds/tree/src/test/simple-tree/api/create.spec.ts index a93d91266a5d..4294473f8441 100644 --- a/packages/dds/tree/src/test/simple-tree/api/create.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/create.spec.ts @@ -13,6 +13,7 @@ import { import { SchemaFactory } from "../../../simple-tree/index.js"; import { validateUsageError } from "../../utils.js"; import { singleJsonCursor } from "../../json/index.js"; +import { validateAssertionError } from "@fluidframework/test-runtime-utils/internal"; describe("simple-tree create", () => { describe("createFromCursor", () => { @@ -21,10 +22,20 @@ describe("simple-tree create", () => { createFromCursor(SchemaFactory.string, cursor); }); - it("Failure", () => { + it("Failure: unknown schema", () => { const cursor = singleJsonCursor("Hello world"); assert.throws( () => createFromCursor(SchemaFactory.number, cursor), + (e: Error) => validateAssertionError(e, /Tree does not conform to schema/), + ); + }); + + it("Failure: out of schema", () => { + const factory = new SchemaFactory("test"); + class Obj extends factory.object("Obj", { x: SchemaFactory.string }) {} + const cursor = singleJsonCursor("Hello world"); + assert.throws( + () => createFromCursor(Obj, cursor), validateUsageError(/does not conform to schema/), ); }); diff --git a/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts b/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts index 5b82d5ab8e04..b811fde65453 100644 --- a/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/treeNodeApi.spec.ts @@ -1751,6 +1751,45 @@ describe("treeNodeApi", () => { describe("verbose", () => { describe("importVerbose", () => { + it("unknown schema: leaf", () => { + // Input using schema not included in the context + assert.throws( + () => TreeAlpha.importVerbose(SchemaFactory.number, "x"), + validateUsageError(/Tree does not conform to schema/), + ); + }); + + it("unknown schema: non-leaf", () => { + const factory = new SchemaFactory("Test"); + class A extends factory.object("A", {}) {} + class B extends factory.object("B", {}) {} + // Input using schema not included in the context + assert.throws( + () => TreeAlpha.importVerbose(A, { type: B.identifier, fields: {} }), + validateUsageError(/Tree does not conform to schema/), + ); + }); + + it("invalid with known schema", () => { + const factory = new SchemaFactory("Test"); + class A extends factory.object("A", { a: SchemaFactory.string }) {} + assert.throws( + () => TreeAlpha.importVerbose(A, { type: A.identifier, fields: { wrong: "x" } }), + validateUsageError( + `Failed to parse VerboseTree due to unexpected key "wrong" on type "Test.A".`, + ), + ); + }); + + it("missing field with default", () => { + const factory = new SchemaFactory("Test"); + class A extends factory.object("A", { a: factory.identifier }) {} + assert.throws( + () => TreeAlpha.importVerbose(A, { type: A.identifier, fields: {} }), + validateUsageError(/Field_MissingRequiredChild/), + ); + }); + it("undefined", () => { // Valid assert.equal(TreeAlpha.importVerbose(schema.optional([]), undefined), undefined); @@ -1762,7 +1801,7 @@ describe("treeNodeApi", () => { // Undefined required, not provided assert.throws( () => TreeAlpha.importVerbose(schema.optional([]), 1), - validateUsageError(/does not conform to schema/), + validateUsageError(/Tree does not conform to schema/), ); }); @@ -1772,7 +1811,7 @@ describe("treeNodeApi", () => { // invalid assert.throws( () => TreeAlpha.importVerbose([schema.null, schema.number], "x"), - validateUsageError(/does not conform to schema/), + validateUsageError(/Tree does not conform to schema/), ); }); @@ -1821,10 +1860,14 @@ describe("treeNodeApi", () => { const testTree = new Point3D({ x: 1, y: 2, z: 3 }); const exported = TreeAlpha.exportVerbose(testTree); - // TODO:AB#26720 The error here should be more clear. + // TODO:AB#26720 The error here should be more clear: + // perhaps reference allowUnknownOptionalFields and stored keys specifically. assert.throws( () => TreeAlpha.importVerbose(Point2D, exported), - (error: Error) => validateAssertionError(error, /missing field info/), + + validateUsageError( + `Failed to parse VerboseTree due to unexpected key "z" on type "com.example.Point".`, + ), ); }); }); diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index 07d0ee1f9e8c..dbb3d8798e07 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -94,8 +94,6 @@ import { type NormalizedFieldUpPath, type ExclusiveMapTree, type MapTree, - aboveRootPlaceholder, - rootFieldKey, SchemaVersion, } from "../core/index.js"; import { typeboxValidator } from "../external-utilities/index.js"; @@ -115,6 +113,10 @@ import { cursorForJsonableTreeField, chunkFieldSingle, makeSchemaCodec, + mapTreeWithField, + type MinimalMapTreeNodeView, + jsonableTreeFromCursor, + cursorForMapTreeNode, } from "../feature-libraries/index.js"; import { type CheckoutEvents, @@ -821,10 +823,7 @@ function createCheckoutWithContent( }, ): { checkout: TreeCheckout; logger: IMockLoggerExt } { const fieldCursor = normalizeNewFieldContent(content.initialTree); - const roots: MapTree = { - type: aboveRootPlaceholder, - fields: new Map([[rootFieldKey, mapTreeFieldFromCursor(fieldCursor)]]), - }; + const roots: MapTree = mapTreeWithField(mapTreeFieldFromCursor(fieldCursor)); const schema = new TreeStoredSchemaRepository(content.schema); const forest = buildTestForest({ additionalAsserts: args?.additionalAsserts ?? true, @@ -892,10 +891,7 @@ export function buildTestForest(options: { export function forestWithContent(content: TreeStoredContent): IEditableForest { const fieldCursor = normalizeNewFieldContent(content.initialTree); - const roots: MapTree = { - type: aboveRootPlaceholder, - fields: new Map([[rootFieldKey, mapTreeFieldFromCursor(fieldCursor)]]), - }; + const roots: MapTree = mapTreeWithField(mapTreeFieldFromCursor(fieldCursor)); const forest = buildTestForest({ additionalAsserts: true, schema: new TreeStoredSchemaRepository(content.schema), @@ -963,6 +959,23 @@ export function expectJsonTree( } } +export function expectEqualMapTreeViews( + actual: MinimalMapTreeNodeView, + expected: MinimalMapTreeNodeView, +): void { + expectEqualCursors(cursorForMapTreeNode(actual), cursorForMapTreeNode(expected)); +} + +export function expectEqualCursors( + actual: ITreeCursorSynchronous | undefined, + expected: ITreeCursorSynchronous, +): void { + assert(actual !== undefined); + const actualJson = jsonableTreeFromCursor(actual); + const expectedJson = jsonableTreeFromCursor(expected); + assert.deepEqual(actualJson, expectedJson); +} + export function expectNoRemovedRoots(tree: ITreeCheckout): void { assert(tree.getRemovedRoots().length === 0); } @@ -1469,12 +1482,7 @@ export function fieldCursorFromInsertable< ? ImplicitFieldSchema : TSchema & ImplicitFieldSchema, data: InsertableField, - context?: NodeIdentifierManager | undefined, ): ITreeCursorSynchronous { - const mapTree = mapTreeFromNodeData( - data as InsertableField, - schema, - context, - ); + const mapTree = mapTreeFromNodeData(data as InsertableField, schema); return cursorForMapTreeField(mapTree === undefined ? [] : [mapTree]); }