@InjectSpy and @InjectMock cause @Valid behaviour change

[tioback]

Describe the bug

When using @io.quarkus.test.junit.mockito.InjectMock or @io.quarkus.test.junit.mockito.InjectSpy for a bean that has methods using @javax.validation.Valid, validations are being skipped.

Expected behavior

If @InjectMock along with .callRealMethod(), or if @InjectSpy are used on a method that has @Valid, I expect the system to validate the bean accordingly.

Actual behavior

The system is running as if no validations were set up, leading to errors and failing tests.

To Reproduce

I built a project to reproduce the problem, so just mvn test it.

Configuration

It's all in the project.

Environment (please complete the following information):

Output of uname -a or ver

Darwin Renatos-MacBook-Pro-2.local 19.6.0 Darwin Kernel Version 19.6.0: Thu May  6 00:48:39 PDT 2021; root:xnu-6153.141.33~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.7" 2020-04-14
OpenJDK Runtime Environment (build 11.0.7+10)
OpenJDK 64-Bit Server VM (build 11.0.7+10, mixed mode)

Quarkus version or git rev

2.0.0.Final
1.13.7.Final

Build tool (ie. output of mvnw --version or gradlew --version)

--2021-07-04 21:27:08--  https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar
Resolving repo.maven.apache.org... 199.232.192.215, 199.232.196.215
Connecting to repo.maven.apache.org|199.232.192.215|:443... connected.
OpenSSL: error:1407742E:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert protocol version
Unable to establish SSL connection.
Error: Could not find or load main class org.apache.maven.wrapper.MavenWrapperMain
Caused by: java.lang.ClassNotFoundException: org.apache.maven.wrapper.MavenWrapperMain

Additional context

(Add any other context about the problem here.)

I'm running this through IntelliJ IDEA 2021.1.3.

Despite the sample project using 2.0.0.Final, the same problem happens on 1.13.7.Final.

From what I could tell, when either InjectMock or InjectSpy are used, the generated subclasses are called in an order that affects the Validation behaviour.

More specifically, at validateParameters(T, java.lang.reflect.Executable, java.lang.Object[], java.lang.Class<?>...):

// Dependency: hibernate-validator-6.2.0.Final.jar
package org.hibernate.validator.internal.engine;

public class ValidatorImpl {

    private <T> Set<ConstraintViolation<T>> validateParameters(T object, Executable executable, Object[] parameterValues, Class<?>... groups) {
        sanityCheckGroups(groups);

        @SuppressWarnings("unchecked")
        Class<T> rootBeanClass = object != null ? (Class<T>) object.getClass() : (Class<T>) executable.getDeclaringClass();
        BeanMetaData<T> rootBeanMetaData = beanMetaDataManager.getBeanMetaData(rootBeanClass);

        if (!rootBeanMetaData.hasConstraints()) {
            return Collections.emptySet();
        }

        ExecutableValidationContext<T> validationContext = getValidationContextBuilder().forValidateParameters(
                rootBeanClass,
                rootBeanMetaData,
                object,
                executable,
                parameterValues
        );

        ValidationOrder validationOrder = determineGroupValidationOrder(groups);

        validateParametersInContext(validationContext, parameterValues, validationOrder);

        return validationContext.getFailingConstraints();
    }
}

At the rootBeanMetaData.hasConstraints() check, when using Spies or Mocks, the variable is false.

After some debugging, I noticed that this happens because Spies and Mocks cause the use of org.hibernate.validator.internal.metadata.PredefinedScopeBeanMetaDataManager.UninitializedBeanMetaData.hasConstraints as the BeanMetadData implementation.
Which in turn led me to this:

// Dependency: hibernate-validator-6.2.0.Final.jar
package org.hibernate.validator.internal.metadata;

public class PredefinedScopeBeanMetaDataManager implements BeanMetaDataManager {
    
    @Override
    public <T> BeanMetaData<T> getBeanMetaData(Class<T> beanClass) {
        Class<?> normalizedBeanClass = beanMetaDataClassNormalizer.normalize( beanClass );
        BeanMetaData<T> beanMetaData = (BeanMetaData<T>) beanMetaDataMap.get( normalizedBeanClass );
        if ( beanMetaData == null ) {
            // note that if at least one element of the hierarchy is constrained, the child classes should really be initialized
            // otherwise they will be considered unconstrained.
            beanMetaData = (BeanMetaData<T>) beanMetaDataMap.computeIfAbsent( normalizedBeanClass, UninitializedBeanMetaData::new );
        }
        return beanMetaData;
    }
}

That comment about child classes seems to be the reason this is happening.

Before I start working on a solution, I was wondering if someone has a word on that.

[tioback]

A possible solution would be changing io.quarkus.hibernate.validator.runtime.ArcProxyBeanMetaDataClassNormalizer#normalize in quarkus-hibernate-validator to the following:

public class ArcProxyBeanMetaDataClassNormalizer implements BeanMetaDataClassNormalizer {

    @Override
    public <T> Class<? super T> normalize(Class<T> beanClass) {
        if (!Subclass.class.isAssignableFrom(beanClass)) {
            return beanClass;
        }
        return normalize(beanClass.getSuperclass());
    }

}

This solves the issue for Spies, but Mocks still behave weirdly.

I wonder if that's something we should worry about, though. As per Mockito's documentation, Mocks aren't built for partially mocking an object, specially if the target method throws exceptions, or depends on object state. I believe the highlighted part is what might allow us to leave it as is for Mocks.

[quarkus-bot]

/cc @geoand, @gsmet, @yrodiere