From 195b7224a44456b654ae4c6468913b19c10bb321 Mon Sep 17 00:00:00 2001 From: Abram Sanderson Date: Thu, 12 Dec 2024 15:31:48 -0800 Subject: [PATCH] Rewrite ViewSchema.checkCompatibility to leverage discrepancies (#23192) ## Description Updates `ViewSchema.checkCompatibility` to leverage the `getFieldDiscrepancies` codepath. This also puts in place infrastructure to allow viewing the document when the only differences between view and stored schema are extra optional fields in the stored schema on nodes that the view schema author has declared are OK. --------- Co-authored-by: Abram Sanderson --- .../tree-cli-app/src/test/legacy/index.ts | 9 + .../apps/tree-cli-app/src/test/legacy/v1.ts | 13 + .../apps/tree-cli-app/src/test/schema.spec.ts | 98 +++-- .../dds/tree/api-report/tree.alpha.api.md | 2 +- packages/dds/tree/src/core/index.ts | 1 - .../dds/tree/src/core/schema-stored/schema.ts | 10 + .../dds/tree/src/core/schema-view/index.ts | 1 - .../dds/tree/src/core/schema-view/view.ts | 11 - .../default-schema/defaultSchema.ts | 1 + .../default-schema/schemaChecker.ts | 5 +- .../dds/tree/src/feature-libraries/index.ts | 13 + .../modular-schema/discrepancies.ts | 114 ++++-- .../feature-libraries/modular-schema/index.ts | 13 + .../tree/src/shared-tree/schematizeTree.ts | 16 +- .../src/shared-tree/schematizingTreeView.ts | 32 +- .../dds/tree/src/simple-tree/api/create.ts | 6 +- .../dds/tree/src/simple-tree/api/index.ts | 6 +- .../tree/src/simple-tree/api/schemaFactory.ts | 63 ++- .../tree/src/simple-tree/api/storedSchema.ts | 29 +- packages/dds/tree/src/simple-tree/api/tree.ts | 5 + packages/dds/tree/src/simple-tree/api/view.ts | 248 +++++++++--- packages/dds/tree/src/simple-tree/index.ts | 2 + .../dds/tree/src/simple-tree/objectNode.ts | 28 +- .../tree/src/simple-tree/objectNodeTypes.ts | 5 + .../tree/src/simple-tree/toStoredSchema.ts | 44 ++- .../default-schema/schemaChecker.spec.ts | 2 + .../modular-schema/discrepancies.spec.ts | 26 +- .../schemaEvolutionExamples.spec.ts | 59 +-- .../dds/tree/src/test/schemaFactoryAlpha.ts | 90 +++++ .../test/shared-tree/schematizeTree.spec.ts | 66 ++-- .../shared-tree/schematizingTreeView.spec.ts | 137 ++++++- .../test/simple-tree/api/storedSchema.spec.ts | 3 +- .../src/test/simple-tree/api/treeApi.spec.ts | 84 ++++ .../src/test/simple-tree/schemaTypes.spec.ts | 2 +- .../src/test/simple-tree/toMapTree.spec.ts | 17 + .../src/test/simple-tree/viewSchema.spec.ts | 369 ++++++++++++++++++ .../api-report/fluid-framework.alpha.api.md | 2 +- 37 files changed, 1321 insertions(+), 311 deletions(-) create mode 100644 examples/apps/tree-cli-app/src/test/legacy/index.ts create mode 100644 examples/apps/tree-cli-app/src/test/legacy/v1.ts create mode 100644 packages/dds/tree/src/test/schemaFactoryAlpha.ts create mode 100644 packages/dds/tree/src/test/simple-tree/viewSchema.spec.ts diff --git a/examples/apps/tree-cli-app/src/test/legacy/index.ts b/examples/apps/tree-cli-app/src/test/legacy/index.ts new file mode 100644 index 000000000000..32fc95b7ee8b --- /dev/null +++ b/examples/apps/tree-cli-app/src/test/legacy/index.ts @@ -0,0 +1,9 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +// Using 'export *' in this file increases readability of relevant tests by grouping schema according to their versions. +// The usual concerns of export * around bundling do not apply. +/* eslint-disable no-restricted-syntax */ +export * as v1 from "./v1.js"; diff --git a/examples/apps/tree-cli-app/src/test/legacy/v1.ts b/examples/apps/tree-cli-app/src/test/legacy/v1.ts new file mode 100644 index 000000000000..a737f5303a87 --- /dev/null +++ b/examples/apps/tree-cli-app/src/test/legacy/v1.ts @@ -0,0 +1,13 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { SchemaFactory } from "@fluidframework/tree"; + +const schemaBuilder = new SchemaFactory("com.fluidframework.example.cli"); + +/** + * List node. + */ +export class List extends schemaBuilder.array("List", [schemaBuilder.string]) {} diff --git a/examples/apps/tree-cli-app/src/test/schema.spec.ts b/examples/apps/tree-cli-app/src/test/schema.spec.ts index c31245d6b653..22c63761a347 100644 --- a/examples/apps/tree-cli-app/src/test/schema.spec.ts +++ b/examples/apps/tree-cli-app/src/test/schema.spec.ts @@ -11,55 +11,16 @@ import { typeboxValidator, type ForestOptions, type ICodecOptions, + type ImplicitFieldSchema, type JsonCompatible, // eslint-disable-next-line import/no-internal-modules } from "@fluidframework/tree/alpha"; import { List } from "../schema.js"; -// This file demonstrates how applications can write tests which ensure they maintain compatibility with the schema from previously released versions. - -describe("schema", () => { - it("current schema matches latest historical schema", () => { - const current = extractPersistedSchema(List); - - // For compatibility with deep equality and simple objects, round trip via JSON to erase prototypes. - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const currentRoundTripped: JsonCompatible = JSON.parse(JSON.stringify(current)); - - const previous = historicalSchema.at(-1); - assert(previous !== undefined); - // This ensures that historicalSchema's last entry is up to date with the current application code. - // This can catch: - // 1. Forgetting to update historicalSchema when intentionally making schema changes. - // 2. Accidentally changing schema in a way that impacts document compatibility. - assert.deepEqual(currentRoundTripped, previous.schema); - }); - - it("historical schema can be upgraded to current schema", () => { - const options: ForestOptions & ICodecOptions = { jsonValidator: typeboxValidator }; - - for (let documentIndex = 0; documentIndex < historicalSchema.length; documentIndex++) { - for (let viewIndex = 0; viewIndex < historicalSchema.length; viewIndex++) { - const compat = comparePersistedSchema( - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - historicalSchema[documentIndex]!.schema, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - historicalSchema[viewIndex]!.schema, - options, - false, - ); +import { v1 } from "./legacy/index.js"; - // We do not expect duplicates in historicalSchema. - assert.equal(compat.isEquivalent, documentIndex === viewIndex); - // Currently collaboration is only allowed between identical versions - assert.equal(compat.canView, documentIndex === viewIndex); - // Older versions should be upgradable to newer versions, but not the reverse. - assert.equal(compat.canUpgrade, documentIndex <= viewIndex); - } - } - }); -}); +// This file demonstrates how applications can write tests which ensure they maintain compatibility with the schema from previously released versions. /** * List of schema from previous versions of this application. @@ -67,7 +28,11 @@ describe("schema", () => { * * The `schema` field is generated by passing the schema to `extractPersistedSchema`. */ -const historicalSchema: { version: string; schema: JsonCompatible }[] = [ +const historicalSchema: { + version: string; + schema: JsonCompatible; + viewSchema: ImplicitFieldSchema; +}[] = [ { version: "1.0", schema: { @@ -90,6 +55,7 @@ const historicalSchema: { version: string; schema: JsonCompatible }[] = [ types: ["com.fluidframework.example.cli.List"], }, }, + viewSchema: v1.List, }, { version: "2.0", @@ -140,5 +106,51 @@ const historicalSchema: { version: string; schema: JsonCompatible }[] = [ types: ["com.fluidframework.example.cli.List"], }, }, + viewSchema: List, }, ]; + +describe("schema", () => { + it("current schema matches latest historical schema", () => { + const current = extractPersistedSchema(List); + + // For compatibility with deep equality and simple objects, round trip via JSON to erase prototypes. + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const currentRoundTripped: JsonCompatible = JSON.parse(JSON.stringify(current)); + + const previous = historicalSchema.at(-1); + assert(previous !== undefined); + // This ensures that historicalSchema's last entry is up to date with the current application code. + // This can catch: + // 1. Forgetting to update historicalSchema when intentionally making schema changes. + // 2. Accidentally changing schema in a way that impacts document compatibility. + assert.deepEqual(currentRoundTripped, previous.schema); + }); + + describe("historical schema can be upgraded to current schema", () => { + const options: ForestOptions & ICodecOptions = { jsonValidator: typeboxValidator }; + + for (let documentIndex = 0; documentIndex < historicalSchema.length; documentIndex++) { + for (let viewIndex = 0; viewIndex < historicalSchema.length; viewIndex++) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + it(`document ${historicalSchema[documentIndex]!.version} vs view version ${historicalSchema[viewIndex]!.version}`, () => { + const compat = comparePersistedSchema( + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + historicalSchema[documentIndex]!.schema, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + historicalSchema[viewIndex]!.viewSchema, + options, + false, + ); + + // We do not expect duplicates in historicalSchema. + assert.equal(compat.isEquivalent, documentIndex === viewIndex); + // Currently collaboration is only allowed between identical versions + assert.equal(compat.canView, documentIndex === viewIndex); + // Older versions should be upgradable to newer versions, but not the reverse. + assert.equal(compat.canUpgrade, documentIndex <= viewIndex); + }); + } + } + }); +}); diff --git a/packages/dds/tree/api-report/tree.alpha.api.md b/packages/dds/tree/api-report/tree.alpha.api.md index 128b3b3bb851..21483a31fa1c 100644 --- a/packages/dds/tree/api-report/tree.alpha.api.md +++ b/packages/dds/tree/api-report/tree.alpha.api.md @@ -58,7 +58,7 @@ export interface CommitMetadata { } // @alpha -export function comparePersistedSchema(persisted: JsonCompatible, view: JsonCompatible, options: ICodecOptions, canInitialize: boolean): SchemaCompatibilityStatus; +export function comparePersistedSchema(persisted: JsonCompatible, view: ImplicitFieldSchema, options: ICodecOptions, canInitialize: boolean): SchemaCompatibilityStatus; // @alpha export type ConciseTree = Exclude | THandle | ConciseTree[] | { diff --git a/packages/dds/tree/src/core/index.ts b/packages/dds/tree/src/core/index.ts index a14043ce84d0..496e19aed616 100644 --- a/packages/dds/tree/src/core/index.ts +++ b/packages/dds/tree/src/core/index.ts @@ -207,7 +207,6 @@ export { export { type Adapters, AdaptedViewSchema, - Compatibility, type TreeAdapter, AllowedUpdateType, } from "./schema-view/index.js"; diff --git a/packages/dds/tree/src/core/schema-stored/schema.ts b/packages/dds/tree/src/core/schema-stored/schema.ts index 4e5acb0c565c..b81df693af59 100644 --- a/packages/dds/tree/src/core/schema-stored/schema.ts +++ b/packages/dds/tree/src/core/schema-stored/schema.ts @@ -92,6 +92,16 @@ export interface SchemaPolicy { * If true, new content inserted into the tree should be validated against the stored schema. */ readonly validateSchema: boolean; + + /** + * Whether to allow a document to be opened when a particular stored schema (identified by `identifier`) + * contains optional fields that are not known to the view schema. + * + * @privateRemarks + * Plumbing this in via `SchemaPolicy` avoids needing to walk the view schema representation repeatedly in places + * that need it (schema validation, view vs stored compatibility checks). + */ + allowUnknownOptionalFields(identifier: TreeNodeSchemaIdentifier): boolean; } /** diff --git a/packages/dds/tree/src/core/schema-view/index.ts b/packages/dds/tree/src/core/schema-view/index.ts index bab4be023d2a..22092eef4b96 100644 --- a/packages/dds/tree/src/core/schema-view/index.ts +++ b/packages/dds/tree/src/core/schema-view/index.ts @@ -5,7 +5,6 @@ export { type Adapters, - Compatibility, type TreeAdapter, AdaptedViewSchema, AllowedUpdateType, diff --git a/packages/dds/tree/src/core/schema-view/view.ts b/packages/dds/tree/src/core/schema-view/view.ts index f15dc2b3778e..7bc56008cb49 100644 --- a/packages/dds/tree/src/core/schema-view/view.ts +++ b/packages/dds/tree/src/core/schema-view/view.ts @@ -9,17 +9,6 @@ import type { TreeNodeSchemaIdentifier, TreeStoredSchema } from "../schema-store * APIs for applying `view schema` to documents. */ -/** - * How compatible a particular view schema is for some operation on some specific document. - */ -export enum Compatibility { - Incompatible, - // For write compatibility this can include compatible schema updates to stored schema. - // TODO: separate schema updates from adapters. - // RequiresAdapters, - Compatible, -} - /** * What kinds of updates to stored schema to permit. * diff --git a/packages/dds/tree/src/feature-libraries/default-schema/defaultSchema.ts b/packages/dds/tree/src/feature-libraries/default-schema/defaultSchema.ts index fb849d40ca6e..8da2bbd8a8d2 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/defaultSchema.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/defaultSchema.ts @@ -13,4 +13,5 @@ import { fieldKinds } from "./defaultFieldKinds.js"; export const defaultSchemaPolicy: FullSchemaPolicy = { fieldKinds, validateSchema: false, + allowUnknownOptionalFields: () => false, }; 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 6760c112b8ba..a47b18fa6e6a 100644 --- a/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts +++ b/packages/dds/tree/src/feature-libraries/default-schema/schemaChecker.ts @@ -73,7 +73,10 @@ export function isNodeInSchema( uncheckedFieldsFromNode.delete(fieldKey); } // The node has fields that we did not check as part of looking at every field defined in the node's schema - if (uncheckedFieldsFromNode.size !== 0) { + if ( + uncheckedFieldsFromNode.size !== 0 && + !schemaAndPolicy.policy.allowUnknownOptionalFields(node.type) + ) { return SchemaValidationErrors.ObjectNode_FieldNotInSchema; } } else if (schema instanceof MapNodeStoredSchema) { diff --git a/packages/dds/tree/src/feature-libraries/index.ts b/packages/dds/tree/src/feature-libraries/index.ts index cc3f3a0ae708..d3055fb3214d 100644 --- a/packages/dds/tree/src/feature-libraries/index.ts +++ b/packages/dds/tree/src/feature-libraries/index.ts @@ -76,7 +76,20 @@ export { type FieldKindConfigurationEntry, getAllowedContentDiscrepancies, isRepoSuperset, + type AllowedTypeDiscrepancy, + type FieldKindDiscrepancy, + type ValueSchemaDiscrepancy, + type FieldDiscrepancy, + type NodeDiscrepancy, + type NodeKindDiscrepancy, + type NodeFieldsDiscrepancy, isNeverTree, + type LinearExtension, + type Realizer, + fieldRealizer, + PosetComparisonResult, + comparePosetElements, + posetLte, } from "./modular-schema/index.js"; export { mapRootChanges } from "./deltaUtils.js"; diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/discrepancies.ts b/packages/dds/tree/src/feature-libraries/modular-schema/discrepancies.ts index 43213c9193f9..ccbe4dc86d83 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/discrepancies.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/discrepancies.ts @@ -6,6 +6,7 @@ import { assert } from "@fluidframework/core-utils/internal"; import { + type FieldKey, type FieldKindIdentifier, LeafNodeStoredSchema, MapNodeStoredSchema, @@ -25,6 +26,7 @@ import { brand } from "../../util/index.js"; // Rather than both existing, one of which just returns boolean and the other which returns additional details, a simple comparison which returns everything needed should be used. /** + * Discriminated union (keyed on `mismatch`) of discrepancies between a view and stored schema. * @remarks * * 1. FieldDiscrepancy @@ -60,47 +62,73 @@ export type Discrepancy = FieldDiscrepancy | NodeDiscrepancy; export type NodeDiscrepancy = NodeKindDiscrepancy | NodeFieldsDiscrepancy; +/** + * A discrepancy in the declaration of a field. + */ export type FieldDiscrepancy = | AllowedTypeDiscrepancy | FieldKindDiscrepancy | ValueSchemaDiscrepancy; -export interface AllowedTypeDiscrepancy { - identifier: string | undefined; // undefined indicates root field schema +/** + * Information about where a field discrepancy is located within a collection of schema. + */ +export interface FieldDiscrepancyLocation { + /** + * The {@link TreeNodeSchemaIdentifier} that contains the discrepancy. + * + * Undefined iff the discrepancy is part of the root field schema. + */ + identifier: TreeNodeSchemaIdentifier | undefined; + /** + * The {@link FieldKey} for the field that contains the discrepancy. + * Undefined when: + * - the discrepancy is part of the root field schema + * - the discrepancy is for 'all fields' of a map node + */ + fieldKey: FieldKey | undefined; +} + +/** + * A discrepancy in the allowed types of a field. + * + * @remarks + * This reports the symmetric difference of allowed types in view/stored to enable more efficient checks for compatibility + */ +export interface AllowedTypeDiscrepancy extends FieldDiscrepancyLocation { mismatch: "allowedTypes"; /** - * List of allowed type identifiers in viewed schema + * List of allowed type identifiers in viewed schema which are not allowed in stored schema */ - view: string[]; + view: TreeNodeSchemaIdentifier[]; /** - * List of allowed type identifiers in stored schema + * List of allowed type identifiers in stored schema which are not allowed in view schema */ - stored: string[]; + stored: TreeNodeSchemaIdentifier[]; } -export interface FieldKindDiscrepancy { - identifier: string | undefined; // undefined indicates root field schema +export interface FieldKindDiscrepancy extends FieldDiscrepancyLocation { mismatch: "fieldKind"; view: FieldKindIdentifier; stored: FieldKindIdentifier; } export interface ValueSchemaDiscrepancy { - identifier: string; + identifier: TreeNodeSchemaIdentifier; mismatch: "valueSchema"; view: ValueSchema | undefined; stored: ValueSchema | undefined; } export interface NodeKindDiscrepancy { - identifier: string; + identifier: TreeNodeSchemaIdentifier; mismatch: "nodeKind"; view: SchemaFactoryNodeKind | undefined; stored: SchemaFactoryNodeKind | undefined; } export interface NodeFieldsDiscrepancy { - identifier: string; + identifier: TreeNodeSchemaIdentifier; mismatch: "fields"; differences: FieldDiscrepancy[]; } @@ -121,26 +149,25 @@ function getNodeSchemaType(nodeSchema: TreeNodeStoredSchema): SchemaFactoryNodeK /** * Finds and reports discrepancies between a view schema and a stored schema. * - * The workflow for finding schema incompatibilities: - * 1. Compare the two root schemas to identify any `FieldDiscrepancy`. - * - * 2. For each node schema in the `view`: - * - Verify if the node schema exists in the stored. If it does, ensure that the `SchemaFactoryNodeKind` are - * consistent. Otherwise this difference is treated as `NodeKindDiscrepancy` - * - If a node schema with the same identifier exists in both view and stored, and their `SchemaFactoryNodeKind` - * are consistent, perform a exhaustive validation to identify all `FieldDiscrepancy`. - * - * 3. For each node schema in the stored, verify if it exists in the view. The overlapping parts were already - * addressed in the previous step. + * See documentation on {@link Discrepancy} for details of possible discrepancies. + * @remarks + * This function does not attempt to distinguish between equivalent representations of a node/field involving extraneous never trees. + * For example, a Forbidden field with allowed type set `[]` is equivalent to an optional field with allowed type set `[]`, + * as well as an optional field with an allowed type set containing only unconstructable types. * - * @returns the discrepancies between two TreeStoredSchema objects + * It is up to the caller to determine whether such discrepancies matter. */ export function* getAllowedContentDiscrepancies( view: TreeStoredSchema, stored: TreeStoredSchema, ): Iterable { // check root schema discrepancies - yield* getFieldDiscrepancies(view.rootFieldSchema, stored.rootFieldSchema); + yield* getFieldDiscrepancies( + view.rootFieldSchema, + stored.rootFieldSchema, + undefined, + undefined, + ); for (const result of compareMaps(view.nodeSchema, stored.nodeSchema)) { switch (result.type) { @@ -195,6 +222,7 @@ function* getNodeDiscrepancies( case "object": { const differences = Array.from( trackObjectNodeDiscrepancies( + identifier, view as ObjectNodeStoredSchema, stored as ObjectNodeStoredSchema, ), @@ -213,6 +241,7 @@ function* getNodeDiscrepancies( (view as MapNodeStoredSchema).mapFields, (stored as MapNodeStoredSchema).mapFields, identifier, + undefined, ); break; case "leaf": { @@ -241,7 +270,8 @@ function* getNodeDiscrepancies( function* getFieldDiscrepancies( view: TreeFieldStoredSchema, stored: TreeFieldStoredSchema, - keyOrRoot?: string, + identifier: TreeNodeSchemaIdentifier | undefined, + fieldKey: FieldKey | undefined, ): Iterable { // Only track the symmetric differences of two sets. const findSetDiscrepancies = ( @@ -256,7 +286,8 @@ function* getFieldDiscrepancies( const [viewExtra, storedExtra] = findSetDiscrepancies(view.types, stored.types); if (viewExtra.length > 0 || storedExtra.length > 0) { yield { - identifier: keyOrRoot, + identifier, + fieldKey, mismatch: "allowedTypes", view: viewExtra, stored: storedExtra, @@ -265,7 +296,8 @@ function* getFieldDiscrepancies( if (view.kind !== stored.kind) { yield { - identifier: keyOrRoot, + identifier, + fieldKey, mismatch: "fieldKind", view: view.kind, stored: stored.kind, @@ -274,6 +306,7 @@ function* getFieldDiscrepancies( } function* trackObjectNodeDiscrepancies( + identifier: TreeNodeSchemaIdentifier, view: ObjectNodeStoredSchema, stored: ObjectNodeStoredSchema, ): Iterable { @@ -298,7 +331,8 @@ function* trackObjectNodeDiscrepancies( break; } yield { - identifier: fieldKey, + identifier, + fieldKey, mismatch: "fieldKind", view: result.value.kind, stored: storedEmptyFieldSchema.kind, @@ -312,7 +346,8 @@ function* trackObjectNodeDiscrepancies( break; } yield { - identifier: fieldKey, + identifier, + fieldKey, mismatch: "fieldKind", view: storedEmptyFieldSchema.kind, stored: result.value.kind, @@ -320,12 +355,11 @@ function* trackObjectNodeDiscrepancies( break; } case "both": { - yield* getFieldDiscrepancies(result.valueA, result.valueB, fieldKey); + yield* getFieldDiscrepancies(result.valueA, result.valueB, identifier, fieldKey); break; } - default: { + default: break; - } } } } @@ -428,13 +462,13 @@ function isFieldDiscrepancyCompatible(discrepancy: FieldDiscrepancy): boolean { * * The linear extension is represented as a lookup from each poset element to its index in the linear extension. */ -type LinearExtension = Map; +export type LinearExtension = Map; /** * A realizer for a partially-ordered set. See: * https://en.wikipedia.org/wiki/Order_dimension */ -type Realizer = LinearExtension[]; +export type Realizer = LinearExtension[]; /** * @privateRemarks @@ -468,7 +502,7 @@ const FieldKindIdentifiers = { * identifier * ``` */ -const fieldRealizer: Realizer = [ +export const fieldRealizer: Realizer = [ [ FieldKindIdentifiers.forbidden, FieldKindIdentifiers.identifier, @@ -485,7 +519,7 @@ const fieldRealizer: Realizer = [ ], ].map((extension) => new Map(extension.map((identifier, index) => [identifier, index]))); -const PosetComparisonResult = { +export const PosetComparisonResult = { Less: "<", Greater: ">", Equal: "=", @@ -494,7 +528,11 @@ const PosetComparisonResult = { type PosetComparisonResult = (typeof PosetComparisonResult)[keyof typeof PosetComparisonResult]; -function comparePosetElements(a: T, b: T, realizer: Realizer): PosetComparisonResult { +export function comparePosetElements( + a: T, + b: T, + realizer: Realizer, +): PosetComparisonResult { let hasLessThanResult = false; let hasGreaterThanResult = false; for (const extension of realizer) { @@ -517,7 +555,7 @@ function comparePosetElements(a: T, b: T, realizer: Realizer): PosetCompar : PosetComparisonResult.Equal; } -function posetLte(a: T, b: T, realizer: Realizer): boolean { +export function posetLte(a: T, b: T, realizer: Realizer): boolean { const comparison = comparePosetElements(a, b, realizer); return ( comparison === PosetComparisonResult.Less || comparison === PosetComparisonResult.Equal diff --git a/packages/dds/tree/src/feature-libraries/modular-schema/index.ts b/packages/dds/tree/src/feature-libraries/modular-schema/index.ts index 8378915b1208..a13179d02c28 100644 --- a/packages/dds/tree/src/feature-libraries/modular-schema/index.ts +++ b/packages/dds/tree/src/feature-libraries/modular-schema/index.ts @@ -77,4 +77,17 @@ export type { export { getAllowedContentDiscrepancies, isRepoSuperset, + type AllowedTypeDiscrepancy, + type FieldKindDiscrepancy, + type ValueSchemaDiscrepancy, + type FieldDiscrepancy, + type NodeDiscrepancy, + type NodeKindDiscrepancy, + type NodeFieldsDiscrepancy, + type LinearExtension, + type Realizer, + fieldRealizer, + PosetComparisonResult, + comparePosetElements, + posetLte, } from "./discrepancies.js"; diff --git a/packages/dds/tree/src/shared-tree/schematizeTree.ts b/packages/dds/tree/src/shared-tree/schematizeTree.ts index 233f55e5aa95..6ac813393303 100644 --- a/packages/dds/tree/src/shared-tree/schematizeTree.ts +++ b/packages/dds/tree/src/shared-tree/schematizeTree.ts @@ -7,7 +7,6 @@ import { assert, unreachableCase } from "@fluidframework/core-utils/internal"; import { AllowedUpdateType, - Compatibility, CursorLocationType, type ITreeCursorSynchronous, type TreeStoredSchema, @@ -24,7 +23,7 @@ import { import { fail, isReadonlyArray } from "../util/index.js"; import type { ITreeCheckout } from "./treeCheckout.js"; -import type { ViewSchema } from "../simple-tree/index.js"; +import { toStoredSchema, type ViewSchema } from "../simple-tree/index.js"; /** * Modify `storedSchema` and invoke `setInitialTree` when it's time to set the tree content. @@ -120,10 +119,7 @@ export function evaluateUpdate( ): UpdateType { const compatibility = viewSchema.checkCompatibility(checkout.storedSchema); - if ( - compatibility.read === Compatibility.Compatible && - compatibility.write === Compatibility.Compatible - ) { + if (compatibility.canUpgrade && compatibility.canView) { // Compatible as is return UpdateType.None; } @@ -133,13 +129,13 @@ export function evaluateUpdate( return UpdateType.Initialize; } - if (compatibility.read !== Compatibility.Compatible) { + if (!compatibility.canUpgrade) { // Existing stored schema permits trees which are incompatible with the view schema, so schema can not be updated return UpdateType.Incompatible; } - assert(compatibility.write === Compatibility.Incompatible, 0x8bd /* unexpected case */); - assert(compatibility.read === Compatibility.Compatible, 0x8be /* unexpected case */); + assert(!compatibility.canView, 0x8bd /* unexpected case */); + assert(compatibility.canUpgrade, 0x8be /* unexpected case */); // eslint-disable-next-line no-bitwise return allowedSchemaModifications & AllowedUpdateType.SchemaCompatible @@ -244,7 +240,7 @@ export function ensureSchema( return false; } case UpdateType.SchemaCompatible: { - checkout.updateSchema(viewSchema.schema); + checkout.updateSchema(toStoredSchema(viewSchema.schema)); return true; } case UpdateType.Initialize: { diff --git a/packages/dds/tree/src/shared-tree/schematizingTreeView.ts b/packages/dds/tree/src/shared-tree/schematizingTreeView.ts index bb49861c7276..5abd224c83d7 100644 --- a/packages/dds/tree/src/shared-tree/schematizingTreeView.ts +++ b/packages/dds/tree/src/shared-tree/schematizingTreeView.ts @@ -12,12 +12,7 @@ import { createEmitter } from "@fluid-internal/client-utils"; import { assert } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; -import { - AllowedUpdateType, - anchorSlot, - Compatibility, - type SchemaPolicy, -} from "../core/index.js"; +import { AllowedUpdateType, anchorSlot, type SchemaPolicy } from "../core/index.js"; import { type NodeKeyManager, defaultSchemaPolicy, @@ -58,7 +53,9 @@ import { HydratedContext, SimpleContextSlot, areImplicitFieldSchemaEqual, + createUnknownOptionalFieldPolicy, } from "../simple-tree/index.js"; + /** * Creating multiple tree views from the same checkout is not supported. This slot is used to detect if one already * exists and error if creating a second. @@ -116,14 +113,14 @@ export class SchematizingSimpleTreeView< } checkout.forest.anchors.slots.set(ViewSlot, this); - const policy = { + this.rootFieldSchema = normalizeFieldSchema(config.schema); + this.schemaPolicy = { ...defaultSchemaPolicy, validateSchema: config.enableSchemaValidation, + allowUnknownOptionalFields: createUnknownOptionalFieldPolicy(this.rootFieldSchema), }; - this.rootFieldSchema = normalizeFieldSchema(config.schema); - this.schemaPolicy = defaultSchemaPolicy; - this.viewSchema = new ViewSchema(policy, {}, toStoredSchema(this.rootFieldSchema)); + this.viewSchema = new ViewSchema(this.schemaPolicy, {}, this.rootFieldSchema); // This must be initialized before `update` can be called. this.currentCompatibility = { canView: false, @@ -166,16 +163,13 @@ export class SchematizingSimpleTreeView< this.nodeKeyManager, { schema: this.checkout.storedSchema, - policy: { - ...defaultSchemaPolicy, - validateSchema: this.config.enableSchemaValidation, - }, + policy: this.schemaPolicy, }, ); prepareContentForHydration(mapTree, this.checkout.forest); initialize(this.checkout, { - schema: this.viewSchema.schema, + schema: toStoredSchema(this.viewSchema.schema), initialTree: mapTree === undefined ? undefined : cursorForMapTreeNode(mapTree), }); }); @@ -202,7 +196,7 @@ export class SchematizingSimpleTreeView< AllowedUpdateType.SchemaCompatible, this.checkout, { - schema: this.viewSchema.schema, + schema: toStoredSchema(this.viewSchema.schema), initialTree: undefined, }, ); @@ -432,11 +426,7 @@ export function requireSchema( { const compatibility = viewSchema.checkCompatibility(checkout.storedSchema); - assert( - compatibility.write === Compatibility.Compatible && - compatibility.read === Compatibility.Compatible, - 0x8c3 /* requireSchema invoked with incompatible schema */, - ); + assert(compatibility.canView, 0x8c3 /* requireSchema invoked with incompatible schema */); } const view = new CheckoutFlexTreeView(checkout, schemaPolicy, nodeKeyManager, onDispose); diff --git a/packages/dds/tree/src/simple-tree/api/create.ts b/packages/dds/tree/src/simple-tree/api/create.ts index d2e962024b3f..100e011c402d 100644 --- a/packages/dds/tree/src/simple-tree/api/create.ts +++ b/packages/dds/tree/src/simple-tree/api/create.ts @@ -32,6 +32,7 @@ import { isFieldInSchema } from "../../feature-libraries/index.js"; import { toStoredSchema } from "../toStoredSchema.js"; import { inSchemaOrThrow, mapTreeFromNodeData } from "../toMapTree.js"; import { getUnhydratedContext } from "../createContext.js"; +import { createUnknownOptionalFieldPolicy } from "../objectNode.js"; /** * Construct tree content that is compatible with the field defined by the provided `schema`. @@ -122,7 +123,10 @@ export function createFromCursor( const flexSchema = context.flexContext.schema; const schemaValidationPolicy: SchemaAndPolicy = { - policy: defaultSchemaPolicy, + policy: { + ...defaultSchemaPolicy, + allowUnknownOptionalFields: createUnknownOptionalFieldPolicy(schema), + }, schema: context.flexContext.schema, }; diff --git a/packages/dds/tree/src/simple-tree/api/index.ts b/packages/dds/tree/src/simple-tree/api/index.ts index 83fda6bf3356..be037d6dce38 100644 --- a/packages/dds/tree/src/simple-tree/api/index.ts +++ b/packages/dds/tree/src/simple-tree/api/index.ts @@ -17,7 +17,11 @@ export { type TreeBranchEvents, asTreeViewAlpha, } from "./tree.js"; -export { SchemaFactory, type ScopedSchemaName } from "./schemaFactory.js"; +export { + SchemaFactory, + type ScopedSchemaName, + type SchemaFactoryObjectOptions, +} from "./schemaFactory.js"; export type { ValidateRecursiveSchema, FixRecursiveArraySchema, diff --git a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts index 8a8efaf86628..ea79af26a0bc 100644 --- a/packages/dds/tree/src/simple-tree/api/schemaFactory.ts +++ b/packages/dds/tree/src/simple-tree/api/schemaFactory.ts @@ -18,6 +18,9 @@ import { getOrCreate, isReadonlyArray, } from "../../util/index.js"; +// This import is required for intellisense in @link doc comments on mouseover in VSCode. +// eslint-disable-next-line unused-imports/no-unused-imports, @typescript-eslint/no-unused-vars +import type { TreeAlpha } from "../../shared-tree/index.js"; import { booleanSchema, @@ -74,6 +77,7 @@ import type { import { createFieldSchemaUnsafe } from "./schemaFactoryRecursive.js"; import { TreeNodeValid } from "../treeNodeValid.js"; import { isLazy } from "../flexList.js"; + /** * Gets the leaf domain schema compatible with a given {@link TreeValue}. */ @@ -97,6 +101,58 @@ export function schemaFromValue(value: TreeValue): TreeNodeSchema { } } +/** + * Options when declaring an {@link SchemaFactory.object|object node}'s schema + */ +export interface SchemaFactoryObjectOptions { + /** + * Opt in to viewing documents which have unknown optional fields for this object's identifier, i.e. optional fields + * in the document schema which are not present in this object schema declaration. + * + * @defaultValue `false` + * @remarks + * The advantage of enabling this option is that it allows an application ecosystem with staged rollout to more quickly + * upgrade documents to include schema for new optional features. + * + * However, it does come with trade-offs that applications should weigh carefully when it comes to interactions between + * code and documents. + * When opening such documents, the API presented is still determined by the view schema. + * This can have implications on the behavior of edits or code which uses portions of the view schema, + * since this may inadvertently drop data which is present in those optional fields in the document schema. + * + * Consider the following example: + * + * ```typescript + * const sf = new SchemaFactory("com.example"); + * class PersonView extends sf.object("Person", { name: sf.string }, { allowUnknownOptionalFields: true }) {} + * class PersonStored extends sf.object("Person", { name: sf.string, nickname: sf.optional(sf.string) }) {} + * + * // Say we have a document which uses `PersonStored` in its schema, and application code constructs + * // a tree view using `PersonView`. If the application for some reason had implemented a function like this: + * function clonePerson(a: PersonView): PersonView { + * return new PersonView({ name: a.name }); + * } + * // ...or even like this: + * function clonePerson(a: PersonView): PersonView { + * return new PersonView({ ...a}) + * } + * // Then the alleged clone wouldn't actually clone the entire person in either case, it would drop the nickname. + * ``` + * + * If an application wants to be particularly careful to preserve all data on a node when editing it, it can use + * {@link TreeAlpha.(importVerbose:2)|import}/{@link TreeAlpha.(exportVerbose:2)|export} APIs with persistent keys. + * + * Note that public API methods which operate on entire nodes (such as `moveTo`, `moveToEnd`, etc. on arrays) do not encounter + * this problem as SharedTree's implementation stores the entire node in its lower layers. It's only when application code + * reaches into a node (either by accessing its fields, spreading it, or some other means) that this problem arises. + */ + allowUnknownOptionalFields?: boolean; +} + +export const defaultSchemaFactoryObjectOptions: Required = { + allowUnknownOptionalFields: false, +}; + /** * The name of a schema produced by {@link SchemaFactory}, including its optional scope prefix. * @@ -333,7 +389,12 @@ export class SchemaFactory< true, T > { - return objectSchema(this.scoped(name), fields, true); + return objectSchema( + this.scoped(name), + fields, + true, + defaultSchemaFactoryObjectOptions.allowUnknownOptionalFields, + ); } /** diff --git a/packages/dds/tree/src/simple-tree/api/storedSchema.ts b/packages/dds/tree/src/simple-tree/api/storedSchema.ts index 7a9fe7353b89..e26802c6a324 100644 --- a/packages/dds/tree/src/simple-tree/api/storedSchema.ts +++ b/packages/dds/tree/src/simple-tree/api/storedSchema.ts @@ -4,7 +4,7 @@ */ import type { ICodecOptions } from "../../codec/index.js"; -import { Compatibility, type TreeStoredSchema } from "../../core/index.js"; +import type { TreeStoredSchema } from "../../core/index.js"; import { defaultSchemaPolicy, encodeTreeSchema, @@ -59,7 +59,7 @@ export function extractPersistedSchema(schema: ImplicitFieldSchema): JsonCompati * opening a document that used the `persisted` schema and provided `view` to {@link ViewableTree.viewWith}. * * @param persisted - Schema persisted for a document. Typically persisted alongside the data and assumed to describe that data. - * @param view - Schema which would be used to view persisted content. Use {@link extractPersistedSchema} to convert the view schema into this format. + * @param view - Schema which would be used to view persisted content. * @param options - {@link ICodecOptions} used when parsing the provided schema. * @param canInitialize - Passed through to the return value unchanged and otherwise unused. * @returns The {@link SchemaCompatibilityStatus} a {@link TreeView} would report for this combination of schema. @@ -75,7 +75,7 @@ export function extractPersistedSchema(schema: ImplicitFieldSchema): JsonCompati * assert( * comparePersistedSchema( * require("./schema.json"), - * extractPersistedSchema(MySchema), + * MySchema, * { jsonValidator: typeboxValidator }, * false, * ).canUpgrade, @@ -85,14 +85,13 @@ export function extractPersistedSchema(schema: ImplicitFieldSchema): JsonCompati */ export function comparePersistedSchema( persisted: JsonCompatible, - view: JsonCompatible, + view: ImplicitFieldSchema, options: ICodecOptions, canInitialize: boolean, ): SchemaCompatibilityStatus { const schemaCodec = makeSchemaCodec(options); const stored = schemaCodec.decode(persisted as Format); - const viewParsed = schemaCodec.decode(view as Format); - const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, viewParsed); + const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, view); return comparePersistedSchemaInternal(stored, viewSchema, canInitialize); } @@ -105,22 +104,8 @@ export function comparePersistedSchemaInternal( viewSchema: ViewSchema, canInitialize: boolean, ): SchemaCompatibilityStatus { - const result = viewSchema.checkCompatibility(stored); - - // TODO: AB#8121: Weaken this check to support viewing under additional circumstances. - // In the near term, this should support viewing documents with additional optional fields in their schema on object types. - // Longer-term (as demand arises), we could also add APIs to constructing view schema to allow for more flexibility - // (e.g. out-of-schema content handlers could allow support for viewing docs which have extra allowed types in a particular field) - const canView = - result.write === Compatibility.Compatible && result.read === Compatibility.Compatible; - const canUpgrade = result.read === Compatibility.Compatible; - const isEquivalent = canView && canUpgrade; - const compatibility: SchemaCompatibilityStatus = { - canView, - canUpgrade, - isEquivalent, + return { + ...viewSchema.checkCompatibility(stored), canInitialize, }; - - return compatibility; } diff --git a/packages/dds/tree/src/simple-tree/api/tree.ts b/packages/dds/tree/src/simple-tree/api/tree.ts index 9dba1ea95b10..529b1312b19e 100644 --- a/packages/dds/tree/src/simple-tree/api/tree.ts +++ b/packages/dds/tree/src/simple-tree/api/tree.ts @@ -12,6 +12,10 @@ import type { RevertibleFactory, } from "../../core/index.js"; +// This is referenced by doc comments. +// eslint-disable-next-line @typescript-eslint/no-unused-vars, unused-imports/no-unused-imports +import type { TreeAlpha } from "../../shared-tree/index.js"; + import { type ImplicitFieldSchema, type InsertableField, @@ -31,6 +35,7 @@ import { markSchemaMostDerived } from "./schemaFactory.js"; import { fail, getOrCreate } from "../../util/index.js"; import type { MakeNominal } from "../../util/index.js"; import { walkFieldSchema } from "../walkFieldSchema.js"; + /** * A tree from which a {@link TreeView} can be created. * diff --git a/packages/dds/tree/src/simple-tree/api/view.ts b/packages/dds/tree/src/simple-tree/api/view.ts index 108066ef76d5..4cde4a003932 100644 --- a/packages/dds/tree/src/simple-tree/api/view.ts +++ b/packages/dds/tree/src/simple-tree/api/view.ts @@ -5,32 +5,56 @@ import { AdaptedViewSchema, + type TreeNodeStoredSchema, type Adapters, - Compatibility, type TreeFieldStoredSchema, type TreeNodeSchemaIdentifier, - type TreeNodeStoredSchema, type TreeStoredSchema, } from "../../core/index.js"; import { fail } from "../../util/index.js"; import { + FieldKinds, type FullSchemaPolicy, - allowsRepoSuperset, + type FieldDiscrepancy, + getAllowedContentDiscrepancies, isNeverTree, + PosetComparisonResult, + fieldRealizer, + comparePosetElements, } from "../../feature-libraries/index.js"; +import { + normalizeFieldSchema, + type FieldSchema, + type ImplicitFieldSchema, +} from "../schemaTypes.js"; +import { toStoredSchema } from "../toStoredSchema.js"; +import { unreachableCase } from "@fluidframework/core-utils/internal"; +import type { SchemaCompatibilityStatus } from "./tree.js"; /** * A collection of View information for schema, including policy. */ export class ViewSchema { /** - * @param schema - Cached conversion of view schema in the stored schema format. + * Cached conversion of the view schema in the stored schema format. + */ + private readonly viewSchemaAsStored: TreeStoredSchema; + /** + * Normalized view schema (implicitly allowed view schema types are converted to their canonical form). + */ + public readonly schema: FieldSchema; + + /** + * @param viewSchema - Schema for the root field of this view. */ public constructor( public readonly policy: FullSchemaPolicy, public readonly adapters: Adapters, - public readonly schema: TreeStoredSchema, - ) {} + viewSchema: ImplicitFieldSchema, + ) { + this.schema = normalizeFieldSchema(viewSchema); + this.viewSchemaAsStored = toStoredSchema(this.schema); + } /** * Determines the compatibility of a stored document @@ -42,53 +66,168 @@ export class ViewSchema { * TODO: this API violates the parse don't validate design philosophy. * It should be wrapped with (or replaced by) a parse style API. */ - public checkCompatibility(stored: TreeStoredSchema): { - read: Compatibility; - write: Compatibility; - writeAllowingStoredSchemaUpdates: Compatibility; - } { + public checkCompatibility( + stored: TreeStoredSchema, + ): Omit { // TODO: support adapters // const adapted = this.adaptRepo(stored); - const read = allowsRepoSuperset(this.policy, stored, this.schema) - ? Compatibility.Compatible - : // TODO: support adapters - // : allowsRepoSuperset(this.policy, adapted.adaptedForViewSchema, this.storedSchema) - // ? Compatibility.RequiresAdapters - Compatibility.Incompatible; - // TODO: Extract subset of adapters that are valid to use on stored - // TODO: separate adapters from schema updates - const write = allowsRepoSuperset(this.policy, this.schema, stored) - ? Compatibility.Compatible - : // TODO: support adapters - // : allowsRepoSuperset(this.policy, this.storedSchema, adapted.adaptedForViewSchema) - // TODO: IThis assumes adapters are bidirectional. - // Compatibility.RequiresAdapters - Compatibility.Incompatible; - - // TODO: compute this properly (and maybe include the set of schema changes needed for it?). - // Maybe updates would happen lazily when needed to store data? - // When willingness to updates can avoid need for some adapters, - // how should it be decided if the adapter should be used to avoid the update? - // TODO: is this case actually bi-variant, making this correct if we did it for each schema independently? - let writeAllowingStoredSchemaUpdates = - // TODO: This should consider just the updates needed - // (ex: when view covers a subset of stored after stored has a update to that subset). - allowsRepoSuperset(this.policy, stored, this.schema) - ? Compatibility.Compatible - : // TODO: this assumes adapters can translate in both directions. In general this will not be true. - // TODO: this also assumes that schema updates to the adapted repo would translate to - // updates on the stored schema, which is also likely untrue. - // // TODO: support adapters - // allowsRepoSuperset(this.policy, adapted.adaptedForViewSchema, this.storedSchema) - // ? Compatibility.RequiresAdapters // Requires schema updates. TODO: consider adapters that can update writes. - Compatibility.Incompatible; - - // Since the above does not consider partial updates, - // we can improve the tolerance a bit by considering the op-op update: - writeAllowingStoredSchemaUpdates = Math.max(writeAllowingStoredSchemaUpdates, write); - - return { read, write, writeAllowingStoredSchemaUpdates }; + // View schema allows a subset of documents that stored schema does, and the discrepancies are allowed by policy + // determined by the view schema (i.e. objects with extra optional fields in the stored schema have opted into allowing this. + // In the future, this would also include things like: + // - fields with more allowed types in the stored schema than in the view schema have out-of-schema "unknown content" adapters + let canView = true; + // View schema allows a superset of documents that stored schema does, hence the document could be upgraded to use a persisted version + // of this view schema as its stored schema. + let canUpgrade = true; + + const updateCompatibilityFromFieldDiscrepancy = (discrepancy: FieldDiscrepancy): void => { + switch (discrepancy.mismatch) { + case "allowedTypes": { + // Since we only track the symmetric difference between the allowed types in the view and + // stored schemas, it's sufficient to check if any extra allowed types still exist in the + // stored schema. + if ( + discrepancy.stored.some( + (identifier) => + !isNeverTree(this.policy, stored, stored.nodeSchema.get(identifier)), + ) + ) { + // Stored schema has extra allowed types that the view schema does not. + canUpgrade = false; + canView = false; + } + + if ( + discrepancy.view.some( + (identifier) => + !isNeverTree( + this.policy, + this.viewSchemaAsStored, + this.viewSchemaAsStored.nodeSchema.get(identifier), + ), + ) + ) { + // View schema has extra allowed types that the stored schema does not. + canView = false; + } + break; + } + case "fieldKind": { + const result = comparePosetElements( + discrepancy.stored, + discrepancy.view, + fieldRealizer, + ); + + if (result === PosetComparisonResult.Greater) { + // Stored schema is more relaxed than view schema. + canUpgrade = false; + if ( + discrepancy.view === FieldKinds.forbidden.identifier && + discrepancy.identifier !== undefined && + this.policy.allowUnknownOptionalFields(discrepancy.identifier) + ) { + // When the application has opted into it, we allow viewing documents which have additional + // optional fields in the stored schema that are not present in the view schema. + } else { + canView = false; + } + } + + if (result === PosetComparisonResult.Less) { + // View schema is more relaxed than stored schema. + canView = false; + } + + if (result === PosetComparisonResult.Incomparable) { + canUpgrade = false; + canView = false; + } + + break; + } + case "valueSchema": { + canView = false; + canUpgrade = false; + break; + } + default: + unreachableCase(discrepancy); + } + }; + + for (const discrepancy of getAllowedContentDiscrepancies( + this.viewSchemaAsStored, + stored, + )) { + if (!canView && !canUpgrade) { + break; + } + + switch (discrepancy.mismatch) { + case "nodeKind": { + const viewNodeSchema = this.viewSchemaAsStored.nodeSchema.get( + discrepancy.identifier, + ); + const storedNodeSchema = stored.nodeSchema.get(discrepancy.identifier); + // We conservatively do not allow node types to change. + // The only time this might be valid in the sense that the data canonically converts is converting an object node + // to a map node over the union of all the object fields' types. + if (discrepancy.stored === undefined) { + const viewIsNever = + viewNodeSchema !== undefined + ? isNeverTree(this.policy, this.viewSchemaAsStored, viewNodeSchema) + : true; + if (!viewIsNever) { + // View schema has added a node type that the stored schema doesn't know about. + canView = false; + } + } else if (discrepancy.view === undefined) { + const storedIsNever = + storedNodeSchema !== undefined + ? isNeverTree(this.policy, stored, storedNodeSchema) + : true; + if (!storedIsNever) { + // Stored schema has a node type that the view schema doesn't know about. + canUpgrade = false; + } + } else { + // Node type exists in both schemas but has changed. We conservatively never allow this. + const storedIsNever = + storedNodeSchema !== undefined + ? isNeverTree(this.policy, stored, storedNodeSchema) + : true; + const viewIsNever = + viewNodeSchema !== undefined + ? isNeverTree(this.policy, this.viewSchemaAsStored, viewNodeSchema) + : true; + if (!storedIsNever || !viewIsNever) { + canView = false; + canUpgrade = false; + } + } + break; + } + case "valueSchema": + case "allowedTypes": + case "fieldKind": { + updateCompatibilityFromFieldDiscrepancy(discrepancy); + break; + } + case "fields": { + discrepancy.differences.forEach(updateCompatibilityFromFieldDiscrepancy); + break; + } + // No default + } + } + + return { + canView, + canUpgrade, + isEquivalent: canView && canUpgrade, + }; } /** @@ -102,8 +241,15 @@ export class ViewSchema { // since there never is a reason to have a never type as an adapter input, // and its impossible for an adapter to be correctly implemented if its output type is never // (unless its input is also never). + for (const adapter of this.adapters?.tree ?? []) { - if (isNeverTree(this.policy, this.schema, this.schema.nodeSchema.get(adapter.output))) { + if ( + isNeverTree( + this.policy, + this.viewSchemaAsStored, + this.viewSchemaAsStored.nodeSchema.get(adapter.output), + ) + ) { fail("tree adapter for stored adapter.output should not be never"); } } diff --git a/packages/dds/tree/src/simple-tree/index.ts b/packages/dds/tree/src/simple-tree/index.ts index 708def9ac6bd..fe762268e014 100644 --- a/packages/dds/tree/src/simple-tree/index.ts +++ b/packages/dds/tree/src/simple-tree/index.ts @@ -35,6 +35,7 @@ export { type SchemaCompatibilityStatus, type ITreeConfigurationOptions, SchemaFactory, + type SchemaFactoryObjectOptions, type ScopedSchemaName, type ValidateRecursiveSchema, type FixRecursiveArraySchema, @@ -170,6 +171,7 @@ export { type ObjectFromSchemaRecord, type TreeObjectNode, setField, + createUnknownOptionalFieldPolicy, } from "./objectNode.js"; export type { TreeMapNode, MapNodeInsertableData } from "./mapNode.js"; export { diff --git a/packages/dds/tree/src/simple-tree/objectNode.ts b/packages/dds/tree/src/simple-tree/objectNode.ts index 5d692dc6d13a..db62bb46eecb 100644 --- a/packages/dds/tree/src/simple-tree/objectNode.ts +++ b/packages/dds/tree/src/simple-tree/objectNode.ts @@ -6,7 +6,7 @@ import { assert, Lazy } from "@fluidframework/core-utils/internal"; import { UsageError } from "@fluidframework/telemetry-utils/internal"; -import type { FieldKey } from "../core/index.js"; +import type { FieldKey, SchemaPolicy } from "../core/index.js"; import { FieldKinds, type FlexTreeField, @@ -42,7 +42,11 @@ import { } from "./core/index.js"; import { mapTreeFromNodeData, type InsertableContent } from "./toMapTree.js"; import { type RestrictiveStringRecord, fail, type FlattenKeys } from "../util/index.js"; -import type { ObjectNodeSchema, ObjectNodeSchemaInternalData } from "./objectNodeTypes.js"; +import { + isObjectNodeSchema, + type ObjectNodeSchema, + type ObjectNodeSchemaInternalData, +} from "./objectNodeTypes.js"; import { TreeNodeValid, type MostDerivedData } from "./treeNodeValid.js"; import { getUnhydratedContext } from "./createContext.js"; @@ -330,6 +334,7 @@ export function objectSchema< identifier: TName, info: T, implicitlyConstructable: ImplicitlyConstructable, + allowUnknownOptionalFields: boolean, ): ObjectNodeSchema & ObjectNodeSchemaInternalData { // Ensure no collisions between final set of property keys, and final set of stored keys (including those // implicitly derived from property keys) @@ -368,6 +373,7 @@ export function objectSchema< ]), ); public static readonly identifierFieldKeys: readonly FieldKey[] = identifierFieldKeys; + public static readonly allowUnknownOptionalFields: boolean = allowUnknownOptionalFields; public static override prepareInstance( this: typeof TreeNodeValid, @@ -509,3 +515,21 @@ function assertUniqueKeys< derivedStoredKeys.add(storedKey); } } + +/** + * Creates a policy for allowing unknown optional fields on an object node which delegates to the policy defined + * on the object node's internal schema data. + */ +export function createUnknownOptionalFieldPolicy( + schema: ImplicitFieldSchema, +): SchemaPolicy["allowUnknownOptionalFields"] { + const context = getUnhydratedContext(schema); + return (identifier) => { + const storedSchema = context.schema.get(identifier); + return ( + storedSchema !== undefined && + isObjectNodeSchema(storedSchema) && + storedSchema.allowUnknownOptionalFields + ); + }; +} diff --git a/packages/dds/tree/src/simple-tree/objectNodeTypes.ts b/packages/dds/tree/src/simple-tree/objectNodeTypes.ts index 58743b6f1bf7..acf1ab5bb510 100644 --- a/packages/dds/tree/src/simple-tree/objectNodeTypes.ts +++ b/packages/dds/tree/src/simple-tree/objectNodeTypes.ts @@ -55,6 +55,11 @@ export interface ObjectNodeSchemaInternalData { * Stored keys which hold identifiers. */ readonly identifierFieldKeys: readonly FieldKey[]; + + /** + * Whether to tolerate (and preserve) additional unknown optional fields in instances of this object node. + */ + readonly allowUnknownOptionalFields: boolean; } export const ObjectNodeSchema = { diff --git a/packages/dds/tree/src/simple-tree/toStoredSchema.ts b/packages/dds/tree/src/simple-tree/toStoredSchema.ts index a657c96ec712..eaff11f9beb3 100644 --- a/packages/dds/tree/src/simple-tree/toStoredSchema.ts +++ b/packages/dds/tree/src/simple-tree/toStoredSchema.ts @@ -20,7 +20,7 @@ import { type TreeTypeSet, } from "../core/index.js"; import { FieldKinds, type FlexFieldKind } from "../feature-libraries/index.js"; -import { brand, fail, isReadonlyArray } from "../util/index.js"; +import { brand, fail, getOrCreate, isReadonlyArray } from "../util/index.js"; import { NodeKind, type TreeNodeSchema } from "./core/index.js"; import { FieldKind, @@ -33,29 +33,35 @@ import { LeafNodeSchema } from "./leafNodeSchema.js"; import { isObjectNodeSchema } from "./objectNodeTypes.js"; import { normalizeFlexListEager } from "./flexList.js"; +const viewToStoredCache = new WeakMap(); + /** * Converts a {@link ImplicitFieldSchema} into a {@link TreeStoredSchema}. */ export function toStoredSchema(root: ImplicitFieldSchema): TreeStoredSchema { - const nodeSchema: Map = new Map(); - walkFieldSchema(root, { - node(schema) { - if (nodeSchema.has(brand(schema.identifier))) { - // Use JSON.stringify to quote and escape identifier string. - throw new UsageError( - `Multiple schema encountered with the identifier ${JSON.stringify( - schema.identifier, - )}. Remove or rename them to avoid the collision.`, - ); - } - nodeSchema.set(brand(schema.identifier), getStoredSchema(schema)); - }, - }); + return getOrCreate(viewToStoredCache, root, () => { + const nodeSchema: Map = new Map(); + walkFieldSchema(root, { + node(schema) { + if (nodeSchema.has(brand(schema.identifier))) { + // Use JSON.stringify to quote and escape identifier string. + throw new UsageError( + `Multiple schema encountered with the identifier ${JSON.stringify( + schema.identifier, + )}. Remove or rename them to avoid the collision.`, + ); + } + nodeSchema.set(brand(schema.identifier), getStoredSchema(schema)); + }, + }); - return { - nodeSchema, - rootFieldSchema: convertField(root), - }; + const result: TreeStoredSchema = { + nodeSchema, + rootFieldSchema: convertField(root), + }; + viewToStoredCache.set(root, result); + return result; + }); } /** diff --git a/packages/dds/tree/src/test/feature-libraries/default-schema/schemaChecker.spec.ts b/packages/dds/tree/src/test/feature-libraries/default-schema/schemaChecker.spec.ts index 92d5018e2cba..09955f2cda64 100644 --- a/packages/dds/tree/src/test/feature-libraries/default-schema/schemaChecker.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/default-schema/schemaChecker.spec.ts @@ -55,6 +55,8 @@ function createSchemaAndPolicy( // Note: the value of 'validateSchema' doesn't matter for the tests in this file because they're testing a // layer where we already decided that we are doing validation and are validating that it works correctly. validateSchema: true, + // TODO: unit test other options in this file + allowUnknownOptionalFields: () => false, }, }; } diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/discrepancies.spec.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/discrepancies.spec.ts index 2c25effee617..588c8aee8107 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/discrepancies.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/discrepancies.spec.ts @@ -216,18 +216,21 @@ describe("Schema Discrepancies", () => { [ { identifier: undefined, + fieldKey: undefined, mismatch: "fieldKind", view: "Optional", stored: "Value", }, { identifier: testTreeNodeIdentifier, + fieldKey: undefined, mismatch: "allowedTypes", view: [], stored: ["string"], }, { identifier: testTreeNodeIdentifier, + fieldKey: undefined, mismatch: "fieldKind", view: "Value", stored: "Optional", @@ -240,6 +243,7 @@ describe("Schema Discrepancies", () => { [ { identifier: testTreeNodeIdentifier, + fieldKey: undefined, mismatch: "allowedTypes", view: ["number"], stored: ["array"], @@ -349,19 +353,22 @@ describe("Schema Discrepancies", () => { mismatch: "fields", differences: [ { - identifier: "x", + fieldKey: "x", + identifier: testTreeNodeIdentifier, mismatch: "allowedTypes", view: ["number"], stored: ["string"], }, { - identifier: "x", + fieldKey: "x", + identifier: testTreeNodeIdentifier, mismatch: "fieldKind", view: "Value", stored: "Optional", }, { - identifier: "y", + fieldKey: "y", + identifier: testTreeNodeIdentifier, mismatch: "fieldKind", view: "Forbidden", stored: "Optional", @@ -443,6 +450,7 @@ describe("Schema Discrepancies", () => { assert.deepEqual(Array.from(getAllowedContentDiscrepancies(neverTree, mapNodeSchema)), [ { identifier: testTreeNodeIdentifier, + fieldKey: undefined, mismatch: "allowedTypes", view: [], stored: ["number"], @@ -457,7 +465,8 @@ describe("Schema Discrepancies", () => { mismatch: "fields", differences: [ { - identifier: "x", + identifier: testTreeNodeIdentifier, + fieldKey: "x", mismatch: "allowedTypes", view: [], stored: ["number"], @@ -498,7 +507,8 @@ describe("Schema Discrepancies", () => { mismatch: "fields", differences: [ { - identifier: "x", + identifier: testTreeNodeIdentifier, + fieldKey: "x", mismatch: "fieldKind", view: "Forbidden", stored: "Value", @@ -516,13 +526,15 @@ describe("Schema Discrepancies", () => { mismatch: "fields", differences: [ { - identifier: "x", + identifier: testTreeNodeIdentifier, + fieldKey: "x", mismatch: "allowedTypes", view: [], stored: ["number"], }, { - identifier: "x", + identifier: testTreeNodeIdentifier, + fieldKey: "x", mismatch: "fieldKind", view: "Forbidden", stored: "Value", diff --git a/packages/dds/tree/src/test/feature-libraries/modular-schema/schemaEvolutionExamples.spec.ts b/packages/dds/tree/src/test/feature-libraries/modular-schema/schemaEvolutionExamples.spec.ts index 7ee6f2c484b0..6bf1451d36d1 100644 --- a/packages/dds/tree/src/test/feature-libraries/modular-schema/schemaEvolutionExamples.spec.ts +++ b/packages/dds/tree/src/test/feature-libraries/modular-schema/schemaEvolutionExamples.spec.ts @@ -7,7 +7,6 @@ import { strict as assert } from "node:assert"; import { type Adapters, - Compatibility, EmptyKey, type TreeFieldStoredSchema, type TreeNodeSchemaIdentifier, @@ -116,14 +115,10 @@ describe("Schema Evolution Examples", () => { * (since adapters are not implemented yet, and they are the nice way to handle that). */ it("basic usage", () => { - // Collect our view schema. - // This will represent our view schema for a simple canvas application. - const viewCollection = toStoredSchema(root); - // This is where legacy schema handling logic for schematize. const adapters: Adapters = {}; // Compose all the view information together. - const view = new ViewSchema(defaultSchemaPolicy, adapters, viewCollection); + const view = new ViewSchema(defaultSchemaPolicy, adapters, root); // Now lets imagine using this application on a new empty document. // TreeStoredSchemaRepository defaults to a state that permits no document states at all. @@ -138,18 +133,8 @@ describe("Schema Evolution Examples", () => { // Sadly for our application, we did not allow empty roots in our view schema, // nor did we provide an adapter capable of handling empty roots. // This means our application is unable to view this document. - assertEnumEqual(Compatibility, compat.read, Compatibility.Incompatible); - - // And since the view schema currently excludes empty roots, its also incompatible for writing: - assertEnumEqual(Compatibility, compat.write, Compatibility.Incompatible); - - // Additionally even updating the document schema can't save this app, - // since the new schema would be incompatible with the existing document content. - assertEnumEqual( - Compatibility, - compat.writeAllowingStoredSchemaUpdates, - Compatibility.Incompatible, - ); + // And since the view schema currently excludes empty roots, its also incompatible for upgrading: + assert.deepEqual(compat, { canView: false, canUpgrade: false, isEquivalent: false }); // This is where the app would inform the user that the document // is not compatible with their version of the application. @@ -165,25 +150,16 @@ describe("Schema Evolution Examples", () => { // This example picks the first approach. // Lets simulate the developers of the app making this change by modifying the view schema: - const viewCollection2 = toStoredSchema(tolerantRoot); - const view2 = new ViewSchema(defaultSchemaPolicy, adapters, viewCollection2); + const view2 = new ViewSchema(defaultSchemaPolicy, adapters, tolerantRoot); // When we open this document, we should check it's compatibility with our application: const compat = view2.checkCompatibility(stored); // The adjusted view schema can be used read this document, no adapters needed. - assertEnumEqual(Compatibility, compat.read, Compatibility.Compatible); + assert.equal(compat.canUpgrade, true); // However the document just has its empty root schema, // so the app could make changes that could not be written back. - assertEnumEqual(Compatibility, compat.write, Compatibility.Incompatible); - - // However, it is possible to update the schema in the document to match our schema - // (since the existing data in compatible with our schema) - assertEnumEqual( - Compatibility, - compat.writeAllowingStoredSchemaUpdates, - Compatibility.Compatible, - ); + assert.equal(compat.canView, false); // The app can consider this compatible and proceed if it is ok with updating schema on write. // There are a few approaches apps might want to take here, but we will assume one that seems reasonable: @@ -212,11 +188,12 @@ describe("Schema Evolution Examples", () => { // which will notify and applications with the document open. // They can recheck their compatibility: const compatNew = view2.checkCompatibility(stored); - const report = Array.from(getAllowedContentDiscrepancies(viewCollection2, stored)); + const report = Array.from( + getAllowedContentDiscrepancies(toStoredSchema(tolerantRoot), stored), + ); assert.deepEqual(report, []); - assertEnumEqual(Compatibility, compatNew.read, Compatibility.Compatible); // It is now possible to write our date into the document. - assertEnumEqual(Compatibility, compatNew.write, Compatibility.Compatible); + assert.deepEqual(compatNew, { canView: true, canUpgrade: true, isEquivalent: true }); // Now lets imagine some time passes, and the developers want to add a second content type: @@ -231,18 +208,11 @@ describe("Schema Evolution Examples", () => { // And canvas is still the same storage wise, but its view schema references the updated positionedCanvasItem2: const canvas2 = builder.array("Canvas", positionedCanvasItem2); // Once again we will simulate reloading the app with different schema by modifying the view schema. - const viewCollection3 = toStoredSchema(builder.optional(canvas2)); - const view3 = new ViewSchema(defaultSchemaPolicy, adapters, viewCollection3); + const view3 = new ViewSchema(defaultSchemaPolicy, adapters, builder.optional(canvas2)); // With this new schema, we can load the document just like before: const compat2 = view3.checkCompatibility(stored); - assertEnumEqual(Compatibility, compat2.read, Compatibility.Compatible); - assertEnumEqual(Compatibility, compat2.write, Compatibility.Incompatible); - assertEnumEqual( - Compatibility, - compat2.writeAllowingStoredSchemaUpdates, - Compatibility.Compatible, - ); + assert.deepEqual(compat2, { canView: false, canUpgrade: true, isEquivalent: false }); // This is the same case as above where we can choose to do a schema update if we want: assert(stored.tryUpdateTreeSchema(positionedCanvasItem2)); @@ -250,12 +220,11 @@ describe("Schema Evolution Examples", () => { // And recheck compat: const compat3 = view3.checkCompatibility(stored); - assertEnumEqual(Compatibility, compat3.read, Compatibility.Compatible); - assertEnumEqual(Compatibility, compat3.write, Compatibility.Compatible); + assert.deepEqual(compat3, { canView: true, canUpgrade: true, isEquivalent: true }); } }); - // TODO: support adapters + // TODO: support adapters. // function makeTolerantRootAdapter(view: TreeStoredSchema): FieldAdapter { // return { diff --git a/packages/dds/tree/src/test/schemaFactoryAlpha.ts b/packages/dds/tree/src/test/schemaFactoryAlpha.ts new file mode 100644 index 000000000000..14b3a8ce95ca --- /dev/null +++ b/packages/dds/tree/src/test/schemaFactoryAlpha.ts @@ -0,0 +1,90 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import type { ScopedSchemaName, InsertableObjectFromSchemaRecord } from "../internalTypes.js"; +import { + SchemaFactory, + type SchemaFactoryObjectOptions, + type TreeNodeSchemaClass, + type NodeKind, + type Unenforced, + type ImplicitFieldSchema, +} from "../simple-tree/index.js"; +// eslint-disable-next-line import/no-internal-modules +import { defaultSchemaFactoryObjectOptions } from "../simple-tree/api/schemaFactory.js"; +// eslint-disable-next-line import/no-internal-modules +import { type TreeObjectNode, objectSchema } from "../simple-tree/objectNode.js"; +import type { RestrictiveStringRecord } from "../util/index.js"; + +/** + * Copy of {@link SchemaFactory} with additional alpha APIs. + * + * @privateRemarks + * Not currently exported to the public API surface as doing so produces errors in API-extractor. + * + * Can be removed once additional object node features are deemed stable and on the base class. + */ +export class SchemaFactoryAlpha< + out TScope extends string | undefined = string | undefined, + TName extends number | string = string, +> extends SchemaFactory { + // TS has trouble with subclassing schema factory and produces errors in the definition of objectRecursive without + // explicit type annotations saying the return type is "the same as the parent class". There's not a great way to do + // that AFAICT without using an instantiation expression, but `super` is unsupported in such expressions. + private readonly baseKludge: SchemaFactory = this; + private scoped2(name: Name): ScopedSchemaName { + return ( + this.scope === undefined ? `${name}` : `${this.scope}.${name}` + ) as ScopedSchemaName; + } + + /** + * Define a {@link TreeNodeSchemaClass} for a {@link TreeObjectNode}. + * + * @param name - Unique identifier for this schema within this factory's scope. + * @param fields - Schema for fields of the object node's schema. Defines what children can be placed under each key. + */ + public override object< + const Name extends TName, + const T extends RestrictiveStringRecord, + >( + name: Name, + fields: T, + options?: SchemaFactoryObjectOptions, + ): TreeNodeSchemaClass< + ScopedSchemaName, + NodeKind.Object, + TreeObjectNode>, + object & InsertableObjectFromSchemaRecord, + true, + T + > { + return objectSchema( + this.scoped2(name), + fields, + true, + options?.allowUnknownOptionalFields ?? + defaultSchemaFactoryObjectOptions.allowUnknownOptionalFields, + ); + } + + /** + * {@inheritdoc} + */ + public override objectRecursive< + const Name extends TName, + const T extends Unenforced>, + >( + name: Name, + t: T, + options?: SchemaFactoryObjectOptions, + ): ReturnType> { + return this.object( + name, + t as T & RestrictiveStringRecord, + options, + ) as unknown as ReturnType>; + } +} diff --git a/packages/dds/tree/src/test/shared-tree/schematizeTree.spec.ts b/packages/dds/tree/src/test/shared-tree/schematizeTree.spec.ts index b5d415101257..cd97df01a0a1 100644 --- a/packages/dds/tree/src/test/shared-tree/schematizeTree.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/schematizeTree.spec.ts @@ -15,7 +15,7 @@ import { TreeStoredSchemaRepository, type AnchorSetRootEvents, } from "../../core/index.js"; -import { singleJsonCursor } from "../json/index.js"; +import { JsonUnion, singleJsonCursor } from "../json/index.js"; import { FieldKinds, allowsRepoSuperset, @@ -47,18 +47,17 @@ import { } from "../../simple-tree/index.js"; // eslint-disable-next-line import/no-internal-modules import { toStoredSchema } from "../../simple-tree/toStoredSchema.js"; -import { jsonSequenceRootSchema } from "../sequenceRootUtils.js"; import type { Transactor } from "../../shared-tree-core/index.js"; const builder = new SchemaFactory("test"); const root = builder.number; -const schema = toStoredSchema(root); +const schema = root; -const schemaGeneralized = toStoredSchema(builder.optional([root, builder.string])); -const schemaValueRoot = toStoredSchema([root, builder.string]); +const schemaGeneralized = builder.optional([root, builder.string]); +const schemaValueRoot = [root, builder.string]; // Schema for tree that must always be empty. -const emptySchema = toStoredSchema(builder.optional([])); +const emptySchema = builder.optional([]); function expectSchema(actual: TreeStoredSchema, expected: TreeStoredSchema): void { // Check schema match @@ -139,15 +138,24 @@ describe("schematizeTree", () => { }); } - testInitialize("optional-empty", { schema, initialTree: undefined }); - testInitialize("optional-full", { schema, initialTree: singleJsonCursor(5) }); - testInitialize("value", { schema: schemaValueRoot, initialTree: singleJsonCursor(6) }); + testInitialize("optional-empty", { + schema: toStoredSchema(schema), + initialTree: undefined, + }); + testInitialize("optional-full", { + schema: toStoredSchema(schema), + initialTree: singleJsonCursor(5), + }); + testInitialize("value", { + schema: toStoredSchema(schemaValueRoot), + initialTree: singleJsonCursor(6), + }); // TODO: Test schema validation of initial tree (once we have a utility for it) }); - function mockCheckout(InputSchema: TreeStoredSchema, isEmpty: boolean): ITreeCheckout { - const storedSchema = new TreeStoredSchemaRepository(InputSchema); + function mockCheckout(InputSchema: ImplicitFieldSchema, isEmpty: boolean): ITreeCheckout { + const storedSchema = new TreeStoredSchemaRepository(toStoredSchema(InputSchema)); const checkout: ITreeCheckout = { storedSchema, // eslint-disable-next-line @typescript-eslint/consistent-type-assertions @@ -185,13 +193,13 @@ describe("schematizeTree", () => { describe("evaluateUpdate", () => { describe("test cases", () => { - const testCases: [string, TreeStoredSchema, boolean][] = [ + const testCases: [string, ImplicitFieldSchema, boolean][] = [ ["empty", emptySchema, true], ["basic-optional-empty", schema, true], ["basic-optional", schema, false], ["basic-value", schemaValueRoot, false], - ["complex-empty", jsonSequenceRootSchema, true], - ["complex", jsonSequenceRootSchema, false], + ["complex-empty", JsonUnion, true], + ["complex", builder.arrayRecursive("root", JsonUnion), false], ]; for (const [name, data, isEmpty] of testCases) { it(name, () => { @@ -248,7 +256,7 @@ describe("schematizeTree", () => { describe("ensureSchema", () => { it("compatible empty schema", () => { const checkout = checkoutWithContent({ - schema: emptySchema, + schema: toStoredSchema(emptySchema), initialTree: undefined, }); const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, emptySchema); @@ -256,22 +264,22 @@ describe("schematizeTree", () => { }); it("initialize optional root", () => { - const emptyContent = { - schema: emptySchema, + const emptyContent: TreeStoredContent = { + schema: toStoredSchema(emptySchema), initialTree: undefined, }; const emptyCheckout = checkoutWithContent(emptyContent); const content: TreeStoredContent = { - schema: schemaGeneralized, + schema: toStoredSchema(schemaGeneralized), initialTree: singleJsonCursor(5), }; const initializedCheckout = checkoutWithContent(content); // Schema upgraded, but content not initialized const upgradedCheckout = checkoutWithContent({ - schema: schemaGeneralized, + schema: toStoredSchema(schemaGeneralized), initialTree: undefined, }); - const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, content.schema); + const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, schemaGeneralized); // Non updating cases { @@ -338,18 +346,18 @@ describe("schematizeTree", () => { }); it("initialize required root", () => { - const emptyContent = { - schema: emptySchema, + const emptyContent: TreeStoredContent = { + schema: toStoredSchema(emptySchema), initialTree: undefined, }; const emptyCheckout = checkoutWithContent(emptyContent); const content: TreeStoredContent = { - schema: schemaValueRoot, + schema: toStoredSchema(schemaValueRoot), initialTree: singleJsonCursor(5), }; const initializedCheckout = checkoutWithContent(content); - const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, content.schema); + const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, schemaValueRoot); // Non updating cases { @@ -398,23 +406,23 @@ describe("schematizeTree", () => { }); it("update non-empty", () => { - const initialContent = { - schema, + const initialContent: TreeStoredContent = { + schema: toStoredSchema(schema), get initialTree() { return singleJsonCursor(5); }, }; const initialCheckout = checkoutWithContent(initialContent); const content: TreeStoredContent = { - schema: schemaGeneralized, + schema: toStoredSchema(schemaGeneralized), initialTree: singleJsonCursor("Should not be used"), }; const updatedCheckout = checkoutWithContent({ - schema: schemaGeneralized, + schema: toStoredSchema(schemaGeneralized), initialTree: initialContent.initialTree, }); - const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, content.schema); + const viewSchema = new ViewSchema(defaultSchemaPolicy, {}, schemaGeneralized); // Non updating case { 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 7858c7f5e5c1..64e52c64e444 100644 --- a/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/schematizingTreeView.spec.ts @@ -29,6 +29,7 @@ import { } from "../utils.js"; import { insert } from "../sequenceRootUtils.js"; import type { TreeCheckout, TreeStoredContent } from "../../shared-tree/index.js"; +import { SchemaFactoryAlpha } from "../schemaFactoryAlpha.js"; const schema = new SchemaFactory("com.example"); const config = new TreeViewConfiguration({ schema: schema.number }); @@ -144,8 +145,6 @@ describe("SchematizingSimpleTreeView", () => { assert.deepEqual(log, [["rootChanged", 6]]); }); - // TODO: AB#8121: When adding support for additional optional fields, we may want a variant of this test which does the analogous flow using - // an intermediate state where canView is true but canUpgrade is false. it("Schema becomes un-upgradeable then exact match again", () => { const checkout = checkoutWithInitialTree(config, 5); const view = new SchematizingSimpleTreeView(checkout, config, new MockNodeKeyManager()); @@ -180,6 +179,140 @@ describe("SchematizingSimpleTreeView", () => { view.dispose(); }); + it("Open document whose stored schema has additional optional fields", () => { + // This sort of scenario might be reasonably encountered when an "older" version of an application opens + // up a document that has been created and/or edited by a "newer" version of an application (which has + // expanded the schema to include more information). + const factory = new SchemaFactoryAlpha(undefined); + class PersonGeneralized extends factory.object("Person", { + name: factory.string, + age: factory.number, + address: factory.optional(factory.string), + }) {} + class PersonSpecific extends factory.object( + "Person", + { + name: factory.string, + age: factory.number, + }, + { allowUnknownOptionalFields: true }, + ) {} + + const personConfig = new TreeViewConfiguration({ + schema: PersonSpecific, + }); + const personConfigGeneralied = new TreeViewConfiguration({ + schema: PersonGeneralized, + }); + const checkout = checkoutWithInitialTree( + personConfigGeneralied, + new PersonGeneralized({ name: "Alice", age: 42, address: "123 Main St" }), + ); + const viewSpecific = new SchematizingSimpleTreeView( + checkout, + personConfig, + new MockNodeKeyManager(), + ); + + assert.deepEqual(viewSpecific.compatibility, { + canView: true, + canUpgrade: false, + isEquivalent: false, + canInitialize: false, + }); + + assert.equal(Object.keys(viewSpecific.root).length, 2); + assert.equal(Object.entries(viewSpecific.root).length, 2); + assert.equal(viewSpecific.root.name, "Alice"); + assert.equal(viewSpecific.root.age, 42); + + viewSpecific.dispose(); + const viewGeneralized = new SchematizingSimpleTreeView( + checkout, + personConfigGeneralied, + new MockNodeKeyManager(), + ); + assert.deepEqual(viewGeneralized.compatibility, { + canView: true, + canUpgrade: true, + isEquivalent: true, + canInitialize: false, + }); + assert.equal(Object.keys(viewGeneralized.root).length, 3); + assert.equal(Object.entries(viewGeneralized.root).length, 3); + assert.equal(viewGeneralized.root.name, "Alice"); + assert.equal(viewGeneralized.root.age, 42); + assert.equal(viewGeneralized.root.address, "123 Main St"); + }); + + it("Calling moveToEnd on a more specific schema preserves a node's optional fields that were unknown to that schema", () => { + const factorySpecific = new SchemaFactoryAlpha(undefined); + const factoryGeneral = new SchemaFactoryAlpha(undefined); + class PersonGeneralized extends factorySpecific.object("Person", { + name: factoryGeneral.string, + age: factoryGeneral.number, + address: factoryGeneral.optional(factoryGeneral.string), + }) {} + class PersonSpecific extends factorySpecific.object( + "Person", + { + name: factorySpecific.string, + age: factorySpecific.number, + }, + { allowUnknownOptionalFields: true }, + ) {} + + const peopleConfig = new TreeViewConfiguration({ + schema: factorySpecific.array(PersonSpecific), + }); + const peopleGeneralizedConfig = new TreeViewConfiguration({ + schema: factoryGeneral.array(PersonGeneralized), + }); + const checkout = checkoutWithInitialTree(peopleGeneralizedConfig, [ + new PersonGeneralized({ name: "Alice", age: 42, address: "123 Main St" }), + new PersonGeneralized({ name: "Bob", age: 24 }), + ]); + const viewSpecific = new SchematizingSimpleTreeView( + checkout, + peopleConfig, + new MockNodeKeyManager(), + ); + + assert.deepEqual(viewSpecific.compatibility, { + canView: true, + canUpgrade: false, + isEquivalent: false, + canInitialize: false, + }); + + viewSpecific.root.moveRangeToEnd(0, 1); + + // To the view that doesn't have "address" in its schema, the node appears as if it doesn't + // have an address... + assert.equal(Object.keys(viewSpecific.root[1]).length, 2); + assert.equal(viewSpecific.root[1].name, "Alice"); + assert.equal(viewSpecific.root[1].age, 42); + viewSpecific.dispose(); + + const viewGeneralized = new SchematizingSimpleTreeView( + checkout, + peopleGeneralizedConfig, + new MockNodeKeyManager(), + ); + assert.deepEqual(viewGeneralized.compatibility, { + canView: true, + canUpgrade: true, + isEquivalent: true, + canInitialize: false, + }); + + // ...however, despite that client making an edit to Alice, the field is preserved via the move APIs. + assert.equal(Object.keys(viewGeneralized.root[1]).length, 3); + assert.equal(viewGeneralized.root[1].name, "Alice"); + assert.equal(viewGeneralized.root[1].age, 42); + assert.equal(viewGeneralized.root[1].address, "123 Main St"); + }); + it("Open upgradable document, then upgrade schema", () => { const checkout = checkoutWithInitialTree(config, 5); const view = new SchematizingSimpleTreeView( diff --git a/packages/dds/tree/src/test/simple-tree/api/storedSchema.spec.ts b/packages/dds/tree/src/test/simple-tree/api/storedSchema.spec.ts index ad911b7d7bee..e7fa6bde1535 100644 --- a/packages/dds/tree/src/test/simple-tree/api/storedSchema.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/storedSchema.spec.ts @@ -27,10 +27,9 @@ describe("simple-tree storedSchema", () => { // but might as will give it a simple smoke test for the various test schema. it(`comparePersistedSchema to self ${test.name}`, () => { const persistedA = extractPersistedSchema(test.schema); - const persistedB = extractPersistedSchema(test.schema); const status = comparePersistedSchema( persistedA, - persistedB, + test.schema, { jsonValidator: typeboxValidator, }, diff --git a/packages/dds/tree/src/test/simple-tree/api/treeApi.spec.ts b/packages/dds/tree/src/test/simple-tree/api/treeApi.spec.ts index 72de90a45980..f8dd3d15fc7a 100644 --- a/packages/dds/tree/src/test/simple-tree/api/treeApi.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/api/treeApi.spec.ts @@ -46,6 +46,7 @@ import { testSimpleTrees } from "../../testTrees.js"; import { FluidClientVersion } from "../../../codec/index.js"; import { ajvValidator } from "../../codec/index.js"; import { TreeAlpha } from "../../../shared-tree/index.js"; +import { SchemaFactoryAlpha } from "../../schemaFactoryAlpha.js"; const schema = new SchemaFactory("com.example"); @@ -1301,6 +1302,36 @@ describe("treeNodeApi", () => { }); } } + + describe("with misaligned view and stored schema", () => { + it("does not preserve additional optional fields", () => { + // (because stored keys are not being used, see analogous test in roundtrip-stored) + const sf1 = new SchemaFactoryAlpha("com.example"); + const sf2 = new SchemaFactoryAlpha("com.example"); + class Point2D extends sf1.object( + "Point", + { + x: sf1.number, + y: sf1.number, + }, + { allowUnknownOptionalFields: true }, + ) {} + class Point3D extends sf2.object("Point", { + x: sf2.number, + y: sf2.number, + z: sf2.optional(sf2.number), + }) {} + + 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. + assert.throws( + () => TreeAlpha.importVerbose(Point2D, exported), + /missing field info/, + ); + }); + }); }); describe("roundtrip-stored", () => { @@ -1320,6 +1351,59 @@ describe("treeNodeApi", () => { }); } } + + describe("with misaligned view and stored schema", () => { + const sf1 = new SchemaFactoryAlpha("com.example"); + class Point3D extends sf1.object("Point", { + x: sf1.number, + y: sf1.number, + z: sf1.optional(sf1.number), + }) {} + + it("preserves additional allowed optional fields", () => { + const sf2 = new SchemaFactoryAlpha("com.example"); + + class Point2D extends sf2.object( + "Point", + { + x: sf2.number, + y: sf2.number, + }, + { allowUnknownOptionalFields: true }, + ) {} + const testTree = new Point3D({ x: 1, y: 2, z: 3 }); + const exported = TreeAlpha.exportVerbose(testTree, { useStoredKeys: true }); + const imported = TreeAlpha.importVerbose(Point2D, exported, { useStoredKeys: true }); + const exported2 = TreeAlpha.exportVerbose(imported, { useStoredKeys: true }); + const imported2 = TreeAlpha.importVerbose(Point3D, exported2, { + useStoredKeys: true, + }); + assert.deepEqual(exported, exported2); + assert.deepEqual(Object.keys(imported), ["x", "y"]); + assert.deepEqual(Object.keys(imported2), ["x", "y", "z"]); + assert.equal(imported2.z, 3); + }); + + it("errors on additional disallowed optional fields", () => { + const sf2 = new SchemaFactoryAlpha("com.example"); + + class Point2D extends sf2.object( + "Point", + { + x: sf2.number, + y: sf2.number, + }, + { allowUnknownOptionalFields: false }, + ) {} + const testTree = new Point3D({ x: 1, y: 2, z: 3 }); + const exported = TreeAlpha.exportVerbose(testTree, { useStoredKeys: true }); + + assert.throws( + () => TreeAlpha.importVerbose(Point2D, exported, { useStoredKeys: true }), + /Tree does not conform to schema./, + ); + }); + }); }); }); diff --git a/packages/dds/tree/src/test/simple-tree/schemaTypes.spec.ts b/packages/dds/tree/src/test/simple-tree/schemaTypes.spec.ts index eff7319dd90a..2e6fb0d55938 100644 --- a/packages/dds/tree/src/test/simple-tree/schemaTypes.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/schemaTypes.spec.ts @@ -227,7 +227,7 @@ describe("schemaTypes", () => { } // Class that implements both TreeNodeSchemaNonClass and TreeNodeSchemaNonClass - class CustomizedBoth extends objectSchema("B", { x: [schema.number] }, true) { + class CustomizedBoth extends objectSchema("B", { x: [schema.number] }, true, false) { public customized = true; } diff --git a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts index 983f730ec153..e0b9f37e9c1f 100644 --- a/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts +++ b/packages/dds/tree/src/test/simple-tree/toMapTree.spec.ts @@ -1294,6 +1294,9 @@ describe("toMapTree", () => { policy: { fieldKinds, validateSchema: true, + // toMapTree drops all extra fields, so varying this policy is unnecessary + // (schema validation only occurs after converting to a MapTree) + allowUnknownOptionalFields: () => false, }, }; } @@ -1400,6 +1403,20 @@ describe("toMapTree", () => { outOfSchemaExpectedError, ); }); + + it("Only imports data in the schema", () => { + const schemaValidationPolicy = createSchemaAndPolicyForObjectNode(); + // Note that despite the content containing keys not in the object schema, this test passes. + // This is by design: if an app author wants to preserve data that isn't in the schema (ex: to + // collaborate with other clients that have newer schema without erasing auxiliary data), they + // can use import/export tree APIs as noted in `SchemaFactoryObjectOptions`. + mapTreeFromNodeData( + { foo: "Hello world", notInSchemaKey: 5, anotherNotInSchemaKey: false }, + [myObjectSchema, schemaFactory.string], + new MockNodeKeyManager(), + schemaValidationPolicy, + ); + }); }); describe("Map node", () => { diff --git a/packages/dds/tree/src/test/simple-tree/viewSchema.spec.ts b/packages/dds/tree/src/test/simple-tree/viewSchema.spec.ts new file mode 100644 index 000000000000..5f0cdd33928f --- /dev/null +++ b/packages/dds/tree/src/test/simple-tree/viewSchema.spec.ts @@ -0,0 +1,369 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "node:assert"; +import { + storedEmptyFieldSchema, + type Adapters, + type TreeStoredSchema, +} from "../../core/index.js"; +import { defaultSchemaPolicy, type FullSchemaPolicy } from "../../feature-libraries/index.js"; +import { + toStoredSchema, + ViewSchema, + type ImplicitFieldSchema, + type SchemaCompatibilityStatus, +} from "../../simple-tree/index.js"; +import { createUnknownOptionalFieldPolicy } from "../../simple-tree/index.js"; +import { SchemaFactoryAlpha } from "../schemaFactoryAlpha.js"; + +const noAdapters: Adapters = {}; +const emptySchema: TreeStoredSchema = { + nodeSchema: new Map(), + rootFieldSchema: storedEmptyFieldSchema, +}; + +const factory = new SchemaFactoryAlpha(""); + +function expectCompatibility( + { view, stored }: { view: ImplicitFieldSchema; stored: TreeStoredSchema }, + expected: ReturnType, + policy: FullSchemaPolicy = { + ...defaultSchemaPolicy, + allowUnknownOptionalFields: createUnknownOptionalFieldPolicy(view), + }, +) { + const viewSchema = new ViewSchema(policy, noAdapters, view); + const compatibility = viewSchema.checkCompatibility(stored); + assert.deepEqual(compatibility, expected); +} + +describe("viewSchema", () => { + describe(".checkCompatibility", () => { + it("works with never trees", () => { + class NeverObject extends factory.objectRecursive("NeverObject", { + foo: factory.requiredRecursive([() => NeverObject]), + }) {} + + const neverField = factory.required([]); + expectCompatibility( + { view: NeverObject, stored: emptySchema }, + { canView: false, canUpgrade: false, isEquivalent: false }, + ); + + expectCompatibility( + { view: neverField, stored: emptySchema }, + { canView: false, canUpgrade: false, isEquivalent: false }, + ); + + // We could reasonably detect these cases as equivalent and update the test expectation here. + // Doing so would amount to normalizing optional fields to forbidden fields when they do not + // contain any constructible types. + // Until we have a use case for it, we can leave it as is (i.e. be stricter with compatibility + // in cases that realistic users probably won't encounter). + expectCompatibility( + { view: factory.optional(NeverObject), stored: emptySchema }, + { canView: false, canUpgrade: true, isEquivalent: false }, + ); + expectCompatibility( + { view: factory.optional([]), stored: emptySchema }, + { canView: false, canUpgrade: true, isEquivalent: false }, + ); + }); + + describe("recognizes identical schema as equivalent", () => { + function expectSelfEquivalent(view: ImplicitFieldSchema) { + expectCompatibility( + { view, stored: toStoredSchema(view) }, + { canView: true, canUpgrade: true, isEquivalent: true }, + ); + } + it("empty schema", () => { + expectSelfEquivalent(factory.optional([])); + expectSelfEquivalent(factory.required([])); + }); + + it("object", () => { + expectSelfEquivalent( + factory.object("foo", { x: factory.number, y: factory.number, baz: factory.string }), + ); + }); + + it("map", () => { + expectSelfEquivalent(factory.map("foo", [factory.number, factory.boolean])); + }); + + it("array", () => { + expectSelfEquivalent(factory.array(factory.number)); + }); + + it("leaf", () => { + expectSelfEquivalent(factory.number); + expectSelfEquivalent(factory.boolean); + expectSelfEquivalent(factory.string); + }); + + it("recursive", () => { + class RecursiveObject extends factory.objectRecursive("foo", { + x: factory.optionalRecursive([() => RecursiveObject]), + }) {} + expectSelfEquivalent(RecursiveObject); + }); + }); + + describe("allows upgrades but not viewing when the view schema allows a strict superset of the stored schema", () => { + const expected: Omit = { + canView: false, + canUpgrade: true, + isEquivalent: false, + }; + + // Add allowed types to map node + it("view: SomethingMap ⊃ stored: NeverMap", () => { + class NeverMap extends factory.map("TestNode", []) {} + class SomethingMap extends factory.mapRecursive("TestNode", [factory.number]) {} + expectCompatibility( + { view: SomethingMap, stored: toStoredSchema(NeverMap) }, + expected, + ); + }); + + // Add allowed types to object node + it("view: FlexibleObject ⊃ stored: StricterObject", () => { + class StricterObject extends factory.object("TestNode", { + x: factory.number, + }) {} + class FlexibleObject extends factory.object("TestNode", { + x: [factory.number, factory.string], + }) {} + expectCompatibility( + { view: FlexibleObject, stored: toStoredSchema(StricterObject) }, + expected, + ); + }); + // Add optional field to existing schema + it("view: optional 3d Point ⊃ stored: 2d Point", () => { + class Point2D extends factory.object("Point", { + x: factory.number, + y: factory.number, + }) {} + class Point3D extends factory.object("Point", { + x: factory.number, + y: factory.number, + z: factory.optional(factory.number), + }) {} + expectCompatibility({ view: Point3D, stored: toStoredSchema(Point2D) }, expected); + }); + + describe("due to field kind relaxation", () => { + it("view: required field ⊃ stored: identifier field", () => { + // Identifiers are strings, so they should only be relaxable to fields which support strings. + expectCompatibility( + { + view: factory.required(factory.string), + stored: toStoredSchema(factory.identifier), + }, + expected, + ); + expectCompatibility( + { + view: factory.required(factory.number), + stored: toStoredSchema(factory.identifier), + }, + { canView: false, canUpgrade: false, isEquivalent: false }, + ); + }); + it("view: optional field ⊃ stored: required field", () => { + expectCompatibility( + { + view: factory.optional(factory.number), + stored: toStoredSchema(factory.required(factory.number)), + }, + expected, + ); + }); + it("view: optional field ⊃ stored: forbidden field", () => { + expectCompatibility( + { + view: factory.optional(factory.number), + stored: emptySchema, + }, + expected, + ); + }); + // Note: despite optional fields being relaxable to sequence fields in the stored schema representation, + // this is not possible to recreate using the current public API due to differences in array and sequence design + }); + }); + + describe("allows viewing but not upgrading when the view schema has opted into allowing the differences", () => { + it("due to additional optional fields in the stored schema", () => { + class Point2D extends factory.object( + "Point", + { + x: factory.number, + y: factory.number, + }, + { allowUnknownOptionalFields: true }, + ) {} + class Point3D extends factory.object("Point", { + x: factory.number, + y: factory.number, + z: factory.optional(factory.number), + }) {} + expectCompatibility( + { view: Point2D, stored: toStoredSchema(Point3D) }, + { canView: true, canUpgrade: false, isEquivalent: false }, + ); + }); + }); + + describe("forbids viewing and upgrading", () => { + describe("when the view schema and stored schema are incomparable", () => { + // (i.e. neither is a subset of the other, hence each allows documents the other does not) + function expectIncomparability(a: ImplicitFieldSchema, b: ImplicitFieldSchema): void { + const expected: Omit = { + canView: false, + canUpgrade: false, + isEquivalent: false, + }; + expectCompatibility({ view: a, stored: toStoredSchema(b) }, expected); + expectCompatibility({ view: b, stored: toStoredSchema(a) }, expected); + } + + describe("due to an allowed type difference", () => { + it("at the root", () => { + expectIncomparability(factory.number, factory.string); + }); + + it("in an object", () => { + class IncompatibleObject1 extends factory.object("TestNode", { + x: factory.number, + }) {} + class IncompatibleObject2 extends factory.objectRecursive("TestNode", { + x: factory.optionalRecursive([() => IncompatibleObject2]), + }) {} + expectIncomparability(IncompatibleObject1, IncompatibleObject2); + }); + + it("in a map", () => { + class IncompatibleMap1 extends factory.map("TestNode", [ + factory.null, + factory.number, + ]) {} + class IncompatibleMap2 extends factory.map("TestNode", [ + factory.null, + factory.string, + ]) {} + expectIncomparability(IncompatibleMap1, IncompatibleMap2); + }); + }); + + it("due to array vs not array differences", () => { + expectIncomparability(factory.array(factory.number), factory.number); + expectIncomparability( + factory.array(factory.number), + factory.optional(factory.number), + ); + expectIncomparability(factory.array(factory.string), factory.identifier); + }); + + describe("due to a field kind difference", () => { + it("view: identifier vs stored: forbidden", () => { + expectIncomparability(factory.identifier, factory.required([])); + }); + + it("view: 2d Point vs stored: required 3d Point", () => { + class Point2D extends factory.object("Point", { + x: factory.number, + y: factory.number, + }) {} + class Point3D extends factory.object("Point", { + x: factory.number, + y: factory.number, + z: factory.number, + }) {} + expectIncomparability(Point2D, Point3D); + }); + }); + }); + + describe("when the view schema allows a subset of the stored schema's documents but in ways that misalign with allowed viewing policies", () => { + const expected: Omit = { + canView: false, + canUpgrade: false, + isEquivalent: false, + }; + + // Note: the decision to not allow is policy. See + // "allows viewing but not upgrading when the view schema has opted into allowing the differences" above. + it("stored schema has additional optional fields which view schema did not allow", () => { + class Point2D extends factory.object("Point", { + x: factory.number, + y: factory.number, + }) {} + class Point3D extends factory.object("Point", { + x: factory.number, + y: factory.number, + z: factory.optional(factory.number), + }) {} + expectCompatibility({ view: Point2D, stored: toStoredSchema(Point3D) }, expected); + }); + + // This case demonstrates some need for care when allowing view schema to open documents with more flexible stored schema + it("stored schema has optional fields where view schema expects content", () => { + expectCompatibility( + { + view: factory.identifier, + stored: toStoredSchema(factory.optional(factory.string)), + }, + expected, + ); + expectCompatibility( + { view: factory.number, stored: toStoredSchema(factory.optional(factory.number)) }, + expected, + ); + }); + + describe("stored schema has additional unadapted allowed types", () => { + it("at the root", () => { + expectCompatibility( + { + view: factory.number, + stored: toStoredSchema(factory.required([factory.number, factory.string])), + }, + expected, + ); + }); + + it("in an object", () => { + class IncompatibleObject1 extends factory.object("TestNode", { + x: factory.number, + }) {} + class IncompatibleObject2 extends factory.object("TestNode", { + x: [factory.number, factory.string], + }) {} + expectCompatibility( + { view: IncompatibleObject1, stored: toStoredSchema(IncompatibleObject2) }, + expected, + ); + }); + + it("in a map", () => { + class IncompatibleMap1 extends factory.map("TestNode", [factory.number]) {} + class IncompatibleMap2 extends factory.map("TestNode", [ + factory.number, + factory.string, + ]) {} + expectCompatibility( + { view: IncompatibleMap1, stored: toStoredSchema(IncompatibleMap2) }, + expected, + ); + }); + }); + }); + }); + }); +}); diff --git a/packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md b/packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md index 752b27794015..4d6bc3b4d57d 100644 --- a/packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md +++ b/packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md @@ -65,7 +65,7 @@ export interface CommitMetadata { } // @alpha -export function comparePersistedSchema(persisted: JsonCompatible, view: JsonCompatible, options: ICodecOptions, canInitialize: boolean): SchemaCompatibilityStatus; +export function comparePersistedSchema(persisted: JsonCompatible, view: ImplicitFieldSchema, options: ICodecOptions, canInitialize: boolean): SchemaCompatibilityStatus; // @alpha export type ConciseTree = Exclude | THandle | ConciseTree[] | {