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

Added the FQCN to the @Size-annotation. #975

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

denvitaharen
Copy link
Contributor

  • You have read the contributors guide
  • Your code is properly formatted according to our code style
  • Pull Request title contains the target branch if not targeting main: [0.9.x] Subject
  • Pull Request contains link to the issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket

Thanks to @ricardozanini for pointing me in the right direction.

The solution is to post process the models and find the @Size annotation and replace them with the FQCN instead.

This fixed bug: #925

I took the update from #926 that @joschi made to reproduce the problem and used it to verify that the fix works. So that pull request can be closed when this is merge.

I tried to write an test to verify that the Size annotation is there, but since the Size annotation is not on the variable listOfStrings but rather on the type of the list, I can't really understand how I can check that.

Copy link

@joschi joschi left a comment

Choose a reason for hiding this comment

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

I think post-processing the generated models is unnecessary and it's merely a configuration issue.

After reporting this bug, I've dropped this project and used OpenAPI Generator directly to generate the respective models.

This is the working configuration:

annotationLibrary: "none"
asyncNative: true
dateLibrary: "java8"
disableMultipart: true
documentationProvider: "none"
generateBuilders: true
hideGenerationTimestamp: true
library: "microprofile"
microprofileMutiny: true
microprofileRestClientVersion: "3.0"
openApiNullable: false
serializationLibrary: "jackson"
useBeanValidation: true
useJakartaEe: true
useRuntimeException: true

The missing piece is probably microprofileRestClientVersion: "3.0" and potentially useJakartaEe: true.
https://github.com/OpenAPITools/openapi-generator/blob/v7.11.0/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java#L289

@ricardozanini
Copy link
Member

ricardozanini commented Jan 30, 2025

@joschi @denvitaharen, then we can set/expose these configurations since it seems that the generator already sets the correct variables based on the set of properties. microprofileRestClientVersion: "3.0" and useJakartaEe: true should be the default on our model, perhaps immutable like the Quarkus platform.

@denvitaharen
Copy link
Contributor Author

denvitaharen commented Jan 31, 2025

I think post-processing the generated models is unnecessary and it's merely a configuration issue.

After reporting this bug, I've dropped this project and used OpenAPI Generator directly to generate the respective models.

This is the working configuration:

annotationLibrary: "none"
asyncNative: true
dateLibrary: "java8"
disableMultipart: true
documentationProvider: "none"
generateBuilders: true
hideGenerationTimestamp: true
library: "microprofile"
microprofileMutiny: true
microprofileRestClientVersion: "3.0"
openApiNullable: false
serializationLibrary: "jackson"
useBeanValidation: true
useJakartaEe: true
useRuntimeException: true

The missing piece is probably microprofileRestClientVersion: "3.0" and potentially useJakartaEe: true. https://github.com/OpenAPITools/openapi-generator/blob/v7.11.0/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/JavaClientCodegen.java#L289

From my own experience with the OpenAPI generator (I have used it my self before changing to this), the useJakartaEe: true only changes from where the imports will come (javax vs Jakarta), but the annotations will be the same. useJakartaEeand microprofikeRestClientVersion also effects the template engine that's used in the OpenAPI generator.

My understanding of this extension is that it doesn't use the OpenAPI generator to generate the classes, the OpenAPI generator is used to convert the OpenAPI file to an set of java objects which then is pass to the Qute template engine that generates the java classes.

Of course, I tried this and I removed my post processing and added the configuration, but made no difference and we where back to square one with an private List<@Size(min = 1)String> listOfStrings = new ArrayList<>();.

@joschi
Copy link

joschi commented Jan 31, 2025

My understanding of this extension is that it doesn't use the OpenAPI generator to generate the classes, the OpenAPI generator is used to convert the OpenAPI file to an set of java objects which then is pass to the Qute template engine that generates the java classes.

This is a simplified view of the world. In reality JavaCodegen and AbstractJavaCodegen already generate significant parts of the code, for example the problematic annotation:

https://github.com/OpenAPITools/openapi-generator/blob/v7.11.0/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java#L1199-L1203

The custom templates of this extension are just incomplete with regards to these generated parts of the code.

In other words: The templates have to be fixed and be aligned with the upstream Mustache templates if you still want to use the upstream code generator.

I've updated #926 with the fixed template.

@ricardozanini
Copy link
Member

Indeed, a review of Qute templates to follow some mustache changes might be needed. But please remember that the files generated here aim to be closer to the Quarkus Ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug [Bean Validation]: Compilation error with array of strings with validations
4 participants