Set RetryState Key to null if Necessary

See 9a95da2b70

If the key generator returns null, we need to set the key to null so the
state is not stored.

pom.xml

@@ -159,6 +159,12 @@
 			<version>2.3</version>
 			<scope>test</scope>
 		</dependency>
+		<dependency>
+			<groupId>org.mockito</groupId>
+			<artifactId>mockito-core</artifactId>
+			<version>1.10.19</version>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>org.aspectj</groupId>
 			<artifactId>aspectjrt</artifactId>

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),

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<String> list = new ArrayList<String>();
 		((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<Object>() {
+			@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<DefaultRetryState> 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<Collection<String>>() {
+			@Override
 			public Collection<String> 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<String> transform(String in) throws Exception {
 			count++;
 			if (count < 2) {