search

Nexmo / oas_parser

An open source Open API Spec 3 Definition Parser
MIT License
51 stars 16 forks source link

Handle circular references when resolving #32

Closed piao-mdsol closed 5 years ago

piao-mdsol commented 5 years ago

Resolves https://github.com/Nexmo/oas_parser/issues/24, also another implementation of #25

Overview

Parser now handles circular/self reference by returning the $ref as is instead of expanding it indefinitely.

Background

Historically, my colleague @ssteeg-mdsol had sent a Pull Request (https://github.com/braintree/open_api_parser/pull/10) to open_api_parser to handle the exact same circular references. We had been using it in our various projects ever since and had no problem.

We found this gem when we migrate to OpenAPI V3. Later I also found out that this project was originated from open_api_parser, the codebase shares the similarity which makes applying the same patch super easy. So the changes here are almost identical to https://github.com/braintree/open_api_parser/pull/10.

Notes for Reviewers

I added a changelog as I think it'd be helpful for developers to understand the notable changes between each release. I didn't back-filled the old releases. (Please feel free to modify / amend 🙏 )

As for how the circular reference should be handled, I couldn't find a universally agreed upon answer. right now I'm keeping it as is. The code is flexible to adopt to other strategies like removing it entirely. Probably in the future, if we see the needs we could add OasParser.circule_reference_strategy = :removal or :nullify and let the users decide which way to go. But for now, think that'd be an overkill 🙂.

cc @ssteeg-mdsol @ykitamura-mdsol @jfeltesse-mdsol

piao-mdsol commented 5 years ago

@mheap Gentle ping~ 🙏

mheap commented 5 years ago

Hey @piao-mdsol, thanks for the contribution! A quick look at the tests look good, but I'll need a few more days to clone and test locally

mheap commented 5 years ago

@piao-mdsol This is released as 0.19.0 - thanks for your contribution and patience!

piao-mdsol commented 5 years ago

@mheap Yay! Thanks a lot! 🎉

MarioRuiz commented 5 years ago

This is still an issue. On https://github.com/Azure/azure-rest-api-specs/tree/master/specification/network/resource-manager/Microsoft.Network/stable/2019-04-01 try to resolve: networkInterface.json

mheap commented 5 years ago

@MarioRuiz Could you open a PR with a failing test case and I'll take a look

MarioRuiz commented 5 years ago

I cannot find out where the circular reference is from that swagger so not possible to create a test