Port to use ajv?

[ropensci]

Port to use ajv?

[karawoo]

I've lost a bit of time this week from not realizing that the newer schema specs weren't supported, so I'm very 👍 on adding support for ajv. I don't have any experience wrapping js libraries in R, but @richfitz if there's anything I can do to help with this let me know!

[richfitz]

Any help would be appreciated, for sure.

I have been using the version on the use-ajv for a project without issue for a while but the issues in this thread remain. Please try that branch out with your use cases and let me know if it works or not!

[richfitz]

Hi @ijlyttle:

if a newer version of the AJV library were to be integrated into this package, is there a process in place to do that, and also to note which version of AJV is currently being used?

The js directory contains a package.json file, running make inst/bundle.js in the
package root should update things. The version number is not reported out anywhere sensible at this point.

what sorts of things need to be done to accept all this into master, and eventually to CRAN?

If one of you could look over #17, which incorporates your previous work, I will release to CRAN

[richfitz]

not worth it at this point.

[eibanez]

I don't know if this package is actively developed, but it looks like ajv would be a good alternative given that is-my-json-valid does not support newer json-schema specs.

[richfitz]

Yeah, given #5, this is probably worth doing. I don't have a lot of spare capacity at the moment so if someone wants to work up a PR that'd be 💯, but otherwise I may get it done in the next month or two

[richfitz]

Thanks all - I'll bump this up the queue

[richfitz]

☝️ @karawoo - I've got going here and have the new javascript code into the package but doing nothing yet

What would be super useful is some examples of things that are not working with jsonvalidate added as failing tests - if you're able to add some that would be amazing. In any case I can start wiring up the currently embarrassingly small amount of testing to work with ajv

Work is on the use-ajv branch

[karawoo]

Sure thing, I’ll put together a PR today

[richfitz]

Thanks for the tests @karawoo - I can get the validator to work just fine with the draft-07 metaschema but I can't get it to fail with the old metaschemas, even after getting them loaded! But progress in any case :)

It's up on the use-ajv branch if anyone would like to try with a real example

[karawoo]

@richfitz did you see this section of the ajv docs? It looks like there are some additional commands needed when validating against older versions of the json schema spec. Maybe there needs to be a metaschema_version argument or something, though it seems weird that that should be necessary when the metaschema version is right there in the $schema field...

[richfitz]

Yeah - I did that here:

jsonvalidate/js/in.js

Lines 5 to 8 in fa92c4c

	global.AjvGenerator =
	new Ajv({allErrors: true, schemaId: 'auto', verbose: true});
	global.AjvGenerator.addMetaSchema(AjvSchema4);
	global.AjvGenerator.addMetaSchema(AjvSchema6);

(if you don't then you get an error when ajv tries to resolve the reference) but it doesn't actually use the right version :(

[ijlyttle]

Hi @richfitz and @karawoo,

I have been lurking on this issue for a while (I gave myself away ☝️), and was wondering if I can be useful at all to this effort. I'd like to be able to validate Vega and Vega-Lite specs, which I understand use ajv.

Happy to do what I can, but happy to stay out of the way too.

[ijlyttle]

I still need to test on my use-cases; I thought I might start by trying to understand the issues on this thread.

I think I have wrapped my head around it; I have adapted one of your tests to create a reprex on Observable.

Question: have I identified the issue you have in mind?

If I have the right issue, and I have the right understanding (that there is an issue with ajv), perhaps a JS example might help bring things into focus (if it is, indeed, their issue).

[karawoo]

Thanks for looking into this @ijlyttle. I think the issue might be slightly different. I'm not sure if validating the schemas themselves would necessarily have different behavior between draft 4 and draft 6 metaschemas. I believe schemas can contain keywords that aren't used for validation, so it would be ok to have "const" in a draft 4 schema.

I think the difference should come when validating data against the schema. If I understand correctly, the data {"a" : "bar"} should be invalid against the schema that uses draft 6, because with draft 6 "a" is required to be the constant "foo". But {"a" : "bar"} should be valid against the schema that uses draft 4, because in draft 4 "const" isn't a validation keyword, so {"a" : "bar"} isn't breaking any validation rules. Right now though, ajv says that {"a" : "bar"} is invalid against both the draft 6 and draft 4 versions of the schema.

It's very possible I'm misunderstanding the expected behavior, though.

[ijlyttle]

Hi @karawoo, thanks for patiently walking me through the issue. This is subtle (or at least seems so to me).

It will be no problem for me to rework the example to hew to your explanation - I'll update this thread once I have made the changes.

[ijlyttle]

I have updated the Observable notebook.

Assuming that this view of things represents your view of things, would you like me to put together a Stack Overflow question using the notebook as a reference? Should I take a different action (or no action)? Thanks!

[karawoo]

That would be great!

[ijlyttle]

Change of plans - I in my SO search, I found this post that pointed me towards the ajv 5.0.0 release notes, which recommends the removeKeyword() function.

I reworked the Observable notebook to show how this might be implemented - it would pass the tests (esp. the commented-out tests).

The jsonvalidate tests refer only to the keywords - the ajv release notes make some additional recommendations. I will propose a PR that deals only with the keywords.

It should be no problem to adapt the PR to the other recommendations, but I figure you may wish to weigh in on them.

(update: PR at #16)

[karawoo]

That is interesting and not what I would have expected. Thank you for digging into this!

[ijlyttle]

Thank you for the review and kind words!

I am curious now about a couple of things:

• if a newer version of the AJV library were to be integrated into this package, is there a process in place to do that, and also to note which version of AJV is currently being used?

• what sorts of things need to be done to accept all this into master, and eventually to CRAN?

Of course, I am happy to help with any of this.

[richfitz]

Fixed in #17