If retry state key is null, context is not cached

If the retry state has a null key it could be an error, but there
are real work scenarios where that just means "no information"
and the safest thing to do in that case is to simply not cache the
retry context (so the retry callback is executed "naturally" and
the exception bubbles up).

See comment in gh-49 about Spring AMQP use case

src/main/java/org/springframework/retry/support/RetryTemplate.java

@@ -411,13 +411,16 @@ public class RetryTemplate implements RetryOperations {
 	private void registerContext(RetryContext context, RetryState state) {
 		if (state != null) {
 			Object key = state.getKey();
-			if (context.getRetryCount() > 0 && !this.retryContextCache.containsKey(key)) {
-				throw new RetryException(
-						"Inconsistent state for failed item key: cache key has changed. "
-								+ "Consider whether equals() or hashCode() for the key might be inconsistent, "
-								+ "or if you need to supply a better key");
+			if (key != null) {
+				if (context.getRetryCount() > 0
+						&& !this.retryContextCache.containsKey(key)) {
+					throw new RetryException(
+							"Inconsistent state for failed item key: cache key has changed. "
+									+ "Consider whether equals() or hashCode() for the key might be inconsistent, "
+									+ "or if you need to supply a better key");
+				}
+				this.retryContextCache.put(key, context);
 			}
-			this.retryContextCache.put(key, context);
 		}
 	}
 

src/test/java/org/springframework/retry/policy/StatefulRetryIntegrationTests.java

@@ -52,8 +52,9 @@ public class StatefulRetryIntegrationTests {
 		RetryTemplate retryTemplate = new RetryTemplate();
 		MapRetryContextCache cache = new MapRetryContextCache();
 		retryTemplate.setRetryContextCache(cache);
-		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(1, Collections
-				.<Class<? extends Throwable>, Boolean> singletonMap(Exception.class, true)));
+		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(1,
+				Collections.<Class<? extends Throwable>, Boolean>singletonMap(
+						Exception.class, true)));
 
 		assertFalse(cache.containsKey("foo"));
 
@@ -94,8 +95,9 @@ public class StatefulRetryIntegrationTests {
 		RetryTemplate retryTemplate = new RetryTemplate();
 		MapRetryContextCache cache = new MapRetryContextCache();
 		retryTemplate.setRetryContextCache(cache);
-		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(2, Collections
-				.<Class<? extends Throwable>, Boolean> singletonMap(Exception.class, true)));
+		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(2,
+				Collections.<Class<? extends Throwable>, Boolean>singletonMap(
+						Exception.class, true)));
 
 		assertFalse(cache.containsKey("foo"));
 
@@ -136,8 +138,7 @@ public class StatefulRetryIntegrationTests {
 						throw new Exception("Fail");
 					}
 				}, new RecoveryCallback<String>() {
-					public String recover(RetryContext context)
-							throws Exception {
+					public String recover(RetryContext context) throws Exception {
 						return null;
 					}
 				}, retryState);
@@ -151,11 +152,42 @@ public class StatefulRetryIntegrationTests {
 		assertTrue(times.get(2) - times.get(1) >= 150);
 	}
 
+	@Test
+	public void testExternalRetryWithFailAndNoRetryWhenKeyIsNull() throws Throwable {
+		MockRetryCallback callback = new MockRetryCallback();
+
+		RetryState retryState = new DefaultRetryState(null);
+
+		RetryTemplate retryTemplate = new RetryTemplate();
+		MapRetryContextCache cache = new MapRetryContextCache();
+		retryTemplate.setRetryContextCache(cache);
+		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(1,
+				Collections.<Class<? extends Throwable>, Boolean>singletonMap(
+						Exception.class, true)));
+
+		try {
+			retryTemplate.execute(callback, retryState);
+			// The first failed attempt...
+			fail("Expected RuntimeException");
+		}
+		catch (RuntimeException e) {
+			assertEquals(null, e.getMessage());
+		}
+
+		retryTemplate.execute(callback, retryState);
+		// The second attempt is successful by design...
+
+		// Callback is called twice because its state is null: the recovery path should
+		// not be called...
+		assertEquals(2, callback.attempts);
+	}
+
 	/**
 	 * @author Dave Syer
 	 * 
 	 */
-	private static final class MockRetryCallback implements RetryCallback<String, Exception> {
+	private static final class MockRetryCallback
+			implements RetryCallback<String, Exception> {
 		int attempts = 0;
 
 		public String doWithRetry(RetryContext context) throws Exception {