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

JsonParsingException:null while using custom validations with ARRAY instance type #35

Closed ghost closed 5 years ago

ghost commented 5 years ago

I am trying to implement a custom validation by implementing FormatAttribute. It works fin when the InstanceType is STRING. However, if I try to apply a format for an InstanceType ARRAY, then I get a JsonParsingException:null

My implementation

  @Override
  public InstanceType valueType() {
    return InstanceType.ARRAY;
  }

Exception Received

javax.json.stream.JsonParsingException: null
liberty    |    at org.leadpony.justify.internal.base.json.DefaultJsonReader.newParsingException(DefaultJsonReader.java:148)
liberty    |    at org.leadpony.justify.internal.base.json.DefaultJsonReader.readValue(DefaultJsonReader.java:122)
leadpony commented 5 years ago

Sorry @adityamandhare The current implementation has a restriction on custom formats. They can be applied only to simple types such as string, number, boolean, and null. I have updated README and Javadoc in the last commit to clarify this limitation. Do you really need custom format for array or object?

ghost commented 5 years ago

@leadpony So I have a scenario wherein I have an array of Customer objects. And Customer object can be be of type Manufacturer, Retailer, Distributor and END_USER. The customer object also has other attributes along with type like firstName, lastName etc. I need to make sure that this array of customers must contain Manufacturer and Retailer customers. So I was thinking that for the array of Customers I can implement a custom validation to make this check and return error if Manufacturer or Retailer customer is missing. Does this validation library have a better way to support validating this array of objects instead of writing a custom validation?

leadpony commented 5 years ago

Hello @adityamandhare I have imagined the expected JSON documents based on the story you told me.

Good JSON

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

Bad JSON (Retailer type is missing)

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

Are these examples appropriate?

atomictag commented 5 years ago

Perhaps you need something along the lines of:

{
   ...
  "allOf" : [
       { "contains" : { "properties" : { "type" : { "const" : "Manufacturer" } } } },
       { "contains" : { "properties" : { "type" : { "const" : "Retailer" } } } },
       { "contains" : { "properties" : { "type" : { "const" : "Distributor" } } } }
   ]
   ...
}

note that the contains schema validates against one or more items in the array, so the above example guarantees that your array items have one item of each type. Actually at least one item of each type, since the above validates perfectly fine with duplicate types (which is not possible to avoid as far as I know - as "uniqueItems" : true works only on a per-item basis, not on a per-key basis).

ghost commented 5 years ago

@leadpony the JSON you have posted based on my story is correct. @atomictag I tried your suggestion. It did not work in my case.

{
  "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"
            }
          }
        }
      }
    ]
  }
}
leadpony commented 5 years ago

Thank you @adityamandhare @atomictag The keyword contains must be applied to the array itself, not to each item in it.

atomictag commented 5 years ago

@adityamandhare I think you need to move that allOf outside of items - the contains assertion is not a property of items, but of the array schema itself.

atomictag commented 5 years ago

oh @leadpony was faster than me on this one :)

ghost commented 5 years ago

ahh, I see. Thanks @leadpony and @atomictag . Moving allOf outside items does work and validate the array for missing objects. However, the errorMessage is too generic At least one of the following sets of problems must be resolved. Also the problem.getPointer() returns null value (which usually returns the field name which has error). The problem.getBranch(0) does help get some useful error message The value must be constant string "Manufacturer". However I would like to give a more user-friendly error message like Missing customer of type 'Manufacturer'. Is there a way I can use contains for identifying the error, but for use my own custom error message?

leadpony commented 5 years ago

Hi @adityamandhare I have tried your modified schema with Justify CLI and got the following problem messages:

Validating the instance "issue35.json"...
At least one of the following sets of problems must be resolved.
1) [3,26][/0/type] The value must be constant string "Retailer".
2) [8,25][/1/type] The value must be constant string "Retailer".
At least 1 problem(s) were found in the instance "issue35.json".

You can try other validator such as https://jsonschema.dev/ and will see similar messages after all. I agree with you that these are not very user-friendly messages. It is difficult for a validator to fully understand the real intent of constraints defined in the complex schema.

ghost commented 5 years ago

@leadpony would it be some cool new feature to add wherein you give freedom to the return a customized errorMessage when a schema validation error with a field is found? Also thanks for looking into this and trying out the validation urself. Appreciate that 🙂

leadpony commented 5 years ago

Unfortunately there is no such a feature in the current release. Please see also the issue #16

ghost commented 5 years ago

@leadpony actually the solutions described in issue #16 helped resolve my issue. I can use the contains to make sure the required customer is present and then use my custom keyword to identify the error field and user friendly error message. Thanks 🙂