Fix ref resolving for oneOf and array schemas

- resolve items subschema refs for renderers and testers - make rootSchema a required parameter in resolveSchema, as it is needed while resolving - use resolved schemas in MaterialOneOfRenderer when computing default value - enable and update tests - remove 'Resolve Remote Schema' example as it's unsupported
eclipsesource · Feb 8, 2022 · 4571c0f · 4571c0f
1 parent d65d7eb
commit 4571c0f
Show file tree

Hide file tree

Showing 15 changed files with 160 additions and 175 deletions.
diff --git a/packages/core/src/generators/uischema.ts b/packages/core/src/generators/uischema.ts
@@ -122,7 +122,7 @@ const generateUISchema = (
 ): UISchemaElement => {
   if (!isEmpty(jsonSchema) && jsonSchema.$ref !== undefined) {
     return generateUISchema(
-      resolveSchema(rootSchema, jsonSchema.$ref),
+      resolveSchema(rootSchema, jsonSchema.$ref, rootSchema),
       schemaElements,
       currentRef,
       schemaName,
@@ -164,7 +164,7 @@ const generateUISchema = (
         let value = jsonSchema.properties[propName];
         const ref = `${nextRef}/${encode(propName)}`;
         if (value.$ref !== undefined) {
-          value = resolveSchema(rootSchema, value.$ref);
+          value = resolveSchema(rootSchema, value.$ref, rootSchema);
         }
         generateUISchema(
           value,

diff --git a/packages/core/src/testers/testers.ts b/packages/core/src/testers/testers.ts
@@ -86,7 +86,10 @@ export const schemaMatches = (
   }
   let currentDataSchema = schema;
   if (hasType(schema, 'object')) {
-    currentDataSchema = resolveSchema(schema, schemaPath);
+    currentDataSchema = resolveSchema(schema, schemaPath, schema);
+  }
+  if (hasType(currentDataSchema, 'array')) {
+    currentDataSchema = { ...currentDataSchema, items: (resolveSchema(currentDataSchema, 'items', schema)) } as JsonSchema
   }
   if (currentDataSchema === undefined) {
     return false;
@@ -105,7 +108,10 @@ export const schemaSubPathMatches = (
   const schemaPath = uischema.scope;
   let currentDataSchema: JsonSchema = schema;
   if (hasType(schema, 'object')) {
-    currentDataSchema = resolveSchema(schema, schemaPath);
+    currentDataSchema = resolveSchema(schema, schemaPath, schema);
+  }
+  if (hasType(currentDataSchema, 'array')) {
+    currentDataSchema = { ...currentDataSchema, items: (resolveSchema(currentDataSchema, 'items', schema)) } as JsonSchema
   }
   currentDataSchema = get(currentDataSchema, subPath);
 
@@ -425,7 +431,7 @@ export const isObjectArrayWithNesting = (
     return false;
   }
   const schemaPath = (uischema as ControlElement).scope;
-  const resolvedSchema = resolveSchema(schema, schemaPath);
+  const resolvedSchema = resolveSchema(schema, schemaPath, schema);
   const wantedNestingByType: { [key: string]: number } = {
     object: 2,
     array: 1
@@ -437,6 +443,9 @@ export const isObjectArrayWithNesting = (
         if (val === schema) {
           return false;
         }
+        if (val.$ref !== undefined) {
+          return false;
+        }
         // we don't support multiple types
         if (typeof val.type !== 'string') {
           return true;

diff --git a/packages/core/src/util/combinators.ts b/packages/core/src/util/combinators.ts
@@ -24,7 +24,7 @@
 */
 
 import { ControlElement, JsonSchema, UISchemaElement } from '../models';
-import { resolveSchema } from './resolvers';
+import { resolveArraySchema, resolveSchema } from './resolvers';
 import { findUISchema, JsonFormsUISchemaRegistryEntry } from '../reducers';
 
 export interface CombinatorSubSchemaRenderInfo {
@@ -58,7 +58,7 @@ export const resolveSubSchemas = (
     return {
       ...schema,
       [keyword]: (schema[keyword] as any[]).map(e =>
-        e.$ref ? resolveSchema(rootSchema, e.$ref) : e
+        e.$ref ? resolveArraySchema(resolveSchema(rootSchema, e.$ref, rootSchema), rootSchema) : e
       )
     };
   }

diff --git a/packages/core/src/util/renderer.ts b/packages/core/src/util/renderer.ts
@@ -439,11 +439,10 @@ export const mapStateToControlProps = (
   const required =
     controlElement.scope !== undefined &&
     isRequired(ownProps.schema, controlElement.scope, rootSchema);
-  const resolvedSchema = Resolve.schema(
-    ownProps.schema || rootSchema,
-    controlElement.scope,
-    rootSchema
-  );
+  const resolvedSchema = Resolve.arraySchema(
+    Resolve.schema(ownProps.schema || rootSchema, controlElement.scope, rootSchema),
+    rootSchema);
+
   const errors = getErrorAt(path, resolvedSchema)(state);
 
   const description =

diff --git a/packages/core/src/util/resolvers.ts b/packages/core/src/util/resolvers.ts
@@ -59,6 +59,17 @@ export const resolveData = (instance: any, dataPath: string): any => {
     }, instance);
 };
 
+export const resolveArraySchema = (schema: JsonSchema,
+  rootSchema: JsonSchema):JsonSchema => {
+  if (schema?.items && (schema.items as any).$ref) {
+    return {
+      ...schema,
+      items: resolveSchema(schema, 'items', rootSchema)
+    } as JsonSchema
+  }
+  return schema;
+}
+
 /**
  * Finds all references inside the given schema.
  *
@@ -111,7 +122,7 @@ const invalidSegment = (pathSegment: string) =>
 export const resolveSchema = (
   schema: JsonSchema,
   schemaPath: string,
-  rootSchema?: JsonSchema
+  rootSchema: JsonSchema
 ): JsonSchema => {
   if (isEmpty(schema)) {
     return undefined;
@@ -123,7 +134,7 @@ export const resolveSchema = (
     resultSchema =
       resultSchema === undefined || resultSchema.$ref === undefined
         ? resultSchema
-        : resolveSchema(schema, resultSchema.$ref);
+        : resolveSchema(schema, resultSchema.$ref, rootSchema);
     if (invalidSegment(pathSegment)) {
       // skip invalid segments
       continue;
@@ -137,7 +148,7 @@ export const resolveSchema = (
         resultSchema?.anyOf ?? []
       );
       for (let item of schemas) {
-        curSchema = resolveSchema(item, validPathSegments.slice(i).map(encode).join('/'));
+        curSchema = resolveSchema(item, validPathSegments.slice(i).map(encode).join('/'), rootSchema);
         if (curSchema) {
           break;
         }
@@ -155,36 +166,10 @@ export const resolveSchema = (
   // with absolute paths here, so that we don't need to keep two different
   // schemas around
   if (resultSchema !== undefined && resultSchema.$ref !== undefined) {
-    try {
-      return retrieveResolvableSchema(schema, resultSchema.$ref);
-    } catch (e) {
-      return retrieveResolvableSchema(rootSchema, resultSchema.$ref);
-    }
+    return resolveSchema(schema, resultSchema.$ref, rootSchema)
+      ?? resolveSchema(rootSchema, resultSchema.$ref, rootSchema)
+      ?? schema;
   }
 
   return resultSchema;
 };
-
-/**
- * Normalizes the schema and resolves the given ref.
- *
- * @param {JsonSchema} full the JSON schema to resolved the reference against
- * @param {string} reference the reference to be resolved
- * @returns {JsonSchema} the resolved sub-schema
- */
-// disable rule because resolve is mutually recursive
-// tslint:disable:only-arrow-functions
-function retrieveResolvableSchema(
-  full: JsonSchema,
-  reference: string
-): JsonSchema {
-  // tslint:enable:only-arrow-functions
-  const child = resolveSchema(full, reference);
-  const allRefs = findAllRefs(child);
-  const innerSelfReference = allRefs[reference];
-  if (innerSelfReference !== undefined) {
-    innerSelfReference.$ref = '#';
-  }
-
-  return child;
-}
diff --git a/packages/core/src/util/util.ts b/packages/core/src/util/util.ts
@@ -28,7 +28,7 @@ import isArray from 'lodash/isArray';
 import includes from 'lodash/includes';
 import find from 'lodash/find';
 import { JsonSchema, Scopable, UISchemaElement } from '..';
-import { resolveData, resolveSchema } from './resolvers';
+import { resolveArraySchema, resolveData, resolveSchema } from './resolvers';
 import { composePaths, toDataPathSegments } from './path';
 import { isEnabled, isVisible } from './runtime';
 import Ajv from 'ajv';
@@ -96,15 +96,21 @@ export const deriveTypes = (jsonSchema: JsonSchema): string[] => {
 * Convenience wrapper around resolveData and resolveSchema.
 */
 export const Resolve: {
- schema(
-   schema: JsonSchema,
-   schemaPath: string,
-   rootSchema?: JsonSchema
- ): JsonSchema;
- data(data: any, path: string): any;
+  schema(
+    schema: JsonSchema,
+    schemaPath: string,
+    rootSchema?: JsonSchema
+  ): JsonSchema;
+  arraySchema(
+    schema: JsonSchema,
+    rootSchema?: JsonSchema
+  ): JsonSchema;
+  data(data: any, path: string): any;
+
 } = {
- schema: resolveSchema,
- data: resolveData
+  schema: resolveSchema,
+  arraySchema: resolveArraySchema,
+  data: resolveData
 };
 
 // Paths --

diff --git a/packages/core/test/testers.test.ts b/packages/core/test/testers.test.ts
@@ -230,6 +230,30 @@ test('schemaMatches should return false for control pointing to invalid subschem
   t.false(schemaMatches(() => false)(uischema, schema));
 });
 
+test('schemaMatches should check type sub-schema of array items ref', t => {
+  const schema: JsonSchema = {
+    definitions: {
+      order: {
+        type: 'string'
+      }
+    },
+    type: 'object',
+    properties: {
+      foo: { type: 'array',
+      items: {
+        $ref: '#/definitions/order'
+      } }
+    }
+  };
+  const uischema: ControlElement = {
+    type: 'Control',
+    scope: '#/properties/foo'
+  };
+  t.true(
+    schemaMatches(subSchema => (subSchema.items as any).type === 'string')(uischema, schema)
+  );
+});
+
 test('scopeEndsWith checks whether the ref of a control ends with a certain string', t => {
   const uischema: ControlElement = {
     type: 'Control',
@@ -423,6 +447,28 @@ test('tester isObjectArrayControl', t => {
     }
   };
   t.true(isObjectArrayControl(control, schema_innerAllOf));
+
+  const schemaWithRefs = {
+    definitions: {
+      order: {
+        type: 'object',
+        properties: {
+          x: { type: 'integer' },
+          y: { type: 'integer' }
+        }
+      }
+    },
+    type: 'object',
+    properties: {
+      foo: {
+        type: 'array',
+        items: {
+          $ref: '#/definitions/order'
+        }
+      }
+    }
+  }
+  t.true(isObjectArrayControl(control, schemaWithRefs));
 });
 
 test('isBooleanControl', t => {
@@ -771,12 +817,39 @@ test('tester schemaSubPathMatches', t => {
     type: 'Control',
     scope: '#'
   };
+
+  const schemaWithRef = {
+    definitions: {
+      foobar: { type: 'number' }
+    },
+    properties: {
+      test: {
+        type: 'array',
+        items: {
+          $ref: "#/definitions/foobar"
+        }
+      }
+    }
+  }
+
+  const refUiSchema: ControlElement = {
+    type: 'Control',
+    scope: '#/properties/test'
+  };
+
   t.true(
     schemaSubPathMatches('items', items => items.type === 'number')(
       uischema,
       schema
     )
   );
+
+  t.true(
+    schemaSubPathMatches('items', items => items.type === 'number')(
+      refUiSchema,
+      schemaWithRef
+    )
+  );
 });
 
 test('tester not', t => {

diff --git a/packages/core/test/util/resolvers.test.ts b/packages/core/test/util/resolvers.test.ts
@@ -56,13 +56,13 @@ test('resolveSchema - resolves schema with any ', t => {
     }
   };
   // test backward compatibility
-  t.deepEqual(resolveSchema(schema, '#/properties/description/oneOf/0/properties/name'), {type: 'string'});
-  t.deepEqual(resolveSchema(schema, '#/properties/description/oneOf/1/properties/index'), {type: 'number'});
+  t.deepEqual(resolveSchema(schema, '#/properties/description/oneOf/0/properties/name', schema), {type: 'string'});
+  t.deepEqual(resolveSchema(schema, '#/properties/description/oneOf/1/properties/index', schema), {type: 'number'});
   // new simple approach
-  t.deepEqual(resolveSchema(schema, '#/properties/description/properties/name'), {type: 'string'});
-  t.deepEqual(resolveSchema(schema, '#/properties/description/properties/index'), {type: 'number'});
-  t.deepEqual(resolveSchema(schema, '#/properties/description/properties/exist'), {type: 'boolean'});
-  t.is(resolveSchema(schema, '#/properties/description/properties/notfound'), undefined);
+  t.deepEqual(resolveSchema(schema, '#/properties/description/properties/name', schema), {type: 'string'});
+  t.deepEqual(resolveSchema(schema, '#/properties/description/properties/index', schema), {type: 'number'});
+  t.deepEqual(resolveSchema(schema, '#/properties/description/properties/exist', schema), {type: 'boolean'});
+  t.is(resolveSchema(schema, '#/properties/description/properties/notfound', schema), undefined);
 });
 
 test('resolveSchema - resolves schema with encoded characters', t => {
@@ -74,6 +74,6 @@ test('resolveSchema - resolves schema with encoded characters', t => {
       }
     }
   };
-  t.deepEqual(resolveSchema(schema, '#/properties/foo ~1 ~0 bar'), {type: 'integer'});
-  t.is(resolveSchema(schema, '#/properties/foo / bar'), undefined);
+  t.deepEqual(resolveSchema(schema, '#/properties/foo ~1 ~0 bar', schema), {type: 'integer'});
+  t.is(resolveSchema(schema, '#/properties/foo / bar', schema), undefined);
 });