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

openapi2,3: support array of types in type field #912

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

brandonbloom
Copy link
Contributor

Resolves #563

The discussion in issue #563 mentions that this is only supported by openapi2, but the JSON Schema validation spec seems to imply that this should still be supported: https://json-schema.org/draft/2020-12/json-schema-validation#name-type

If the value of "type" is an array, then an instance validates successfully if its type matches any of the types indicated by the strings in the array.

This PR is a big and breaking change and only really adds two tests. Putting it up for discussion sake. Totally understood if an alternative approach is preferred.

Copy link
Collaborator

@fenollp fenollp left a comment

Choose a reason for hiding this comment

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

This approach is the way forward to me too. Thanks for tackling this!

Just a comment, really yack shaving on a test...
More importantly, please push the diff generated by that command:

./docs.sh

@@ -40,5 +40,5 @@ func Example() {
Properties["bar"].Value.
Type,
)
// Output: string
// Output: &[string]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's update this test so its output is serialized json (or yaml), so as to show that serialization turns single valued arrays of types into a string

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 think that's precisely what this comment change is doing, right? Not sure I follow the concern.

@fenollp
Copy link
Collaborator

fenollp commented Feb 20, 2024

The discussion in issue #563 mentions that this is only supported by openapi2, but the JSON Schema validation spec seems to imply that this should still be supported

Note that OpenAPIvN when N<3.1 does not use the JSON Schema draft, it defines its own JSON schema.
So in openapi3's schema.go there should be a new Validate() check that returns an error IFF len(schema.Type) >1.
And in the v2-to-v3 code here's the comment: #563 (comment)

@brandonbloom
Copy link
Contributor Author

brandonbloom commented Feb 24, 2024

I've run ./docs.sh and resolved conflicts with mainline changes.

Regarding the legality of types being an array, it's actually valid again in 3.1. See discussion here: mattpolzin/OpenAPIKit#356

If you plan on merging this change, I'd appreciate it you did it sooner rather than later and then we followed up with any smaller remaining improvements. Due to the pervasive nature of this change, conflicts are very likely with other changes, as evidenced by the conflicts I had to resolve before I could update this branch. Thanks!

@brandonbloom
Copy link
Contributor Author

(accidentally closed, reopening now)

@fenollp fenollp changed the title Support for array in type keyword openapi2,3: support array of types in type field Feb 25, 2024
@fenollp fenollp merged commit 672dc32 into getkin:master Feb 25, 2024
13 checks passed
@brandonbloom
Copy link
Contributor Author

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.

Support for array in type keyword
2 participants