[4947][java]: adds support for validation of primitives in arrays #17165

Aliaksie · 2023-11-22T11:56:58Z

Adds Bean validation for primitive type in array see issue and issue

I'm not sure this should be added here but intuitively feel that it should be some how general for Java, also I may not have covered all possible options but I hope that this could be a good start for this story.

I'm looking forward to any recommendations and advice! and if all these changes do not have any sense, just reject this PR.

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/configs/*.yaml
./bin/utils/export_docs_generators.sh
```
(For Windows users, please run the script in Git BASH)
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*.
IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.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.

@wing328 @bbdouglas @sreeshas @jfiala @lukoyanov @cbornet @jeff9finger @karismann @Zomzog @lwlee2608 @martin-mfg

martin-mfg

Thanks for your PR!

I may not have covered all possible options but I hope that this could be a good start for this story.

According to https://swagger.io/docs/specification/data-models/data-types/ there are some more relevant options. e.g. multipleOf, minItems, etc.
It's up to you whether you want to support more options in this PR or not.

...generator/src/test/java/org/openapitools/codegen/java/JavaValidationArrayPrimitivesTest.java

.../openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java

…n_array

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java

martin-mfg · 2023-12-03T14:48:34Z

LGTM.
Let's wait for a maintainer to merge this PR. :)

…n_array

Aliaksie · 2023-12-04T14:28:32Z

@wing328 could you please have a look ?

wing328 · 2023-12-04T14:32:26Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java

+        // the response data types should not contains a bean validation annotation.
+        if (r.dataType.contains("@")) {
+            r.dataType = r.dataType.replaceAll("(?:(?i)@[a-z0-9]*+\\s*)*+", "");
+        }


I wonder if it's a must to do this in the default codegen as this code block seems to handle data type for Java generators only.

What about doing moving these check in postProcessWithModels in abstract java codegen instead?

sure why not, please check if it looks better and i got you correctly.

@wing328, sorry to bother you, have you had a chance to look into it? do I need to make additional changes or can I resolve this conversation ?

wing328 · 2023-12-04T14:48:35Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java

@@ -1420,136 +1420,11 @@ public void setRequestMappingMode(RequestMappingMode requestMappingMode) {
        this.requestMappingMode = requestMappingMode;
    }



cc spring technical committee as the change impacts spring codegen as well

@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) @martin-mfg (2023/08)

welshm

Looks good to me from a Java Spring perspective.

Tests from #17157 should still be passing so the recently added validation should still be working 👍

welshm · 2023-12-04T15:23:34Z

.../openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java

+
+    private String getIntegerBeanValidation(Schema<?> items) {
+        if (items.getMinimum() != null && items.getMaximum() != null) {
+            return String.format(Locale.ROOT, "@Min(%s) @Max(%s)", items.getMinimum(), items.getMaximum());


This is generating code - I don't think it should be taking in a locale, since it should be the same regardless of the machine/user Locale settings

I used Locale.ROOT because I saw a lot of examples in code base, which is why I decided that this is the customary way to use it in the project... I can do it with the usual concatenation... but it seems to me that the code will become less readable... what do you think?

Probably better for @wing328 or someone with broader context to comment it just seems odd to use a Locale to format strings which are supposed to represent code.

If that's the standard for the project, probably best to leave as-is.

@wing328, sorry to bother you, have you had a chance to look into it? do I need to make additional changes or can I resolve this conversation ?

let me take another look tomorrow.

@wing328 hey hey! sorry to bother you again... any updates?

note: I noticed that the tests for some samples in the extended test checks did not pass due to the lack of imports .. that’s why I added imports to the templates .. I hope I found all the places where this is needed, let me know if it’s ok

checking now

some java sample tests still failed. can you please take a look when you've time?

oh sorry for that, I missed that for helidon should use rootJavaEEPackage in templates, fixed hope now all tests will passed. Thanks!

…n_array

wing328 · 2023-12-11T09:48:50Z

.../openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java

@@ -945,6 +946,121 @@ public String getTypeDeclaration(Schema p) {
        return super.getTypeDeclaration(target);
    }

+    private String getBeanValidation(Schema<?> items) {


minor suggestion: add a docstring for this method

wing328 · 2023-12-11T12:56:00Z

just merged the PR. thanks again for the contribution.

wing328 · 2023-12-26T02:23:45Z

@Aliaksie just noticed that this PR seems to break the CI tests.

In samples/server/petstore/jaxrs/jersey2, run mvn clean package resulting in the following errors:

[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/gen/java/org/openapitools/model/ArrayTest.java:[112,50] annotation @javax.validation.Valid not applicable in this type context
[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/gen/java/org/openapitools/model/ArrayTest.java:[127,20] annotation @javax.validation.Valid not applicable in this type context
[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/gen/java/org/openapitools/model/ArrayTest.java:[131,46] annotation @javax.validation.Valid not applicable in this type context
[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/main/java/org/openapitools/api/impl/UserApiServiceImpl.java:[26,52] annotation @javax.validation.Valid not applicable in this type context

can you please take a look when you've time?

ref: https://github.com/OpenAPITools/openapi-generator/actions/runs/7262479579/job/19950668803?pr=17431

Aliaksie · 2023-12-26T11:01:54Z

@Aliaksie just noticed that this PR seems to break the CI tests.

In samples/server/petstore/jaxrs/jersey2, run mvn clean package resulting in the following errors:

[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/gen/java/org/openapitools/model/ArrayTest.java:[112,50] annotation @javax.validation.Valid not applicable in this type context
[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/gen/java/org/openapitools/model/ArrayTest.java:[127,20] annotation @javax.validation.Valid not applicable in this type context
[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/gen/java/org/openapitools/model/ArrayTest.java:[131,46] annotation @javax.validation.Valid not applicable in this type context
[ERROR] /Users/williamcheng/Code/openapi-generator7/samples/server/petstore/jaxrs/jersey2/src/main/java/org/openapitools/api/impl/UserApiServiceImpl.java:[26,52] annotation @javax.validation.Valid not applicable in this type context

can you please take a look when you've time?

ref: https://github.com/OpenAPITools/openapi-generator/actions/runs/7262479579/job/19950668803?pr=17431

@wing328 thanks for let me know!

I briefly looked into the problem you mentioned... and it looks like io.swagger:swagger-jersey2-jaxrs:jar:1.6.5 is transitively pulling up the javax.validation:validation-api:jar:1.1.0.Final:compile, which is causing the compilation error. The pipeline may pass or fail due to fact that also in deps jakarta.validation:jakarta.validation-api:jar:2.0.2.

we probably can exclude the library from the dependency

    <dependency>
      <groupId>io.swagger</groupId>
      <artifactId>swagger-jersey2-jaxrs</artifactId>
      <scope>compile</scope>
      <version>${swagger-core-version}</version>
      <exclusions>
        <exclusion>
          <groupId>javax.validation</groupId>
          <artifactId>validation-api</artifactId>
        </exclusion>
      </exclusions>
    </dependency>

what do you think @wing328 ?

wing328 · 2023-12-26T16:28:54Z

If it's not a lot of work, can you please file a PR with the suggested fix?

Aliaksie · 2023-12-27T10:59:19Z

If it's not a lot of work, can you please file a PR with the suggested fix?

@wing328 sure, please have a look #17479 , hope it works and help!

ChristianCiach · 2024-01-08T12:27:34Z

This MR seems to break the Java native Client when useBeanValidation is used. The parameters now use @Valid annotations, but this annotation class (and other classes from the validation package) is never imported.

I think it misses this block

{{#useBeanValidation}}
import {{javaxPackage}}.validation.constraints.*;
import {{javaxPackage}}.validation.Valid;
{{/useBeanValidation}}

in https://github.com/Aliaksie/openapi-generator/blob/77497ba19267e7c7106126ba9f19aeb5101d1803/modules/openapi-generator/src/main/resources/Java/libraries/native/api.mustache

Aliaksie force-pushed the bugfix/4947_adds_validation_in_array branch from bf53b15 to b5cdc5d Compare November 22, 2023 14:24

Aliaksie added 3 commits November 24, 2023 09:07

[4947][java]: adds support for validation of primitives in arrays

3f87820

[4947][java]: prevents generation '@Valid' for Object

8446e88

[4947][java]: test against different codegens and stick to primitive

c668a11

Aliaksie force-pushed the bugfix/4947_adds_validation_in_array branch from ce7e022 to c668a11 Compare November 24, 2023 09:44

martin-mfg suggested changes Nov 30, 2023

View reviewed changes

Aliaksie and others added 2 commits December 1, 2023 08:15

Merge branch 'OpenAPITools:master' into bugfix/4947_adds_validation_i…

49eb303

…n_array

[4947][java]: code review

c932a6f

martin-mfg reviewed Dec 1, 2023

View reviewed changes

modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java Outdated Show resolved Hide resolved

Aliaksie added 2 commits December 1, 2023 16:07

[4947][java]: enhance getBeanValidation

747dc06

[4947][java]: adds email

e278537

martin-mfg approved these changes Dec 3, 2023

View reviewed changes

Aliaksie and others added 2 commits December 4, 2023 08:50

Merge branch 'OpenAPITools:master' into bugfix/4947_adds_validation_i…

bbecaf9

…n_array

[4947][java]: removes unnecessary override

0810034

wing328 reviewed Dec 4, 2023

View reviewed changes

[4947][java]: adds postProcessResponseWithProperty

3b84e36

welshm reviewed Dec 4, 2023

View reviewed changes

wing328 added Client: Java Enhancement: General labels Dec 5, 2023

wing328 added this to the 7.2.0 milestone Dec 5, 2023

Aliaksie and others added 3 commits December 8, 2023 16:19

[4947][java]: adds missing import {{javaxPackage}}.validation.Valid

c7a83f4

Merge branch 'OpenAPITools:master' into bugfix/4947_adds_validation_i…

e0f50bd

…n_array

[4947][java]: adds missing useBeanValidation

4ae5d42

wing328 reviewed Dec 11, 2023

View reviewed changes

[4947][java]: fix use rootJavaEEPackage for helidon

77497ba

wing328 merged commit 809b333 into OpenAPITools:master Dec 11, 2023
120 checks passed

Aliaksie deleted the bugfix/4947_adds_validation_in_array branch December 18, 2023 19:21

wacker mentioned this pull request Jan 18, 2024

[BUG] [JAVA] "cannot find symbol" syntax error involving @Valid (useBeanValidation) in case of fully qualified type reference #17647

Open

5 tasks

radhakrishnan10192 mentioned this pull request Feb 4, 2024

[REQ] Custom Extension for primitive List #17786

Open

This was referenced Feb 13, 2024

[BUG] V7.2.0 Breaks Schema Validation Imports #17485

Closed

[java][native] Fix missing imports when using list validation #17857

Merged

horaceli mentioned this pull request Apr 8, 2024

[java][resttemplate] Fix missing javax validation imports with list validation #18332

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4947][java]: adds support for validation of primitives in arrays #17165

[4947][java]: adds support for validation of primitives in arrays #17165

Aliaksie commented Nov 22, 2023 •

edited

Loading

martin-mfg left a comment

martin-mfg commented Dec 3, 2023

Aliaksie commented Dec 4, 2023

wing328 Dec 4, 2023

Aliaksie Dec 4, 2023

Aliaksie Dec 7, 2023

wing328 Dec 4, 2023

welshm left a comment

welshm Dec 4, 2023

Aliaksie Dec 4, 2023

welshm Dec 4, 2023

Aliaksie Dec 7, 2023

wing328 Dec 7, 2023

Aliaksie Dec 11, 2023

wing328 Dec 11, 2023

wing328 Dec 11, 2023

Aliaksie Dec 11, 2023

wing328 Dec 11, 2023

Aliaksie Dec 11, 2023

wing328 commented Dec 11, 2023

wing328 commented Dec 26, 2023 •

edited

Loading

Aliaksie commented Dec 26, 2023 •

edited

Loading

wing328 commented Dec 26, 2023

Aliaksie commented Dec 27, 2023

ChristianCiach commented Jan 8, 2024 •

edited

Loading

		@@ -1420,136 +1420,11 @@ public void setRequestMappingMode(RequestMappingMode requestMappingMode) {
		this.requestMappingMode = requestMappingMode;
		}

[4947][java]: adds support for validation of primitives in arrays #17165

[4947][java]: adds support for validation of primitives in arrays #17165

Conversation

Aliaksie commented Nov 22, 2023 • edited Loading

PR checklist

martin-mfg left a comment

Choose a reason for hiding this comment

martin-mfg commented Dec 3, 2023

Aliaksie commented Dec 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

welshm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wing328 commented Dec 11, 2023

wing328 commented Dec 26, 2023 • edited Loading

Aliaksie commented Dec 26, 2023 • edited Loading

wing328 commented Dec 26, 2023

Aliaksie commented Dec 27, 2023

ChristianCiach commented Jan 8, 2024 • edited Loading

Aliaksie commented Nov 22, 2023 •

edited

Loading

wing328 commented Dec 26, 2023 •

edited

Loading

Aliaksie commented Dec 26, 2023 •

edited

Loading

ChristianCiach commented Jan 8, 2024 •

edited

Loading