Switch to better validation tool

[tkurki]

tv4 appears dead as a project and its error reporting is sometimes really confusing.

Consider switching to another JSON Schema validator, such as http://epoberezkin.github.io/ajv/

See also https://github.com/ebdrup/json-schema-benchmark

[bkp7]

@tkurki, just wanted to check we agree on the objectives before I spend too much time.

The end goal is to change from tv4 to ajv (which still looks to be active and good) and to automatically test schemas, samples, tests and the examples to be put in the documentation.

I would also like to make it easier to add and remove tests, which for me would just mean adding or removing a json file and the tests then automatically running. The present arrangement where each test has to be added in js is not straightforward at all. I propose the tests are placed in a folder structure:

• tests
  • full
    • valid
    • invalid
  • delta
    • valid
    • invalid
  • electrical
    • valid
    • invalid
  • sails
    • valid
    • invalid
  • etc ....

I think for unit testing it makes sense to separate the lower level schemas as there ought to be quite a few tests for each. For samples and documentation examples I think they should all be top level schema compliant (ie full, delta, subscribe, unsubscribe, discovery). Do you concur?

It would also be great to build something for the documentation examples so that sections can be minimised automatically (the scaffolding you mention in #320).

At present the tv4 tests have the banUnknownProperties flag set, which along with other issues in tv4 makes reliable testing and validation difficult. With this set, the current 'loose' json schema we have appear to be 'hardened', when other validators do not see it that way. I can't see that ajv has an equivalent flag, see discussion here https://github.com/epoberezkin/ajv/issues/238 . This will mean that changing to ajv will probably require some changes to the schemas to achieve the same results.

Keeping eyes on the end goal I propose:

1. Move/modify existing tests into the new folder layout but still running with tv4 to ensure we don't lose any existing test results
2. migrate to ajv
3. add in handling of documentation examples

Finally, a nice to have, would be a SignalK web page validator into which json can be copied and it validates it. I believe ajv can do this for us.

Your thoughts?

[tkurki]

Please see http://node-master.signalk.org/@signalk/playground/dist/ fir an interactive validation tool. Where would you have found thus link? I mean how ti tell people about it?

To be useful the schema validation must be run so that the data only properties present in the schema. If ajv can not do that I don’t quite see how it would be useful.

This issue is about the validation tool. Reorganising tests is another - but I do agree with you on running all json files against the schema automatically. If you start this way please PR early, let’s review it early.

Documentation samples is another. How about a priof of concept on this first?

[bkp7]

I would have thought this link should be referenced in the specification documentation, probably under 'Developing with Signal K'. There should also be a clear reference to the fact that the Signal K schema is not compliant with any of the json schema (draft) standards, in that it relies on the TV4 'banUnknownProperties' flag. Thus validation cannot be performed with anything other then TV4.

The playground code should also make it clear which version of the schema it is using (currently 1.0.2).

Ideally the schema itself should be modified so it is (draft) json schema compatible, then it will work with any validation tool.

[tkurki]

Great ideas: referencing the validation tools, version in playground. Should activate also Greenkeeper for the playground repo to get automatic PRs for upgrades.

I didn't quite get your comment about json schema draft standards and banning unknown properties - afaik json schema does not define any method for this: json-schema-org/json-schema-spec#326 (comment).

Making the schema json schema compatible is a separate issue afai can see.

[bkp7]

There's no way to do it globally as TV4 offers, but my understanding of the reasons this was rejected was because the global approach creates issues when you use allOf, or oneOf. See this. In any case we can't influence the json schema specs, nor does it make sense for the whole of Signal K to rely on a unique feature in TV4 when that project is not being maintained. I would have thought it would be better for the Signal K schema to be able to be used with any validator.

So we would need to enforce it by for example replacing:

   "numberValue": {
      "type": "object",
      "description": "Data should be of type number.",
      "allOf": [{
        "$ref": "#/definitions/commonValueFields"
      }, {
        "properties": {
          "value": { "type": "number" },
          "values": {
            "type": "object",
            "patternProperties": { ".*": { "$ref": "#/definitions/valuesNumberValue"}}
          }
   }}]}

with:

   "numberValue": {
      "type": "object",
      "description": "Data should be of type number.",
      "allOf": [{
        "$ref": "#/definitions/commonValueFields"
      }, {
        "additionalProperties": false,
        "properties": {
          "timestamp": {}, "$source": {}, "_attr": {}, "meta": {}, "pgn": {}, "sentence": {},
          "value": { "type": "number" },
          "values": {
            "type": "object",
            "patternProperties": { ".*": { "$ref": "#/definitions/valuesNumberValue"}}
          }
   }}]}

Making this change throughout the schema is clearly 'risky' from the point of view of adding errors. That is why I've been wanting to see an easier way to add tests, and my view is that the api validator should support TV4 and AJV validation simultaneously on different versions of the schema. That way we can see that tests pass/fail consistently under the old and new. Finally AJV is able to report coverage so we can work towards having tests which cover the whole schema.