From b599c234043c90f0b40885f6e7aaf26bc8d5c4d8 Mon Sep 17 00:00:00 2001 From: Marius Grama Date: Sat, 13 Apr 2019 15:09:44 +0200 Subject: [PATCH] Expose the MethodInvocation in the MethodInvocationRetryCallback in order to permit to concrete RetryListener implementations to inspect in detail the method called as well as its arguments. This information can be used in the retry listener for either detailed logging or monitoring for the proxied method invocations. --- README.md | 29 ++++ pom.xml | 6 + .../MethodInvocationRetryCallback.java | 53 +++++++ .../RetryOperationsInterceptor.java | 3 +- .../StatefulRetryOperationsInterceptor.java | 21 +-- .../MethodInvocationRetryListenerSupport.java | 77 ++++++++++ .../RetryOperationsInterceptorTests.java | 62 ++++++-- ...odInvocationRetryListenerSupportTests.java | 132 ++++++++++++++++++ 8 files changed, 356 insertions(+), 27 deletions(-) create mode 100644 src/main/java/org/springframework/retry/interceptor/MethodInvocationRetryCallback.java create mode 100644 src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java create mode 100644 src/test/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupportTests.java diff --git a/README.md b/README.md index 4c6ad3f..7354b1a 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,35 @@ The open and close callbacks come before and after the entire retry in the simpl Note that when there is more than one listener, they are in a list, so there is an order. In this case open will be called in the same order while onError and close will be called in reverse order. +### Listeners for reflective method invocations + +When dealing with methods annotated with `@Retryable` or with Spring AOP intercepted methods, +spring-retry provides the possibility to inspect in detail the method invocation within the +`RetryListener` implementation. Such a scenario could be particularly useful when there is a need +to monitor how often a certain method call has been retried and expose it with detailed tagging +information (e.g. : class name, method name, or even parameter values in some exotic cases). + +All that needs to be done is checking whe + +```java + +template.registerListener(new MethodInvocationRetryListenerSupport() { + @Override + protected void doClose(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + monitoringTags.put(labelTagName, callback.getLabel()); + Method method = callback.getInvocation() + .getMethod(); + monitoringTags.put(classTagName, + method.getDeclaringClass().getSimpleName()); + monitoringTags.put(methodTagName, method.getName()); + + // register a monitoring counter with appropriate tags + // ... + } + }); +``` + ## Declarative Retry Sometimes there is some business processing that you know you want to retry every time it happens. The classic example of this is the remote service call. Spring Retry provides an AOP interceptor that wraps a method call in a `RetryOperations` for just this purpose. The `RetryOperationsInterceptor` executes the intercepted method and retries on failure according to the `RetryPolicy` in the provided `RepeatTemplate`. diff --git a/pom.xml b/pom.xml index cf7a4d4..23054e2 100644 --- a/pom.xml +++ b/pom.xml @@ -218,6 +218,12 @@ 4.12 test + + org.hamcrest + hamcrest-all + 1.3 + test + org.easymock easymock diff --git a/src/main/java/org/springframework/retry/interceptor/MethodInvocationRetryCallback.java b/src/main/java/org/springframework/retry/interceptor/MethodInvocationRetryCallback.java new file mode 100644 index 0000000..42b0ae4 --- /dev/null +++ b/src/main/java/org/springframework/retry/interceptor/MethodInvocationRetryCallback.java @@ -0,0 +1,53 @@ +package org.springframework.retry.interceptor; + +import org.aopalliance.intercept.MethodInvocation; +import org.springframework.retry.RetryCallback; +import org.springframework.retry.RetryOperations; +import org.springframework.util.StringUtils; + +/** + * Callback class for a Spring AOP reflective `MethodInvocation` that can be retried using a {@link + * RetryOperations}. + * + * In a concrete {@link org.springframework.retry.RetryListener} implementation, the + * `MethodInvocation` can be analysed for providing insights on the method called as well as its + * parameter values which could then be used for monitoring purposes. + * + * @param the type of object returned by the callback + * @param the type of exception it declares may be thrown + * @see StatefulRetryOperationsInterceptor + * @see RetryOperationsInterceptor + * @see org.springframework.retry.listener.MethodInvocationRetryListenerSupport + * @since 1.3 + */ +public abstract class MethodInvocationRetryCallback + implements RetryCallback { + + protected final MethodInvocation invocation; + + protected final String label; + + /** + * Constructor for the class. + * + * @param invocation the method invocation + * @param label a unique label for statistics reporting. + */ + public MethodInvocationRetryCallback(MethodInvocation invocation, String label) { + this.invocation = invocation; + if (StringUtils.hasText(label)) { + this.label = label; + } else { + this.label = invocation.getMethod().toGenericString(); + } + } + + public MethodInvocation getInvocation() { + return invocation; + } + + public String getLabel() { + return label; + } + +} diff --git a/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java index d868d9d..5ab4609 100644 --- a/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/interceptor/RetryOperationsInterceptor.java @@ -75,7 +75,8 @@ public class RetryOperationsInterceptor implements MethodInterceptor { } final String label = name; - RetryCallback retryCallback = new RetryCallback() { + RetryCallback retryCallback = new MethodInvocationRetryCallback( + invocation, label) { public Object doWithRetry(RetryContext context) throws Exception { diff --git a/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java index 85abac1..8fa1580 100644 --- a/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java @@ -168,7 +168,7 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor { this.rollbackClassifier); Object result = this.retryOperations - .execute(new MethodInvocationRetryCallback(invocation, label), + .execute(new StatefulMethodInvocationRetryCallback(invocation, label), this.recoverer != null ? new ItemRecovererCallback(args, this.recoverer) : null, retryState); @@ -204,21 +204,12 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor { * @author Dave Syer * */ - private static final class MethodInvocationRetryCallback - implements RetryCallback { + private static final class StatefulMethodInvocationRetryCallback + extends MethodInvocationRetryCallback { - private final MethodInvocation invocation; - - private String label; - - private MethodInvocationRetryCallback(MethodInvocation invocation, String label) { - this.invocation = invocation; - if (StringUtils.hasText(label)) { - this.label = label; - } - else { - this.label = invocation.getMethod().toGenericString(); - } + private StatefulMethodInvocationRetryCallback(MethodInvocation invocation, + String label) { + super(invocation, label); } @Override diff --git a/src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java b/src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java new file mode 100644 index 0000000..cbcd365 --- /dev/null +++ b/src/main/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupport.java @@ -0,0 +1,77 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.retry.listener; + +import org.springframework.retry.RetryCallback; +import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryListener; +import org.springframework.retry.interceptor.MethodInvocationRetryCallback; + +/** + *

+ * Empty method implementation of {@link RetryListener} with focus on the AOP reflective + * method invocations providing convenience retry listener type-safe (with a + * `MethodInvocationRetryCallback` callback parameter) specific methods. + *

+ * NOTE that this listener performs an action only when dealing with callbacks that are + * instances of {@link MethodInvocationRetryCallback}. + * + * @since 1.3 + */ +public class MethodInvocationRetryListenerSupport implements RetryListener { + + public void close(RetryContext context, + RetryCallback callback, Throwable throwable) { + if (callback instanceof MethodInvocationRetryCallback) { + MethodInvocationRetryCallback methodInvocationRetryCallback = (MethodInvocationRetryCallback) callback; + doClose(context, methodInvocationRetryCallback, throwable); + } + } + + public void onError(RetryContext context, + RetryCallback callback, Throwable throwable) { + if (callback instanceof MethodInvocationRetryCallback) { + MethodInvocationRetryCallback methodInvocationRetryCallback = (MethodInvocationRetryCallback) callback; + doOnError(context, methodInvocationRetryCallback, throwable); + } + } + + public boolean open(RetryContext context, + RetryCallback callback) { + if (callback instanceof MethodInvocationRetryCallback) { + MethodInvocationRetryCallback methodInvocationRetryCallback = (MethodInvocationRetryCallback) callback; + return doOpen(context, methodInvocationRetryCallback); + } + // in case that the callback is not for a reflective method invocation + // just go forward with the execution + return true; + } + + protected void doClose(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + } + + protected void doOnError(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + } + + protected boolean doOpen(RetryContext context, + MethodInvocationRetryCallback callback) { + return true; + } + +} diff --git a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java index be1a33f..a2bf364 100644 --- a/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java +++ b/src/test/java/org/springframework/retry/interceptor/RetryOperationsInterceptorTests.java @@ -16,22 +16,31 @@ package org.springframework.retry.interceptor; +import static org.hamcrest.collection.IsMapContaining.hasEntry; +import static org.hamcrest.core.AllOf.allOf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.lang.reflect.AccessibleObject; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; - +import java.util.Map; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.Before; import org.junit.Test; - import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.target.SingletonTargetSource; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.listener.MethodInvocationRetryListenerSupport; import org.springframework.retry.listener.RetryListenerSupport; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.policy.SimpleRetryPolicy; @@ -40,13 +49,12 @@ import org.springframework.transaction.support.TransactionSynchronizationAdapter import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.util.ClassUtils; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - public class RetryOperationsInterceptorTests { + private static int count; + + private static int transactionCount; + private RetryOperationsInterceptor interceptor; private Service service; @@ -55,10 +63,6 @@ public class RetryOperationsInterceptorTests { private RetryContext context; - private static int count; - - private static int transactionCount; - @Before public void setUp() throws Exception { this.interceptor = new RetryOperationsInterceptor(); @@ -94,6 +98,42 @@ public class RetryOperationsInterceptorTests { assertEquals("FOO", this.context.getAttribute(RetryContext.NAME)); } + @Test + public void testDefaultInterceptorWithRetryListenerInspectingTheMethodInvocation() + throws Exception { + + final String label = "FOO"; + final String classTagName = "class"; + final String methodTagName = "method"; + final String labelTagName = "label"; + final Map monitoringTags = new HashMap(); + RetryTemplate template = new RetryTemplate(); + template.setRetryPolicy(new SimpleRetryPolicy(2)); + template.registerListener(new MethodInvocationRetryListenerSupport() { + @Override + protected void doClose(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + monitoringTags.put(labelTagName, callback.getLabel()); + Method method = callback.getInvocation().getMethod(); + monitoringTags.put(classTagName, + method.getDeclaringClass().getSimpleName()); + monitoringTags.put(methodTagName, method.getName()); + } + }); + + this.interceptor.setLabel(label); + this.interceptor.setRetryOperations(template); + + ((Advised) this.service).addAdvice(this.interceptor); + this.service.service(); + assertEquals(2, count); + assertEquals(3, monitoringTags.entrySet().size()); + assertThat(monitoringTags, allOf(hasEntry(labelTagName, label), + hasEntry(classTagName, + RetryOperationsInterceptorTests.Service.class.getSimpleName()), + hasEntry(methodTagName, "service"))); + } + @Test public void testDefaultInterceptorWithRecovery() throws Exception { RetryTemplate template = new RetryTemplate(); diff --git a/src/test/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupportTests.java b/src/test/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupportTests.java new file mode 100644 index 0000000..d13c6ae --- /dev/null +++ b/src/test/java/org/springframework/retry/listener/MethodInvocationRetryListenerSupportTests.java @@ -0,0 +1,132 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.retry.listener; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; + +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.Test; +import org.springframework.retry.RetryCallback; +import org.springframework.retry.RetryContext; +import org.springframework.retry.interceptor.MethodInvocationRetryCallback; + +public class MethodInvocationRetryListenerSupportTests { + + @Test + public void testClose() { + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport(); + try { + support.close(null, null, null); + } + catch (Exception e) { + fail("Unexpected exception"); + } + } + + @Test + public void testCloseWithMethodInvocationRetryCallbackShouldCallDoCloseMethod() { + final AtomicInteger callsOnDoCloseMethod = new AtomicInteger(0); + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport() { + @Override + protected void doClose(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + callsOnDoCloseMethod.incrementAndGet(); + } + }; + RetryContext context = mock(RetryContext.class); + MethodInvocationRetryCallback callback = mock( + MethodInvocationRetryCallback.class); + support.close(context, callback, null); + + assertEquals(1, callsOnDoCloseMethod.get()); + } + + @Test + public void testCloseWithRetryCallbackShouldntCallDoCloseMethod() { + final AtomicInteger callsOnDoCloseMethod = new AtomicInteger(0); + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport() { + @Override + protected void doClose(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + callsOnDoCloseMethod.incrementAndGet(); + } + }; + RetryContext context = mock(RetryContext.class); + RetryCallback callback = mock(RetryCallback.class); + support.close(context, callback, null); + + assertEquals(0, callsOnDoCloseMethod.get()); + } + + @Test + public void testOnError() { + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport(); + try { + support.onError(null, null, null); + } + catch (Exception e) { + fail("Unexpected exception"); + } + } + + @Test + public void testOnErrorWithMethodInvocationRetryCallbackShouldCallDoOnErrorMethod() { + final AtomicInteger callsOnDoOnErrorMethod = new AtomicInteger(0); + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport() { + @Override + protected void doOnError(RetryContext context, + MethodInvocationRetryCallback callback, Throwable throwable) { + callsOnDoOnErrorMethod.incrementAndGet(); + } + }; + RetryContext context = mock(RetryContext.class); + MethodInvocationRetryCallback callback = mock( + MethodInvocationRetryCallback.class); + support.onError(context, callback, null); + + assertEquals(1, callsOnDoOnErrorMethod.get()); + } + + @Test + public void testOpen() { + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport(); + assertTrue(support.open(null, null)); + } + + @Test + public void testOpenWithMethodInvocationRetryCallbackShouldCallDoCloseMethod() { + final AtomicInteger callsOnDoOpenMethod = new AtomicInteger(0); + MethodInvocationRetryListenerSupport support = new MethodInvocationRetryListenerSupport() { + @Override + protected boolean doOpen(RetryContext context, + MethodInvocationRetryCallback callback) { + callsOnDoOpenMethod.incrementAndGet(); + return true; + } + }; + RetryContext context = mock(RetryContext.class); + MethodInvocationRetryCallback callback = mock( + MethodInvocationRetryCallback.class); + assertTrue(support.open(context, callback)); + + assertEquals(1, callsOnDoOpenMethod.get()); + } + +}