From 2e293c33c3d60cc2f9bb83f0042b975f4cf4bf0e Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 23 Sep 2016 16:22:32 -0400 Subject: [PATCH] Set RetryState Key to null if Necessary See https://github.com/spring-projects/spring-retry/commit/9a95da2b708d5c7a0d323cfa4faf19ab10b89e81 If the key generator returns null, we need to set the key to null so the state is not stored. --- pom.xml | 6 +++ .../StatefulRetryOperationsInterceptor.java | 9 ++-- ...atefulRetryOperationsInterceptorTests.java | 41 ++++++++++++++++--- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 4522431..bdcc895 100644 --- a/pom.xml +++ b/pom.xml @@ -159,6 +159,12 @@ 2.3 test + + org.mockito + mockito-core + 1.10.19 + test + org.aspectj aspectjrt diff --git a/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java index 77aaefa..5d5eb48 100644 --- a/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2016 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. @@ -22,6 +22,7 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.classify.Classifier; import org.springframework.retry.RecoveryCallback; import org.springframework.retry.RetryCallback; @@ -52,6 +53,7 @@ import org.springframework.util.StringUtils; * {@link RetryTemplate}. * * @author Dave Syer + * @author Gary Russell */ public class StatefulRetryOperationsInterceptor implements MethodInterceptor { @@ -150,8 +152,9 @@ public class StatefulRetryOperationsInterceptor implements MethodInterceptor { defaultKey = args[0]; } - Object key = Arrays.asList(name, this.keyGenerator != null - ? this.keyGenerator.getKey(invocation.getArguments()) : defaultKey ); + Object generatedKey = this.keyGenerator != null ? this.keyGenerator.getKey(invocation.getArguments()) : null; + Object key = this.keyGenerator == null || generatedKey != null + ? Arrays.asList(name, generatedKey != null ? generatedKey : defaultKey ) : null; RetryState retryState = new DefaultRetryState(key, this.newMethodArgumentsIdentifier != null && this.newMethodArgumentsIdentifier.isNew(args), diff --git a/src/test/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptorTests.java b/src/test/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptorTests.java index 617fd73..2d40c47 100644 --- a/src/test/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptorTests.java +++ b/src/test/java/org/springframework/retry/interceptor/StatefulRetryOperationsInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2016 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. @@ -16,8 +16,13 @@ package org.springframework.retry.interceptor; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.Collection; @@ -28,27 +33,33 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; + import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.target.SingletonTargetSource; import org.springframework.retry.ExhaustedRetryException; +import org.springframework.retry.RecoveryCallback; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.RetryOperations; import org.springframework.retry.listener.RetryListenerSupport; import org.springframework.retry.policy.AlwaysRetryPolicy; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.policy.SimpleRetryPolicy; +import org.springframework.retry.support.DefaultRetryState; import org.springframework.retry.support.RetryTemplate; /** * @author Dave Syer - * + * @author Gary Russell + * */ public class StatefulRetryOperationsInterceptorTests { private StatefulRetryOperationsInterceptor interceptor; - private RetryTemplate retryTemplate = new RetryTemplate(); + private final RetryTemplate retryTemplate = new RetryTemplate(); private Service service; @@ -69,9 +80,9 @@ public class StatefulRetryOperationsInterceptorTests { } }); interceptor.setRetryOperations(retryTemplate); - service = (Service) ProxyFactory.getProxy(Service.class, + service = ProxyFactory.getProxy(Service.class, new SingletonTargetSource(new ServiceImpl())); - transformer = (Transformer) ProxyFactory.getProxy(Transformer.class, + transformer = ProxyFactory.getProxy(Transformer.class, new SingletonTargetSource(new TransformerImpl())); count = 0; } @@ -145,6 +156,7 @@ public class StatefulRetryOperationsInterceptorTests { ((Advised) service).addAdvice(interceptor); final List list = new ArrayList(); ((Advised) service).addAdvice(new MethodInterceptor() { + @Override public Object invoke(MethodInvocation invocation) throws Throwable { list.add("chain"); return invocation.proceed(); @@ -231,6 +243,7 @@ public class StatefulRetryOperationsInterceptorTests { } assertEquals(1, count); interceptor.setRecoverer(new MethodInvocationRecoverer() { + @Override public Object recover(Object[] data, Throwable cause) { count++; return null; @@ -240,6 +253,21 @@ public class StatefulRetryOperationsInterceptorTests { assertEquals(2, count); } + @SuppressWarnings("unchecked") + @Test + public void testKeyGeneratorReturningNull() throws Throwable { + this.interceptor.setKeyGenerator(mock(MethodArgumentsKeyGenerator.class)); + this.interceptor.setLabel("foo"); + RetryOperations template = mock(RetryOperations.class); + this.interceptor.setRetryOperations(template); + MethodInvocation invocation = mock(MethodInvocation.class); + when(invocation.getArguments()).thenReturn(new Object[] { new Object() }); + this.interceptor.invoke(invocation); + ArgumentCaptor captor = ArgumentCaptor.forClass(DefaultRetryState.class); + verify(template).execute(any(RetryCallback.class), any(RecoveryCallback.class), captor.capture()); + assertNull(captor.getValue().getKey()); + } + @Test public void testTransformerRecoveryAfterTooManyAttempts() throws Exception { ((Advised) transformer).addAdvice(interceptor); @@ -256,6 +284,7 @@ public class StatefulRetryOperationsInterceptorTests { } assertEquals(1, count); interceptor.setRecoverer(new MethodInvocationRecoverer>() { + @Override public Collection recover(Object[] data, Throwable cause) { count++; return Collections.singleton((String) data[0]); @@ -272,6 +301,7 @@ public class StatefulRetryOperationsInterceptorTests { public static class ServiceImpl implements Service { + @Override public void service(String in) throws Exception { count++; if (count < 2) { @@ -287,6 +317,7 @@ public class StatefulRetryOperationsInterceptorTests { public static class TransformerImpl implements Transformer { + @Override public Collection transform(String in) throws Exception { count++; if (count < 2) {