From 138d69d4289eac889dc64888980a7f1e19f293ec Mon Sep 17 00:00:00 2001 From: Lucas Koehler Date: Tue, 28 Jan 2025 15:21:53 +0100 Subject: [PATCH 1/4] core: Remove AJV usage from combinator mappers - Adapt algorithm to determine the fitting schema index for combinators to no longer use AJV - New heuristic uses identifying properties that should match a const value in the schema - Adapt MaterialOneOfRenderer.test.tsx to fit new heuristic - Describe changes and add examples to migration guide - Adapt some of the anyOf and oneOf examples to custom and new identification properties #2371 --- MIGRATION.md | 112 ++++++++ packages/core/src/mappers/combinators.ts | 104 ++++++++ packages/core/src/mappers/renderer.ts | 52 +--- .../core/test/mappers/combinators.test.ts | 246 +++++++++++++++++- packages/core/test/mappers/renderer.test.ts | 64 ----- packages/examples/src/examples/anyOf.ts | 9 +- packages/examples/src/examples/oneOf.ts | 3 + packages/examples/src/examples/oneOfArray.ts | 4 + .../renderers/MaterialOneOfRenderer.test.tsx | 17 +- 9 files changed, 497 insertions(+), 114 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index c886c8d0a..3c57cd35f 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,5 +1,117 @@ # Migration guide +## Migrating to JSON Forms 3.6 + +### Combinator (anyOf & oneOf) index selection now uses a heuristic instead of AJV + +In this update, we have eliminated the direct usage of AJV to determine the selected subschema for combinator renderers. +To achieve this, the algorithm in `getCombinatorIndexOfFittingSchema` and with this `mapStateToCombinatorRendererProps` was changed. +Thus, custom renderers using either method might have behavior changes. +This rework is part of an ongoing effort to remove mandatory usage of AJV from JSON Forms. + +Before this change, AJV was used to validate the current data against all schemas of the combinator. +This was replaced by using a heuristic which tries to match the schema via an identification property +against a `const` entry in the schema. + +The identification property is determined as follows in descending order of priority: + +1. The schema contains a new custom property `x-jsf-type-property` next to the combinator to define the identification property. +2. The data has any of these properties: `type`, `kind`, `id`. They are considered in the listed order. +3. The data has any string or number property. The first encountered one is used. + +If no combinator schema can be matched, fallback to the first one as before this update. + +Note that this approach can not determine a subschema for non-object subschemas (e.g. ones only defining a primitive property). +Furthermore, subschemas can no longer automatically be selected based on validation results like +produced by different required properties between subschemas. + +#### Example 1: Custom identification property + +Use custom property `x-jsf-type-property` to define which property's content identifies the subschema to select. +In this case, `mytype` is defined as the property to use. The two subschemas in the `anyOf` each define a `const` value for this property. +Meaning a data object with property `mytype: 'user'` results in the second subschema being selected. +The `default` keyword can be used to tell JSON Forms to automatically initialize the property. + +```ts +const schema = { + $schema: 'http://json-schema.org/draft-07/schema#', + type: 'object', + properties: { + addressOrUser: { + 'x-jsf-type-property': 'mytype', + anyOf: [ + { + type: 'object', + properties: { + mytype: { const: 'address', default: 'address' }, + street_address: { type: 'string' }, + city: { type: 'string' }, + state: { type: 'string' }, + }, + }, + { + type: 'object', + properties: { + mytype: { const: 'user', default: 'user' }, + name: { type: 'string' }, + }, + }, + ], + }, + }, +}; + +// Data that results in the second subschema being selected +const dataWithUser = { + addressOrUser: { + mytype: 'user', + name: 'Peter', + }, +}; +``` + +#### Example 2: Use a default identification property + +In this example we use the `kind` property as the identification property. Like in the custom property case, subschemas are matched via a `const` definition in the identification property's schema. However, we do not need to explicitly specify `kind` being used. +The `default` keyword can be used to tell JSON Forms to automatically initialize the property. + +```ts +const schema = { + $schema: 'http://json-schema.org/draft-07/schema#', + type: 'object', + properties: { + addressOrUser: { + anyOf: [ + { + type: 'object', + properties: { + kind: { const: 'address', default: 'address' }, + street_address: { type: 'string' }, + city: { type: 'string' }, + state: { type: 'string' }, + }, + }, + { + type: 'object', + properties: { + kind: { const: 'user', default: 'user' }, + name: { type: 'string' }, + }, + }, + ], + }, + }, +}; + +// Data that results in the second subschema being selected +const dataWithUser = { + addressOrUser: { + kind: 'user', + name: 'Peter', + }, +}; +``` + ## Migrating to JSON Forms 3.5 ### Angular support now targets Angular 18 and Angular 19 diff --git a/packages/core/src/mappers/combinators.ts b/packages/core/src/mappers/combinators.ts index baa5b3367..4563904e2 100644 --- a/packages/core/src/mappers/combinators.ts +++ b/packages/core/src/mappers/combinators.ts @@ -36,6 +36,12 @@ export interface CombinatorSubSchemaRenderInfo { export type CombinatorKeyword = 'anyOf' | 'oneOf' | 'allOf'; +/** Custom schema keyword to define the property identifying different combinator schemas. */ +export const COMBINATOR_TYPE_PROPERTY = 'x-jsf-type-property'; + +/** Default properties that are used to identify combinator schemas. */ +export const COMBINATOR_IDENTIFICATION_PROPERTIES = ['type', 'kind', 'id']; + export const createCombinatorRenderInfos = ( combinatorSubSchemas: JsonSchema[], rootSchema: JsonSchema, @@ -67,3 +73,101 @@ export const createCombinatorRenderInfos = ( `${keyword}-${subSchemaIndex}`, }; }); + +/** + * Returns the identification property of the given data object. + * The following heuristics are applied: + * If the schema defines a `x-jsf-type-property`, it is used as the identification property. + * Otherwise, the first of the following data properties is used: + * - `type` + * - `kind` + * - `id` + * + * If none of the above properties are present, the first string or number property of the data object is used. + */ +export const getCombinatorIdentificationProp = ( + data: any, + schema: JsonSchema +): string | undefined => { + if (typeof data !== 'object' || data === null) { + return undefined; + } + + // Determine the identification property + let idProperty: string | undefined; + if ( + COMBINATOR_TYPE_PROPERTY in schema && + typeof schema[COMBINATOR_TYPE_PROPERTY] === 'string' + ) { + idProperty = schema[COMBINATOR_TYPE_PROPERTY]; + } else { + // Use the first default identification property that is present in the data object + for (const prop of COMBINATOR_IDENTIFICATION_PROPERTIES) { + if (Object.prototype.hasOwnProperty.call(data, prop)) { + idProperty = prop; + break; + } + } + } + + // If no identification property was found, use the first string or number property + // of the data object + if (idProperty === undefined) { + for (const key of Object.keys(data)) { + if (typeof data[key] === 'string' || typeof data[key] === 'number') { + idProperty = key; + break; + } + } + } + + return idProperty; +}; + +/** + * Returns the index of the schema in the given combinator keyword that matches the identification property of the given data object. + * The heuristic only works for data objects with a corresponding schema. If the data is a primitive value or an array, the heuristic does not work. + * + * If the index cannot be determined, `-1` is returned. + * + * @returns the index of the fitting schema or `-1` if no fitting schema was found + */ +export const getCombinatorIndexOfFittingSchema = ( + data: any, + keyword: CombinatorKeyword, + schema: JsonSchema, + rootSchema: JsonSchema +): number => { + let indexOfFittingSchema = -1; + const idProperty = getCombinatorIdentificationProp(data, schema); + if (idProperty === undefined) { + return indexOfFittingSchema; + } + + for (let i = 0; i < schema[keyword]?.length; i++) { + let resolvedSchema = schema[keyword][i]; + if (resolvedSchema.$ref) { + resolvedSchema = Resolve.schema( + rootSchema, + resolvedSchema.$ref, + rootSchema + ); + } + + // Match the identification property against a constant value in resolvedSchema + const maybeConstIdValue = resolvedSchema.properties?.[idProperty]?.const; + + if ( + maybeConstIdValue !== undefined && + data[idProperty] === maybeConstIdValue + ) { + indexOfFittingSchema = i; + console.debug( + `Data matches the resolved schema for property ${idProperty}` + ); + break; + } + } + + return indexOfFittingSchema; +}; diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index f67730334..89119aca4 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -84,7 +84,10 @@ import { getUiSchema, } from '../store'; import { isInherentlyEnabled } from './util'; -import { CombinatorKeyword } from './combinators'; +import { + CombinatorKeyword, + getCombinatorIndexOfFittingSchema, +} from './combinators'; import isEqual from 'lodash/isEqual'; const move = (array: any[], index: number, delta: number) => { @@ -1128,43 +1131,12 @@ export const mapStateToCombinatorRendererProps = ( const { data, schema, rootSchema, i18nKeyPrefix, label, ...props } = mapStateToControlProps(state, ownProps); - const ajv = state.jsonforms.core.ajv; - const structuralKeywords = [ - 'required', - 'additionalProperties', - 'type', - 'enum', - 'const', - ]; - const dataIsValid = (errors: ErrorObject[]): boolean => { - return ( - !errors || - errors.length === 0 || - !errors.find((e) => structuralKeywords.indexOf(e.keyword) !== -1) - ); - }; - let indexOfFittingSchema: number; - // TODO instead of compiling the combinator subschemas we can compile the original schema - // without the combinator alternatives and then revalidate and check the errors for the - // element - for (let i = 0; i < schema[keyword]?.length; i++) { - try { - let _schema = schema[keyword][i]; - if (_schema.$ref) { - _schema = Resolve.schema(rootSchema, _schema.$ref, rootSchema); - } - const valFn = ajv.compile(_schema); - valFn(data); - if (dataIsValid(valFn.errors)) { - indexOfFittingSchema = i; - break; - } - } catch (error) { - console.debug( - "Combinator subschema is not self contained, can't hand it over to AJV" - ); - } - } + const indexOfFittingSchema = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + rootSchema + ); return { data, @@ -1173,7 +1145,9 @@ export const mapStateToCombinatorRendererProps = ( ...props, i18nKeyPrefix, label, - indexOfFittingSchema, + // Fall back to the first schema if none fits + indexOfFittingSchema: + indexOfFittingSchema !== -1 ? indexOfFittingSchema : 0, uischemas: getUISchemas(state), }; }; diff --git a/packages/core/test/mappers/combinators.test.ts b/packages/core/test/mappers/combinators.test.ts index a10a7f2f5..91a0156ba 100644 --- a/packages/core/test/mappers/combinators.test.ts +++ b/packages/core/test/mappers/combinators.test.ts @@ -1,6 +1,9 @@ import test from 'ava'; import { ControlElement } from '../../src/models'; -import { createCombinatorRenderInfos } from '../../src/mappers'; +import { + createCombinatorRenderInfos, + getCombinatorIndexOfFittingSchema, +} from '../../src/mappers'; const rootSchema = { type: 'object', @@ -111,3 +114,244 @@ test('createCombinatorRenderInfos - uses keyword + index when no labels provided t.deepEqual(duaRenderInfo.label, 'anyOf-0'); t.deepEqual(lipaRenderInfo.label, 'anyOf-1'); }); + +const schemaWithCustomIdProperty = { + properties: { + customId: { const: '123' }, + }, +}; + +const schemaWithId = { + properties: { + id: { const: '123' }, + }, +}; + +const schemaWithIdWithoutConst = { + properties: { + type: { type: 'string' }, + }, +}; + +const schemaWithType = { + properties: { + type: { const: 'typeValue' }, + }, +}; + +const schemaWithKind = { + properties: { + kind: { const: 'kindValue' }, + }, +}; + +const schemaWithFirstString = { + properties: { + obj: { type: 'object' }, + name: { const: 'John' }, + }, +}; + +const schemaWithFirstNumber = { + properties: { + obj: { type: 'object' }, + identity: { const: 123 }, + }, +}; + +const schemaWithFirstNumberWithoutConst = { + properties: { + obj: { type: 'object' }, + identity: { type: 'number' }, + }, +}; + +const indexRootSchema = { + definitions: { + schemaWithCustomIdProperty, + schemaWithId, + schemaWithIdWithoutConst, + schemaWithType, + schemaWithKind, + schemaWithFirstString, + schemaWithFirstNumber, + schemaWithFirstNumberWithoutConst, + }, +}; + +test('getCombinatorIndexOfFittingSchema - schema with x-jsf-type-property', (t) => { + const data = { customId: '123' }; + const keyword = 'anyOf'; + const schema = { + anyOf: [schemaWithId, schemaWithCustomIdProperty], + 'x-jsf-type-property': 'customId', + }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 1); +}); + +test('getCombinatorIndexOfFittingSchema - data with id property', (t) => { + const data = { id: '123' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithId, schemaWithKind] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 0); +}); + +test('getCombinatorIndexOfFittingSchema - data with id property without const', (t) => { + const data = { id: '123', type: 'typeValue' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithIdWithoutConst, schemaWithKind] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + // First schema does not have a const and, thus, cannot match + t.is(result, -1); +}); + +test('getCombinatorIndexOfFittingSchema - data with unfitting id property value', (t) => { + const data = { id: '321' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithId, schemaWithKind] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, -1); +}); + +test('getCombinatorIndexOfFittingSchema - data with type property', (t) => { + const data = { type: 'typeValue' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithId, schemaWithType] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 1); +}); + +test('getCombinatorIndexOfFittingSchema - data with unfitting type property value', (t) => { + const data = { type: 'wrongTypeValue' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithId, schemaWithType] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, -1); +}); + +test('getCombinatorIndexOfFittingSchema - schema with refs and data with type property', (t) => { + const data = { type: 'typeValue' }; + const keyword = 'anyOf'; + const schema = { + anyOf: [ + { $ref: '#/definitions/schemaWithId' }, + { $ref: '#/definitions/schemaWithType' }, + ], + }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 1); +}); + +test('getCombinatorIndexOfFittingSchema - data with kind property', (t) => { + const data = { kind: 'kindValue' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithKind] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 0); +}); + +test('getCombinatorIndexOfFittingSchema - data with unfitting kind property value', (t) => { + const data = { kind: 'wrongKindValue' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithKind] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, -1); +}); + +test('getCombinatorIndexOfFittingSchema - data with first string property', (t) => { + const data = { obj: {}, name: 'John' }; + const keyword = 'anyOf'; + const schema = { anyOf: [{}, schemaWithFirstString] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 1); +}); + +test('getCombinatorIndexOfFittingSchema - data with first number property', (t) => { + const data = { obj: {}, identity: 123 }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithFirstNumber] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, 0); +}); + +test('getCombinatorIndexOfFittingSchema - no matching schema', (t) => { + const data = { name: 'Doe' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithFirstString] }; + + const result = getCombinatorIndexOfFittingSchema( + data, + keyword, + schema, + indexRootSchema + ); + t.is(result, -1); +}); diff --git a/packages/core/test/mappers/renderer.test.ts b/packages/core/test/mappers/renderer.test.ts index 9d0dd516c..c42b5b341 100644 --- a/packages/core/test/mappers/renderer.test.ts +++ b/packages/core/test/mappers/renderer.test.ts @@ -63,7 +63,6 @@ import { mapStateToLayoutProps, mapStateToMultiEnumControlProps, mapStateToOneOfEnumControlProps, - mapStateToOneOfProps, } from '../../src/mappers'; import { clearAllIds, convertDateToString, createAjv } from '../../src/util'; import { rankWith } from '../../src'; @@ -1210,69 +1209,6 @@ test('mapStateToLayoutProps - hidden via state with path from ownProps ', (t) => t.false(props.visible); }); -test("mapStateToOneOfProps - indexOfFittingSchema should not select schema if enum doesn't match", (t) => { - const uischema: ControlElement = { - type: 'Control', - scope: '#/properties/method', - }; - - const ownProps = { - uischema, - }; - - const state = { - jsonforms: { - core: { - ajv: createAjv(), - schema: { - type: 'object', - properties: { - method: { - oneOf: [ - { - title: 'Injection', - type: 'object', - properties: { - method: { - title: 'Method', - type: 'string', - enum: ['Injection'], - default: 'Injection', - }, - }, - required: ['method'], - }, - { - title: 'Infusion', - type: 'object', - properties: { - method: { - title: 'Method', - type: 'string', - enum: ['Infusion'], - default: 'Infusion', - }, - }, - required: ['method'], - }, - ], - }, - }, - }, - data: { - method: { - method: 'Infusion', - }, - }, - uischema, - }, - }, - }; - - const oneOfProps = mapStateToOneOfProps(state, ownProps); - t.is(oneOfProps.indexOfFittingSchema, 1); -}); - test('mapStateToMultiEnumControlProps - oneOf items', (t) => { const uischema: ControlElement = { type: 'Control', diff --git a/packages/examples/src/examples/anyOf.ts b/packages/examples/src/examples/anyOf.ts index 31eb17a3b..e45bd4828 100644 --- a/packages/examples/src/examples/anyOf.ts +++ b/packages/examples/src/examples/anyOf.ts @@ -31,6 +31,7 @@ export const schema = { address: { type: 'object', properties: { + mytype: { const: 'address', default: 'address' }, street_address: { type: 'string' }, city: { type: 'string' }, state: { type: 'string' }, @@ -40,6 +41,7 @@ export const schema = { user: { type: 'object', properties: { + mytype: { const: 'user', default: 'user' }, name: { type: 'string' }, mail: { type: 'string' }, }, @@ -63,6 +65,7 @@ export const schema = { { $ref: '#/definitions/address' }, { $ref: '#/definitions/user' }, ], + 'x-jsf-type-property': 'mytype', }, addressesOrUsers: { anyOf: [ @@ -104,9 +107,9 @@ export const uischema = { const data = { addressOrUser: { - street_address: '1600 Pennsylvania Avenue NW', - city: 'Washington', - state: 'DC', + mytype: 'user', + name: 'Peter', + email: 'peter@example.org', }, }; diff --git a/packages/examples/src/examples/oneOf.ts b/packages/examples/src/examples/oneOf.ts index 174ea834c..b7c920551 100644 --- a/packages/examples/src/examples/oneOf.ts +++ b/packages/examples/src/examples/oneOf.ts @@ -31,6 +31,7 @@ export const schema = { address: { type: 'object', properties: { + type: { const: 'address', default: 'address' }, street_address: { type: 'string' }, city: { type: 'string' }, state: { type: 'string' }, @@ -41,6 +42,7 @@ export const schema = { user: { type: 'object', properties: { + type: { const: 'user', default: 'user' }, name: { type: 'string' }, mail: { type: 'string' }, }, @@ -80,6 +82,7 @@ export const uischema = { const data = { name: 'test', addressOrUser: { + type: 'user', name: 'User', mail: 'mail@example.com', }, diff --git a/packages/examples/src/examples/oneOfArray.ts b/packages/examples/src/examples/oneOfArray.ts index ec6d8f783..6ad0c91a5 100644 --- a/packages/examples/src/examples/oneOfArray.ts +++ b/packages/examples/src/examples/oneOfArray.ts @@ -31,6 +31,7 @@ export const schema = { address: { type: 'object', properties: { + kind: { const: 'address', default: 'address' }, street_address: { type: 'string' }, city: { type: 'string' }, state: { type: 'string' }, @@ -40,6 +41,7 @@ export const schema = { user: { type: 'object', properties: { + kind: { const: 'user', default: 'user' }, name: { type: 'string' }, mail: { type: 'string' }, }, @@ -77,11 +79,13 @@ const data = { name: 'test', addressOrUsers: [ { + kind: 'address', street_address: '1600 Pennsylvania Avenue NW', city: 'Washington', state: 'DC', }, { + kind: 'user', name: 'User', mail: 'user@user.user', }, diff --git a/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx b/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx index 765967e93..eba98a293 100644 --- a/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx +++ b/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx @@ -114,7 +114,7 @@ describe('Material oneOf renderer', () => { expect(firstTab.html()).toContain('Mui-selected'); }); - it('should render and select second tab due to datatype', () => { + it('should render and select first tab for primitive combinator data', () => { const schema = { type: 'object', properties: { @@ -150,11 +150,11 @@ describe('Material oneOf renderer', () => { ); expect(wrapper.find(MaterialOneOfRenderer).length).toBeTruthy(); - const secondTab = wrapper.find(Tab).at(1); + const secondTab = wrapper.find(Tab).at(0); expect(secondTab.html()).toContain('Mui-selected'); }); - it('should render and select second tab due to schema with additionalProperties', () => { + it('should render and select second tab due to string prop with matching const value', () => { const schema = { type: 'object', properties: { @@ -164,7 +164,7 @@ describe('Material oneOf renderer', () => { title: 'String', type: 'object', properties: { - foo: { type: 'string' }, + foo: { type: 'string', const: 'foo' }, }, additionalProperties: false, }, @@ -172,7 +172,7 @@ describe('Material oneOf renderer', () => { title: 'Number', type: 'object', properties: { - bar: { type: 'string' }, + bar: { type: 'string', const: 'bar' }, }, additionalProperties: false, }, @@ -202,7 +202,7 @@ describe('Material oneOf renderer', () => { expect(secondTab.html()).toContain('Mui-selected'); }); - it('should render and select second tab due to schema with required', () => { + it('should render and select second tab due const custom property', () => { const schema = { type: 'object', properties: { @@ -213,6 +213,7 @@ describe('Material oneOf renderer', () => { type: 'object', properties: { foo: { type: 'string' }, + myprop: { const: 'foo' }, }, required: ['foo'], }, @@ -221,10 +222,12 @@ describe('Material oneOf renderer', () => { type: 'object', properties: { bar: { type: 'string' }, + myprop: { const: 'bar' }, }, required: ['bar'], }, ], + 'x-jsf-type-property': 'myprop', }, }, }; @@ -234,7 +237,7 @@ describe('Material oneOf renderer', () => { scope: '#/properties/value', }; const data = { - value: { bar: 'bar' }, + value: { bar: 'bar', myprop: 'bar' }, }; wrapper = mount( Date: Fri, 31 Jan 2025 14:34:08 +0100 Subject: [PATCH 2/4] Remove mapStateToCombinatorRendererProps usage from mapStateToAllOfProps In contrast to oneOf and anyOf rendererer, allOf renderers do not need the `indexOfFittingSchema` because all schemas apply at once. Thus, remove using mapStateToCombinatorRendererProps from mapStateToAllOfProps and, with this, the unnecessary calculation of the index. Part of #2371 --- packages/core/src/mappers/renderer.ts | 27 +++++++++++++++++-- .../src/complex/MaterialAllOfRenderer.tsx | 4 +-- packages/react/src/JsonFormsContext.tsx | 7 ++--- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index 89119aca4..0bde958cb 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -1123,6 +1123,11 @@ export interface StatePropsOfCombinator extends StatePropsOfControl { data: any; } +export type StatePropsOfAllOfRenderer = Omit< + StatePropsOfCombinator, + 'indexOfFittingSchema' +>; + export const mapStateToCombinatorRendererProps = ( state: JsonFormsState, ownProps: OwnPropsOfControl, @@ -1155,6 +1160,12 @@ export const mapStateToCombinatorRendererProps = ( export interface CombinatorRendererProps extends StatePropsOfCombinator, DispatchPropsOfControl {} + +export type AllOfRendererProps = Omit< + CombinatorRendererProps, + 'indexOfFittingSchema' +>; + /** * Map state to all of renderer props. * @param state the store's state @@ -1164,8 +1175,20 @@ export interface CombinatorRendererProps export const mapStateToAllOfProps = ( state: JsonFormsState, ownProps: OwnPropsOfControl -): StatePropsOfCombinator => - mapStateToCombinatorRendererProps(state, ownProps, 'allOf'); +): StatePropsOfAllOfRenderer => { + const { data, schema, rootSchema, i18nKeyPrefix, label, ...props } = + mapStateToControlProps(state, ownProps); + + return { + data, + schema, + rootSchema, + ...props, + i18nKeyPrefix, + label, + uischemas: getUISchemas(state), + }; +}; export const mapStateToAnyOfProps = ( state: JsonFormsState, diff --git a/packages/material-renderers/src/complex/MaterialAllOfRenderer.tsx b/packages/material-renderers/src/complex/MaterialAllOfRenderer.tsx index 213fe41c0..5434d54d4 100644 --- a/packages/material-renderers/src/complex/MaterialAllOfRenderer.tsx +++ b/packages/material-renderers/src/complex/MaterialAllOfRenderer.tsx @@ -31,7 +31,7 @@ import { JsonSchema, RankedTester, rankWith, - StatePropsOfCombinator, + StatePropsOfAllOfRenderer, } from '@jsonforms/core'; import { JsonFormsDispatch, withJsonFormsAllOfProps } from '@jsonforms/react'; @@ -44,7 +44,7 @@ export const MaterialAllOfRenderer = ({ path, uischemas, uischema, -}: StatePropsOfCombinator) => { +}: StatePropsOfAllOfRenderer) => { const delegateUISchema = findMatchingUISchema(uischemas)( schema, uischema.scope, diff --git a/packages/react/src/JsonFormsContext.tsx b/packages/react/src/JsonFormsContext.tsx index b10323b65..53980a5b4 100644 --- a/packages/react/src/JsonFormsContext.tsx +++ b/packages/react/src/JsonFormsContext.tsx @@ -81,6 +81,7 @@ import { arrayDefaultTranslations, getArrayTranslations, ArrayTranslations, + AllOfRendererProps, } from '@jsonforms/core'; import debounce from 'lodash/debounce'; import React, { @@ -561,12 +562,12 @@ const withContextToAnyOfProps = ( }; const withContextToAllOfProps = ( - Component: ComponentType + Component: ComponentType ): ComponentType => function WithContextToAllOfProps({ ctx, props, - }: JsonFormsStateContext & CombinatorRendererProps) { + }: JsonFormsStateContext & AllOfRendererProps) { const allOfProps = ctxToAllOfProps(ctx, props); const dispatchProps = ctxDispatchToControlProps(ctx.dispatch); return ; @@ -764,7 +765,7 @@ export const withJsonFormsAnyOfProps = ( ); export const withJsonFormsAllOfProps = ( - Component: ComponentType, + Component: ComponentType, memoize = true ): ComponentType => withJsonFormsContext( From 659c47e5a8f6c9a812ed36d4375340883e79107d Mon Sep 17 00:00:00 2001 From: Lucas Koehler Date: Fri, 28 Feb 2025 11:57:21 +0100 Subject: [PATCH 3/4] review: Rely on schema instead of data, remove string/number prop usage - remove usage of any number or string property as fallback - choose type, kind default prop based on combinator schema with const entry instead of presence in the data object - Adapt tests - remove usage of id as a default prop --- MIGRATION.md | 10 +- packages/core/src/mappers/combinators.ts | 93 +++++------ .../core/test/mappers/combinators.test.ts | 158 ++++++++---------- .../renderers/MaterialOneOfRenderer.test.tsx | 8 +- 4 files changed, 119 insertions(+), 150 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 3c57cd35f..ebc33fe62 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -10,14 +10,12 @@ Thus, custom renderers using either method might have behavior changes. This rework is part of an ongoing effort to remove mandatory usage of AJV from JSON Forms. Before this change, AJV was used to validate the current data against all schemas of the combinator. -This was replaced by using a heuristic which tries to match the schema via an identification property -against a `const` entry in the schema. +This was replaced by a heuristic which tries to match the schema via an identification property against a `const` entry in the schema. The identification property is determined as follows in descending order of priority: 1. The schema contains a new custom property `x-jsf-type-property` next to the combinator to define the identification property. -2. The data has any of these properties: `type`, `kind`, `id`. They are considered in the listed order. -3. The data has any string or number property. The first encountered one is used. +2. At least one of the combinator schemas has this property with a const declaration: `type`, `kind`. They are considered in the listed order. If no combinator schema can be matched, fallback to the first one as before this update. @@ -72,7 +70,9 @@ const dataWithUser = { #### Example 2: Use a default identification property -In this example we use the `kind` property as the identification property. Like in the custom property case, subschemas are matched via a `const` definition in the identification property's schema. However, we do not need to explicitly specify `kind` being used. +In this example we use the `kind` property as the identification property. +Like in the custom property case, subschemas are matched via a `const` definition in the identification property's schema. +However, we do not need to explicitly specify `kind` being used. The `default` keyword can be used to tell JSON Forms to automatically initialize the property. ```ts diff --git a/packages/core/src/mappers/combinators.ts b/packages/core/src/mappers/combinators.ts index 4563904e2..52485b7c7 100644 --- a/packages/core/src/mappers/combinators.ts +++ b/packages/core/src/mappers/combinators.ts @@ -40,7 +40,7 @@ export type CombinatorKeyword = 'anyOf' | 'oneOf' | 'allOf'; export const COMBINATOR_TYPE_PROPERTY = 'x-jsf-type-property'; /** Default properties that are used to identify combinator schemas. */ -export const COMBINATOR_IDENTIFICATION_PROPERTIES = ['type', 'kind', 'id']; +export const COMBINATOR_IDENTIFICATION_PROPERTIES = ['type', 'kind']; export const createCombinatorRenderInfos = ( combinatorSubSchemas: JsonSchema[], @@ -75,22 +75,41 @@ export const createCombinatorRenderInfos = ( }); /** - * Returns the identification property of the given data object. + * Returns the index of the schema in the given combinator keyword that matches the identification property of the given data object. + * The heuristic only works for data objects with a corresponding schema. If the data is a primitive value or an array, the heuristic does not work. + * * The following heuristics are applied: * If the schema defines a `x-jsf-type-property`, it is used as the identification property. - * Otherwise, the first of the following data properties is used: + * Otherwise, the first of the following properties is used if it exists in at least one combinator schema and has a `const` entry: * - `type` * - `kind` - * - `id` * - * If none of the above properties are present, the first string or number property of the data object is used. + * If the index cannot be determined, `-1` is returned. + * + * @returns the index of the fitting schema or `-1` if no fitting schema was found */ -export const getCombinatorIdentificationProp = ( +export const getCombinatorIndexOfFittingSchema = ( data: any, - schema: JsonSchema -): string | undefined => { + keyword: CombinatorKeyword, + schema: JsonSchema, + rootSchema: JsonSchema +): number => { if (typeof data !== 'object' || data === null) { - return undefined; + return -1; + } + + // Resolve all schemas in the combinator. + const resolvedCombinatorSchemas = []; + for (let i = 0; i < schema[keyword]?.length; i++) { + let resolvedSchema = schema[keyword][i]; + if (resolvedSchema.$ref) { + resolvedSchema = Resolve.schema( + rootSchema, + resolvedSchema.$ref, + rootSchema + ); + } + resolvedCombinatorSchemas.push(resolvedSchema); } // Determine the identification property @@ -101,58 +120,25 @@ export const getCombinatorIdentificationProp = ( ) { idProperty = schema[COMBINATOR_TYPE_PROPERTY]; } else { - // Use the first default identification property that is present in the data object - for (const prop of COMBINATOR_IDENTIFICATION_PROPERTIES) { - if (Object.prototype.hasOwnProperty.call(data, prop)) { - idProperty = prop; - break; - } - } - } - - // If no identification property was found, use the first string or number property - // of the data object - if (idProperty === undefined) { - for (const key of Object.keys(data)) { - if (typeof data[key] === 'string' || typeof data[key] === 'number') { - idProperty = key; - break; + // Use the first default identification property that has a const entry in at least one of the schemas + for (const potentialIdProp of COMBINATOR_IDENTIFICATION_PROPERTIES) { + for (const resolvedSchema of resolvedCombinatorSchemas) { + if (resolvedSchema.properties?.[potentialIdProp]?.const !== undefined) { + idProperty = potentialIdProp; + break; + } } } } - return idProperty; -}; - -/** - * Returns the index of the schema in the given combinator keyword that matches the identification property of the given data object. - * The heuristic only works for data objects with a corresponding schema. If the data is a primitive value or an array, the heuristic does not work. - * - * If the index cannot be determined, `-1` is returned. - * - * @returns the index of the fitting schema or `-1` if no fitting schema was found - */ -export const getCombinatorIndexOfFittingSchema = ( - data: any, - keyword: CombinatorKeyword, - schema: JsonSchema, - rootSchema: JsonSchema -): number => { let indexOfFittingSchema = -1; - const idProperty = getCombinatorIdentificationProp(data, schema); if (idProperty === undefined) { return indexOfFittingSchema; } - for (let i = 0; i < schema[keyword]?.length; i++) { - let resolvedSchema = schema[keyword][i]; - if (resolvedSchema.$ref) { - resolvedSchema = Resolve.schema( - rootSchema, - resolvedSchema.$ref, - rootSchema - ); - } + // Check if the data matches the identification property of one of the resolved schemas + for (let i = 0; i < resolvedCombinatorSchemas.length; i++) { + const resolvedSchema = resolvedCombinatorSchemas[i]; // Match the identification property against a constant value in resolvedSchema const maybeConstIdValue = resolvedSchema.properties?.[idProperty]?.const; @@ -162,9 +148,6 @@ export const getCombinatorIndexOfFittingSchema = ( data[idProperty] === maybeConstIdValue ) { indexOfFittingSchema = i; - console.debug( - `Data matches the resolved schema for property ${idProperty}` - ); break; } } diff --git a/packages/core/test/mappers/combinators.test.ts b/packages/core/test/mappers/combinators.test.ts index 91a0156ba..13c81cf5a 100644 --- a/packages/core/test/mappers/combinators.test.ts +++ b/packages/core/test/mappers/combinators.test.ts @@ -1,5 +1,5 @@ import test from 'ava'; -import { ControlElement } from '../../src/models'; +import { ControlElement, JsonSchema } from '../../src/models'; import { createCombinatorRenderInfos, getCombinatorIndexOfFittingSchema, @@ -127,15 +127,15 @@ const schemaWithId = { }, }; -const schemaWithIdWithoutConst = { +const schemaWithType = { properties: { - type: { type: 'string' }, + type: { const: 'typeValue' }, }, }; -const schemaWithType = { +const schemaWithTypeWithoutConst = { properties: { - type: { const: 'typeValue' }, + type: { type: 'string' }, }, }; @@ -152,38 +152,21 @@ const schemaWithFirstString = { }, }; -const schemaWithFirstNumber = { - properties: { - obj: { type: 'object' }, - identity: { const: 123 }, - }, -}; - -const schemaWithFirstNumberWithoutConst = { - properties: { - obj: { type: 'object' }, - identity: { type: 'number' }, - }, -}; - const indexRootSchema = { definitions: { schemaWithCustomIdProperty, schemaWithId, - schemaWithIdWithoutConst, schemaWithType, schemaWithKind, schemaWithFirstString, - schemaWithFirstNumber, - schemaWithFirstNumberWithoutConst, }, }; test('getCombinatorIndexOfFittingSchema - schema with x-jsf-type-property', (t) => { const data = { customId: '123' }; - const keyword = 'anyOf'; + const keyword = 'oneOf'; const schema = { - anyOf: [schemaWithId, schemaWithCustomIdProperty], + oneOf: [schemaWithId, schemaWithCustomIdProperty], 'x-jsf-type-property': 'customId', }; @@ -198,8 +181,8 @@ test('getCombinatorIndexOfFittingSchema - schema with x-jsf-type-property', (t) test('getCombinatorIndexOfFittingSchema - data with id property', (t) => { const data = { id: '123' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithId, schemaWithKind] }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithId, schemaWithKind] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -207,28 +190,14 @@ test('getCombinatorIndexOfFittingSchema - data with id property', (t) => { schema, indexRootSchema ); - t.is(result, 0); -}); - -test('getCombinatorIndexOfFittingSchema - data with id property without const', (t) => { - const data = { id: '123', type: 'typeValue' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithIdWithoutConst, schemaWithKind] }; - - const result = getCombinatorIndexOfFittingSchema( - data, - keyword, - schema, - indexRootSchema - ); - // First schema does not have a const and, thus, cannot match + // Id property is not one of the default identification properties t.is(result, -1); }); -test('getCombinatorIndexOfFittingSchema - data with unfitting id property value', (t) => { - const data = { id: '321' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithId, schemaWithKind] }; +test('getCombinatorIndexOfFittingSchema - data with type property', (t) => { + const data = { type: 'typeValue' }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithId, schemaWithType] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -236,13 +205,13 @@ test('getCombinatorIndexOfFittingSchema - data with unfitting id property value' schema, indexRootSchema ); - t.is(result, -1); + t.is(result, 1); }); -test('getCombinatorIndexOfFittingSchema - data with type property', (t) => { - const data = { type: 'typeValue' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithId, schemaWithType] }; +test('getCombinatorIndexOfFittingSchema - data with unfitting type property value', (t) => { + const data = { type: 'wrongTypeValue' }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithId, schemaWithType] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -250,13 +219,13 @@ test('getCombinatorIndexOfFittingSchema - data with type property', (t) => { schema, indexRootSchema ); - t.is(result, 1); + t.is(result, -1); }); -test('getCombinatorIndexOfFittingSchema - data with unfitting type property value', (t) => { - const data = { type: 'wrongTypeValue' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithId, schemaWithType] }; +test('getCombinatorIndexOfFittingSchema - schema with type property without const', (t) => { + const data = { type: 'sometype' }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithTypeWithoutConst] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -269,9 +238,9 @@ test('getCombinatorIndexOfFittingSchema - data with unfitting type property valu test('getCombinatorIndexOfFittingSchema - schema with refs and data with type property', (t) => { const data = { type: 'typeValue' }; - const keyword = 'anyOf'; + const keyword = 'oneOf'; const schema = { - anyOf: [ + oneOf: [ { $ref: '#/definitions/schemaWithId' }, { $ref: '#/definitions/schemaWithType' }, ], @@ -288,8 +257,8 @@ test('getCombinatorIndexOfFittingSchema - schema with refs and data with type pr test('getCombinatorIndexOfFittingSchema - data with kind property', (t) => { const data = { kind: 'kindValue' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithKind] }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithKind] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -302,8 +271,8 @@ test('getCombinatorIndexOfFittingSchema - data with kind property', (t) => { test('getCombinatorIndexOfFittingSchema - data with unfitting kind property value', (t) => { const data = { kind: 'wrongKindValue' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithKind] }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithKind] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -314,10 +283,10 @@ test('getCombinatorIndexOfFittingSchema - data with unfitting kind property valu t.is(result, -1); }); -test('getCombinatorIndexOfFittingSchema - data with first string property', (t) => { - const data = { obj: {}, name: 'John' }; - const keyword = 'anyOf'; - const schema = { anyOf: [{}, schemaWithFirstString] }; +test('getCombinatorIndexOfFittingSchema - no matching schema', (t) => { + const data = { name: 'Doe' }; + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithFirstString] }; const result = getCombinatorIndexOfFittingSchema( data, @@ -325,33 +294,50 @@ test('getCombinatorIndexOfFittingSchema - data with first string property', (t) schema, indexRootSchema ); - t.is(result, 1); + t.is(result, -1); }); -test('getCombinatorIndexOfFittingSchema - data with first number property', (t) => { - const data = { obj: {}, identity: 123 }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithFirstNumber] }; +test('getCombinatorIndexOfFittingSchema - non-object data', (t) => { + const keyword = 'oneOf'; + const schema = { oneOf: [schemaWithId, schemaWithType] }; - const result = getCombinatorIndexOfFittingSchema( - data, - keyword, - schema, - indexRootSchema + t.is( + getCombinatorIndexOfFittingSchema(42, keyword, schema, indexRootSchema), + -1 + ); + t.is( + getCombinatorIndexOfFittingSchema( + 'string', + keyword, + schema, + indexRootSchema + ), + -1 + ); + t.is( + getCombinatorIndexOfFittingSchema( + undefined, + keyword, + schema, + indexRootSchema + ), + -1 ); - t.is(result, 0); }); -test('getCombinatorIndexOfFittingSchema - no matching schema', (t) => { - const data = { name: 'Doe' }; - const keyword = 'anyOf'; - const schema = { anyOf: [schemaWithFirstString] }; +test('getCombinatorIndexOfFittingSchema - combinator schemas are absent', (t) => { + const data = { someProp: 'value' }; + const schema = {}; + const rootSchema = {}; + t.is( + getCombinatorIndexOfFittingSchema(data, 'oneOf', schema, rootSchema), + -1 + ); - const result = getCombinatorIndexOfFittingSchema( - data, - keyword, - schema, - indexRootSchema + // Empty oneOf array + const emptySchema = { oneOf: [] } as JsonSchema; + t.is( + getCombinatorIndexOfFittingSchema(data, 'oneOf', emptySchema, rootSchema), + -1 ); - t.is(result, -1); }); diff --git a/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx b/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx index eba98a293..35e761b66 100644 --- a/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx +++ b/packages/material-renderers/test/renderers/MaterialOneOfRenderer.test.tsx @@ -154,7 +154,7 @@ describe('Material oneOf renderer', () => { expect(secondTab.html()).toContain('Mui-selected'); }); - it('should render and select second tab due to string prop with matching const value', () => { + it('should render and select second tab due to type prop with matching const value', () => { const schema = { type: 'object', properties: { @@ -164,7 +164,7 @@ describe('Material oneOf renderer', () => { title: 'String', type: 'object', properties: { - foo: { type: 'string', const: 'foo' }, + type: { const: 'foo' }, }, additionalProperties: false, }, @@ -172,7 +172,7 @@ describe('Material oneOf renderer', () => { title: 'Number', type: 'object', properties: { - bar: { type: 'string', const: 'bar' }, + type: { const: 'bar' }, }, additionalProperties: false, }, @@ -186,7 +186,7 @@ describe('Material oneOf renderer', () => { scope: '#/properties/value', }; const data = { - value: { bar: 'bar' }, + value: { type: 'bar' }, }; wrapper = mount( Date: Mon, 3 Mar 2025 16:10:31 +0100 Subject: [PATCH 4/4] review: fix vue-vanilla one of renderer tests --- packages/vue-vanilla/tests/unit/complex/OneOfRenderer.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/vue-vanilla/tests/unit/complex/OneOfRenderer.spec.ts b/packages/vue-vanilla/tests/unit/complex/OneOfRenderer.spec.ts index d1bb27878..d0f824def 100644 --- a/packages/vue-vanilla/tests/unit/complex/OneOfRenderer.spec.ts +++ b/packages/vue-vanilla/tests/unit/complex/OneOfRenderer.spec.ts @@ -3,6 +3,7 @@ import { mountJsonForms } from '../util'; const schema = { title: 'My Object', + 'x-jsf-type-property': 'variant', oneOf: [ { title: 'Variant A',