Added test cases from Foundations of JSON Schema research paper

[iainbeeston]

trafficstars

In the 25th internationl world wide web conference there was a paper named "Foundations of JSON Schema", presented by:

• Felipe Pezoa ([email protected])
• Juan L. Reutter ([email protected])
• Fernando Suarez ([email protected])
• Martín Ugarte ([email protected])
• Domagoj Vrgo ([email protected])

See:

http://www2016.net/proceedings/proceedings/p263.pdf

This paper compared a number of json schema implementations, using a set of 5 tests which highlighted inconsistencies between the implementations. I have reproduced those tests here. All credit should go to the authors of the paper for highlighting these inconsistencies and publishing the details of them online.

[iainbeeston]

Fixes #115

[Julian]

Awesome! Very nice work, really appreciated, will definitely have a look. I'm sure it's in the paper which I haven't read, so forgive me for being lazy, but just in case -- what's the license on the tests from the paper?

[iainbeeston]

That's a good question - there is no mention of a license in the research paper or in the source code from in the appendix. For all of the tests I've treated them as a bug report and written the smallest, cleanest test that can reproduce the issue, rather than copying any of them directly into the test suite.

[Relequestual]

As a note, I was in direct contact with several of the authors, and they wanted the tests to be integrated into the official test suite, but hadn't been able to put any time into it.

[iainbeeston]

@Relequestual @Julian Should we get formal approval from the authors? Is there any way to get one of them to 👍 this PR?

Furthermore, @Julian can you confirm that I (and the paper's authors) have interpreted the spec correctly in the tests in this pull request?

[iainbeeston]

Incidentally, these tests will fail on a lot of implementations, as shown by the paper. I've also tested them on the implementation I contribute to (https://github.com/ruby-json-schema/json-schema/) and even the latest version does not pass all of them

[Relequestual]

Somewhat worrying. I think @Julian will need to review for sure. As for contacting the paper authors, I'm not sure they use github... Would suggest emailing them.

[DomagojVrgoc]

Dear All,

this is Domagoj, one of the authors of the paper. Iain contacted me already, but just to restate what I said to him, we do maintain a webpage with more informal introduction to our formalization at http://cswr.github.io/JsonSchema/ in case you want to take a look. Over there, at http://cswr.github.io/JsonSchema/spec/why/, you can also find a bit more details on the tests you were discussing, and some thoughts on what we think was going on with the implementations.

Best, Domagoj.

[iainbeeston]

Thanks @DomagojVrgoc !

[iainbeeston]

I didn't include test T3 from the paper because I couldn't see a succinct way of doing it... The original test checks that a schema for an object ignores value-based properties (eg. multipleOf)

I could add a test to every json schema property that refers to values and make sure they do nothing when used on an object or array? (eg minimum, maximum, multipleOf etc) but it would make for a lot of extra tests. I'd appreciate hearing opinions on the best course of action on that test.

[Relequestual]

I can't claim I've fully read and understood all of the above, but I would think that a validator would check the schema is valid against the schema for schemas, if that makes sense. So a check would be done to make sure that properties such as minimum are only where they are relevant.

[iainbeeston]

So - rather than simply ignore those properties, we would fail validation because the schema itself is invalid? (Is that correct?)

Is that how we handle invalid schemas in the test suite already? (Are there even any tests for schemas that are invalid?)

[Relequestual]

If I were writing a library, that's how I'd do it. If you get part way through and find it's partially invalid, you've wasted time. If you take a softer approach, and ignore some parts based on some rules you've made up, then you're asking for unexpected behaviour. I would think it's better to let the user know they have done something wrong.

I don't remember seeing any tests to confirm this behaviour, which is OK, as I also don't remember it being part of the spec. I feel it probably should / could be though. Maybe I should raise an issue to ask the question?

In terms of my experience developing a collaboriative international API, I tend to read rules in a stricter, less forgiving manner than others. Make of that what you will.

[iainbeeston]

Yeah I think it's worth raising as another issue.

FYI the ruby json-schema gem validates the user's schema against the appropriate meta schema (eg http://json-schema.org/draft-04/schema), before validating the data against the user's schema. It works well.

If you agree with that way of doing things, if the meta schema says a schema with type: object can have a multipleOf property, the it is valid and it's up to the validator (and the common test suite) to handle that appropriately.

[Relequestual]

New issue logged.

Hum. yeah, that's a good point. Should the spec say things like "unless the object is of type x, then properties y and z should be ignore."? If so, then it would make sense to bake that into the metaschema, I would feel.

[iainbeeston]

Thanks for your very thorough feedback @Relequestual @epoberezkin and @awwright - I've removed several tests that used invalid schemas, and corrected one test that @epoberezkin pointed out had a mistake in it.

I believe this should be ready to merge now but please let me know if I should make further changes.

[iainbeeston]

@epoberezkin I've added these tests to draft6 now (in addition to draft4)

[handrews]

@iainbeeston @epoberezkin @Julian can anyone summarize what's going on here and where we are? We'll also want to copy these to draft-07 (which is backwards-compatible with draft-06, so that should be trivial.

[Julian]

The last time I looked I think there were a bunch of stuff here that needed cleaning up after review comments, but will have to have another look to see exactly what / how.

[gregsdennis]

Are these tests still desired? It's been... 6 years since anyone looked at them. A rebase is also required.

If not, we should close it.

[iainbeeston]

@gregsdennis I've rebased this, added the same tests to draft7 and removed the two ref-based tests that were controversial. This means there are only 2 new test cases added (to drafts 4, 6 and 7)

[gregsdennis]

Thanks, @iainbeeston. One last thing: could you add those tests to

• draft2019-09
• draft2020-12
• draft-next

please? You'll need to change dependencies to either dependentSchemas or dependentProperties as appropriate and change any definitions to $defs.

[iainbeeston]

@gregsdennis I've done that now, hopefully it's ready to merge? 🤞

[gregsdennis]

Thank you. I still need to run it on my implementation (or someone else can run it on theirs). I can probably do that tomorrow. For now, it's Friday night.

Thanks for updating the PR.