Skip to content
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

Errors with different path got overwritten #81

Open
a1png opened this issue Jul 30, 2020 · 3 comments
Open

Errors with different path got overwritten #81

a1png opened this issue Jul 30, 2020 · 3 comments
Labels

Comments

@a1png
Copy link

a1png commented Jul 30, 2020

When there are errors of more than one different paths in ajv error list, the helpers.makeTree function will overwrite the previous errors in the path with the new one in the reduce function:

paths &&
      paths.reduce((obj, path, i) => {
        obj.children[path] = obj.children[path] || { children: {}, errors: [] };
        if (i === paths.length - 1) {
          obj.children[path].errors.push(ajvError);
        }
        return obj.children[path];
      }, root);

Is this an intended design?

@torifat torifat added the bug label Aug 6, 2020
@torifat
Copy link
Collaborator

torifat commented Aug 6, 2020

@a1png it would be really helpful if you can provide an example 🙂

@a1png
Copy link
Author

a1png commented Aug 6, 2020

@torifat Here is an example:
The schema:

const schema = {
    type: 'object',
    properties: {
      test: {
        anyOf: [
          {
            type: "object",
            properties: {
              a: {
                type: "string",
                enum: ["test_1", "test_2"]
              }
            },
            required: [ "a" ],
            "additionalProperties": false
          },
          {
            type: "object",
            properties: {
              a: {
                type: "string",
                enum: ["b"]
              },
              b: {
                type: "string",
                enum: ["test_1"]
              }
            },
            required: ["a", "b"],
            "additionalProperties": false
          }
        ],
      },
    },
    "additionalProperties": false
  };

The validated data:

const test_data = {
  test: {
    a: "wrong input",
  }
}

The error in ajv.erros:

[ { keyword: 'enum',
    dataPath: '/test/a',
    schemaPath: '#/properties/test/anyOf/0/properties/a/enum',
    params: { allowedValues: [Array] },
    message: 'should be equal to one of the allowed values' },
  { keyword: 'enum',
    dataPath: '/test/a',
    schemaPath: '#/properties/test/anyOf/1/properties/a/enum',
    params: { allowedValues: [Array] },
    message: 'should be equal to one of the allowed values' },
  { keyword: 'required',
    dataPath: '/test',
    schemaPath: '#/properties/test/anyOf/1/required',
    params: { missingProperty: 'b' },
    message: 'should have required property \'b\'' },
  { keyword: 'anyOf',
    dataPath: '/test',
    schemaPath: '#/properties/test/anyOf',
    params: {},
    message: 'should match some schema in anyOf' } ]

The error message returned from the library:

[ '/test should have required property \'b\'' ]

@a1png
Copy link
Author

a1png commented Aug 31, 2020

@torifat After looking into the code, I found that the problem is actually not with the tree generation, but in filterRedundantErrors. In the function, 'required' is set as first priority and other errors are filtered out once this required error is presented:

  /**
   * If there is a `required` error then we can just skip everythig else.
   * And, also `required` should have more priority than `anyOf`. @see #8
   */
  (0, _utils.getErrors)(root).forEach(error => {
    if ((0, _utils.isRequiredError)(error)) {
      root.errors = [error];
      root.children = {};
    }
  });

So in this situation, the error tree generated is:

{
  "children": {
    "/test": {
      "children": {
        "/a": {
          "children": {},
          "errors": [
            {
              "keyword": "enum",
              "dataPath": "/test/a",
              "schemaPath": "#/properties/test/anyOf/0/properties/a/enum",
              "params": {
                "allowedValues": [
                  "test_1",
                  "test_2"
                ]
              },
              "message": "should be equal to one of the allowed values"
            },
            {
              "keyword": "enum",
              "dataPath": "/test/a",
              "schemaPath": "#/properties/test/anyOf/1/properties/a/enum",
              "params": {
                "allowedValues": [
                  "b"
                ]
              },
              "message": "should be equal to one of the allowed values"
            }
          ]
        }
      },
      "errors": [
        {
          "keyword": "required",
          "dataPath": "/test",
          "schemaPath": "#/properties/test/anyOf/1/required",
          "params": {
            "missingProperty": "b"
          },
          "message": "should have required property 'b'"
        },
        {
          "keyword": "anyOf",
          "dataPath": "/test",
          "schemaPath": "#/properties/test/anyOf",
          "params": {},
          "message": "should match some schema in anyOf"
        }
      ]
    }
  }
}

When 'required' is detected under /test, the errors under /test/a is removed. However, in this case required should not be the first priority, especially higher than anyOf, because it is one of two conditions under anyOf and the field required is not necessarily required across all fields under anyOf .

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

No branches or pull requests

2 participants