Skip to content

openapi3: allow Extensions next to $ref in SchemaRef#901

Merged
fenollp merged 1 commit intogetkin:masterfrom
paulmach:schemaref
Jun 24, 2024
Merged

openapi3: allow Extensions next to $ref in SchemaRef#901
fenollp merged 1 commit intogetkin:masterfrom
paulmach:schemaref

Conversation

@paulmach
Copy link
Copy Markdown
Contributor

@paulmach paulmach commented Jan 30, 2024 •
edited
Loading

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 into SchemaRef.Extensions

type: object
properties:
 id:
   $ref: "#/components/schemas/id"
   x-order: 1
 name:
   type: string

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:

schemas:
  id:
    type: integer
    x-order: 2
  thing:
    type: object
    properties:
     thing_id:
       $ref: "#/components/schemas/id"
       type: string
       x-order: 1
     name:
       type: string

Doesn't change: thingIDRef.JSONLookup("type") will continue to return "integer" as overriding the type next to the ref is not allowed and bad practice, in my opinion. There could be a use-case for stuff like nullable, 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 be 1 (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 the ProhibitExtensionsWithRef option.

@paulmach paulmach mentioned this pull request Jan 30, 2024
Closed
@paulmach
Copy link
Copy Markdown
Contributor Author

paulmach commented Jan 30, 2024

Oh snap, looks like refs.go is auto generated. Please see if you like this change, then I make the change to refs.sh.

fenollp
fenollp reviewed Jan 31, 2024
View reviewed changes
fenollp
fenollp reviewed Jan 31, 2024
View reviewed changes
fenollp
fenollp requested changes Jan 31, 2024
View reviewed changes
@paulmach
Copy link
Copy Markdown
Contributor Author

paulmach commented Feb 1, 2024

I made the following changes:

  • moved tests in ref_test.go into refs_issue222_test.go and refs_issue247_test.go
  • added validation options AllowExtensionsWithRef (default) and ProhibitExtensionsWithRef
  • updated refs.sh to generate the code
  • add refs_test.sh to generate tests
  • reran docs.sh and added refs_test.sh as part of the github workflow.

I think the only "question" is if ref.Extensions should capture ^x- fields or all extra fields. As mentioned above, there could be some impact for "weird" schemas if we capture all fields. I propose to only capture ^x- fields for now as more backwards compatible. We can always change it based on feedback.

@fenollp
Copy link
Copy Markdown
Collaborator

fenollp commented Feb 1, 2024

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

@fenollp fenollp linked an issue Feb 2, 2024 that may be closed by this pull request
v3: allow extensions next to $ref in SchemaRef #900
Closed
@paulmach paulmach requested a review from fenollp February 7, 2024 01:03
@paulmach
Copy link
Copy Markdown
Contributor Author

paulmach commented Apr 23, 2024

I rebased on the latest master. Hoping to get this into the next release.

@paulmach
Copy link
Copy Markdown
Contributor Author

paulmach commented Jun 9, 2024

Hi @fenollp, I rebased again on the latest master. Hoping to get this into the next release.

@paulmach paulmach mentioned this pull request Jun 10, 2024
Closed
@paulmach paulmach force-pushed the schemaref branch 2 times, most recently from ba91e23 to 63ca84c Compare June 23, 2024 05:26
fenollp
fenollp approved these changes Jun 24, 2024
View reviewed changes
openapi3/validation_options.go
// error if extensions (fields starting with 'x-') are found as
// siblings for $ref fields. Non-extension fields are prohibited
// unless allowed explicitly with the AllowExtraSiblingFields option.
func ProhibitExtensionsWithRef() ValidationOption {
Copy link
Copy Markdown
Collaborator

@fenollp fenollp Feb 1, 2024

Choose a reason for hiding this comment

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

Do you mind renaming to ˋDisallow..ˋ? The other options in there start with Allow.. or Disallow.. (well, most)

@fenollp fenollp merged commit 57624b3 into getkin:master Jun 24, 2024
@paulmach paulmach mentioned this pull request Jul 13, 2024
Merged
@irees irees mentioned this pull request Aug 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fenollp fenollp fenollp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

v3: allow extensions next to $ref in SchemaRef

2 participants

@paulmach @fenollp