Open tpburch opened 4 years ago
There are now 2 PRs for this (#183 and this one).
Can we make sure we converge on one?
@tlivings @sprootshift Agreed - we should probably determine an overall direction. This PR is definitely a bigger overall design change, which may not be the direction you want to go on this. My goal is to centralize the logic differences needed to handle different schema versions.
I think the approach in https://github.com/krakenjs/hapi-openapi/pull/183 is equally valid. It will likely result in fewer overall changes, but the specifics of both OAS2 and OAS3 schemas will be more spread out throughout the library.
It looks like https://github.com/krakenjs/hapi-openapi/pull/183 is handling the schema version differences in regards to the interaction with enjoi. There are a few other areas where OAS2 & OAS3 differ that need to be handled, assuming the goal is feature parity for schema versions.
Which way would you like to go with this?
Can @tpburch and @sprootshift come to an alignment on the change?
Thanks.
I think these are somewhat independent PRs. I agree with @tpburch re "https://github.com/krakenjs/hapi-openapi/pull/183 likely result in fewer overall changes, but the specifics of both OAS2 and OAS3 schemas will be more spread out throughout the library". I consider https://github.com/krakenjs/hapi-openapi/pull/183 a feature expansion and https://github.com/krakenjs/hapi-openapi/pull/181 a refactoring to isolate OpenAPI 2 and 3 schema differences and hide behind a common interface. Although some isolation is already provided by swagger-parser.
With the recent announcement that Hapi will be sunset, my use case for these changes has basically evaporated. Based on that, it probably makes sense to go ahead with https://github.com/krakenjs/hapi-openapi/pull/183 for the overall functionality.
@sprootshift - Having said that, feel free to use any or all of the changes in this PR if you feel like it will help and/or add additional functionality.
Hapi will continue to be supported for 2 years minimum and is unlikely to be going away in the near term.
Is this project dead?
No, but I’m the only maintainer left from the team and don’t have a lot of time to focus on it these days.
I appreciate and try to keep up with PRs as they come but ideally a couple of additional maintainers can start helping out.
I guess we can close this ont @tlivings
Addresses https://github.com/krakenjs/hapi-openapi/issues/146
When completed, this PR will add support for OpenAPI 3. This necessarily a very invasive change to the codebase. Due to this, I wanted to get a draft of the partial changes out for initial feedback on the approach I am taking.
The major changes in this initial draft are:
I created an abstraction to separate the swagger/openapi logic from the Hapi logic. ApiDto exposes an interface that is used to register the Hapi components. The Dto uses the strategy pattern to map either a swagger or openapi document into the interface.
I did some significant refactoring of
index.js
, mostly to help me understand what all needed to be done. For the most part, this file was just rearranged, and can be put back if desired.routes.js
was rewritten to consume ApiDto. Part of the existing logic here was moved into the new mappers, while the Hapi-facing logic was all kept inroutes.js
.Unit testing: most of the existing unit tests have been updated to run against both an oas2 and oas3 schema. This involved branching /test/fixtures/defs into "oas2" and "oas3" directories, where the "oas2" directory are the pre-existing versions and the "oas3" directory contains a converted version of the same schema. Each test is then run once for each schema file. While this makes the unit tests a little more involved, I thought the benefit of ensuring compatibility across the board was worth it. With this pattern established, it will be easier to ensure future changes are compatible with both schema versions.
Next steps:
Some more work needs to be done on unifying the structure of parameters across schema versions. The goal is to simplify the logic of what we send to enjoi, what modifications we have to make to the resulting joi schema, and how parameters are handled in preRequest coercion.
Parameter serialization: the schema support for this is pretty different between oas2 and oas3. The cases currently unit tested work, but others aren't supported yet. Some more testing needs to be added for other serialization types.
File uploads: this is another area where oas2 & oas3 are fairly different.
As this is a draft PR - I am very interested in any comments, concerns, gotchas, etc.