atlassian / better-ajv-errors

JSON Schema validation for Human πŸ‘¨β€πŸŽ€
https://atlassian.github.io/better-ajv-errors/
Other
232 stars 44 forks source link

fix: enumeration in anyOf #85

Closed ext closed 2 years ago

ext commented 3 years ago

In the current version of better-ajv-errors enumeration errors are only handled if it is the only type of errors present. This does not mesh well with anyOf (or similar construct).

Consider the following schema where id can either be foo or bar (e.g. "id": "foo") or it can be an array of those values (e.g. "id": ["bar", "foo"]):

{
  "type": "object",
  "properties": {
    "id": {
      "anyOf": [
        { "enum": ["foo", "bar"] },
        {
          "type": "array",
          "items": { "enum": ["foo", "bar"] }
        }
      ]
    }
  }
}

Given the data { id: "baz" }, Ajv will yield a list of three errors:

[
  {
    "keyword": "enum",
    "dataPath": "/id",
    "schemaPath": "#/properties/id/anyOf/0/enum",
    "params": {
      "allowedValues": ["foo", "bar"]
    },
    "message": "should be equal to one of the allowed values"
  },
  {
    "keyword": "type",
    "dataPath": "/id",
    "schemaPath": "#/properties/id/anyOf/1/type",
    "params": {
      "type": "array"
    },
    "message": "should be array"
  },
  {
    "keyword": "anyOf",
    "dataPath": "/id",
    "schemaPath": "#/properties/id/anyOf",
    "params": {},
    "message": "should match some schema in anyOf"
  }
]

The current code tests if all errors are enum (as far as I can tell because some other edge cases but the previous commits were a bit sparse on details) and only handles enum if and only if all errors are enum.

In my use-case this causes the above test to fail and thus the enum error is handled by DefaultValidationError instead of EnumValidationError which gives a less precise error:

ENUM should be equal to one of the allowed values

  1 | {
> 2 |   "id": "baz"
    |         ^^^^^ πŸ‘ˆπŸ½  enum should be equal to one of the allowed values
  3 | }

[snip]

This PR adds the enum case back to the else-block so enum is once again handled by EnumValidationError giving the intended message again:

ENUM should be equal to one of the allowed values
(foo, bar)

  1 | {
> 2 |   "id": "baz"
    |         ^^^^^ πŸ‘ˆπŸ½  Did you mean bar here?
  3 | }

[snip]

With that said I do not fully understand the previous case matching only if all errors are enum and thus I'm not fully sure there are no regressions with this PR but at least all other test-cases passed without any changes. Please do let me know if there are any issues with this approach.

torifat commented 3 years ago

Thanks again for raising the PR. I vaguely remember that there was a reason for it but I can't recall it now.

I need some time to review and test this out. I have been away from this project for quite some time. I was waiting for a change in the ajv so that I can resume rewriting it using typescript https://github.com/torifat/better-ajv-errors/tree/typescript.