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

Adding NotNull annotation for required fields #810

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

michalkolenda
Copy link
Contributor

Following changes concern generating POJOs using client with use-bean-validation property set to true.

There is a functionality of annotating required fields with Jakarta @NotNull. According to my tests, it does not work in case when the field has no other constraints (like minLength, maxLength, etc.). I expect that required fields are annotated with NotNull no matter it has any other constraints or not.

I would suggest a little modification in pojo.qute in order to annotate every required field.

@michalkolenda michalkolenda requested a review from a team as a code owner September 26, 2024 18:49
Field size = ValidatedObject.class.getDeclaredField("size");

assertThat(Arrays.stream(ValidatedObject.class.getFields())
assertThat(Stream.of(id, name, secondName, size)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be sure, have you changed this only for performance reasons?

Copy link
Contributor Author

@michalkolenda michalkolenda Sep 28, 2024

Choose a reason for hiding this comment

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

Actually not.

getFields method returns only public fields and the generated class consists only of private ones. It makes ValidatedObject.class.getFields() always returning an empty array.

By this reason the following assertion has no chance to fail.

assertThat(Arrays.stream(UnvalidatedObject.class.getFields())
    .allMatch(f -> f.isAnnotationPresent(NotNull.class)))

getDeclaredFields() method would be more appropriate (instead of getFields), but I decided to utilize the objects returned by getDeclaredField calls a few lines above.

I've just added a similar change also in testValidationAnnotationsAreSkippedModel test method.

getFields method returns only `public` fields and the generated class consists only of private ones.

It makes ValidatedObject.class.getFields() always returning an empty array.

Following assertion has no chance to fail
assertThat(Arrays.stream(UnvalidatedObject.class.getFields())
                .noneMatch(f -> f.isAnnotationPresent(NotNull.class))).isTrue();
@ricardozanini
Copy link
Member

@allcontributors add @michalkolenda for code

Copy link
Contributor

@ricardozanini

I've put up a pull request to add @michalkolenda! 🎉

@ricardozanini ricardozanini merged commit 9521d8d into quarkiverse:main Sep 30, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants