Skip to content

Commit

Permalink
Hacking
Browse files Browse the repository at this point in the history
  • Loading branch information
snicoll committed Jun 19, 2019
1 parent 73c2eb1 commit 68e3656
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ private Object execute(final CacheOperationInvoker invoker, Method method, Cache
Object key = generateKey(context, CacheOperationExpressionEvaluator.NO_RESULT);
Cache cache = context.getCaches().iterator().next();
try {
return wrapCacheValue(method, cache.get(key, () -> unwrapReturnValue(invokeOperation(invoker))));
return wrapCacheValue(method, doGetSync(invoker, cache, key));
}
catch (Cache.ValueRetrievalException ex) {
// The invoker wraps any Throwable in a ThrowableWrapper instance so we
Expand Down Expand Up @@ -435,6 +435,23 @@ private Object execute(final CacheOperationInvoker invoker, Method method, Cache
return returnValue;
}

@Nullable
private Object doGetSync(CacheOperationInvoker invoker, Cache cache, Object key) {
try {
return cache.get(key, () -> unwrapReturnValue(invokeOperation(invoker)));
}
catch (RuntimeException ex) {
// Make sure method invocation exceptions are not managed by the error handler
if (ex instanceof Cache.ValueRetrievalException) {
throw ex;
}
getErrorHandler().handleCacheGetError(ex, cache, key);

This comment has been minimized.

Copy link
@chrisirhc

chrisirhc Nov 25, 2023

One possible improvement might be to wrap this exception in a new SynchronizedCacheException type. So that error handlers can treat this new behavior differently/accordingly.
Another potential thing to consider is to add a way to opt-in to either the new behaviour so de-risk existing deployments.

// If the exception is handled, we're just invoking the method and ignore
// the cache infrastructure failure
return unwrapReturnValue(invokeOperation(invoker));
}
}

@Nullable
private Object wrapCacheValue(Method method, @Nullable Object cacheValue) {
if (method.getReturnType() == Optional.class &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.cache.interceptor;

import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicLong;

import org.junit.Before;
Expand All @@ -38,6 +39,8 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.willReturn;
import static org.mockito.BDDMockito.willThrow;
Expand Down Expand Up @@ -77,6 +80,17 @@ public void getFail() {
verify(this.cache).put(0L, result); // result of the invocation
}

@Test
public void getSyncFail() {
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
willThrow(exception).given(this.cache).get(eq(0L), any(Callable.class));

Object result = this.simpleService.getSync(0L);
assertThat(result).isEqualTo(0L);
verify(this.errorHandler).handleCacheGetError(exception, cache, 0L);
verify(this.cache).get(eq(0L), any(Callable.class));
}

@Test
public void getAndPutFail() {
UnsupportedOperationException exception = new UnsupportedOperationException("Test exception on get");
Expand Down Expand Up @@ -208,6 +222,11 @@ public Object get(long id) {
return this.counter.getAndIncrement();
}

@Cacheable(sync = true)
public Object getSync(long id) {
return this.counter.getAndIncrement();
}

@CachePut
public Object put(long id) {
return this.counter.getAndIncrement();
Expand Down

0 comments on commit 68e3656

Please sign in to comment.