From 203ed166fae098aa66d4103e59e52ca1edac56fa Mon Sep 17 00:00:00 2001 From: Lucas Koehler Date: Thu, 31 Oct 2024 15:32:46 +0100 Subject: [PATCH] WIP 2371 combinator schema index heuristic - 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 WIP: - check TODO - add example and test - potentially add info to migration guide --- packages/core/src/mappers/combinators.ts | 91 +++++++++ packages/core/src/mappers/renderer.ts | 52 ++---- .../core/test/mappers/combinators.test.ts | 172 +++++++++++++++++- packages/core/test/mappers/renderer.test.ts | 64 ------- 4 files changed, 275 insertions(+), 104 deletions(-) diff --git a/packages/core/src/mappers/combinators.ts b/packages/core/src/mappers/combinators.ts index baa5b3367..bfb6a7216 100644 --- a/packages/core/src/mappers/combinators.ts +++ b/packages/core/src/mappers/combinators.ts @@ -36,6 +36,8 @@ export interface CombinatorSubSchemaRenderInfo { export type CombinatorKeyword = 'anyOf' | 'oneOf' | 'allOf'; +export const COMBINATOR_TYPE_PROPERTY = 'x-jsf-type-property'; + export const createCombinatorRenderInfos = ( combinatorSubSchemas: JsonSchema[], rootSchema: JsonSchema, @@ -67,3 +69,92 @@ 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 properties is used: + * - `id` + * - `type` + * - `kind` + * + * 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 => { + // 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 { + for (const prop of ['id', 'type', 'kind']) { + if (Object.prototype.hasOwnProperty.call(data, prop)) { + idProperty = prop; + break; + } + } + } + + // If no identification property was found, use the first string or number property + // TODO should this iterate until it finds a property with a const in the schema? + 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 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 ( + idProperty !== undefined && + 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 9883d5063..b9fe8d13c 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'; const move = (array: any[], index: number, delta: number) => { @@ -1058,43 +1061,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, @@ -1103,7 +1075,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..7c1bc9f78 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,170 @@ test('createCombinatorRenderInfos - uses keyword + index when no labels provided t.deepEqual(duaRenderInfo.label, 'anyOf-0'); t.deepEqual(lipaRenderInfo.label, 'anyOf-1'); }); + +const schemaWithTypeProperty = { + 'x-jsf-type-property': 'customId', + properties: { + customId: { const: '123' }, + }, +}; + +const schemaWithId = { + properties: { + id: { const: '123' }, + }, +}; + +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 indexRootSchema = { + definitions: { + schemaWithTypeProperty, + schemaWithId, + schemaWithType, + schemaWithKind, + schemaWithFirstString, + schemaWithFirstNumber, + }, +}; + +test('getCombinatorIndexOfFittingSchema - schema with x-jsf-type-property', (t) => { + const data = { customId: '123' }; + const keyword = 'anyOf'; + const schema = { anyOf: [schemaWithId, schemaWithTypeProperty] }; + + 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 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 - 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 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 8e686e884..350f29be3 100644 --- a/packages/core/test/mappers/renderer.test.ts +++ b/packages/core/test/mappers/renderer.test.ts @@ -62,7 +62,6 @@ import { mapStateToLayoutProps, mapStateToMultiEnumControlProps, mapStateToOneOfEnumControlProps, - mapStateToOneOfProps, } from '../../src/mappers'; import { clearAllIds, convertDateToString, createAjv } from '../../src/util'; import { rankWith } from '../../src'; @@ -964,69 +963,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',