Skip to content
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: process discriminator mapping values as refs #1022

Merged
merged 2 commits into from
Oct 24, 2024

Conversation

jgresty
Copy link
Contributor

@jgresty jgresty commented Oct 16, 2024

While the type of the discriminator mapping values is a string in the upstream specs, it contains a jsonschema reference to a schema object. It is surprising behaviour that these refs are not handled when calling functions such as InternalizeRefs.

This patch adds the data structures to store the ref internally, and updates the Loader and InternalizeRefs to handle this case. There may be several more functions that need to be updated that I am not aware of.

Since it is not a full Ref object we have to do some fudging to make it work with all the existing ref handling code.

This is a proposal for #1021 , I do not expect it to be the actual implementation.

openapi3/discriminator.go Outdated Show resolved Hide resolved
This allows us to support more use cases than just a string, allowing us to
attach more metadata onto these StringMap types that we cannot encode in just a
string.
While the type of the discriminator mapping values is a string in the
upstream specs, it contains a jsonschema reference to a schema object.
It is surprising behaviour that these refs are not handled when calling
functions such as InternalizeRefs.

This patch adds the data structures to store the ref internally, and
updates the Loader and InternalizeRefs to handle this case. There may be
several more functions that need to be updated that I am not aware of.

Since it is not a full Ref object we have to do some fudging to make it
work with all the existing ref handling code.
@fenollp
Copy link
Collaborator

fenollp commented Nov 8, 2024

Hi @jgresty
I had to revert this change due to #1028
Please feel free to give this another try when you have the time. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants