New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-214: Retryable on JPA Repos with proxyTargetClass [DO NOT MERGE] #246
Conversation
Resolves spring-projects#214 When `proxyTargetClass` is `true` (Boot's default), the JPA interfaces are not copied to the outer retry proxy created by auto-proxy. Add a BPP to fix up the proxy, when needed. Also, the pointcut filter must match on the `SimpleJpaRepository`.
cc/ @mp911de |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if we want to dig into a discussion to make this work for all Spring Data repository implementations. Would be probably easier than keeping code for all kinds of Spring Data modules around.
@@ -596,6 +596,13 @@ line to your `build.gradle` file: | |||
runtime('org.aspectj:aspectjweaver:1.8.13') | |||
``` | |||
|
|||
### Using Declarative Retry on Spring JPA Repository Interfaces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest to generally allow Spring Data repositories.
@@ -276,6 +276,12 @@ | |||
<version>1.2.17</version> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.springframework.data</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest using Spring Data Commons to enable retry functionality for all Spring Data modules.
|
||
@Override | ||
public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { | ||
if (bean instanceof Retryable && bean instanceof Advised) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could generally check if the bean implements org.springframework.data.repository.Repository
(as marker for Spring Data repositories) to make this work for all Spring Data modules. Not sure why JpaRepositoryImplementation` needs to be excluded here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Mark; without removing that interface, I get
Caused by: java.lang.IllegalStateException: No MethodInvocation found: Check that an AOP invocation is in progress, and that the CrudMethodMetadataPopulatingMethodInterceptor is upfront in the interceptor chain.
at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.currentInvocation(CrudMethodMetadataPostProcessor.java:129) ~[spring-data-jpa-2.5.1.jar:2.5.1]
at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$ThreadBoundTargetSource.getTarget(CrudMethodMetadataPostProcessor.java:322) ~[spring-data-jpa-2.5.1.jar:2.5.1]
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:195) ~[spring-aop-5.3.7.jar:5.3.7]
I am going to close this PR for now, and open an issue against spring-aop, to see if we can get the root cause fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spring-projects/spring-framework#27044
Let's see what the framework guys say before doing any more.
Resolves #214
When
proxyTargetClass
istrue
(Boot's default), the JPA interfacesare not copied to the outer retry proxy created by auto-proxy.
Add a BPP to fix up the proxy, when needed.
Also, the pointcut filter must match on the
SimpleJpaRepository
.