User-friendly validation messages

[tfesenko]

Validation messages are composed by JSON-Schema validation and often times are not user (==human)-friendly and don't provide all necessary information.
We want to rephrase them and augment with information which can be obtained from JSON Schema. For example, if a value is enumeration, don't just say that it's not allowed, but also provide a list of available values.

Ted can provide or correct the exact text of the validation messages. We have to point him to the place it code or in tests where he can see them all.

We also would like to make the messages generic, not Swagger-specific and contribute it back to the validator if possible.

[ghillairet]

We mentioned in our discussion that we could do that by forking the json-schema-validator repository. This repository is available here https://github.com/fge/json-schema-validator.

Looks like there has not been much development on it and the owner is looking for a new maintainer. Should we fork it under ModelSolv?

[ghillairet]

We could do it without having to fork this project, processing error messages should be possible to create more meaningful messages.

# fixture 4
# invalid responses type, should be an object

swagger: '2.0'
info:
  version: 0.0.0
  title: Simple API
paths:
  /:
    get:
      responses: 'Hello'

This is the error object that result from the validation of the above yaml:

{
  "level": "error",
  "schema": {
    "loadingURI": "http://swagger.io/v2/schema.json#",
    "pointer": "/definitions/responses"
  },
  "instance": {
    "pointer": "/paths/~1/get/responses"
  },
  "domain": "validation",
  "keyword": "type",
  "message": "instance type (string) does not match any allowed primitive type (allowed: [\"object\"])",
  "found": "string",
  "expected": [
    "object"
  ]
}

The default message says that the value for responses should be an object and not a string (here Hello).

But it should be doable to get a better message by getting meaningful information from the schema. The relevant part of the schema is given in the error field schema. Here the part of the schema:

"responses": {
      "type": "object",
      "description": "Response objects names can either be any valid HTTP status code or 'default'.",
      "minProperties": 1,
      "additionalProperties": false,
      "patternProperties": {
        "^([0-9]{3})$|^(default)$": {
          "$ref": "#/definitions/responseValue"
        },
        "^x-": {
          "$ref": "#/definitions/vendorExtension"
        }
      },
      "not": {
        "type": "object",
        "additionalProperties": false,
        "patternProperties": {
          "^x-": {
            "$ref": "#/definitions/vendorExtension"
          }
        }
      }
    }

[tfesenko]

A list of validation messages from JsonSchema Validation (provided by @ghillairet ) - https://github.com/fge/json-schema-validator/blob/master/src/main/resources/com/github/fge/jsonschema/validator/validation.properties

[tfesenko]

We decided to create a suite of tests for validation messages.
The tests will be specification - the YAML model under test and validation message should be in the same file. For example, line in Xtext Xpect or in https://github.com/ghillairet/emfjson/blob/emfjson-0.9.0/emfjson-jackson/src/test/java/org/emfjson/jackson/junit/tests/EnumTest.xtend#L38 when model text is pasted to the test.

JSON Schema for Swagger is here - https://github.com/swagger-api/swagger-spec/blob/master/schemas/v2.0/schema.json . It can be used to retrieve additional information for the validation message.

[tedepstein]

@tfesenko , I can provide better error messages, but think I will need some guidance as to how I can specify them, to make use of the information available in the schema. In some cases, I will need to use variables that don't seem to be defined yet.

Let's talk after tomorrow's SUM.

[tfesenko]

@tedepstein , @ghillairet started the "tests as documentation" in ValidationMessageTest.

The expected message is placed above the model text. In the model text, the " #validation error marker" are placed right above the place where we expect to see a validation error. It's for human convenience only and will be ignored by the test.

Does this format suit you? Do you have any suggestions to improve readability?

[tedepstein]

@tfesenko , this looks fine to me. Want to complete the inventory of tests, and then I'll review? Or do you want my input now?

[tfesenko]

@tedepstein , I don't plan to add more validation examples today.

[tedepstein]

I've added my comments. See commit f2a852c .

[ghillairet]

Some of the messages have been updated, as you can see in this commit 9988e42

It concerns the most common error messages.

The most interesting would be that this previous kind of messages

instance failed to match exactly one schema (matched 0 out of 2)

will be replaced by more meaningful ones, such as

value of type integer is not allowed, value should be of type string
object has properties "description" which are not allowed
object has missing required properties "$ref"

This excerpt represents in single message that replaces the previous one. It includes all the errors that apply at a position of the document.

[tedepstein]

@ghillairet , w.r.t. your last comment, I opened #60, which I think is partially caused by the replacement of "instance failed to match exactly one schema" with a list of errors related to each failed match.

We could try listing the schemas individually, like this:

Failed to match exactly one schema:
  schema 1 :
    - value of type integer is not allowed, value should be of type string
  schema 2:
    - object has properties "description" which are not allowed
    - object has missing required properties "$ref"

It's not ideal, because we cannot easily name or describe the schemas. The user has to guess what the different schemas are looking for.

But it's more informative than a single "failed to match", and it organizes the error messages in a way that might make it easier to interpret.

Thoughts?

[tedepstein]

Also, the example in #60 seems to show that schema validation failures can be recursive. We see "failed to match" errors, presumably arising from attempted schema validation of contained definitions.

I don't think it's wise to try to "unwrap" schema validation errors recursively. We could get a really large tree, and could even encounter cycles, unless we're sure that this kind of validation is acyclic... Regardless, it seems like we should not try to take the unwrapping too far.

[tedepstein]

@tfesenko , as discussed, please review and extract to separate issues if needed. Note that we already have #60 open for error messages on oneOf validation errors.