spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.62k stars 38.13k forks source link

@MockBean does not work with request-scoped Supplier<T> without explicit name #30043

Open mwisnicki opened 1 year ago

mwisnicki commented 1 year ago

Trying to define mock for request-scoped supplier does not work unless I explicitly name the mock.

The problem with hardcoding bean name is that name can be dependent on configuration (for example via use conditions).

If there is no RequestScope or I use custom interface there is no such problem.

Spring Boot 2.7.8 + JDK17

Simplified program:

package com.example.testspringoverridebeantest;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Primary;
import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Service;
import org.springframework.web.context.annotation.RequestScope;

import java.util.function.Supplier;

@SpringBootApplication
public class TestSpringOverrideBeanTestApplication {

    @Bean
    @RequestScope
    Supplier<String> word() {
        return () -> "app";
    }

    @Bean
    @RequestScope
    @Profile("test") // just an example condition
    @Primary
    Supplier<String> testWord() {
        return () -> "testapp";
    }

    public static void main(String[] args) {
        SpringApplication.run(TestSpringOverrideBeanTestApplication.class, args);
    }

}

@Service
record Greeter(Supplier<String> word) {
    String hello() {
        return "hello, " + word.get();
    }
}
package com.example.testspringoverridebeantest;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;

import java.util.function.Supplier;

@SpringBootTest
class TestSpringOverrideBeanTestApplicationTests {

    @Autowired
    Greeter greeter;

    @MockBean// (name = "testWord") // unbreaks but the name can vary!
    Supplier<String> word;

    @Test
    void canOverrideBeanForTest() {
        Mockito.when(word.get()).thenReturn("test");
        Assertions.assertEquals("hello, test", greeter.hello());
    }

}
wilkinsona commented 1 year ago

Thanks for the reproducer.

When the bean's type is Supplier<String>, MockitoPostProcessor asks the bean factory for the names of all beans of type Supplier<String>. The result is a single name: scopedTarget.word. This is then filtered out due to the fix for https://github.com/spring-projects/spring-boot/issues/5724. If I update the reproducer to introduce a custom WordSupplier interface and replace Supplier<String> with WordSupplier, when asked for the names of all beans of type WordSupplier, the bean factory responds with both scopedTarget.word and word. After filtering, we're left with word and the mocking works as expected. I need to dig a bit more, but this looks like a Framework limitation or bug.

wilkinsona commented 1 year ago

word is a org.springframework.aop.scope.ScopedProxyFactoryBean. When the type is Supplier<String>, AbstractBeanFactory.isTypeMatch("word", java.util.function.Supplier<java.lang.String>, false) is called. It ends up checking if Supplier<String> is assignable from Supplier. It isn't so the word bean is skipped. If there are no generics in the type signature of the request-scoped bean, this type match succeeds. We'll need to get the Framework team to investigate.

wilkinsona commented 1 year ago

More minimal test that shows the difference in Framework's behavior:

package com.example;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.function.Supplier;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.ResolvableType;
import org.springframework.web.context.annotation.RequestScope;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;

class RequestScopedBeansOfTypeTests {

    @Test
    void requestScopedGenericSupplier() {
        ResolvableType type = ResolvableType.forClassWithGenerics(Supplier.class, String.class);
        assertBeansAreFound(GenericSupplierConfiguration.class, type);
    }

    @Test
    void requestScopedCustomSupplier() {
        ResolvableType type = ResolvableType.forClass(CustomSupplier.class);
        assertBeansAreFound(CustomSupplierConfiguration.class, type);
    }

    void assertBeansAreFound(Class<?> config, ResolvableType type) {
        try (AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext()) {
            context.register(config);
            context.refresh();
            ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
            String[] names = beanFactory.getBeanNamesForType(type, true, false);
            assertThat(names).containsExactlyInAnyOrder("scopedTarget.requestScopedBean", "requestScopedBean", "bean");
        }
    }

    @Configuration(proxyBeanMethods = false)
    static class GenericSupplierConfiguration {

        @Bean
        @RequestScope
        Supplier<String> requestScopedBean() {
            return () -> "value";
        }

        @Bean
        Supplier<String> bean() {
            return () -> "value";
        }

    }

    @Configuration(proxyBeanMethods = false)
    static class CustomSupplierConfiguration {

        @Bean
        @RequestScope
        CustomSupplier requestScopedBean() {
            return () -> "value";
        }

        @Bean
        CustomSupplier bean() {
            return () -> "value";
        }

    }

    static interface CustomSupplier extends Supplier<String> {

    }

}
snicoll commented 10 months ago

There's indeed a problematic shortcut with this case as the factory bean creates a proxy for the scope that does not carry the full generic information that is required for the algorithm to match. Looking at the underlying bean definition that's created, I can see that the the resolvedTargetType is the factory bean class, but I wonder if it shouldn't be the beanClass instead, with the target type being the return type of the method with its full generic information.

Even if we did that, we still need to modify the algorithm, perhaps checking higher in the stack if the type to match has a generic.

Thoughts @jhoeller?