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.

Sign up for GitHub

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

Jump to bottom

[Java][Spring] fix @Operation content for array response #14201

Merged
merged 1 commit into from
Jan 2, 2023
Merged

[Java][Spring] fix @Operation content for array response #14201

merged 1 commit into from
Jan 2, 2023

Conversation

borsch
Copy link
Member

@borsch borsch commented Dec 7, 2022

Fix for #14178

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples: 
    ./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09)

@borsch borsch force-pushed the spring-fix-swagger-ui-for-arrays branch 2 times, most recently from d52a319 to d5c6133 Compare December 7, 2022 16:08
msg2santoshsagar
msg2santoshsagar approved these changes Dec 9, 2022
View reviewed changes
Copy link

@msg2santoshsagar msg2santoshsagar left a comment

Choose a reason for hiding this comment

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

Looks good..

@borsch
Copy link
Member Author

borsch commented Dec 10, 2022

@wing328 please review

welshm
welshm approved these changes Dec 10, 2022
View reviewed changes
Copy link
Contributor

@welshm welshm left a comment

Choose a reason for hiding this comment

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

Nice!

samples/client/others/typescript-axios/with-separate-models-and-api-inheritance/common.ts Outdated
@@ -85,6 +85,7 @@ export const setOAuthToObject = async function (object: any, name: string, scope
}

function setFlattenedQueryParams(urlSearchParams: URLSearchParams, parameter: any, key: string = ""): void {
if (parameter == null) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems unrelated to what you changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, without this "samples up-to date" pipeline is failling

modules/openapi-generator/src/main/resources/JavaSpring/api.mustache Outdated
@@ -161,7 +162,7 @@ public interface {{classname}} {
{{#responses}}
@ApiResponse(responseCode = "{{{code}}}", description = "{{{message}}}"{{#baseType}}, content = {
{{#produces}}
@Content(mediaType = "{{{mediaType}}}", schema = @Schema(implementation = {{{baseType}}}.class)){{^-last}},{{/-last}}
@Content(mediaType = "{{{mediaType}}}", {{#isArray}}array = @ArraySchema( {{/isArray}}schema = @Schema(implementation = {{{baseType}}}.class){{#isArray}}){{/isArray}}){{^-last}},{{/-last}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this require any version checks for Swagger? Or it should have always been there?

Copy link
Member Author

Choose a reason for hiding this comment

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

seems like it's in all versions of v3 package

https://github.com/swagger-api/swagger-core/blob/master/modules/swagger-annotations/src/main/java/io/swagger/v3/oas/annotations/media/Content.java#L70

Copy link
Contributor

Choose a reason for hiding this comment

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

The while section is wrapped in a {{#swagger2AnnotationLibrary}} Block. So it is only in effect with Swagger v2.x / OAS v3.

@borsch borsch force-pushed the spring-fix-swagger-ui-for-arrays branch from d5c6133 to 14d7a7a Compare December 21, 2022 07:07
@cachescrubber
Copy link
Contributor

Related issue? #13332.

@borsch borsch force-pushed the spring-fix-swagger-ui-for-arrays branch 3 times, most recently from d9aaed8 to b9dab63 Compare January 1, 2023 19:12
@borsch borsch force-pushed the spring-fix-swagger-ui-for-arrays branch from b9dab63 to 9be7efd Compare January 1, 2023 19:25
@borsch
Copy link
Member Author

borsch commented Jan 2, 2023

@wing328 could you please review and merge?

@wing328
Copy link
Member

wing328 commented Jan 2, 2023

cc @OpenAPITools/generator-core-team as the change impacts example generator

@wing328 wing328 added this to the 6.3.0 milestone Jan 2, 2023
@wing328
Copy link
Member

wing328 commented Jan 2, 2023

LGTM and I've not tested it locally

@wing328 wing328 merged commit dc99a45 into OpenAPITools:master Jan 2, 2023
@borsch borsch deleted the spring-fix-swagger-ui-for-arrays branch January 2, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msg2santoshsagar msg2santoshsagar msg2santoshsagar approved these changes

@cachescrubber cachescrubber cachescrubber approved these changes

@welshm welshm welshm approved these changes

Assignees
No one assigned
Labels
Issue: Bug Server: Java Server: Spring
Projects
None yet
Milestone
6.3.0
Development

Successfully merging this pull request may close these issues.
5 participants
@borsch @cachescrubber @wing328 @welshm @msg2santoshsagar