leadpony / justify

Justify is a JSON validator based on JSON Schema Specification and Jakarta JSON Processing API (JSON-P).
Apache License 2.0
96 stars 18 forks source link

Incorrect problem schema and error message and error type returned for array element validation #37

Closed ghost closed 4 years ago

ghost commented 4 years ago

I have the following JSON schema in which I am trying to validate that customer types Manufacturer and Retailer exist.

{
  "type": "array",
  "title": "The Customer Schema",
  "minItems": 2,
  "items": {
    "type": "object",
    "required": [
      "firstName",
      "lastName",
      "type"
    ],
    "properties": {
      "firstName": {
        "type": "string"
      },
      "lastName": {
        "type": "string"
      },
      "type": {
        "type": "string",
        "enum": [
          "Manufacturer",
          "Retailer",
          "Distributor"
        ]
      }
    }
  },
  "allOf": [
    {
      "contains": {
        "properties": {
          "type": {
            "const": "Manufacturer"
          }
        }
      }
    },
    {
      "contains": {
        "properties": {
          "type": {
            "const": "Retailer"
          }
        }
      }
    }
  ]
}

This works fine if the array contains either 0, 2 or more elements (customer objects). However, if the array only contains 1 element (customer object) of any type say Distributor then instead of returning a validation error against contains keyword it returns a validation against const keyword with error messages "The value must be constant string "Manufacturer." and "The value must be constant string "Retailer." with error field type. So basically instead of pointing out the missing customer types Manufacturer and Reseller it asks me to convert the customer type of Distributor to Manufacturer and Reseller. If the only element in the array is of type Manufacturer, the the returned error message will be: "The value must be constant string "Retailer." and again pointing to the type field.

The validation works fine against the contains only if we have 0 or at least 2 elements in the array. Say the array was empty, then it returns error message: At least one of the following sets of problems must be resolved., At least one of the following sets of problems must be resolved. one message each for the missing Manufacturer and Retailer, which is good.

Say we had Manufacturer and Distributor, then the error message would be At least one of the following sets of problems must be resolved. and it points out that Retailer is missing which is fine.

leadpony commented 4 years ago

Hello @adityamandhare I wrote a JSON instance as follows:

[
  {
    "firstName": "Doe",
    "lastName":  "John",
    "type": "Distributor"
  }
]

And validated it against your schema. The validation produces the following error messages:

[7,1][] The array must have at least 2 element(s), but actual number is 1.
[5,25][/0/type] The value must be constant string "Manufacturer".
[5,25][/0/type] The value must be constant string "Retailer".

It looks good for me considering what your schema says. What error messages are you expecting? You can also test the schema and the instance with other JSON validator online at https://jsonschema.dev/. You will see the similar messages there.

ghost commented 4 years ago

@leadpony in case where the array contains 0 or 2 elements, and a mandatory customer is missing, then validation returns a correct error message saying At least one of the following sets of problems must be resolved.. This error messages points out that one of the mandatory customers is missing. Also there is nothing wrong with the existing customers in the array which is fine.

However, now if there is only 1 element in the array say Manufacturer, then the returned error message is "The value must be constant string "Retailer." wherein it asks to replace the type value of Manufacturer to Retailer. The returned error message states that there is something wrong with the existing element in the array instead of simply pointing out that a mandatory customer Retailer is missing. Now, if you see, there is nothing wrong with the one Manufacturer element present in the array. I am expecting it to return an error message: At least one of the following sets of problems must be resolved. pointing out that the Retailer customer is missing.

leadpony commented 4 years ago

@adityamandhare Thank you for your reply. Here is a JSON instance which is an array containing 2 items.

[
  {
    "firstName": "Doe",
    "lastName":  "John",
    "type": "Distributor"
  },
  {
    "firstName": "Doe",
    "lastName":  "Jane",
    "type": "Distributor"
  }
]

The validator produces messages as follows.

At least one of the following sets of problems must be resolved.
1) [5,25][/0/type] The value must be constant string "Manufacturer".
2) [10,25][/1/type] The value must be constant string "Manufacturer".
At least one of the following sets of problems must be resolved.
1) [5,25][/0/type] The value must be constant string "Retailer".
2) [10,25][/1/type] The value must be constant string "Retailer".

Please note that the first 3 lines come from the first contains keyword and the last 3 lines come from the second contains keyword in your schema. Here is a JSON instance which is an array containing only 1 item.

[
  {
    "firstName": "Doe",
    "lastName":  "John",
    "type": "Distributor"
  }
]

The validator produces messages as follows.

[7,1][] The array must have at least 2 element(s), but actual number is 1.
[5,25][/0/type] The value must be constant string "Manufacturer".
[5,25][/0/type] The value must be constant string "Retailer".

Do you expect the following messages?

[7,1][] The array must have at least 2 element(s), but actual number is 1.
At least one of the following sets of problems must be resolved.
1) [5,25][/0/type] The value must be constant string "Manufacturer".
At least one of the following sets of problems must be resolved.
1) [5,25][/0/type] The value must be constant string "Retailer".

Please note that the 2nd and 3rd lines come from the first contains keyword and the 4th and 5th lines come from the second contains keyword, exactly same as the array containing 2 items.

The current validator implementation omits the lines At least one of the following sets of problems must be resolved., because there is only 1) line and no 2) line, that means there is single choice to resolve a problem in order to satisfy each contains keyword. What you really need is these omitted lines?

ghost commented 4 years ago

@leadpony it is more about the error field that it points to and the problem schema that it returns in case there is 1 or multiple elements. For example: When there is just one element, the error field pointed to is const. And the problem schema returned is:

{"const": "Retailer"}

When there is 0 or 2 elements, the error field pointed out is contains and the problem schema returned is:

{
      "contains": {
        "properties": {
          "type": {
            "const": "Retailer"
          }
        }
      }
    }

I expect the same error field to be pointed to and the problem schema to be returned whether there is 0, 1 or 2 customers. The thing is that I have been trying to insert some custom schema elements (like a custom error message) to read from the returned problemSchema. Because of the discrepancy of the retuned problem schema, my custom schema elements are sometimes a part of problem schema and sometimes not. Also it becomes difficult to classify the type of validation error since sometimes the error field is const (which points to some invalid format) and sometimes contains (which points to missing input).

leadpony commented 4 years ago

@adityamandhare For array of 0 or 2 elements, the problems returned have the list of the child problems and each of them should have the schema {"const": "Retailer"}. Please check these leaf problems by using the method Problem#hasBranches().

leadpony commented 4 years ago

In Justify, each problem returned can be a tree of problems if there exist multiple solutions for the problem.

ghost commented 4 years ago

@leadpony thanks for the info on leaf problems. However, I would still say that the root problem schema in case of 1 element should still be:

{
  "contains": {
    "properties": {
      "type": {
        "const": "Retailer"
      }
    }
  }
}

and the root error field should still be contains. This is because the problem is still with missing customer type Retailer and replacing type field of a valid customer present in the array is not a valid solution.

leadpony commented 4 years ago

@adityamandhare OK, I will check the missing intermediate (not leaf) problem and fix it if needed. Thank you for detailed report.

ghost commented 4 years ago

Thanks @leadpony 🙂Always appreciate your quick response!

leadpony commented 4 years ago

@adityamandhare I believe the problem you reported has been fixed in the latest release 2.0.0. Please try it.