-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[Java Spring OAS3] Fix numerous OAS3 related Bugs #11181
Changes from 15 commits
77ae456
450de73
c24d436
da3b5b1
b8ff798
2964470
709690b
5b02424
040f2fe
847e4d9
3fa4f14
72df450
04d59c8
2eb0916
9e98ebd
ba1e04c
f8eac23
a02b77c
d56bfec
1bdd9cc
caf5c47
7e9d7cb
de95f90
70abe11
b0f452b
98a15e7
f894674
dbb8b1e
c8193ca
c9b38a3
2b0133b
ff7afa4
9f825b0
4f5d316
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
generatorName: spring | ||
outputDir: samples/openapi3/server/petstore/springboot | ||
inputSpec: modules/openapi-generator/src/test/resources/3_0/petstore.yaml | ||
templateDir: modules/openapi-generator/src/main/resources/JavaSpring | ||
additionalProperties: | ||
artifactId: springboot | ||
snapshotVersion: "true" | ||
oas3: "true" | ||
hideGenerationTimestamp: "true" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
generatorName: spring | ||
library: spring-cloud | ||
outputDir: samples/openapi3/client/petstore/spring-cloud-oas3-fakeapi | ||
inputSpec: modules/openapi-generator/src/test/resources/2_0/petstore-with-fake-endpoints-models-for-testing.yaml | ||
templateDir: modules/openapi-generator/src/main/resources/JavaSpring | ||
additionalProperties: | ||
artifactId: spring-cloud-oas3 | ||
interfaceOnly: "true" | ||
singleContentTypes: "true" | ||
hideGenerationTimestamp: "true" | ||
oas3: "true" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
generatorName: spring | ||
library: spring-cloud | ||
outputDir: samples/openapi3/client/petstore/spring-cloud | ||
inputSpec: modules/openapi-generator/src/test/resources/3_0/petstore.yaml | ||
templateDir: modules/openapi-generator/src/main/resources/JavaSpring | ||
additionalProperties: | ||
artifactId: spring-cloud-oas3 | ||
interfaceOnly: "true" | ||
singleContentTypes: "true" | ||
hideGenerationTimestamp: "true" | ||
oas3: "true" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{{#allowableValues}}allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this handle enums and containers properly? I don't see a change to "values.mustache" but maybe that's already correct There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no values.mustache. Which file do you refer to? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was making an assumption about the reference to #values - does this work properly with enum values and containers then? |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,6 @@ import io.swagger.v3.oas.annotations.Operation; | |
import io.swagger.v3.oas.annotations.Parameter; | ||
import io.swagger.v3.oas.annotations.media.Content; | ||
import io.swagger.v3.oas.annotations.media.Schema; | ||
import io.swagger.v3.oas.annotations.parameters.RequestBody; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there's value in having both annotations present - but open to removing if no one else feels the same way There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was my proposed fix https://github.com/OpenAPITools/openapi-generator/pull/10766/files There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would address issue when we would actually need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is used - by deleting this import the "@RequestBody" annotation is changing to org.springframework.web.bind.annotation.RequestBody from the import org.springframework.web.bind.annotation.* There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this is how it should be -
If we need them both, they should be fully qualified to disambiguate. For example
Question is : Do we actually need to support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it is supported by SpringDoc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thoughts on hvaing both annotations? |
||
import io.swagger.v3.oas.annotations.responses.ApiResponse; | ||
import io.swagger.v3.oas.annotations.security.SecurityRequirement; | ||
import io.swagger.v3.oas.annotations.tags.Tag; | ||
|
@@ -153,13 +152,13 @@ public interface {{classname}} { | |
produces = { {{#produces}}"{{{mediaType}}}"{{^-last}}, {{/-last}}{{/produces}} }{{/hasProduces}}{{#hasConsumes}}, | ||
consumes = { {{#consumes}}"{{{mediaType}}}"{{^-last}}, {{/-last}}{{/consumes}} }{{/hasConsumes}}{{/singleContentTypes}} | ||
) | ||
{{#jdk8-default-interface}}default {{/jdk8-default-interface}}{{#responseWrapper}}{{.}}<{{/responseWrapper}}ResponseEntity<{{>returnTypes}}>{{#responseWrapper}}>{{/responseWrapper}} {{#delegate-method}}_{{/delegate-method}}{{operationId}}({{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{>cookieParams}}{{^-last}},{{/-last}}{{#-last}}{{#reactive}}, {{/reactive}}{{/-last}}{{/allParams}}{{#reactive}}{{#oas3}}@Hidden{{/oas3}}{{^oas3}}@springfox.documentation.annotations.ApiIgnore{{/oas3}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}, {{#oas3}}@Hidden{{/oas3}}{{^oas3}}@springfox.documentation.annotations.ApiIgnore{{/oas3}} final org.springframework.data.domain.Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}}{{#unhandledException}} throws Exception{{/unhandledException}} { | ||
{{#jdk8-default-interface}}default {{/jdk8-default-interface}}{{#responseWrapper}}{{.}}<{{/responseWrapper}}ResponseEntity<{{>returnTypes}}>{{#responseWrapper}}>{{/responseWrapper}} {{#delegate-method}}_{{/delegate-method}}{{operationId}}({{#allParams}}{{>queryParams}}{{>pathParams}}{{>headerParams}}{{>bodyParams}}{{>formParams}}{{>cookieParams}}{{^-last}},{{/-last}}{{#-last}}{{#reactive}}, {{/reactive}}{{/-last}}{{/allParams}}{{#reactive}}{{#useSpringfox}}@springfox.documentation.annotations.ApiIgnore{{/useSpringfox}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}, {{#useSpringfox}}@springfox.documentation.annotations.ApiIgnore{{/useSpringfox}} final org.springframework.data.domain.Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{^jdk8-default-interface}};{{/jdk8-default-interface}}{{#jdk8-default-interface}}{{#unhandledException}} throws Exception{{/unhandledException}} { | ||
{{#delegate-method}} | ||
return {{operationId}}({{#allParams}}{{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, {{/hasParams}}exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}, pageable{{/vendorExtensions.x-spring-paginated}}); | ||
} | ||
|
||
// Override this method | ||
{{#jdk8-default-interface}}default {{/jdk8-default-interface}} {{#responseWrapper}}{{.}}<{{/responseWrapper}}ResponseEntity<{{>returnTypes}}>{{#responseWrapper}}>{{/responseWrapper}} {{operationId}}({{#allParams}}{{^isFile}}{{^isBodyParam}}{{>optionalDataType}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{{dataType}}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{/isFile}}{{#isFile}}{{#reactive}}Flux<Part>{{/reactive}}{{^reactive}}MultipartFile{{/reactive}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, {{/hasParams}}{{#oas3}}@Hidden{{/oas3}}{{^oas3}}@springfox.documentation.annotations.ApiIgnore{{/oas3}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}, {{#oas3}}@Hidden{{/oas3}}{{^oas3}}@springfox.documentation.annotations.ApiIgnore{{/oas3}} final org.springframework.data.domain.Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}} { | ||
{{#jdk8-default-interface}}default {{/jdk8-default-interface}} {{#responseWrapper}}{{.}}<{{/responseWrapper}}ResponseEntity<{{>returnTypes}}>{{#responseWrapper}}>{{/responseWrapper}} {{operationId}}({{#allParams}}{{^isFile}}{{^isBodyParam}}{{>optionalDataType}}{{/isBodyParam}}{{#isBodyParam}}{{^reactive}}{{{dataType}}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}}{{/isBodyParam}}{{/isFile}}{{#isFile}}{{#reactive}}Flux<Part>{{/reactive}}{{^reactive}}MultipartFile{{/reactive}}{{/isFile}} {{paramName}}{{^-last}}, {{/-last}}{{/allParams}}{{#reactive}}{{#hasParams}}, {{/hasParams}}{{#useSpringfox}}@springfox.documentation.annotations.ApiIgnore{{/useSpringfox}} final ServerWebExchange exchange{{/reactive}}{{#vendorExtensions.x-spring-paginated}}, {{#useSpringfox}}@springfox.documentation.annotations.ApiIgnore{{/useSpringfox}} final org.springframework.data.domain.Pageable pageable{{/vendorExtensions.x-spring-paginated}}){{#unhandledException}} throws Exception{{/unhandledException}} { | ||
{{/delegate-method}} | ||
{{^isDelegate}} | ||
{{>methodBody}} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
{{#isBodyParam}}{{#oas3}}@Parameter(name ={{/oas3}}{{^oas3}}@ApiParam(value ={{/oas3}} "{{{description}}}"{{#required}}, required = true{{/required}} {{^isContainer}}{{#defaultValue}}, defaultValue = "{{{.}}}"{{/defaultValue}}{{/isContainer}}{{#allowableValues}}, {{#oas3}}schema = @Schema({{/oas3}}allowableValues = "{{{.}}}"{{#oas3}}){{/oas3}}{{/allowableValues}}) {{#useBeanValidation}} @Valid{{/useBeanValidation}} @RequestBody{{^required}}(required = false){{/required}} {{^reactive}}{{{dataType}}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}} {{paramName}}{{/isBodyParam}} | ||
{{#isBodyParam}}{{>paramDoc}}{{#useBeanValidation}} @Valid{{/useBeanValidation}} @RequestBody{{^required}}(required = false){{/required}} {{^reactive}}{{{dataType}}}{{/reactive}}{{#reactive}}{{^isArray}}Mono<{{{dataType}}}>{{/isArray}}{{#isArray}}Flux<{{{baseType}}}>{{/isArray}}{{/reactive}} {{paramName}}{{/isBodyParam}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
{{#isCookieParam}}{{#useBeanValidation}}{{>beanValidationQueryParams}}{{/useBeanValidation}}{{#oas3}}@Parameter(name = "{{{baseName}}}", description = {{/oas3}}{{^oas3}}@ApiParam(value = {{/oas3}}"{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#enumVars}}{{#lambdaEscapeDoubleQuote}}{{{value}}}{{/lambdaEscapeDoubleQuote}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/enumVars}}"{{/allowableValues}}{{^isContainer}}{{#defaultValue}}, defaultValue = "{{{.}}}"{{/defaultValue}}{{/isContainer}}) @CookieValue("{{baseName}}") {{>optionalDataType}} {{paramName}}{{/isCookieParam}} | ||
{{#isCookieParam}}{{#useBeanValidation}}{{>beanValidationQueryParams}}{{/useBeanValidation}}{{>paramDoc}} @CookieValue("{{baseName}}") {{>optionalDataType}} {{paramName}}{{/isCookieParam}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,4 @@ | ||
{{#isFormParam}}{{^isFile}}{{#oas3}}@Parameter(name = "{{{baseName}}}", description = {{/oas3}}{{^oas3}}@ApiParam(value = {{/oas3}}"{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}}{{^isContainer}}{{#defaultValue}}, defaultValue = "{{{.}}}"{{/defaultValue}}{{/isContainer}}) {{#useBeanValidation}}@Valid{{/useBeanValidation}} @RequestPart(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{{dataType}}} {{paramName}}{{/isFile}}{{#isFile}}{{#oas3}}@Parameter(name = "{{{baseName}}}", description = {{/oas3}}{{^oas3}}@ApiParam(value = {{/oas3}}"{{{description}}}") {{#useBeanValidation}}@Valid{{/useBeanValidation}} @RequestPart(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{#isArray}}List<{{/isArray}}{{#reactive}}Flux<Part>{{/reactive}}{{^reactive}}MultipartFile{{/reactive}}{{#isArray}}>{{/isArray}} {{baseName}}{{/isFile}}{{/isFormParam}} | ||
{{#isFormParam}} | ||
{{^isFile}}{{>paramDoc}}{{#useBeanValidation}} @Valid{{/useBeanValidation}} @RequestPart(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{{dataType}}} {{paramName}}{{/isFile}} | ||
{{#isFile}}{{>paramDoc}} @RequestPart(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}) {{#isArray}}List<{{/isArray}}{{#reactive}}Flux<Part>{{/reactive}}{{^reactive}}MultipartFile{{/reactive}}{{#isArray}}>{{/isArray}} {{baseName}}{{/isFile}} | ||
{{/isFormParam}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
{{#isHeaderParam}}{{#oas3}}@Parameter(description ={{/oas3}}{{^oas3}}@ApiParam(value ={{/oas3}} "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}}{{^isContainer}}{{#defaultValue}}, defaultValue = "{{{.}}}"{{/defaultValue}}{{/isContainer}}) @RequestHeader(value = "{{baseName}}", required = {{#required}}true{{/required}}{{^required}}false{{/required}}) {{>optionalDataType}} {{paramName}}{{/isHeaderParam}} | ||
{{#isHeaderParam}}{{>paramDoc}} @RequestHeader(value = "{{baseName}}", required = {{#required}}true{{/required}}{{^required}}false{{/required}}) {{>optionalDataType}} {{paramName}}{{/isHeaderParam}} |
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.
I feel it would be more correct to avoid adding the imports in the first place, but I'm happy to look into that separately
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.
Actually this is how it is - the swagger2 imports are hard coded in
AbstractJavaCodegen
and can only be removed reliably in a post processor. They are not controlled by a mustache template. To change this, all Java based generators would have to be modified.