Spring Revise AdvisedSupport.MethodCacheKey.equals() implementation

spring-aop AdvisedSupport.MethodCacheKey.equals() use hashCode

Comment From: pivotal-cla

@3163116959 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

Comment From: pivotal-cla

@3163116959 Thank you for signing the Contributor License Agreement!

Comment From: 3163116959

getLocalDateQuarter method has two method references, and both references point to the same interceptor, and each method has similar circumstances, as long as it is being proxied

Comment From: sbrannen

Hi @3163116959,

Congratulations on submitting your first pull request ever! 👍

Regarding the implementation of AdvisedSupport.MethodCacheKey.equals(), it's based on the method by design.

If you believe you have discovered a bug, please provide a sample application that demonstrates the issue (preferably something that we can download and run, such as a Git repository or a ZIP file attached to this issue).

Comment From: 3163116959

Hi @3163116959,

Congratulations on submitting your first pull request ever! 👍

Regarding the implementation of AdvisedSupport.MethodCacheKey.equals(), it's based on the method by design.

If you believe you have discovered a bug, please provide a sample application that demonstrates the issue (preferably something that we can download and run, such as a Git repository or a ZIP file attached to this issue).

thanks! Actually, my project is a blank web project. I've included web-starter in the pom.xml. Then I created a new controller with a method annotated with @GetMapping, without any other additional operations.

Comment From: 3163116959

Hi @3163116959,

Congratulations on submitting your first pull request ever! 👍

Regarding the implementation of AdvisedSupport.MethodCacheKey.equals(), it's based on the method by design.

If you believe you have discovered a bug, please provide a sample application that demonstrates the issue (preferably something that we can download and run, such as a Git repository or a ZIP file attached to this issue).

During my debugging process, I noticed that methdCache inserts asynchronously during startup. After that, when the method is accessed for the first time, due to the equals method comparing method==, it leads to the same method having two instances in the cache.

Comment From: sbrannen

Hi @3163116959,

Thanks for the feedback.

Actually, my project is a blank web project. I've included web-starter in the pom.xml. Then I created a new controller with a method annotated with @GetMapping, without any other additional operations.

Based on the screenshot you provided, it appears that more than just that is going on.

For example, it appears that you've made your controller @Transactional and have added an @Async controller method as well.

During my debugging process, I noticed that methodCache inserts asynchronously during startup. After that, when the method is accessed for the first time, due to the equals method comparing method==, it leads to the same method having two instances in the cache.

I have not been able to reproduce that.

Based on your above feedback and screenshot, I attempted to recreate the scenario with the following standalone test case using the core Spring Framework (main branch).

@SpringJUnitWebConfig
class AdvisedSupportTests {

    MockMvc mockMvc;

    @BeforeEach
    void setUpMockMvc(WebApplicationContext wac) {
        this.mockMvc = webAppContextSetup(wac).build();
    }

    @Test
    void test() throws Exception {
        this.mockMvc.perform(get("/foo")).andExpect(content().string("bar"));

        MvcResult mvcResult = this.mockMvc.perform(get("/asyncFoo"))
                .andExpect(status().isOk())
                .andExpect(request().asyncStarted())
                .andExpect(request().asyncResult("async bar"))
                .andReturn();

        this.mockMvc.perform(asyncDispatch(mvcResult))
                .andExpect(status().isOk())
                .andExpect(content().string("async bar"));
    }

    @Configuration
    @EnableWebMvc
    @EnableAsync
    @EnableTransactionManagement
    @Import(MyController.class)
    static class Config {

        @Bean
        PlatformTransactionManager transactionManager(DataSource dataSource) {
            return new DataSourceTransactionManager(dataSource);
        }

        @Bean
        DataSource dataSource() {
            return new EmbeddedDatabaseBuilder().generateUniqueName(true).build();
        }
    }

    @Transactional
    @RestController
    static class MyController {

        @GetMapping("/foo")
        String foo() {
            return "bar";
        }

        @Async
        @GetMapping("/asyncFoo")
        CompletableFuture<String> asyncFoo() {
            return CompletableFuture.completedFuture("async bar");
        }
    }

}

I also debugged AdvisedSupport, and the following shows the contents of the methodCache field at various stages.

AdvisedSupport: {java.lang.String example.AdvisedSupportTests$MyController.foo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046]}

AdvisedSupport: {java.lang.String example.AdvisedSupportTests$MyController.foo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046], java.util.concurrent.CompletableFuture example.AdvisedSupportTests$MyController.asyncFoo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046]}

AdvisedSupport: {public java.lang.String java.lang.Object.toString()=[], java.lang.String example.AdvisedSupportTests$MyController.foo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046], java.util.concurrent.CompletableFuture example.AdvisedSupportTests$MyController.asyncFoo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046]}

AdvisedSupport: {public java.lang.String java.lang.Object.toString()=[], java.lang.String example.AdvisedSupportTests$MyController.foo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046], protected native java.lang.Object java.lang.Object.clone() throws java.lang.CloneNotSupportedException=[], java.util.concurrent.CompletableFuture example.AdvisedSupportTests$MyController.asyncFoo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046]}

ProxyFactory: {java.lang.String example.AdvisedSupportTests$MyController.foo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046]}

ProxyFactory: {java.lang.String example.AdvisedSupportTests$MyController.foo()=[org.springframework.transaction.interceptor.TransactionInterceptor@379ce046], java.util.concurrent.CompletableFuture example.AdvisedSupportTests$MyController.asyncFoo()=[org.springframework.scheduling.annotation.AnnotationAsyncExecutionInterceptor@4148ce43, org.springframework.transaction.interceptor.TransactionInterceptor@379ce046]}

And I don't see any duplicate cache entries for the MyController.foo() method.

As I stated earlier:

If you believe you have discovered a bug, please provide a sample application that demonstrates the issue (preferably something that we can download and run, such as a Git repository or a ZIP file attached to this issue).

I'm afraid I cannot spend any more time on this issue without a sample application that reproduces the behavior you have described.

In light of that, I am closing this PR.

However, if you are able provide a sample application that reproduces that behavior, we would be willing to look into this again.

Regards,

Sam