Skip to content
This repository has been archived by the owner on Feb 25, 2021. It is now read-only.

Consider json-schema gem over json_schema #7

Closed
bf4 opened this issue Sep 7, 2017 · 5 comments
Closed

Consider json-schema gem over json_schema #7

bf4 opened this issue Sep 7, 2017 · 5 comments

Comments

@bf4
Copy link

bf4 commented Sep 7, 2017

I'm going to make some assertions for discussion:

While I like @brandur 's json_schema gem, and even wrote an ActiveModelSerializers integration against it, I've proposing that the json-schema should be preferred for a couple of (unsubstantiated) reasons:

  • It's complete; json_schema was never completed esp re: reference expanding and may never be
  • It's more actively maintained
  • It's up to date
  • It has better errors
  • It's easy to use
  • json_schema, like prmd and associated earlier hypermedia heroku gems allow and sometimes expect invalid schema ids

See apivore for a nice json-schema implementation

ref:

fwiw, also see https://github.com/stoplightio/api-spec-converter

@brandur
Copy link

brandur commented Sep 7, 2017

Well, I wouldn't take it personally. I try to keep json_schema up-to-date according to what people need, but it does seem valuable to have a dependency that has more regular contributors. I'm still using the gem in a number of places, but it might make sense to see if it would be possible to start phasing it out.

I'm not sure saying that json-schema is more "up to date" than json_schema is necessarily correct. A quick perusal of their README shows that it doesn't support formats defined in the latest JSON schema draft, which is something that json_schema is also guilty of.

I still stand behind what I said in brandur/json_schema#46 though! It's pretty dangerous and very possibly performance impactful to start requesting data from arbitrary URLs during validation.

@bf4
Copy link
Author

bf4 commented Sep 8, 2017

First off, @brandur thanks for your thoughts. I really like your work.

it might make sense to see if it would be possible to start phasing it out.

Though would be a discussion over in json-schema, I'd be happy to help.

I'm not sure saying that json-schema is more "up to date" than json_schema is necessarily correct.

Guilty. When I was TDD'ing our api against a JSON Schema using json_schema about 6 months ago, I remember thinking this was the case, but I don't remember why. I also recall I had trouble getting 'true positives' from my tests. i.e. The only way I could confirm my schema was correct was to create invalid schemas, and they didn't always fail. I wonder if perhaps the problem was that I referenced the hyper schema draft but didn't vendor and load it? I believe I also had a lot of trouble composing schemas. I think I also had trouble using the json_schema/prmd schemas for generating a dummy JSON API.

It's pretty dangerous and very possibly performance impactful to start requesting data from arbitrary URLs during validation.

(I'm happy to have this discussion elsewhere :) I can see perf problems, but by dangerous, do you mean security? One idea would be to do something in the spirit of what the 'approvals' gem does: collect references, and then verify you want them downloaded, then download them to a cached location.

re: errors, I once wrote a hack to try to get the real error message out... https://gist.github.com/bf4/e0a76253c3af96713ca33001de984806#file-validate_json_api_schema_test-rb-L136-L146

@brandur
Copy link

brandur commented Sep 8, 2017

First off, @brandur thanks for your thoughts. I really like your work.

Though would be a discussion over in json-schema, I'd be happy to help.

Thanks and thanks!

I might try a PR at some point to get Committee moved over to json_schema to see how it looks. Your gem almost certainly has the superior architecture given that I wrote the bulk of mine in about four hours from a cafe in Berlin.

Guilty. When I was TDD'ing our api against a JSON Schema using json_schema about 6 months ago, I remember thinking this was the case, but I don't remember why. I also recall I had trouble getting 'true positives' from my tests. i.e. The only way I could confirm my schema was correct was to create invalid schemas, and they didn't always fail. I wonder if perhaps the problem was that I referenced the hyper schema draft but didn't vendor and load it? I believe I also had a lot of trouble composing schemas. I think I also had trouble using the json_schema/prmd schemas for generating a dummy JSON API.

Yeah, it's possible there was something — open an issue if you happen to come across it again. There's definitely some new features on the newer drafts that I'm not supporting yet, and enough has changed at this point that I might need to add a version selector so people can lock themselves into one in particular.

(I'm happy to have this discussion elsewhere :) I can see perf problems, but by dangerous, do you mean security?

Security's a big part of, but I also don't like the lack of transparency. You might hit a URL that hits another URL that hits another URL ... it's completely opaque looking at a single JSON schema just how big your dependency tree might be.

One idea would be to do something in the spirit of what the 'approvals' gem does: collect references, and then verify you want them downloaded, then download them to a cached location.

Yeah, that seems like a good idea. It does make things a little harder though because you need to find a way for your program to be interactive, which might be more difficult as a library.

@handrews
Copy link

handrews commented Oct 3, 2017

Security's a big part of, but I also don't like the lack of transparency. You might hit a URL that hits another URL that hits another URL ... it's completely opaque looking at a single JSON schema just how big your dependency tree might be.

I'd say this is more of a reason to include the ability to manage the concern rather than to forbid it outright. For instance, an API using a well-defined set of schemas hosted under the same hostname as the API should be fine.

Note that schemas SHOULD be cacheable indefinitely, and will often be packaged and pre-distributed to avoid needing to download them over the network.

Regarding indefinite caching: if a schema is ever updated for bug-fixes, you can't/shouldn't assume that anyone who already has it will ever download it again. Changes that clients must take into account need to be given a new schema version.

@jfeltesse-mdsol
Copy link
Contributor

This has been addressed in PR #9 so I think this issue can be closed now.

@bf4 bf4 closed this as completed Nov 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants