Decouple exception messages for sync=true from @Cacheable
This commit is contained in:
parent
038dda97f8
commit
bbcc788f60
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2022 the original author or authors.
|
* Copyright 2002-2023 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -177,9 +177,9 @@ public @interface Cacheable {
|
||||||
* <li>Only one cache may be specified</li>
|
* <li>Only one cache may be specified</li>
|
||||||
* <li>No other cache-related operation can be combined</li>
|
* <li>No other cache-related operation can be combined</li>
|
||||||
* </ol>
|
* </ol>
|
||||||
* This is effectively a hint and the actual cache provider that you are
|
* This is effectively a hint and the chosen cache provider might not actually
|
||||||
* using may not support it in a synchronized fashion. Check your provider
|
* support it in a synchronized fashion. Check your provider documentation for
|
||||||
* documentation for more details on the actual semantics.
|
* more details on the actual semantics.
|
||||||
* @since 4.3
|
* @since 4.3
|
||||||
* @see org.springframework.cache.Cache#get(Object, Callable)
|
* @see org.springframework.cache.Cache#get(Object, Callable)
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -214,7 +214,7 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
|
||||||
@Override
|
@Override
|
||||||
public void afterSingletonsInstantiated() {
|
public void afterSingletonsInstantiated() {
|
||||||
if (getCacheResolver() == null) {
|
if (getCacheResolver() == null) {
|
||||||
// Lazily initialize cache resolver via default cache manager...
|
// Lazily initialize cache resolver via default cache manager
|
||||||
Assert.state(this.beanFactory != null, "CacheResolver or BeanFactory must be set on cache aspect");
|
Assert.state(this.beanFactory != null, "CacheResolver or BeanFactory must be set on cache aspect");
|
||||||
try {
|
try {
|
||||||
setCacheManager(this.beanFactory.getBean(CacheManager.class));
|
setCacheManager(this.beanFactory.getBean(CacheManager.class));
|
||||||
|
@ -307,22 +307,22 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return a bean with the specified name and type. Used to resolve services that
|
* Retrieve a bean with the specified name and type.
|
||||||
* are referenced by name in a {@link CacheOperation}.
|
* Used to resolve services that are referenced by name in a {@link CacheOperation}.
|
||||||
* @param beanName the name of the bean, as defined by the operation
|
* @param name the name of the bean, as defined by the cache operation
|
||||||
* @param expectedType type for the bean
|
* @param serviceType the type expected by the operation's service reference
|
||||||
* @return the bean matching that name
|
* @return the bean matching the expected type, qualified by the given name
|
||||||
* @throws org.springframework.beans.factory.NoSuchBeanDefinitionException if such bean does not exist
|
* @throws org.springframework.beans.factory.NoSuchBeanDefinitionException if such bean does not exist
|
||||||
* @see CacheOperation#getKeyGenerator()
|
* @see CacheOperation#getKeyGenerator()
|
||||||
* @see CacheOperation#getCacheManager()
|
* @see CacheOperation#getCacheManager()
|
||||||
* @see CacheOperation#getCacheResolver()
|
* @see CacheOperation#getCacheResolver()
|
||||||
*/
|
*/
|
||||||
protected <T> T getBean(String beanName, Class<T> expectedType) {
|
protected <T> T getBean(String name, Class<T> serviceType) {
|
||||||
if (this.beanFactory == null) {
|
if (this.beanFactory == null) {
|
||||||
throw new IllegalStateException(
|
throw new IllegalStateException(
|
||||||
"BeanFactory must be set on cache aspect for " + expectedType.getSimpleName() + " retrieval");
|
"BeanFactory must be set on cache aspect for " + serviceType.getSimpleName() + " retrieval");
|
||||||
}
|
}
|
||||||
return BeanFactoryAnnotationUtils.qualifiedBeanOfType(this.beanFactory, expectedType, beanName);
|
return BeanFactoryAnnotationUtils.qualifiedBeanOfType(this.beanFactory, serviceType, name);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -388,12 +388,11 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
// No caching required, only call the underlying method
|
// No caching required, just call the underlying method
|
||||||
return invokeOperation(invoker);
|
return invokeOperation(invoker);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
// Process any early evictions
|
// Process any early evictions
|
||||||
processCacheEvicts(contexts.get(CacheEvictOperation.class), true,
|
processCacheEvicts(contexts.get(CacheEvictOperation.class), true,
|
||||||
CacheOperationExpressionEvaluator.NO_RESULT);
|
CacheOperationExpressionEvaluator.NO_RESULT);
|
||||||
|
@ -641,21 +640,21 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
|
||||||
if (syncEnabled) {
|
if (syncEnabled) {
|
||||||
if (this.contexts.size() > 1) {
|
if (this.contexts.size() > 1) {
|
||||||
throw new IllegalStateException(
|
throw new IllegalStateException(
|
||||||
"@Cacheable(sync=true) cannot be combined with other cache operations on '" + method + "'");
|
"A sync=true operation cannot be combined with other cache operations on '" + method + "'");
|
||||||
}
|
}
|
||||||
if (cacheOperationContexts.size() > 1) {
|
if (cacheOperationContexts.size() > 1) {
|
||||||
throw new IllegalStateException(
|
throw new IllegalStateException(
|
||||||
"Only one @Cacheable(sync=true) entry is allowed on '" + method + "'");
|
"Only one sync=true operation is allowed on '" + method + "'");
|
||||||
}
|
}
|
||||||
CacheOperationContext cacheOperationContext = cacheOperationContexts.iterator().next();
|
CacheOperationContext cacheOperationContext = cacheOperationContexts.iterator().next();
|
||||||
CacheableOperation operation = (CacheableOperation) cacheOperationContext.getOperation();
|
CacheOperation operation = cacheOperationContext.getOperation();
|
||||||
if (cacheOperationContext.getCaches().size() > 1) {
|
if (cacheOperationContext.getCaches().size() > 1) {
|
||||||
throw new IllegalStateException(
|
throw new IllegalStateException(
|
||||||
"@Cacheable(sync=true) only allows a single cache on '" + operation + "'");
|
"A sync=true operation is restricted to a single cache on '" + operation + "'");
|
||||||
}
|
}
|
||||||
if (StringUtils.hasText(operation.getUnless())) {
|
if (operation instanceof CacheableOperation cacheable && StringUtils.hasText(cacheable.getUnless())) {
|
||||||
throw new IllegalStateException(
|
throw new IllegalStateException(
|
||||||
"@Cacheable(sync=true) does not support unless attribute on '" + operation + "'");
|
"A sync=true operation does not support the unless attribute on '" + operation + "'");
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
@ -884,13 +883,13 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Internal holder class for recording that a cache method was invoked.
|
* Internal holder class for recording that a cache method was invoked.
|
||||||
*/
|
*/
|
||||||
private static class InvocationAwareResult {
|
private static class InvocationAwareResult {
|
||||||
|
|
||||||
boolean invoked;
|
boolean invoked;
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2021 the original author or authors.
|
* Copyright 2002-2023 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -48,8 +48,9 @@ public class CacheSyncFailureTests {
|
||||||
|
|
||||||
private SimpleService simpleService;
|
private SimpleService simpleService;
|
||||||
|
|
||||||
|
|
||||||
@BeforeEach
|
@BeforeEach
|
||||||
public void setUp() {
|
public void setup() {
|
||||||
this.context = new AnnotationConfigApplicationContext(Config.class);
|
this.context = new AnnotationConfigApplicationContext(Config.class);
|
||||||
this.simpleService = this.context.getBean(SimpleService.class);
|
this.simpleService = this.context.getBean(SimpleService.class);
|
||||||
}
|
}
|
||||||
|
@ -61,39 +62,40 @@ public class CacheSyncFailureTests {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void unlessSync() {
|
public void unlessSync() {
|
||||||
assertThatIllegalStateException().isThrownBy(() ->
|
assertThatIllegalStateException()
|
||||||
this.simpleService.unlessSync("key"))
|
.isThrownBy(() -> this.simpleService.unlessSync("key"))
|
||||||
.withMessageContaining("@Cacheable(sync=true) does not support unless attribute");
|
.withMessageContaining("A sync=true operation does not support the unless attribute");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void severalCachesSync() {
|
public void severalCachesSync() {
|
||||||
assertThatIllegalStateException().isThrownBy(() ->
|
assertThatIllegalStateException()
|
||||||
this.simpleService.severalCachesSync("key"))
|
.isThrownBy(() -> this.simpleService.severalCachesSync("key"))
|
||||||
.withMessageContaining("@Cacheable(sync=true) only allows a single cache");
|
.withMessageContaining("A sync=true operation is restricted to a single cache");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void severalCachesWithResolvedSync() {
|
public void severalCachesWithResolvedSync() {
|
||||||
assertThatIllegalStateException().isThrownBy(() ->
|
assertThatIllegalStateException()
|
||||||
this.simpleService.severalCachesWithResolvedSync("key"))
|
.isThrownBy(() -> this.simpleService.severalCachesWithResolvedSync("key"))
|
||||||
.withMessageContaining("@Cacheable(sync=true) only allows a single cache");
|
.withMessageContaining("A sync=true operation is restricted to a single cache");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void syncWithAnotherOperation() {
|
public void syncWithAnotherOperation() {
|
||||||
assertThatIllegalStateException().isThrownBy(() ->
|
assertThatIllegalStateException()
|
||||||
this.simpleService.syncWithAnotherOperation("key"))
|
.isThrownBy(() -> this.simpleService.syncWithAnotherOperation("key"))
|
||||||
.withMessageContaining("@Cacheable(sync=true) cannot be combined with other cache operations");
|
.withMessageContaining("A sync=true operation cannot be combined with other cache operations");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void syncWithTwoGetOperations() {
|
public void syncWithTwoGetOperations() {
|
||||||
assertThatIllegalStateException().isThrownBy(() ->
|
assertThatIllegalStateException()
|
||||||
this.simpleService.syncWithTwoGetOperations("key"))
|
.isThrownBy(() -> this.simpleService.syncWithTwoGetOperations("key"))
|
||||||
.withMessageContaining("Only one @Cacheable(sync=true) entry is allowed");
|
.withMessageContaining("Only one sync=true operation is allowed");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -131,6 +133,7 @@ public class CacheSyncFailureTests {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@Configuration
|
@Configuration
|
||||||
@EnableCaching
|
@EnableCaching
|
||||||
static class Config implements CachingConfigurer {
|
static class Config implements CachingConfigurer {
|
||||||
|
|
Loading…
Reference in New Issue