fix: differentiate between array typres and standard structs #134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Was able to drop the need to recurse twice on request bodies by passing a the |
dylanhitt
commented
Jun 26, 2024
dylanhitt
commented
Jun 26, 2024
| @@ -67,6 +161,8 @@ func TestServer_generateOpenAPI(t *testing.T) { | |||
| require.NotNil(t, document.Paths.Find("/")) | |||
| require.Nil(t, document.Paths.Find("/unknown")) | |||
| require.NotNil(t, document.Paths.Find("/post")) | |||
| require.Equal(t, document.Paths.Find("/post").Post.Responses.Value("200").Value.Content["application/json"].Schema.Value.Type, "array") | |||
| require.Equal(t, document.Paths.Find("/post").Post.Responses.Value("200").Value.Content["application/json"].Schema.Value.Items.Ref, "#/components/schemas/MyStruct") | |||
There was a problem hiding this comment.
Need to do more to validate the actual schema reference here
dylanhitt
commented
Jun 26, 2024
|
@EwenQuim when you have a moment to do you mind taking a look. |
Ignore this error as it is not possible with the current instationation of of the openapi generator used. Unless cyclic err is set or a custom schema customizer is provided it is not possible to error.
dylanhitt
commented
Jul 1, 2024
| if v == nil { | ||
| return "unknown-interface" | ||
| s.getOrCreateSchema("unknown-interface", struct{}{}) |
There was a problem hiding this comment.
Need test to ensure an unknown-interface component schema is being added to the spec via test. I did not pick this up until my own diff
…lect the struct and not it's array/ptrs
|
I'll look at it omorrow morning, thanks for this investigation. It is indeed complex. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This did turn out to be pretty complex.
The following is by no means finished. More looking for a discussion. If you take a look #133 the bug is fairly clear. We do not respect response nor return types when they are arrays.
The only way I can think of solving this is that we must create the ref to the schema and link it at the bottom of a recursion tree. If you do not do this you will end up with improper refs, with the original approach the input for the first type wins so if an array of
MyStructis passed the schema will be a list ofMyStructrather than just the simple Schema ofMyStruct. SeeschemaDive. Currently this does require us to dive twice on request bodies. Since we seem to check request bodies if they areunknown-type/string, which I am unsure why (wouldn't we want to have []string be a valid requestBody)? As long as that check exists I'm not sure how to avoid both recursive steps.This does however give the proper diff
This does need more tests, but I'd like another pair of eyes too look at the approach @EwenQuim if you don't mind. Could you please take a look. I'm sure this can be simplified. I've been staring at it too long at this point.
@epentland feel free to chime since you brought this to our attention
Thank you