Skip to content

Commit 2eda5d7

Browse files
committed
Handle low-level errors for sync/flux/mono/future gets
This change adds 3 protected methods to `AbstractCacheInvoker` that wrap additional `Cache#retrieve` and `Cache#get` calls with `handleCacheGetError` in case the Cache call itself fails. For example, if the cache is remote and a connection to it cannot be established. Closes gh-21590
1 parent 5e72ee3 commit 2eda5d7

File tree

3 files changed

+126
-5
lines changed

3 files changed

+126
-5
lines changed

spring-context/src/main/java/org/springframework/cache/interceptor/AbstractCacheInvoker.java

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616

1717
package org.springframework.cache.interceptor;
1818

19+
import java.util.concurrent.Callable;
20+
import java.util.concurrent.CompletableFuture;
21+
import java.util.function.Supplier;
22+
1923
import org.springframework.cache.Cache;
2024
import org.springframework.lang.Nullable;
2125
import org.springframework.util.function.SingletonSupplier;
@@ -78,6 +82,52 @@ protected Cache.ValueWrapper doGet(Cache cache, Object key) {
7882
}
7983
}
8084

85+
@Nullable
86+
protected <T> T doGet(Cache cache, Object key, Callable<T> valueLoader) {
87+
try {
88+
return cache.get(key, valueLoader);
89+
}
90+
catch (Cache.ValueRetrievalException ex) {
91+
throw ex;
92+
}
93+
catch (RuntimeException ex) {
94+
getErrorHandler().handleCacheGetError(ex, cache, key);
95+
try {
96+
return valueLoader.call();
97+
}
98+
catch (Exception ex2) {
99+
throw new RuntimeException(ex2);
100+
}
101+
}
102+
}
103+
104+
@Nullable
105+
protected CompletableFuture<?> doRetrieve(Cache cache, Object key) {
106+
try {
107+
return cache.retrieve(key);
108+
}
109+
catch (Cache.ValueRetrievalException ex) {
110+
throw ex;
111+
}
112+
catch (RuntimeException ex) {
113+
getErrorHandler().handleCacheGetError(ex, cache, key);
114+
return null;
115+
}
116+
}
117+
118+
protected <T> CompletableFuture<T> doRetrieve(Cache cache, Object key, Supplier<CompletableFuture<T>> valueLoader) {
119+
try {
120+
return cache.retrieve(key, valueLoader);
121+
}
122+
catch (Cache.ValueRetrievalException ex) {
123+
throw ex;
124+
}
125+
catch (RuntimeException ex) {
126+
getErrorHandler().handleCacheGetError(ex, cache, key);
127+
return valueLoader.get();
128+
}
129+
}
130+
81131
/**
82132
* Execute {@link Cache#put(Object, Object)} on the specified {@link Cache}
83133
* and invoke the error handler if an exception occurs.

spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ private Object executeSynchronized(CacheOperationInvoker invoker, Method method,
456456
Object key = generateKey(context, CacheOperationExpressionEvaluator.NO_RESULT);
457457
Cache cache = context.getCaches().iterator().next();
458458
if (CompletableFuture.class.isAssignableFrom(method.getReturnType())) {
459-
return cache.retrieve(key, () -> (CompletableFuture<?>) invokeOperation(invoker));
459+
return doRetrieve(cache, key, () -> (CompletableFuture<?>) invokeOperation(invoker));
460460
}
461461
if (this.reactiveCachingHandler != null) {
462462
Object returnValue = this.reactiveCachingHandler.executeSynchronized(invoker, method, cache, key);
@@ -465,7 +465,7 @@ private Object executeSynchronized(CacheOperationInvoker invoker, Method method,
465465
}
466466
}
467467
try {
468-
return wrapCacheValue(method, cache.get(key, () -> unwrapReturnValue(invokeOperation(invoker))));
468+
return wrapCacheValue(method, doGet(cache, key, () -> unwrapReturnValue(invokeOperation(invoker))));
469469
}
470470
catch (Cache.ValueRetrievalException ex) {
471471
// Directly propagate ThrowableWrapper from the invoker,
@@ -515,7 +515,7 @@ private Object findInCaches(CacheOperationContext context, Object key,
515515

516516
for (Cache cache : context.getCaches()) {
517517
if (CompletableFuture.class.isAssignableFrom(context.getMethod().getReturnType())) {
518-
CompletableFuture<?> result = cache.retrieve(key);
518+
CompletableFuture<?> result = doRetrieve(cache, key);
519519
if (result != null) {
520520
return result.exceptionally(ex -> {
521521
getErrorHandler().handleCacheGetError((RuntimeException) ex, cache, key);
@@ -1144,7 +1144,7 @@ public Object findInCaches(CacheOperationContext context, Cache cache, Object ke
11441144

11451145
ReactiveAdapter adapter = this.registry.getAdapter(context.getMethod().getReturnType());
11461146
if (adapter != null) {
1147-
CompletableFuture<?> cachedFuture = cache.retrieve(key);
1147+
CompletableFuture<?> cachedFuture = doRetrieve(cache, key);
11481148
if (cachedFuture == null) {
11491149
return null;
11501150
}

spring-context/src/test/java/org/springframework/cache/interceptor/CacheErrorHandlerTests.java

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,15 @@
1717
package org.springframework.cache.interceptor;
1818

1919
import java.util.Collections;
20+
import java.util.concurrent.Callable;
21+
import java.util.concurrent.CompletableFuture;
2022
import java.util.concurrent.atomic.AtomicLong;
2123

2224
import org.junit.jupiter.api.AfterEach;
2325
import org.junit.jupiter.api.BeforeEach;
2426
import org.junit.jupiter.api.Test;
27+
import reactor.core.publisher.Flux;
28+
import reactor.core.publisher.Mono;
2529

2630
import org.springframework.cache.Cache;
2731
import org.springframework.cache.CacheManager;
@@ -39,6 +43,8 @@
3943

4044
import static org.assertj.core.api.Assertions.assertThat;
4145
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
46+
import static org.mockito.ArgumentMatchers.any;
47+
import static org.mockito.ArgumentMatchers.eq;
4248
import static org.mockito.BDDMockito.given;
4349
import static org.mockito.BDDMockito.willReturn;
4450
import static org.mockito.BDDMockito.willThrow;
@@ -83,11 +89,56 @@ void getFail() {
8389
willThrow(exception).given(this.cache).get(0L);
8490

8591
Object result = this.simpleService.get(0L);
86-
verify(this.errorHandler).handleCacheGetError(exception, cache, 0L);
92+
verify(this.errorHandler).handleCacheGetError(exception, this.cache, 0L);
8793
verify(this.cache).get(0L);
8894
verify(this.cache).put(0L, result); // result of the invocation
8995
}
9096

97+
@Test
98+
public void getSyncFail() {
99+
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
100+
willThrow(exception).given(this.cache).get(eq(0L), any(Callable.class));
101+
102+
Object result = this.simpleService.getSync(0L);
103+
assertThat(result).isEqualTo(0L);
104+
verify(this.errorHandler).handleCacheGetError(exception, this.cache, 0L);
105+
verify(this.cache).get(eq(0L), any(Callable.class));
106+
}
107+
108+
@Test
109+
public void getCompletableFutureFail() {
110+
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
111+
willThrow(exception).given(this.cache).retrieve(eq(0L));
112+
113+
Object result = this.simpleService.getFuture(0L).join();
114+
assertThat(result).isEqualTo(0L);
115+
verify(this.errorHandler).handleCacheGetError(exception, this.cache, 0L);
116+
verify(this.cache).retrieve(eq(0L));
117+
}
118+
119+
@Test
120+
public void getMonoFail() {
121+
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
122+
willThrow(exception).given(this.cache).retrieve(eq(0L));
123+
124+
Object result = this.simpleService.getMono(0L).block();
125+
assertThat(result).isEqualTo(0L);
126+
verify(this.errorHandler).handleCacheGetError(exception, this.cache, 0L);
127+
verify(this.cache).retrieve(eq(0L));
128+
}
129+
130+
131+
@Test
132+
public void getFluxFail() {
133+
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
134+
willThrow(exception).given(this.cache).retrieve(eq(0L));
135+
136+
Object result = this.simpleService.getFlux(0L).blockLast();
137+
assertThat(result).isEqualTo(0L);
138+
verify(this.errorHandler).handleCacheGetError(exception, this.cache, 0L);
139+
verify(this.cache).retrieve(eq(0L));
140+
}
141+
91142
@Test
92143
void getAndPutFail() {
93144
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
@@ -220,6 +271,26 @@ public Object get(long id) {
220271
return this.counter.getAndIncrement();
221272
}
222273

274+
@Cacheable(sync = true)
275+
public Object getSync(long id) {
276+
return this.counter.getAndIncrement();
277+
}
278+
279+
@Cacheable
280+
public CompletableFuture<Long> getFuture(long id) {
281+
return CompletableFuture.completedFuture(this.counter.getAndIncrement());
282+
}
283+
284+
@Cacheable
285+
public Mono<Long> getMono(long id) {
286+
return Mono.just(this.counter.getAndIncrement());
287+
}
288+
289+
@Cacheable
290+
public Flux<Long> getFlux(long id) {
291+
return Flux.just(this.counter.getAndIncrement(), 0L);
292+
}
293+
223294
@CachePut
224295
public Object put(long id) {
225296
return this.counter.getAndIncrement();

0 commit comments

Comments
 (0)