Feat/jooby hibernate validator by kliushnichenko · Pull Request #3508 · jooby-project/jooby

kliushnichenko · 2024-08-24T12:54:48Z

Still need some docs, tests, cleanup, but the basic logic seems done

# Conflicts: # modules/jooby-hbv/pom.xml # modules/jooby-hbv/src/main/java/io/jooby/hbv/HbvModule.java

agentgt · 2024-08-24T14:31:14Z

I'm not sure I like the name of jooby-validation if it is going to have a hard dependency on jakarta.validation. Is the plan to make the validation pluggable in that module and it is just jakarta for now?

Otherwise I think it should be jooby-jakarta-validation.

Also the jooby-apt module should not have any dependency on basically anything ideally. I haven't code dived to see why you made it that way. (you don't need the actual classes you depend on with generated code)

EDIT actually maybe you changed it. When you get a chance @kliushnichenko maybe squash the commits to one or two?

modules/jooby-apt/pom.xml

modules/jooby-validation/pom.xml

cleanup cleanup cleanup

SentryMan · 2024-08-24T18:48:21Z

The way I've done it with other libs I've worked was to have an extensible Validator interface which clients can extend to provide custom validation. Example hibernate impl and avaje validation impl . Perhaps you guys could try something similar if you want to eliminate the dependencies

jknack · 2024-08-24T20:24:40Z

@SentryMan thank you. I feel here isn't necessary to add another abstraction over the jakarta one. Leaving jakarta-validator outside of core is enough.

kliushnichenko · 2024-08-26T19:05:14Z

Updated. Pls verify the changes and I'll proceed with the documentation

kliushnichenko · 2024-08-26T20:18:32Z

@SentryMan could you pls shed some light on avaje-validator. I thought it is fully compliant with jakarta-validation-api. But I can't find how I can get the instance of jakarta.validation.Validator baked by avaje-validator. All the entry points are available only at your custom abstraction io.avaje.validation.Validator.

Is it a trade-off of a reflection-free solution or maybe you can provide us jakarta-compliant entry point in the future release?

SentryMan · 2024-08-26T21:54:25Z

Is it a trade-off of a reflection-free solution

Jakarta validation is mainly reflection-based so we couldn't fully implement its interfaces. Hence why we took a page from spring in our http solution by defining another abstraction so we could support both jakarta and avaje validation

jknack

looking good

modules/jooby-hbv/src/main/java/io/jooby/hbv/HbvModule.java

modules/jooby-hbv/src/main/java/module-info.java

...s/jooby-jakarta-validation/src/main/java/io/jooby/validation/ConstraintViolationHandler.java

jknack · 2024-08-28T21:28:30Z

...-jakarta-validation/src/test/java/io/jooby/validation/app/PasswordsShouldMatchValidator.java

+public class PasswordsShouldMatchValidator implements ConstraintValidator<PasswordsShouldMatch, NewAccountRequest> {
+
+    @Override
+    public boolean isValid(NewAccountRequest request, ConstraintValidatorContext constraintContext) {


Is there a way to access to jooby registry from ConstraintValidatorContext?????

I don't see how we can put it into ConstraintValidatorContext, but there are other options for accessing the registry. It is possible by implementing a custom ConstraintValidatorFactory .

I don't think we need to support it out of the box bc it most likely will require introducing another custom abstraction.
I can add an example in the docs of how to do this. Maybe it is good enough for now?

Additionally, if the project utilizes some DI it is not a problem to inject any service/repo/etc. into custom validator

Additionally, if the project utilizes some DI it is not a problem to inject any service/repo/etc. into custom validator

An example of that on docs will be nice.

kliushnichenko · 2024-08-30T18:29:28Z

Getting back to avaje-validator support...

As SentryMan clarified, there is no way to smoothly support both (hbv/avaje) relying solely on jakarta.validation.Validator
So, my initial plan didn't quite work out 😄

Options that are running through my mind in this regard:

Write a custom implementation of the jakarta.validation.Validator that will trigger io.avaje.validation.Validator under the hood
Concerns:
way too odd. Everything except one method validate() will be stubbed as Unsupported.
Also, it will hide the rest of avaje-validator options, not ok.
Each module can implement its own helper for validation. In jooby-apt pick the one that is available in classpath.
Concerns:
Complicates jooby-apt, probably duplicates some logic in two places, error-prone, complicates further support.
Introduce one more abstraction, as SentryMan pointed out, to cover jooby-apt logic over a single interface.
Concerns:
plus one more abstraction, a bit more cumbersome, but, seems the most acceptable compromise

@jknack what's your take on it? maybe some other options?

jknack · 2024-08-30T19:04:49Z

Seems 3 it is OK. jooby-apt should call io.jooby.validation.BeanValidator and extra logic goes here (jooby-jakarta-validation) and probably on the jooby-avaje-validator module.

So jooby-apt will do what is doing here, wrap the call around BeanValidator

kliushnichenko · 2024-09-01T19:36:51Z

Updated.

Please note the following:

since avaje-validator has its own set of custom entities (Validator, ConstraintViolation, ConstraintViolationException), the BeanValidator helper no longer relies on either jakarta or avaje entities, making the module even simpler.
for the same reason, error handlers should be implemented separately and placed in their corresponding modules.
jooby-jakarta-validation has been renamed back to jooby-validation since it no longer has any dependency on jakarta.
a new MvcValidator interface has been introduced to isolate BeanValidator logic from specific implementations. It includes a single validate() method that performs validation and throws an exception. The type of exception thrown depends on the implementation (Jakarta or Avaje).

Naming is getting tough with a lot of Validator abstractions, any suggestions are welcomed.
@jknack pls confirm we are good with this solution and can move on

jknack

few minor comments

jknack · 2024-09-01T22:20:20Z

modules/jooby-apt/src/main/java/io/jooby/apt/JoobyProcessor.java

+                "Unable to load 'BeanValidator' class. " +
+                "Bean validation usage (@Valid) was detected, but the appropriate dependency is missing. " +
+                "Please ensure that you have added the corresponding validation dependency " +
+                "(e.g., jooby-hbv).");


typo: jooby-hibernate-validator

jknack · 2024-09-01T22:41:09Z

modules/jooby-validation/src/main/java/io/jooby/validation/MvcValidator.java

@@ -0,0 +1,6 @@
+package io.jooby.validation;
+
+public interface MvcValidator<E extends Throwable> {


probably make E RuntimeException

jknack · 2024-09-01T22:41:50Z

modules/jooby-validation/src/main/java/io/jooby/validation/MvcValidator.java

+
+public interface MvcValidator<E extends Throwable> {
+
+    void validate(Object bean) throws E;


well not sure if we need a generic for exception, RuntimeExceptoion or Exception will be OK

kliushnichenko · 2024-09-05T12:47:17Z

minor improvements, javadoc, asciidoc has been added. Ready to final review/merge.
avaje-validator-module I'll send in a separate PR

jknack

@kliushnichenko love what you did! Let me know if we can merge.

kliushnichenko · 2024-09-05T13:15:22Z

Yes, we can. Just a minor thing, master and my branch are under 3.2.10-SNAPSHOT, but I see 3.3.0 has already been released, is it expected, or we need to update both to 3.3.1-SNAPSHOT?

jknack · 2024-09-05T13:53:02Z

oh yea, let me push the version bump

jknack · 2024-09-05T13:54:15Z

@kliushnichenko done

kliushnichenko · 2024-09-05T15:28:27Z

@jknack updated, feel free to merge

kliushnichenko added 7 commits August 6, 2024 22:32

jooby-hbv module draft

daca727

jooby-hbv module draft

1bf0e00

rollback core changes

887dc9d

cleanup hbv module

8d71495

basic implementation for MVC

06eb67e

rebase

7adc2a2

Merge remote-tracking branch 'origin/feat/jooby-hbv' into feat/jooby-hbv

951a459

# Conflicts: # modules/jooby-hbv/pom.xml # modules/jooby-hbv/src/main/java/io/jooby/hbv/HbvModule.java

kliushnichenko marked this pull request as draft August 24, 2024 12:55

kliushnichenko mentioned this pull request Aug 24, 2024

WIP: jooby-hbv module #3494

Closed

jknack reviewed Aug 24, 2024

View reviewed changes

modules/jooby-apt/pom.xml Show resolved Hide resolved

modules/jooby-validation/pom.xml Outdated Show resolved Hide resolved

rollback core changes

646f355

cleanup cleanup cleanup

kliushnichenko force-pushed the feat/jooby-hibernate-validator branch from 5f1dd9b to 646f355 Compare August 24, 2024 17:14

error handler, tests, refactoring

229b584

jknack reviewed Aug 28, 2024

View reviewed changes

kliushnichenko added 2 commits August 31, 2024 18:11

resolve discussions

a470d9b

mvc validation revisited

baf422e

jknack approved these changes Sep 1, 2024

View reviewed changes

kliushnichenko added 2 commits September 2, 2024 21:03

javadoc, support configuration over hocon config

c23d4b0

hibernate-validator module ascii doc

8b79308

kliushnichenko marked this pull request as ready for review September 5, 2024 12:47

jknack approved these changes Sep 5, 2024

View reviewed changes

kliushnichenko added 2 commits September 5, 2024 18:16

Merge branch '3.x' into feat/jooby-hibernate-validator

84c8f93

update version to 3.3.1-SNAPSHOT

4d73e31

jknack merged commit 22a0f5a into jooby-project:3.x Sep 5, 2024

jknack added the validation label Sep 7, 2024

jknack added this to the 3.3.1 milestone Sep 7, 2024

		@@ -0,0 +1,6 @@
		package io.jooby.validation;

		public interface MvcValidator<E extends Throwable> {


		public interface MvcValidator<E extends Throwable> {

		void validate(Object bean) throws E;

Uh oh!

Comments

Conversation

kliushnichenko commented Aug 24, 2024

Uh oh!

agentgt commented Aug 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SentryMan commented Aug 24, 2024

Uh oh!

jknack commented Aug 24, 2024

Uh oh!

kliushnichenko commented Aug 26, 2024

Uh oh!

kliushnichenko commented Aug 26, 2024

Uh oh!

SentryMan commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jknack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jknack Aug 28, 2024

Choose a reason for hiding this comment

Uh oh!

kliushnichenko Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

jknack Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

kliushnichenko commented Aug 30, 2024

Uh oh!

jknack commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kliushnichenko commented Sep 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jknack left a comment

Choose a reason for hiding this comment

Uh oh!

jknack Sep 1, 2024

Choose a reason for hiding this comment

Uh oh!

jknack Sep 1, 2024

Choose a reason for hiding this comment

Uh oh!

jknack Sep 1, 2024

Choose a reason for hiding this comment

Uh oh!

kliushnichenko commented Sep 5, 2024

Uh oh!

jknack left a comment

Choose a reason for hiding this comment

Uh oh!

kliushnichenko commented Sep 5, 2024

Uh oh!

jknack commented Sep 5, 2024

Uh oh!

jknack commented Sep 5, 2024

Uh oh!

kliushnichenko commented Sep 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

agentgt commented Aug 24, 2024 •

edited

Loading

SentryMan commented Aug 26, 2024 •

edited

Loading

jknack commented Aug 30, 2024 •

edited

Loading

kliushnichenko commented Sep 1, 2024 •

edited

Loading