From 9492d8827011546b8db2e5fc0b5686607b9c1990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Mon, 29 Apr 2024 11:48:52 +0200 Subject: [PATCH] Stop wrapping low-level exceptions in CacheAspectSupport initialization This commit replaces the IllegalStateException thrown in CacheAspectSupport when a CacheManager cannot be determined. These were added to provide a dedicated error message, but it is possible to do so without hiding the more adequate exception type. Closes gh-22442 --- .../AspectJEnableCachingIsolatedTests.java | 11 +++++++---- .../cache/interceptor/CacheAspectSupport.java | 13 +++++++++---- .../cache/config/EnableCachingTests.java | 18 +++++++++--------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java b/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java index 75647051d42..df8e1d3588a 100644 --- a/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java +++ b/spring-aspects/src/test/java/org/springframework/cache/aspectj/AspectJEnableCachingIsolatedTests.java @@ -20,6 +20,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.cache.CacheManager; import org.springframework.cache.annotation.CachingConfigurer; import org.springframework.cache.annotation.EnableCaching; @@ -91,8 +93,9 @@ class AspectJEnableCachingIsolatedTests { try { load(MultiCacheManagerConfig.class); } - catch (IllegalStateException ex) { - assertThat(ex.getMessage()).contains("bean of type CacheManager"); + catch (NoUniqueBeanDefinitionException ex) { + assertThat(ex.getMessage()).contains( + "no CacheResolver specified and expected a single CacheManager bean, but found 2: [cm1,cm2]"); } } @@ -116,8 +119,8 @@ class AspectJEnableCachingIsolatedTests { try { load(EmptyConfig.class); } - catch (IllegalStateException ex) { - assertThat(ex.getMessage()).contains("no bean of type CacheManager"); + catch (NoSuchBeanDefinitionException ex) { + assertThat(ex.getMessage()).contains("no CacheResolver specified"); } } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index 75f2293e7e7..475f6e47b94 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -270,12 +270,17 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker setCacheManager(this.beanFactory.getBean(CacheManager.class)); } catch (NoUniqueBeanDefinitionException ex) { - throw new IllegalStateException("No CacheResolver specified, and no unique bean of type " + - "CacheManager found. Mark one as primary or declare a specific CacheManager to use.", ex); + StringBuilder message = new StringBuilder("no CacheResolver specified and expected a single CacheManager bean, but found "); + message.append(ex.getNumberOfBeansFound()); + if (ex.getBeanNamesFound() != null) { + message.append(": [").append(StringUtils.collectionToCommaDelimitedString(ex.getBeanNamesFound())).append("]"); + } + message.append(" - mark one as primary or declare a specific CacheManager to use."); + throw new NoUniqueBeanDefinitionException(CacheManager.class, ex.getNumberOfBeansFound(), message.toString()); } catch (NoSuchBeanDefinitionException ex) { - throw new IllegalStateException("No CacheResolver specified, and no bean of type CacheManager found. " + - "Register a CacheManager bean or remove the @EnableCaching annotation from your configuration.", ex); + throw new NoSuchBeanDefinitionException(CacheManager.class, "no CacheResolver specified - " + + "register a CacheManager bean or remove the @EnableCaching annotation from your configuration."); } } this.initialized = true; diff --git a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java index 28475925ba5..8762ec2d41a 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -89,13 +89,12 @@ class EnableCachingTests extends AbstractCacheAnnotationTests { @Test void multipleCacheManagerBeans() { - @SuppressWarnings("resource") AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(MultiCacheManagerConfig.class); assertThatThrownBy(ctx::refresh) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("no unique bean of type CacheManager") - .hasCauseInstanceOf(NoUniqueBeanDefinitionException.class); + .isInstanceOf(NoUniqueBeanDefinitionException.class) + .hasMessageContaining("no CacheResolver specified and expected a single CacheManager bean, but found 2: [cm1,cm2]") + .hasNoCause(); } @Test @@ -117,13 +116,14 @@ class EnableCachingTests extends AbstractCacheAnnotationTests { @Test void noCacheManagerBeans() { - @SuppressWarnings("resource") AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(EmptyConfig.class); assertThatThrownBy(ctx::refresh) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("no bean of type CacheManager") - .hasCauseInstanceOf(NoSuchBeanDefinitionException.class); + .isInstanceOf(NoSuchBeanDefinitionException.class) + .hasMessageContaining("no CacheResolver specified") + .hasMessageContaining( + "register a CacheManager bean or remove the @EnableCaching annotation from your configuration.") + .hasNoCause(); } @Test