Skip to content

indexOfFittingSchema for oneOf doesn't work anymore if $ref is used #1991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Fankhauser-Dominik opened this issue Jul 28, 2022 · 3 comments · Fixed by #2001
Closed

indexOfFittingSchema for oneOf doesn't work anymore if $ref is used #1991

Fankhauser-Dominik opened this issue Jul 28, 2022 · 3 comments · Fixed by #2001
Assignees
Milestone

Comments

@Fankhauser-Dominik
Copy link

Describe the bug

Since Version 3.0.0-beta.2, the indexOfFittingSchema returns undefined instead, for example 0 or 1.
The Previous Versions worked fine.

If I use the same schema without any refs, it works fine.

oneOf with $ref (official oneOf Example)
{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "definitions": {
    "address": {
      "type": "object",
      "properties": {
        "street_address": {
          "type": "string"
        },
        "city": {
          "type": "string"
        },
        "state": {
          "type": "string"
        }
      },
      "required": [
        "street_address",
        "city",
        "state"
      ],
      "additionalProperties": false
    },
    "user": {
      "type": "object",
      "properties": {
        "name": {
          "type": "string"
        },
        "mail": {
          "type": "string"
        }
      },
      "required": [
        "name",
        "mail"
      ],
      "additionalProperties": false
    }
  },
  "type": "object",
  "properties": {
    "name": {
      "type": "string"
    },
    "addressOrUser": {
      "oneOf": [
        {
          "$ref": "#/definitions/address"
        },
        {
          "$ref": "#/definitions/user"
        }
      ]
    }
  },
  "required": [
    "name"
  ]
}
oneOf without $ref
{
  "type": "object",
  "properties": {
    "name": {
      "type": "string"
    },
    "addressOrUser": {
      "oneOf": [
        {
          "type": "object",
          "properties": {
            "street_address": {
              "type": "string"
            },
            "city": {
              "type": "string"
            },
            "state": {
              "type": "string"
            }
          },
          "required": [
            "street_address",
            "city",
            "state"
          ],
          "additionalProperties": false
        },
        {
          "type": "object",
          "properties": {
            "name": {
              "type": "string"
            },
            "mail": {
              "type": "string"
            }
          },
          "required": [
            "name",
            "mail"
          ],
          "additionalProperties": false
        }
      ]
    }
  },
  "required": [
    "name"
  ]
}

Expected behavior

If the bound data matches a oneOf Schema, then it should have an indexOfFittingSchema.

Steps to reproduce the issue

  1. Go to https://codesandbox.io/s/vigilant-rgb-unncn8?file=/src/schema.json
  2. Because of the bound data it should render the ONEOF-1 Tab, but it renders the ONEOF-0 Tab (default behavior).
  3. Change the JsonForms versions to 3.0.0-beta.1
  4. You will see, that it now works properly

Screenshots

No response

In which browser are you experiencing the issue?

Browser independent

Framework

Core

RendererSet

Material

Additional context

No response

@golinski
Copy link

golinski commented Aug 5, 2022

I've bisected and stepped through the code a bit and found the culprit in #1829.

Before the merge mapStateToCombinatorRendererProps used the following logic to find the fitting subschema:

  ...
  const resolvedSchema = Resolve.schema(
    ownProps.schema || rootSchema,
    uischema.scope,
    rootSchema
  );
  ...
  const schema = resolvedSchema || rootSchema;
  const _schema = resolveSubSchemas(schema, rootSchema, keyword);
  ...
  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 {
      const valFn = ajv.compile(_schema[keyword][i]);
      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");
    }
  }

which works, because _schema contains full schemas that can be compiled. For reference, in the example by @Fankhauser-Dominik this is:

{
    "oneOf": [
        {
            "type": "object",
            "properties": {
                "street_address": {
                    "type": "string"
                },
                "city": {
                    "type": "string"
                },
                "state": {
                    "type": "string"
                }
            },
            "required": [
                "street_address",
                "city",
                "state"
            ],
            "additionalProperties": false
        },
        {
            "type": "object",
            "properties": {
                "name": {
                    "type": "string"
                },
                "mail": {
                    "type": "string"
                }
            },
            "required": [
                "name",
                "mail"
            ],
            "additionalProperties": false
        }
    ]
}

After #1829 was merged, the logic was changed to

  const resolvedSchema = Resolve.schema(
    ownProps.schema || rootSchema,
    uischema.scope,
    rootSchema
  );
  ...
  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 < resolvedSchema[keyword]?.length; i++) {
    try {
      const valFn = ajv.compile(resolvedSchema[keyword][i]);
      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");
    }
  }

This fails with the given debug message as resolvedSchema is now:

{
    "oneOf": [
        {
            "$ref": "#/definitions/address"
        },
        {
            "$ref": "#/definitions/user"
        }
    ]
}

@sdirix
Copy link
Member

sdirix commented Aug 8, 2022

Thanks for the analysis! We'll take a look.

In case you need a workaround until the fix is done you can use json-refs to resolve your JSON Schemas before handing them to JSON Forms.

@sdirix sdirix added this to the 3.0 milestone Aug 8, 2022
LukasBoll added a commit to LukasBoll/jsonforms that referenced this issue Aug 15, 2022
@sdirix sdirix linked a pull request Aug 19, 2022 that will close this issue
@sdirix
Copy link
Member

sdirix commented Aug 19, 2022

Within the combinator mappings we try to determine the best fitting schema so that the
renderer can decide which tab to show. With the removal of the auto-resolving this no
longer worked in cases where the combinator contained references.

The situation is now improved by resolving references on the combinator level. This
should help in many use cases but still does not restore the old behavior completely.
Fully checking and resolving the whole sub schema is out of the scope with the new
simplified resolving architecture. If that is needed the user either needs use a custom
renderer or restore the old behavior by fully resolving their JSON Schema before handing
it over to JSON Forms, for example via json-refs.

Fixed in #2001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants