From 7d1865f78a5e21e59213e2ae4545fc77db489a43 Mon Sep 17 00:00:00 2001 From: Krasimir Chobantonov Date: Sun, 1 Dec 2024 02:24:56 -0500 Subject: [PATCH 1/6] fixes createDefaultValue doesn't follow anyOf, oneOf, allOf #2401 --- packages/core/src/mappers/renderer.ts | 48 ++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index 9883d50632..b5b6aa0218 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -168,9 +168,13 @@ export const showAsRequired = ( */ export const createDefaultValue = ( schema: JsonSchema, - rootSchema: JsonSchema -) => { - const resolvedSchema = Resolve.schema(schema, schema.$ref, rootSchema); + rootSchema: JsonSchema, + defaultValue: any = {} +): any => { + const resolvedSchema = + typeof schema.$ref === 'string' + ? Resolve.schema(rootSchema, schema.$ref, rootSchema) + : schema; if (resolvedSchema.default !== undefined) { return extractDefaults(resolvedSchema, rootSchema); } @@ -196,8 +200,44 @@ export const createDefaultValue = ( return extractDefaults(resolvedSchema, rootSchema); } else if (hasType(resolvedSchema, 'null')) { return null; + } else if ( + schema.oneOf && + Array.isArray(schema.oneOf) && + schema.oneOf.length > 0 + ) { + for (const combineSchema of schema.oneOf) { + const result = createDefaultValue(combineSchema, rootSchema, undefined); + if (result !== undefined) { + // return the first one that have type information + return result; + } + } + } else if ( + schema.anyOf && + Array.isArray(schema.anyOf) && + schema.anyOf.length > 0 + ) { + for (const combineSchema of schema.anyOf) { + const result = createDefaultValue(combineSchema, rootSchema, undefined); + if (result !== undefined) { + // return the first one that have type information + return result; + } + } + } else if ( + schema.allOf && + Array.isArray(schema.allOf) && + schema.allOf.length > 0 + ) { + for (const combineSchema of schema.allOf) { + const result = createDefaultValue(combineSchema, rootSchema, undefined); + if (result !== undefined) { + // return the first one that have type information + return result; + } + } } else { - return {}; + return defaultValue; } }; From 308bde1577a922ae538e7e3cf238d4f2ffd230c8 Mon Sep 17 00:00:00 2001 From: Krasimir Chobantonov Date: Thu, 5 Dec 2024 12:03:34 -0500 Subject: [PATCH 2/6] fixes based on the suggestions --- packages/core/src/mappers/renderer.ts | 80 ++++++++++++++++----------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index b5b6aa0218..26ef27a2f7 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -200,44 +200,58 @@ export const createDefaultValue = ( return extractDefaults(resolvedSchema, rootSchema); } else if (hasType(resolvedSchema, 'null')) { return null; - } else if ( - schema.oneOf && - Array.isArray(schema.oneOf) && - schema.oneOf.length > 0 - ) { - for (const combineSchema of schema.oneOf) { - const result = createDefaultValue(combineSchema, rootSchema, undefined); - if (result !== undefined) { - // return the first one that have type information - return result; - } - } - } else if ( - schema.anyOf && - Array.isArray(schema.anyOf) && - schema.anyOf.length > 0 - ) { - for (const combineSchema of schema.anyOf) { - const result = createDefaultValue(combineSchema, rootSchema, undefined); - if (result !== undefined) { - // return the first one that have type information - return result; - } - } - } else if ( - schema.allOf && - Array.isArray(schema.allOf) && - schema.allOf.length > 0 + } + + let combinatorDefault = undefined; + + combinatorDefault = createDefaultValueForCombinatorSchema( + schema.oneOf, + rootSchema + ); + if (combinatorDefault !== undefined) { + return combinatorDefault; + } + + combinatorDefault = createDefaultValueForCombinatorSchema( + schema.anyOf, + rootSchema + ); + if (combinatorDefault !== undefined) { + return combinatorDefault; + } + + combinatorDefault = createDefaultValueForCombinatorSchema( + schema.allOf, + rootSchema + ); + if (combinatorDefault !== undefined) { + return combinatorDefault; + } + + return defaultValue; +}; + +const createDefaultValueForCombinatorSchema = ( + combinatorSchema: JsonSchema[], + rootSchema: JsonSchema +) => { + if ( + combinatorSchema && + Array.isArray(combinatorSchema) && + combinatorSchema.length > 0 ) { - for (const combineSchema of schema.allOf) { - const result = createDefaultValue(combineSchema, rootSchema, undefined); - if (result !== undefined) { + const noDefaultValue = Symbol.for('noDefaultValue'); + for (const combineSchema of combinatorSchema) { + const result = createDefaultValue( + combineSchema, + rootSchema, + noDefaultValue + ); + if (result !== noDefaultValue) { // return the first one that have type information return result; } } - } else { - return defaultValue; } }; From c3fbea05f2f59d0ef8e71651e92e37c5151c592b Mon Sep 17 00:00:00 2001 From: Krasimir Chobantonov Date: Thu, 5 Dec 2024 12:28:11 -0500 Subject: [PATCH 3/6] add explicit return value --- packages/core/src/mappers/renderer.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index 26ef27a2f7..e980c83642 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -253,6 +253,9 @@ const createDefaultValueForCombinatorSchema = ( } } } + + // no default value found + return undefined; }; /** From 1b614da3a93dbbaf96dcf938a8597ec40b347bb7 Mon Sep 17 00:00:00 2001 From: Krasimir Chobantonov Date: Thu, 5 Dec 2024 13:51:55 -0500 Subject: [PATCH 4/6] use doCreateDefaultValue which can return undefined when no default value is found --- packages/core/src/mappers/renderer.ts | 34 +++++++++++++++------------ 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index e980c83642..2b0e1f7da8 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -168,13 +168,21 @@ export const showAsRequired = ( */ export const createDefaultValue = ( schema: JsonSchema, - rootSchema: JsonSchema, - defaultValue: any = {} -): any => { - const resolvedSchema = - typeof schema.$ref === 'string' - ? Resolve.schema(rootSchema, schema.$ref, rootSchema) - : schema; + rootSchema: JsonSchema +) => { + const defaultValue = doCreateDefaultValue(schema, rootSchema); + + // preserve the backward compatibility where it is returning an empty object if we can't determine the default value + return defaultValue === undefined ? {} : defaultValue; +}; + +/** + * Create a default value based on the given schema. + * @param schema the schema for which to create a default value. + * @returns undefined if no default value, the default value to use otherwise + */ +const doCreateDefaultValue = (schema: JsonSchema, rootSchema: JsonSchema) => { + const resolvedSchema = Resolve.schema(schema, schema.$ref, rootSchema); if (resolvedSchema.default !== undefined) { return extractDefaults(resolvedSchema, rootSchema); } @@ -228,7 +236,8 @@ export const createDefaultValue = ( return combinatorDefault; } - return defaultValue; + // no default value found + return undefined; }; const createDefaultValueForCombinatorSchema = ( @@ -240,14 +249,9 @@ const createDefaultValueForCombinatorSchema = ( Array.isArray(combinatorSchema) && combinatorSchema.length > 0 ) { - const noDefaultValue = Symbol.for('noDefaultValue'); for (const combineSchema of combinatorSchema) { - const result = createDefaultValue( - combineSchema, - rootSchema, - noDefaultValue - ); - if (result !== noDefaultValue) { + const result: any = doCreateDefaultValue(combineSchema, rootSchema); + if (result !== undefined) { // return the first one that have type information return result; } From d805688be2c2fc66bfec36b61fc98770d5d40f0d Mon Sep 17 00:00:00 2001 From: Krasimir Chobantonov Date: Thu, 5 Dec 2024 13:56:05 -0500 Subject: [PATCH 5/6] only resolve the schema if the $ref is of type string --- packages/core/src/mappers/renderer.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index 2b0e1f7da8..2031db9ea3 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -182,7 +182,10 @@ export const createDefaultValue = ( * @returns undefined if no default value, the default value to use otherwise */ const doCreateDefaultValue = (schema: JsonSchema, rootSchema: JsonSchema) => { - const resolvedSchema = Resolve.schema(schema, schema.$ref, rootSchema); + const resolvedSchema = + typeof schema.$ref === 'string' + ? Resolve.schema(rootSchema, schema.$ref, rootSchema) + : schema; if (resolvedSchema.default !== undefined) { return extractDefaults(resolvedSchema, rootSchema); } From 86bccc005f2615647235c13a7c08567325d71181 Mon Sep 17 00:00:00 2001 From: Stefan Dirix Date: Fri, 6 Dec 2024 14:40:11 +0100 Subject: [PATCH 6/6] add testcases, add allof extraction --- packages/core/src/mappers/renderer.ts | 94 +++++++------- packages/core/test/mappers/renderer.test.ts | 128 ++++++++++++++++++++ 2 files changed, 177 insertions(+), 45 deletions(-) diff --git a/packages/core/src/mappers/renderer.ts b/packages/core/src/mappers/renderer.ts index 2031db9ea3..ceedad8144 100644 --- a/packages/core/src/mappers/renderer.ts +++ b/packages/core/src/mappers/renderer.ts @@ -179,9 +179,12 @@ export const createDefaultValue = ( /** * Create a default value based on the given schema. * @param schema the schema for which to create a default value. - * @returns undefined if no default value, the default value to use otherwise + * @returns the default value to use, undefined if none was found */ -const doCreateDefaultValue = (schema: JsonSchema, rootSchema: JsonSchema) => { +export const doCreateDefaultValue = ( + schema: JsonSchema, + rootSchema: JsonSchema +) => { const resolvedSchema = typeof schema.$ref === 'string' ? Resolve.schema(rootSchema, schema.$ref, rootSchema) @@ -198,45 +201,34 @@ const doCreateDefaultValue = (schema: JsonSchema, rootSchema: JsonSchema) => { return convertDateToString(new Date(), resolvedSchema.format); } return ''; - } else if ( - hasType(resolvedSchema, 'integer') || - hasType(resolvedSchema, 'number') - ) { + } + if (hasType(resolvedSchema, 'integer') || hasType(resolvedSchema, 'number')) { return 0; - } else if (hasType(resolvedSchema, 'boolean')) { + } + if (hasType(resolvedSchema, 'boolean')) { return false; - } else if (hasType(resolvedSchema, 'array')) { + } + if (hasType(resolvedSchema, 'array')) { return []; - } else if (hasType(resolvedSchema, 'object')) { - return extractDefaults(resolvedSchema, rootSchema); - } else if (hasType(resolvedSchema, 'null')) { - return null; } - - let combinatorDefault = undefined; - - combinatorDefault = createDefaultValueForCombinatorSchema( - schema.oneOf, - rootSchema - ); - if (combinatorDefault !== undefined) { - return combinatorDefault; + if (hasType(resolvedSchema, 'object')) { + return extractDefaults(resolvedSchema, rootSchema); } - - combinatorDefault = createDefaultValueForCombinatorSchema( - schema.anyOf, - rootSchema - ); - if (combinatorDefault !== undefined) { - return combinatorDefault; + if (hasType(resolvedSchema, 'null')) { + return null; } - combinatorDefault = createDefaultValueForCombinatorSchema( - schema.allOf, - rootSchema - ); - if (combinatorDefault !== undefined) { - return combinatorDefault; + const combinators: CombinatorKeyword[] = ['oneOf', 'anyOf', 'allOf']; + for (const combinator of combinators) { + if (schema[combinator] && Array.isArray(schema[combinator])) { + const combinatorDefault = createDefaultValueForCombinatorSchema( + schema[combinator], + rootSchema + ); + if (combinatorDefault !== undefined) { + return combinatorDefault; + } + } } // no default value found @@ -244,18 +236,14 @@ const doCreateDefaultValue = (schema: JsonSchema, rootSchema: JsonSchema) => { }; const createDefaultValueForCombinatorSchema = ( - combinatorSchema: JsonSchema[], + combinatorSchemas: JsonSchema[], rootSchema: JsonSchema -) => { - if ( - combinatorSchema && - Array.isArray(combinatorSchema) && - combinatorSchema.length > 0 - ) { - for (const combineSchema of combinatorSchema) { - const result: any = doCreateDefaultValue(combineSchema, rootSchema); +): any => { + if (combinatorSchemas.length > 0) { + for (const combinatorSchema of combinatorSchemas) { + const result = doCreateDefaultValue(combinatorSchema, rootSchema); if (result !== undefined) { - // return the first one that have type information + // return the first one with type information return result; } } @@ -278,10 +266,26 @@ export const extractDefaults = (schema: JsonSchema, rootSchema: JsonSchema) => { const resolvedProperty = property.$ref ? Resolve.schema(rootSchema, property.$ref, rootSchema) : property; - if (resolvedProperty.default !== undefined) { + if (resolvedProperty && resolvedProperty.default !== undefined) { result[key] = cloneDeep(resolvedProperty.default); } } + // there could be more properties in allOf schemas + if (schema.allOf && Array.isArray(schema.allOf)) { + schema.allOf.forEach((allOfSchema) => { + if (allOfSchema && allOfSchema.properties) { + for (const key in allOfSchema.properties) { + const property = allOfSchema.properties[key]; + const resolvedProperty = property.$ref + ? Resolve.schema(rootSchema, property.$ref, rootSchema) + : property; + if (resolvedProperty && resolvedProperty.default !== undefined) { + result[key] = cloneDeep(resolvedProperty.default); + } + } + } + }); + } return result; } return cloneDeep(schema.default); diff --git a/packages/core/test/mappers/renderer.test.ts b/packages/core/test/mappers/renderer.test.ts index 8e686e884b..f8eee2982d 100644 --- a/packages/core/test/mappers/renderer.test.ts +++ b/packages/core/test/mappers/renderer.test.ts @@ -654,6 +654,134 @@ test('createDefaultValue', (t) => { bool: true, array: ['a', 'b', 'c'], }); + + const schemaOneOf: JsonSchema = { + oneOf: [ + { + type: 'string', + default: 'oneOfString', + }, + { + type: 'number', + default: 42, + }, + ], + }; + const rootSchemaOneOf: JsonSchema = { + definitions: {}, + }; + const defaultValueOneOf = createDefaultValue(schemaOneOf, rootSchemaOneOf); + t.is(defaultValueOneOf, 'oneOfString'); + + const schemaAnyOf: JsonSchema = { + anyOf: [ + { + type: 'number', + }, + { + type: 'string', + default: 'anyOfString', + }, + ], + }; + const rootSchemaAnyOf: JsonSchema = { + definitions: {}, + }; + const defaultValueAnyOf = createDefaultValue(schemaAnyOf, rootSchemaAnyOf); + t.is(defaultValueAnyOf, 0); + + console.log('testcase allof'); + const schemaAllOf: JsonSchema = { + allOf: [ + { + properties: { + foo: { + type: 'string', + default: 'foo', + }, + }, + }, + { + properties: { + bar: { + type: 'number', + default: 42, + }, + }, + }, + ], + }; + const rootSchemaAllOf: JsonSchema = { + definitions: {}, + }; + const defaultValueAllOf = createDefaultValue(schemaAllOf, rootSchemaAllOf); + t.deepEqual(defaultValueAllOf, { foo: 'foo', bar: 42 }); + + const schemaOneOfEmpty: JsonSchema = { + oneOf: [ + { + type: 'string', + }, + { + type: 'number', + }, + ], + }; + const rootSchemaOneOfEmpty: JsonSchema = { + definitions: {}, + }; + const defaultValueOneOfEmpty = createDefaultValue( + schemaOneOfEmpty, + rootSchemaOneOfEmpty + ); + t.deepEqual(defaultValueOneOfEmpty, ''); + + const schemaAnyOfEmpty: JsonSchema = { + anyOf: [ + { + type: 'string', + }, + { + type: 'number', + }, + ], + }; + const rootSchemaAnyOfEmpty: JsonSchema = { + definitions: {}, + }; + const defaultValueAnyOfEmpty = createDefaultValue( + schemaAnyOfEmpty, + rootSchemaAnyOfEmpty + ); + t.deepEqual(defaultValueAnyOfEmpty, ''); + + const schemaAllOfEmpty: JsonSchema = { + allOf: [ + { + properties: { + foo: { + type: 'string', + }, + }, + }, + { + properties: { + bar: { + type: 'number', + }, + }, + }, + ], + }; + const rootSchemaAllOfEmpty: JsonSchema = { + definitions: {}, + }; + const defaultValueAllOfEmpty = createDefaultValue( + schemaAllOfEmpty, + rootSchemaAllOfEmpty + ); + console.log('defaultValueAllOfEmpty', defaultValueAllOfEmpty); + t.deepEqual(defaultValueAllOfEmpty, {}); }); test(`mapStateToJsonFormsRendererProps should use registered UI schema given ownProps schema`, (t) => {