mikunn / openapi-schema-to-json-schema

Converts OpenAPI Schema Object to JSON Schema
75 stars 6 forks source link

Types are incorrect #4

Closed philsturgeon closed 6 years ago

philsturgeon commented 6 years ago

Hey @mikunn, this is a great job, but you're doing a bunch of work you don't need to do. :)

The OpenAPI Spec is confusing when it talks about types.

screen shot 2018-04-02 at 7 01 39 pm

That common names column is not actually relevant to anything, other than "this is what some languages call this sort of data". For example, type: password is not a thing, at all, so you don't need to code for that.

JSON Schema says "You can have a type and a format, but there are only a handful of items officially defined: https://tools.ietf.org/html/draft-fge-json-schema-validation-00#section-7.3

These actually match the ones that exist in OpenAPI, but OpenAPI just happens to have a lot more.

You don't need to convert anything, you can just delete all this code and the tests that go with it. :)

string_types, numeric_types, etc, and everything in this switch.

mikunn commented 6 years ago

Thank you for bringing this up!

I'm aware of this, but the problem is that there were (and still are) multiple APIs that actually define types incorrectly using the common name and the conversion was made to support those.

But it's true that the conversion is unnecessary assuming the OpenAPI spec is used correctly.

I guess I need to bump the version to 2.0.0 after fixing this to not break the clients that depend on the conversion.

philsturgeon commented 6 years ago

That's fair! I'm glad to hear you were doing it to support others who made the mistake. You're smarter than me, it's tricked me in the past! :D

I'm suggesting that column is removed from 3.0.2 to avoid it happening to others.

https://github.com/OAI/OpenAPI-Specification/pull/1522

Btw the reason I'm sniffing around your code, is I've made a converter to go the other way, entirely using your code, flipping the tests, and deleting some stuff.

https://github.com/philsturgeon/json-schema-to-openapi-schema

This will be going under the https://github.com/wework organization when its ready for the prime time, but it seeems to work for now.

mikunn commented 6 years ago

Some of the APIs were designed by me and some weren't, so it has tricked quite a lot of people :)

Thanks for opening an issue for the removal on the OpenAPI side! It's easy to think that for example dateTime is a shortcut for string type + date-time format. That's at least what I thought initially.

Cool that you are making the reverse and thanks for adding a link to this in the readme, I'll do the same :)

philsturgeon commented 6 years ago

Awesome, thank you!