From 3cda355e7f4dcdbfa613e93bde0b9f933e81de2d Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 8 Apr 2014 17:03:31 +0200 Subject: [PATCH] polishing This commit fixes the handling of cached exceptions in the JSR-107 advisor. Such exceptions are now properly propagated instead of being wrapped in a RuntimeException. Issue: SPR-9616 --- .../cache/aspectj/JCacheCacheAspect.aj | 19 +++++++++--- .../AspectJJcacheNamespaceConfigTests.java | 2 +- .../config/AnnotatedJCacheableService.java | 29 ++++++++++++++----- .../config/AbstractJCacheAnnotationTests.java | 23 +++++++++++++++ .../jcache/config/JCacheableService.java | 4 +++ .../AnnotatedJCacheableService.java | 19 +++++++++++- 6 files changed, 82 insertions(+), 14 deletions(-) diff --git a/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj b/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj index 6b75a12acb..3bb86d5453 100644 --- a/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj +++ b/spring-aspects/src/main/java/org/springframework/cache/aspectj/JCacheCacheAspect.aj @@ -67,10 +67,8 @@ public aspect JCacheCacheAspect extends JCacheAspectSupport { return execute(aspectJInvoker, thisJoinPoint.getTarget(), method, thisJoinPoint.getArgs()); } catch (CacheOperationInvoker.ThrowableWrapper th) { - if (th.getOriginal() instanceof RuntimeException) { - throw (RuntimeException) th.getOriginal(); - } - throw th; // Lose original checked exception + AnyThrow.throwUnchecked(th.getOriginal()); + return null; // never reached } } @@ -109,4 +107,17 @@ public aspect JCacheCacheAspect extends JCacheAspectSupport { private pointcut executionOfCacheRemoveAllMethod() : execution(@CacheRemoveAll * *(..)); + + private static class AnyThrow { + + private static void throwUnchecked(Throwable e) { + AnyThrow.throwAny(e); + } + + @SuppressWarnings("unchecked") + private static void throwAny(Throwable e) throws E { + throw (E)e; + } + } + } \ No newline at end of file diff --git a/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJJcacheNamespaceConfigTests.java b/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJJcacheNamespaceConfigTests.java index 16d1096295..9eb2a9a35f 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJJcacheNamespaceConfigTests.java +++ b/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJJcacheNamespaceConfigTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.cache.jcache.config.aspectj; +package org.springframework.cache.aspectj; import org.springframework.cache.jcache.config.AbstractJCacheAnnotationTests; import org.springframework.context.ApplicationContext; diff --git a/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedJCacheableService.java b/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedJCacheableService.java index de3bca6ccf..dc3deb0add 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedJCacheableService.java +++ b/spring-aspects/src/test/java/org/springframework/cache/config/AnnotatedJCacheableService.java @@ -16,6 +16,10 @@ package org.springframework.cache.config; +import java.io.IOException; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; + import javax.cache.annotation.CacheDefaults; import javax.cache.annotation.CacheKey; import javax.cache.annotation.CachePut; @@ -23,8 +27,6 @@ import javax.cache.annotation.CacheRemove; import javax.cache.annotation.CacheRemoveAll; import javax.cache.annotation.CacheResult; import javax.cache.annotation.CacheValue; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicLong; import org.springframework.cache.Cache; import org.springframework.cache.jcache.config.JCacheableService; @@ -64,6 +66,13 @@ public class AnnotatedJCacheableService implements JCacheableService { return 0L; // Never reached } + @Override + @CacheResult(exceptionCacheName = "exception", nonCachedExceptions = NullPointerException.class) + public Long cacheWithCheckedException(@CacheKey String id, boolean matchFilter) throws IOException { + throwCheckedException(matchFilter); + return 0L; // Never reached + } + @Override @CacheResult(skipGet = true) public Long cacheAlwaysInvoke(String id) { @@ -177,12 +186,6 @@ public class AnnotatedJCacheableService implements JCacheableService { public void noAnnotation() { } - @CacheRemove - @CacheRemoveAll - public void multiAnnotations() { - - } - @Override public long exceptionInvocations() { return exceptionCounter.get(); @@ -198,4 +201,14 @@ public class AnnotatedJCacheableService implements JCacheableService { } } + private void throwCheckedException(boolean matchFilter) throws IOException { + long count = exceptionCounter.getAndIncrement(); + if (matchFilter) { + throw new IOException("Expected exception (" + count + ")"); + } + else { + throw new NullPointerException("Expected exception (" + count + ")"); + } + } + } diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/config/AbstractJCacheAnnotationTests.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/config/AbstractJCacheAnnotationTests.java index bfffbad945..3bb27f06cc 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/config/AbstractJCacheAnnotationTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/jcache/config/AbstractJCacheAnnotationTests.java @@ -18,6 +18,7 @@ package org.springframework.cache.jcache.config; import static org.junit.Assert.*; +import java.io.IOException; import java.util.concurrent.ConcurrentHashMap; import org.junit.Before; @@ -103,6 +104,28 @@ public abstract class AbstractJCacheAnnotationTests { assertNull(cache.get(key)); } + @Test + public void cacheCheckedException() { + String keyItem = name.getMethodName(); + Cache cache = getCache(EXCEPTION_CACHE); + + Object key = createKey(keyItem); + assertNull(cache.get(key)); + + try { + service.cacheWithCheckedException(keyItem, true); + fail("Should have thrown an exception"); + } + catch (IOException e) { + // This is what we expect + } + + Cache.ValueWrapper result = cache.get(key); + assertNotNull(result); + assertEquals(IOException.class, result.get().getClass()); + } + + @SuppressWarnings("ThrowableResultOfMethodCallIgnored") @Test public void cacheExceptionRewriteCallStack() { diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheableService.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheableService.java index 8b5709ef4a..adf1d35bcf 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheableService.java +++ b/spring-context-support/src/test/java/org/springframework/cache/jcache/config/JCacheableService.java @@ -16,6 +16,8 @@ package org.springframework.cache.jcache.config; +import java.io.IOException; + /** * @author Stephane Nicoll */ @@ -25,6 +27,8 @@ public interface JCacheableService { T cacheWithException(String id, boolean matchFilter); + T cacheWithCheckedException(String id, boolean matchFilter) throws IOException; + T cacheAlwaysInvoke(String id); T cacheWithPartialKey(String id, boolean notUsed); diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/AnnotatedJCacheableService.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/AnnotatedJCacheableService.java index 5ea84fa6b0..e3e0caf2cb 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/AnnotatedJCacheableService.java +++ b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/AnnotatedJCacheableService.java @@ -16,6 +16,7 @@ package org.springframework.cache.jcache.interceptor; +import java.io.IOException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; @@ -32,7 +33,6 @@ import org.springframework.cache.jcache.config.JCacheableService; import org.springframework.cache.jcache.support.TestableCacheKeyGenerator; import org.springframework.cache.jcache.support.TestableCacheResolverFactory; - /** * Repository sample with a @CacheDefaults annotation * @@ -64,6 +64,13 @@ public class AnnotatedJCacheableService implements JCacheableService { return 0L; // Never reached } + @Override + @CacheResult(exceptionCacheName = "exception", nonCachedExceptions = NullPointerException.class) + public Long cacheWithCheckedException(@CacheKey String id, boolean matchFilter) throws IOException { + throwCheckedException(matchFilter); + return 0L; // Never reached + } + @Override @CacheResult(skipGet = true) public Long cacheAlwaysInvoke(String id) { @@ -192,4 +199,14 @@ public class AnnotatedJCacheableService implements JCacheableService { } } + private void throwCheckedException(boolean matchFilter) throws IOException { + long count = exceptionCounter.getAndIncrement(); + if (matchFilter) { + throw new IOException("Expected exception (" + count + ")"); + } + else { + throw new NullPointerException("Expected exception (" + count + ")"); + } + } + }