-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fix map handling with allOf
/oneOf
+ upgrade pb33f/libopenapi
#101
Conversation
// All schema refs must be a local, file, or http resolve type | ||
if p.SchemaRef != "" && index.DetermineReferenceResolveType(p.SchemaRef) < 0 { | ||
result = errors.Join(result, errors.New("'schema_ref' must be a valid JSON schema reference")) | ||
} | ||
|
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 is the only removed functionality from upgrading. The index
package in pb33f/libopenapi
has been significantly refactored and I can't find an equivalent function for this.
In reality, this validation wasn't super useful anyways, since it never actually checked the spec for the existence of a reference, it just validates that the reference "could" be valid via the syntax.
If we want to replace this validation in the future, we should probably just load the OpenAPI spec provided and check if the reference exists in the spec.
}), | ||
}, | ||
}), | ||
AdditionalProperties: &base.DynamicValue[*base.SchemaProxy, bool]{ |
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.
The major breaking change was the structure of the AdditionalProperties
field, which has been changed from an any
to a strongly typed value! No more type assertions 🥳
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.
Thank you for the detailed explanations along the way here. Looks good to me. 🚀 Do we want a changelog entry?
Ah! Yes we definitely need a changelog. Thanks for the reminder! ⚡ 584b878 |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Closes #100
This bug was the result of a downstream bug in
pb33f/libopenapi
, where schema's withadditionalProperties
that utilizedallOf
,anyOf
, oroneOf
were not being unmarshaled properly. This resulted in the code generator incorrectly determining that a map was a single nested object with no attributes. This was fixed in pb33f/libopenapi#180 and released inv0.12.0
This PR upgrades
pb33f/libopenapi
, which had some breaking changes to the structure of the code, but no major functional changes outside of the removal of one helper function, which I'll note below.