-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix: Resolve problem with UUID class not found if there is almost one parameter with format as uuid #6617
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
Conversation
4b4c262 to
96638f2
Compare
JFCote
left a comment
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.
See comments. I think we need to validate the changes in the other generators + add the import in the post-processing of the generator code instead of adding specific case in the mustache file.
| #Functions for {{{baseName}}} API | ||
| {{#operations}} | ||
| {{#operation}} | ||
| {{httpMethod}} {{{contextPath}}}{{{path}}} controllers.{{classname}}Controller.{{operationId}}({{#pathParams}}{{paramName}}: {{#isUuid}}java.util.UUID{{/isUuid}}{{^isUuid}}{{{dataType}}}{{/isUuid}}{{#hasMore}}, {{/hasMore}}{{/pathParams}}) |
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 wonder if this should be done in the post processing in JavaPlayFrameworkCodegen.java instead. I think the dataType should always contain everything needed.
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.
@sparta15 have you tried the fullJavaUtil option to see if it would meet your requirement?
https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractJavaCodegen.java#L338
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.
Sorry, I have a lot of work and I couldn't review the comments. I'll try the fullJavaUtiloption too
|
|
||
| if (Boolean.TRUE.equals(cm.isString)) { | ||
| if (Boolean.TRUE.equals(cm.isString) && Boolean.TRUE.equals(cm.isUuid)) { | ||
| r.isUuid = true; |
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.
You will have to generate the other generators to make sure it doesn't break anything. This code is not used only by Play Framework :)
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 isUuid tag is only used in the Play templates so it should be harmless to other generators:
swagger-codegen|master⚡ ⇒ grep -R isUuid modules/swagger-codegen/src/main/resources/*
modules/swagger-codegen/src/main/resources/JavaPlayFramework/conversionBegin.mustache:{{#isBoolean}}Boolean.valueOf({{/isBoolean}}{{#isInteger}}Integer.parseInt({{/isInteger}}{{#isDouble}}Double.parseDouble({{/isDouble}}{{#isLong}}Long.parseLong({{/isLong}}{{#isFloat}}Float.parseFloat({{/isFloat}}{{#isUuid}}UUID.fromString({{/isUuid}}{{#isDateTime}}OffsetDateTime.parse({{/isDateTime}}
modules/swagger-codegen/src/main/resources/JavaPlayFramework/conversionEnd.mustache:{{#isBoolean}}){{/isBoolean}}{{#isInteger}}){{/isInteger}}{{#isDouble}}){{/isDouble}}{{#isLong}}){{/isLong}}{{#isFloat}}){{/isFloat}}{{#isUuid}}){{/isUuid}}{{#isDateTime}}){{/isDateTime}}
modules/swagger-codegen/src/main/resources/JavaPlayFramework/itemConversionBegin.mustache:{{#items.isBoolean}}Boolean.valueOf({{/items.isBoolean}}{{#items.isInteger}}Integer.parseInt({{/items.isInteger}}{{#items.isDouble}}Double.parseDouble({{/items.isDouble}}{{#items.isLong}}Long.parseLong({{/items.isLong}}{{#items.isFloat}}Float.parseFloat({{/items.isFloat}}{{#items.isUuid}}UUID.fromString({{/items.isUuid}}{{#items.isDateTime}}OffsetDateTime.parse({{/items.isDateTime}}
modules/swagger-codegen/src/main/resources/JavaPlayFramework/itemConversionEnd.mustache:{{#items.isBoolean}}){{/items.isBoolean}}{{#items.isInteger}}){{/items.isInteger}}{{#items.isDouble}}){{/items.isDouble}}{{#items.isLong}}){{/items.isLong}}{{#items.isFloat}}){{/items.isFloat}}{{#items.isUuid}}){{/items.isUuid}}{{#items.isDateTime}}){{/items.isDateTime}}
|
|
||
| if (Boolean.TRUE.equals(property.isString)) { | ||
| if (Boolean.TRUE.equals(property.isUuid) && Boolean.TRUE.equals(property.isString)) { | ||
| parameter.isUuid = true; |
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.
same thing here, it might break other generators
|
I've done a quick review of you code. Thanks a lot for the PR! :) |
|
I'll take another look tomorrow and merge this accordingly (after resolving the merge conflicts) |
|
@sparta15 I performed a bit more tests and the result is good. Thanks for the contribution. PR merged into master. |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.3.0.0branch for changes related to OpenAPI spec 3.0. Default:master.Description of the PR
Resolve #6609 . Add
java.util.UUIDinside controller method if the parameter in theurlhasformat:uuidin order to avoid swagger codegen can not find UUID.class