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

Support $ref without a "file:" prefix #11

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ento
Copy link
Contributor

@ento ento commented Nov 26, 2017

This PR adds support for $ref like 'Pet.yaml' and 'definitions.yaml#/Pet', as seen in OpenAPI 2.0's examples. Also fixes some edge cases in the pointer resolution logic.

@drewolson
Copy link
Contributor

This is quite a large PR and will take me some time to review. Thanks for opening it.

@@ -1,5 +1,8 @@
module OpenApiParser
# Responsible for interpreting the fragment portion of a $ref value
# as a JSON Pointer and resolving it within a given document.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind removing comments for consistency with the rest of the library?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean these ones: 9dcc0bd
or including all the other code comments?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the remainder in 0989f6a

if @resolved
fail 'Do not try to resolve an already resolved reference.'
end
@resolved = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we memoize this method such that subsequent calls return the same value rather than failing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried reducing the number of states that the Reference object holds: 134f3f8 so that #resolve can be called any number of times. This way, a Reference is a representation of a {"$ref": ".."} object, which can be resolved in many different contexts.

If you had performance concerns, I can look into caching the reference resolution result at the Document level.

"#/i%5Cj" => 5,
"#/k%22l" => 6,
"#/%20" => 7,
"#/m~0n" => 8,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These example are directly from the JSON Pointer RFC. We need to continue to support them.

Copy link
Contributor Author

@ento ento Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Changed the spec so that it tests by dropping the pound sign, not adding back: 1ea0e73

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants