New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
openapi3: allow Extensions next to $ref in SchemaRef #901
base: master
Are you sure you want to change the base?
Conversation
Oh snap, looks like refs.go is auto generated. Please see if you like this change, then I make the change to |
@@ -559,6 +560,10 @@ func (x *ResponseRef) JSONLookup(token string) (interface{}, error) { | |||
// SchemaRef represents either a Schema or a $ref to a Schema. | |||
// When serializing and both fields are set, Ref is preferred over Value. | |||
type SchemaRef struct { | |||
// Extensions only captures fields starting with 'x-' as no other fields | |||
// are allowed by the openapi spec. | |||
Extensions map[string]interface{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this should live here: this field already exists in the .Value
. Or maybe it'd make more sense to move .Value.Extensions
here, alongside .Ref
.Value
and .extra
. This makes sense to me. You?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this change alone would be its own PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a quick test and it looks like ref.Value
objects are reused for all the places they are referenced. Thus updating them will change them everywhere.
I personally think we should keep it as proposed here. This allows users to know where the extension came from. They can use schemaRef.JSONLookup("x-....")
to do the most common thing, check ref, then fallback to Value.
I made the following changes:
I think the only "question" is if |
This is great work thank you! WRT non x-.. in Extensions you're right: let's just keep the x-.. keys in here for now anyway |
I rebased on the latest master. Hoping to get this into the next release. |
See #900 for full details and use-case. If this approach is agreed upon, I can extend to all *Refs (e.g. CallbackRef, ExampleRef, ResponseRef, etc.)
Captures
x-order
intoSchemaRef.Extensions
Only extensions (those starting with 'x-') are captured. This is because extra attributes are technically not allowed by the spec, but also to maintain the way
JSONLookup
works. For example:Doesn't change:
thingIDRef.JSONLookup("type")
will continue to return "integer" as overriding thetype
next to the ref is not allowed and bad practice, in my opinion. There could be a use-case for stuff likenullable
, but for now I would rather not change the behavior.Does change:
thingIDRef.JSONLookup("x-order")
. Today, 2, as it goes back to the $ref definition. After this change it'll be1
(value next to the $ref).Does change:
Validate()
behavior. It's now okay, by default, to have extensions next to the $ref. To change this behavior, use theProhibitExtensionsWithRef
option.