From 3476d10efad494094829ac28e3bee06c074fc57a Mon Sep 17 00:00:00 2001 From: David Brimley Date: Wed, 11 Jan 2017 16:29:14 +0000 Subject: [PATCH 1/3] Honour ErrorHandler if `Cache.put` fails This commit makes sure that the `ErrorHandler` is invoked if the cache fails to put an element (be it in the main cache or the exception cache). See gh-1292 Issue: SPR-15188 --- .../cache/jcache/interceptor/CacheResultInterceptor.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/CacheResultInterceptor.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/CacheResultInterceptor.java index 7c178edf02d..e42c334b94b 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/CacheResultInterceptor.java +++ b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/CacheResultInterceptor.java @@ -59,7 +59,7 @@ class CacheResultInterceptor extends AbstractKeyCacheInterceptor Date: Mon, 6 Feb 2017 15:24:13 +0100 Subject: [PATCH 2/3] Polish contribution Closes gh-1292 Issue: SPR-15188 --- .../interceptor/JCacheErrorHandlerTests.java | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java index 54b66033a16..0655a5b3535 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java @@ -41,6 +41,7 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import static org.junit.Assert.*; import static org.mockito.BDDMockito.*; /** @@ -53,6 +54,8 @@ public class JCacheErrorHandlerTests { private Cache cache; + private Cache errorCache; + private CacheErrorHandler errorHandler; private SimpleService simpleService; @@ -61,6 +64,7 @@ public class JCacheErrorHandlerTests { public void setup() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Config.class); this.cache = context.getBean("mockCache", Cache.class); + this.errorCache = context.getBean("mockErrorCache", Cache.class); this.errorHandler = context.getBean(CacheErrorHandler.class); this.simpleService = context.getBean(SimpleService.class); } @@ -75,6 +79,35 @@ public class JCacheErrorHandlerTests { verify(errorHandler).handleCacheGetError(exception, cache, key); } + @Test + public void getPutNewElementFail() { + UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on put"); + Object key = SimpleKeyGenerator.generateKey(0L); + given(this.cache.get(key)).willReturn(null); + willThrow(exception).given(this.cache).put(key, 0L); + + this.simpleService.get(0L); + verify(this.errorHandler).handleCachePutError(exception, this.cache, key, 0L); + } + + @Test + public void getFailPutExceptionFail() { + UnsupportedOperationException exceptionOnPut = new UnsupportedOperationException("Test exception on put"); + Object key = SimpleKeyGenerator.generateKey(0L); + given(this.cache.get(key)).willReturn(null); + willThrow(exceptionOnPut).given(this.errorCache).put(key, + SimpleService.TEST_EXCEPTION); + + try { + this.simpleService.getFail(0L); + } + catch (IllegalStateException ex) { + assertEquals("Test exception", ex.getMessage()); + } + verify(this.errorHandler).handleCachePutError(exceptionOnPut, + this.errorCache, key, SimpleService.TEST_EXCEPTION); + } + @Test public void putFail() { UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on put"); @@ -113,7 +146,7 @@ public class JCacheErrorHandlerTests { @Override public CacheManager cacheManager() { SimpleCacheManager cacheManager = new SimpleCacheManager(); - cacheManager.setCaches(Arrays.asList(mockCache())); + cacheManager.setCaches(Arrays.asList(mockCache(), mockErrorCache())); return cacheManager; } @@ -135,10 +168,21 @@ public class JCacheErrorHandlerTests { return cache; } + @Bean + public Cache mockErrorCache() { + Cache cache = mock(Cache.class); + given(cache.getName()).willReturn("error"); + return cache; + } + } @CacheDefaults(cacheName = "test") public static class SimpleService { + + private static final IllegalStateException TEST_EXCEPTION = + new IllegalStateException("Test exception"); + private AtomicLong counter = new AtomicLong(); @CacheResult @@ -146,6 +190,11 @@ public class JCacheErrorHandlerTests { return counter.getAndIncrement(); } + @CacheResult(exceptionCacheName = "error") + public Object getFail(long id) { + throw TEST_EXCEPTION; + } + @CachePut public void put(long id, @CacheValue Object object) { } From 85b20aa08afe2394cfc4028fc301326a1521c530 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 6 Feb 2017 15:26:56 +0100 Subject: [PATCH 3/3] Polish --- .../interceptor/JCacheErrorHandlerTests.java | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java index 0655a5b3535..6e96e5447c2 100644 --- a/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java +++ b/spring-context-support/src/test/java/org/springframework/cache/jcache/interceptor/JCacheErrorHandlerTests.java @@ -62,7 +62,8 @@ public class JCacheErrorHandlerTests { @Before public void setup() { - AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(Config.class); + AnnotationConfigApplicationContext context = + new AnnotationConfigApplicationContext(Config.class); this.cache = context.getBean("mockCache", Cache.class); this.errorCache = context.getBean("mockErrorCache", Cache.class); this.errorHandler = context.getBean(CacheErrorHandler.class); @@ -71,17 +72,19 @@ public class JCacheErrorHandlerTests { @Test public void getFail() { - UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get"); + UnsupportedOperationException exception = + new UnsupportedOperationException("Test exception on get"); Object key = SimpleKeyGenerator.generateKey(0L); - willThrow(exception).given(cache).get(key); + willThrow(exception).given(this.cache).get(key); this.simpleService.get(0L); - verify(errorHandler).handleCacheGetError(exception, cache, key); + verify(this.errorHandler).handleCacheGetError(exception, this.cache, key); } @Test public void getPutNewElementFail() { - UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on put"); + UnsupportedOperationException exception = + new UnsupportedOperationException("Test exception on put"); Object key = SimpleKeyGenerator.generateKey(0L); given(this.cache.get(key)).willReturn(null); willThrow(exception).given(this.cache).put(key, 0L); @@ -92,7 +95,8 @@ public class JCacheErrorHandlerTests { @Test public void getFailPutExceptionFail() { - UnsupportedOperationException exceptionOnPut = new UnsupportedOperationException("Test exception on put"); + UnsupportedOperationException exceptionOnPut = + new UnsupportedOperationException("Test exception on put"); Object key = SimpleKeyGenerator.generateKey(0L); given(this.cache.get(key)).willReturn(null); willThrow(exceptionOnPut).given(this.errorCache).put(key, @@ -110,31 +114,34 @@ public class JCacheErrorHandlerTests { @Test public void putFail() { - UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on put"); + UnsupportedOperationException exception = + new UnsupportedOperationException("Test exception on put"); Object key = SimpleKeyGenerator.generateKey(0L); - willThrow(exception).given(cache).put(key, 234L); + willThrow(exception).given(this.cache).put(key, 234L); this.simpleService.put(0L, 234L); - verify(errorHandler).handleCachePutError(exception, cache, key, 234L); + verify(this.errorHandler).handleCachePutError(exception, this.cache, key, 234L); } @Test public void evictFail() { - UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on evict"); + UnsupportedOperationException exception = + new UnsupportedOperationException("Test exception on evict"); Object key = SimpleKeyGenerator.generateKey(0L); - willThrow(exception).given(cache).evict(key); + willThrow(exception).given(this.cache).evict(key); this.simpleService.evict(0L); - verify(errorHandler).handleCacheEvictError(exception, cache, key); + verify(this.errorHandler).handleCacheEvictError(exception, this.cache, key); } @Test public void clearFail() { - UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on evict"); - willThrow(exception).given(cache).clear(); + UnsupportedOperationException exception = + new UnsupportedOperationException("Test exception on evict"); + willThrow(exception).given(this.cache).clear(); this.simpleService.clear(); - verify(errorHandler).handleCacheClearError(exception, cache); + verify(this.errorHandler).handleCacheClearError(exception, this.cache); } @@ -187,7 +194,7 @@ public class JCacheErrorHandlerTests { @CacheResult public Object get(long id) { - return counter.getAndIncrement(); + return this.counter.getAndIncrement(); } @CacheResult(exceptionCacheName = "error")