microtweak / conditional-validator

An extension package for Bean Validation 2.0 that adds conditional validations
MIT License
6 stars 1 forks source link

@NullWhen throwing Unexpected exception during isValid call #6

Closed paulo-gurjao closed 1 year ago

paulo-gurjao commented 1 year ago

Hello,

I'm trying to use the @NullWhen annotation but I'm getting an jakarta.validation.ValidationException: HV000028: Unexpected exception during isValid call.

I'm using conditional-validator-hv:2.0.0-rc on Quarkus 3.2.0.Final, Hibernate Validator 8.0.1.Final and Jakarta Validation 3.0.2 on Java 17.

It appears that this annotation is having trouble calling the @Null initialize method. Or it could be another problem, I don't know for sure. Could you investigate this for me?

At my DTO class I'm using the @NullWhen annotation as follows:

@ConditionalValidate
public class UpdateDTO {

    private boolean onlySomethingAllowed;

        @NullWhen(expression = "onlySomethingAllowed", message = "someField must be null when other field is present")
        private String someField;

        // Setter of someField that sets onlySomethingAllowed to true given specific value
}

Attached is the stack trace of the exception.

Regards, Paulo

StackTrace.txt

paulo-gurjao commented 1 year ago

I forgot to mention: on this same class I'm using other annotations (ex: @NotNullWhen ) and they work fine. It's only the @NullWhen annotation that throws this exception.

salvadormarcos commented 1 year ago

Hello,

Have you tried using Jarkarta Validation's @Null constraint directly? Does it work?

Regarding Quarkus: does this problem occur with the native image? Or does it also happen when running on top of the JVM?

paulo-gurjao commented 1 year ago

Hi, the @Null annotation works as expected.

I'm using Quarkus on top of the JVM.

paulo-gurjao commented 1 year ago

I've made a small prototype Quarkus project demonstrating the bug. You can clone it at https://github.com/pgurjao/conditional-validation if you want.

salvadormarcos commented 1 year ago

Hello,

Your project was of great help, thank you!

Apparently, Reflections found a ConstraintValidator declared as an anonymous class within ConstraintValidatorManagerImpl. This ContraintValidator is unique to @Null, so it hasn't been detected before.

I'll try to adjust Reflection's scanning and as soon as I have a solution, I'll let you know

paulo-gurjao commented 1 year ago

Great, thanks!

salvadormarcos commented 1 year ago

Hello,

I made the fix to ignore anonymous classes. I tested within your prototype project and everything worked as it should.

Could you test locally in your real project?

paulo-gurjao commented 1 year ago

Sure! Is there a new version? Or the fix was implemented on version 2.0.0-rc?

salvadormarcos commented 1 year ago

I haven't published the new version on Maven Central yet, but it will be 2.0.0-rc2. It would be interesting to test in your environment before releasing a new release.

You can clone this project and run a mvn clean install locally.

salvadormarcos commented 1 year ago

Hi @paulo-gurjao,

Did the fix work in your environment? Can I do a release in Maven Central?

paulo-gurjao commented 1 year ago

Hi @salvadormarcos , sorry for taking too long to answer. In order to test your fix should I include the generated jar file in my build.gradle file replacing the current implementation 'com.github.microtweak:conditional-validator-hv:2.0.0-rc' , is that correct? And the jar file in question is the conditional-validator-microtweak\validator-hv\target\conditional-validator-hv-2.0.0-rc.jar ?

salvadormarcos commented 1 year ago

Yes, it is correct

paulo-gurjao commented 1 year ago

I'm getting the same error but I'm not sure that's because some caching from gradle or if I made some mistake on build.gradle file.

salvadormarcos commented 1 year ago

I have limited experience with Gradle. When I tested, I needed to comment out the mavenCentral() line in build.gradle.

paulo-gurjao commented 1 year ago

Commenting mavenCentral() broke the import of all my dependencies. I'm looking on Gradle Documentation how to properly import a .jar file on build.gradle

paulo-gurjao commented 1 year ago

I've managed to import the .jar file by adding it to build.gradle but when I start Quarkus I'm getting the attached exception.

At first I imagined that was some problem because of the .jar import, but then I tried a different approach by installing an artifact to a specific local repository path on my maven local cache, but neither resolved the issue.

How did you manage to test your fix locally on the prototype I provided? Even there I couldn't make it work...

ClassNotFoundException.txt

salvadormarcos commented 1 year ago

Sorry, I gave wrong information earlier. What I did to make it work was to reverse the order of the mavenCentral() and mavenLocal() repositories.

I believe your build.gradle looks like this:

repositories {
    mavenCentral()
    mavenLocal()
}

You should change it to look like this:

repositories {
    mavenLocal()
    mavenCentral()
}

I believe Gradle prioritizes repositories as they are declared. The first has priority over the second, the second over the third, and so on.

paulo-gurjao commented 1 year ago

Well, I tried switching mavenCentral() and mavenLocal() order but the same exception occurred. I'm fairly confident that what's happening here is some problem with my local environment while importing the patched conditional-validator-hv package (although the package appears on my "external dependencies" folder on Eclipse), and say that for 2 reasons: 1- your patch was pretty small 2- you were able to successfully run the patched package on the prototype I provided

So I think it's reasonable safe to publish the patch to Maven Central and when I import it from there this problem probably will be resolved.

What do you think?

salvadormarcos commented 1 year ago

Of course, we can try...

Tonight I publish on Maven Central. I have the deployment environment configured only on my PC

salvadormarcos commented 1 year ago

Version released! The version number is 2.0.0-rc2

paulo-gurjao commented 1 year ago

I just tested version 2.0.0-rc2 on the prototype I provided above and It worked! And now that it worked I could see that there was a bug on the unit test on the prototype. I fixed the test and it is passing now. Tomorrow I'll test on my environment and let you know.

paulo-gurjao commented 1 year ago

Hi, @salvadormarcos , I just tested this new version on my environment and it worked as well :partying_face: I think we can consider this issue resolved. Many thanks for fixing this and for providing this great lib!

salvadormarcos commented 1 year ago

Hi @paulo-gurjao, great news!!

Thanks for your feedback and sorry for the setback with the local deployment