Skip to content

Commit 4e5979c

Browse files
jhoellerjhoeller
committed
Consistent CacheErrorHandler processing for @Cacheable(sync=true)
Closes gh-34708
1 parent e7db15b commit 4e5979c

File tree

3 files changed

+286
-84
lines changed

3 files changed

+286
-84
lines changed

Diff for: ‎spring-context/src/main/java/org/springframework/cache/interceptor/AbstractCacheInvoker.java

+2-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -105,7 +105,7 @@ protected <T> T doGet(Cache cache, Object key, Callable<T> valueLoader) {
105105
return valueLoader.call();
106106
}
107107
catch (Exception ex2) {
108-
throw new RuntimeException(ex2);
108+
throw new Cache.ValueRetrievalException(key, valueLoader, ex);
109109
}
110110
}
111111
}
@@ -124,16 +124,12 @@ protected CompletableFuture<?> doRetrieve(Cache cache, Object key) {
124124
try {
125125
return cache.retrieve(key);
126126
}
127-
catch (Cache.ValueRetrievalException ex) {
128-
throw ex;
129-
}
130127
catch (RuntimeException ex) {
131128
getErrorHandler().handleCacheGetError(ex, cache, key);
132129
return null;
133130
}
134131
}
135132

136-
137133
/**
138134
* Execute {@link Cache#retrieve(Object, Supplier)} on the specified
139135
* {@link Cache} and invoke the error handler if an exception occurs.
@@ -146,9 +142,6 @@ protected <T> CompletableFuture<T> doRetrieve(Cache cache, Object key, Supplier<
146142
try {
147143
return cache.retrieve(key, valueLoader);
148144
}
149-
catch (Cache.ValueRetrievalException ex) {
150-
throw ex;
151-
}
152145
catch (RuntimeException ex) {
153146
getErrorHandler().handleCacheGetError(ex, cache, key);
154147
return valueLoader.get();

Diff for: ‎spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java

+91-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import java.util.Optional;
2727
import java.util.concurrent.CompletableFuture;
2828
import java.util.concurrent.ConcurrentHashMap;
29+
import java.util.concurrent.atomic.AtomicBoolean;
2930
import java.util.function.Supplier;
3031

3132
import org.apache.commons.logging.Log;
@@ -449,14 +450,41 @@ private Object execute(CacheOperationInvoker invoker, Method method, CacheOperat
449450
return cacheHit;
450451
}
451452

453+
@SuppressWarnings("unchecked")
452454
@Nullable
453455
private Object executeSynchronized(CacheOperationInvoker invoker, Method method, CacheOperationContexts contexts) {
454456
CacheOperationContext context = contexts.get(CacheableOperation.class).iterator().next();
455457
if (isConditionPassing(context, CacheOperationExpressionEvaluator.NO_RESULT)) {
456458
Object key = generateKey(context, CacheOperationExpressionEvaluator.NO_RESULT);
457459
Cache cache = context.getCaches().iterator().next();
458460
if (CompletableFuture.class.isAssignableFrom(method.getReturnType())) {
459-
return doRetrieve(cache, key, () -> (CompletableFuture<?>) invokeOperation(invoker));
461+
AtomicBoolean invokeFailure = new AtomicBoolean(false);
462+
CompletableFuture<?> result = doRetrieve(cache, key,
463+
() -> {
464+
CompletableFuture<?> invokeResult = ((CompletableFuture<?>) invokeOperation(invoker));
465+
if (invokeResult == null) {
466+
return null;
467+
}
468+
return invokeResult.exceptionallyCompose(ex -> {
469+
invokeFailure.set(true);
470+
return CompletableFuture.failedFuture(ex);
471+
});
472+
});
473+
return result.exceptionallyCompose(ex -> {
474+
if (!(ex instanceof RuntimeException rex)) {
475+
return CompletableFuture.failedFuture(ex);
476+
}
477+
try {
478+
getErrorHandler().handleCacheGetError(rex, cache, key);
479+
if (invokeFailure.get()) {
480+
return CompletableFuture.failedFuture(ex);
481+
}
482+
return (CompletableFuture) invokeOperation(invoker);
483+
}
484+
catch (Throwable ex2) {
485+
return CompletableFuture.failedFuture(ex2);
486+
}
487+
});
460488
}
461489
if (this.reactiveCachingHandler != null) {
462490
Object returnValue = this.reactiveCachingHandler.executeSynchronized(invoker, method, cache, key);
@@ -517,9 +545,17 @@ private Object findInCaches(CacheOperationContext context, Object key,
517545
if (CompletableFuture.class.isAssignableFrom(context.getMethod().getReturnType())) {
518546
CompletableFuture<?> result = doRetrieve(cache, key);
519547
if (result != null) {
520-
return result.exceptionally(ex -> {
521-
getErrorHandler().handleCacheGetError((RuntimeException) ex, cache, key);
522-
return null;
548+
return result.exceptionallyCompose(ex -> {
549+
if (!(ex instanceof RuntimeException rex)) {
550+
return CompletableFuture.failedFuture(ex);
551+
}
552+
try {
553+
getErrorHandler().handleCacheGetError(rex, cache, key);
554+
return CompletableFuture.completedFuture(null);
555+
}
556+
catch (Throwable ex2) {
557+
return CompletableFuture.failedFuture(ex2);
558+
}
523559
}).thenCompose(value -> (CompletableFuture<?>) evaluate(
524560
(value != null ? CompletableFuture.completedFuture(unwrapCacheValue(value)) : null),
525561
invoker, method, contexts));
@@ -1097,32 +1133,72 @@ private class ReactiveCachingHandler {
10971133

10981134
private final ReactiveAdapterRegistry registry = ReactiveAdapterRegistry.getSharedInstance();
10991135

1136+
@SuppressWarnings({"rawtypes", "unchecked"})
11001137
@Nullable
11011138
public Object executeSynchronized(CacheOperationInvoker invoker, Method method, Cache cache, Object key) {
1139+
AtomicBoolean invokeFailure = new AtomicBoolean(false);
11021140
ReactiveAdapter adapter = this.registry.getAdapter(method.getReturnType());
11031141
if (adapter != null) {
11041142
if (adapter.isMultiValue()) {
11051143
// Flux or similar
11061144
return adapter.fromPublisher(Flux.from(Mono.fromFuture(
1107-
cache.retrieve(key,
1108-
() -> Flux.from(adapter.toPublisher(invokeOperation(invoker))).collectList().toFuture())))
1109-
.flatMap(Flux::fromIterable));
1145+
doRetrieve(cache, key,
1146+
() -> Flux.from(adapter.toPublisher(invokeOperation(invoker))).collectList().doOnError(ex -> invokeFailure.set(true)).toFuture())))
1147+
.flatMap(Flux::fromIterable)
1148+
.onErrorResume(RuntimeException.class, ex -> {
1149+
try {
1150+
getErrorHandler().handleCacheGetError(ex, cache, key);
1151+
if (invokeFailure.get()) {
1152+
return Flux.error(ex);
1153+
}
1154+
return Flux.from(adapter.toPublisher(invokeOperation(invoker)));
1155+
}
1156+
catch (RuntimeException exception) {
1157+
return Flux.error(exception);
1158+
}
1159+
}));
11101160
}
11111161
else {
11121162
// Mono or similar
11131163
return adapter.fromPublisher(Mono.fromFuture(
1114-
cache.retrieve(key,
1115-
() -> Mono.from(adapter.toPublisher(invokeOperation(invoker))).toFuture())));
1164+
doRetrieve(cache, key,
1165+
() -> Mono.from(adapter.toPublisher(invokeOperation(invoker))).doOnError(ex -> invokeFailure.set(true)).toFuture()))
1166+
.onErrorResume(RuntimeException.class, ex -> {
1167+
try {
1168+
getErrorHandler().handleCacheGetError(ex, cache, key);
1169+
if (invokeFailure.get()) {
1170+
return Mono.error(ex);
1171+
}
1172+
return Mono.from(adapter.toPublisher(invokeOperation(invoker)));
1173+
}
1174+
catch (RuntimeException exception) {
1175+
return Mono.error(exception);
1176+
}
1177+
}));
11161178
}
11171179
}
11181180
if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isSuspendingFunction(method)) {
1119-
return Mono.fromFuture(cache.retrieve(key, () -> {
1120-
Mono<?> mono = ((Mono<?>) invokeOperation(invoker));
1121-
if (mono == null) {
1181+
return Mono.fromFuture(doRetrieve(cache, key, () -> {
1182+
Mono<?> mono = (Mono<?>) invokeOperation(invoker);
1183+
if (mono != null) {
1184+
mono = mono.doOnError(ex -> invokeFailure.set(true));
1185+
}
1186+
else {
11221187
mono = Mono.empty();
11231188
}
11241189
return mono.toFuture();
1125-
}));
1190+
})).onErrorResume(RuntimeException.class, ex -> {
1191+
try {
1192+
getErrorHandler().handleCacheGetError(ex, cache, key);
1193+
if (invokeFailure.get()) {
1194+
return Mono.error(ex);
1195+
}
1196+
return (Mono) invokeOperation(invoker);
1197+
}
1198+
catch (RuntimeException exception) {
1199+
return Mono.error(exception);
1200+
}
1201+
});
11261202
}
11271203
return NOT_HANDLED;
11281204
}
@@ -1137,7 +1213,7 @@ public Object processCacheEvicts(List<CacheOperationContext> contexts, @Nullable
11371213
return NOT_HANDLED;
11381214
}
11391215

1140-
@SuppressWarnings({ "unchecked", "rawtypes" })
1216+
@SuppressWarnings({"rawtypes", "unchecked"})
11411217
@Nullable
11421218
public Object findInCaches(CacheOperationContext context, Cache cache, Object key,
11431219
CacheOperationInvoker invoker, Method method, CacheOperationContexts contexts) {

0 commit comments

Comments
 (0)