From b524539987e0966e0a81ce11e356baebfd18b373 Mon Sep 17 00:00:00 2001 From: Gwenneg Lepage Date: Fri, 14 Jan 2022 00:29:06 +0100 Subject: [PATCH] Allow cache creation from CacheName annotation --- .../cache/deployment/CacheProcessor.java | 54 +++++++------------ .../exception/UnknownCacheNameException.java | 26 --------- .../deployment/DeploymentExceptionsTest.java | 11 +--- .../cache/test/runtime/CacheNamesTest.java | 13 +++-- .../test/runtime/ProgrammaticApiTest.java | 25 +++++---- 5 files changed, 45 insertions(+), 84 deletions(-) delete mode 100644 extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/exception/UnknownCacheNameException.java diff --git a/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java b/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java index ac40c4d63782d..81621cd917c64 100644 --- a/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java +++ b/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java @@ -15,6 +15,7 @@ import static io.quarkus.cache.deployment.CacheDeploymentConstants.REGISTER_REST_CLIENT; import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT; import static io.quarkus.runtime.metrics.MetricsFactory.MICROMETER; +import static org.jboss.jandex.AnnotationTarget.Kind.METHOD; import java.lang.reflect.Modifier; import java.util.ArrayList; @@ -44,7 +45,6 @@ import io.quarkus.cache.CacheManager; import io.quarkus.cache.deployment.exception.ClassTargetException; import io.quarkus.cache.deployment.exception.PrivateMethodTargetException; -import io.quarkus.cache.deployment.exception.UnknownCacheNameException; import io.quarkus.cache.deployment.exception.UnsupportedRepeatedAnnotationException; import io.quarkus.cache.deployment.exception.VoidReturnTypeTargetException; import io.quarkus.cache.runtime.CacheInvalidateAllInterceptor; @@ -102,7 +102,7 @@ void validateCacheAnnotationsAndProduceCacheNames(CombinedIndexBuildItem combine for (DotName bindingName : INTERCEPTOR_BINDINGS) { for (AnnotationInstance binding : combinedIndex.getIndex().getAnnotations(bindingName)) { throwables.addAll(validateInterceptorBindingTarget(binding, binding.target())); - if (binding.target().kind() == Kind.METHOD) { + if (binding.target().kind() == METHOD) { /* * Cache names from the interceptor bindings placed on cache interceptors must not be collected to prevent * the instantiation of a cache with an empty name. @@ -124,7 +124,7 @@ void validateCacheAnnotationsAndProduceCacheNames(CombinedIndexBuildItem combine * Client. Using repeated interceptor bindings on a method from a class annotated with @RegisterRestClient must * therefore be forbidden. */ - if (container.target().kind() == Kind.METHOD) { + if (container.target().kind() == METHOD) { MethodInfo methodInfo = container.target().asMethod(); if (methodInfo.declaringClass().classAnnotation(REGISTER_REST_CLIENT) != null) { throwables.add(new UnsupportedRepeatedAnnotationException(methodInfo)); @@ -133,43 +133,25 @@ void validateCacheAnnotationsAndProduceCacheNames(CombinedIndexBuildItem combine } } - /* - * Before @CacheName can be validated, additional cache names provided by other extensions must be added to the cache - * names collection built above. - */ - for (AdditionalCacheNameBuildItem additionalCacheName : additionalCacheNames) { - names.add(additionalCacheName.getName()); - } - - // @CacheName can now be validated. + // Let's also collect the cache names from the @CacheName annotations. for (AnnotationInstance qualifier : combinedIndex.getIndex().getAnnotations(CACHE_NAME)) { - String cacheName = qualifier.value().asString(); - AnnotationTarget target = qualifier.target(); - switch (target.kind()) { - case FIELD: - if (!names.contains(cacheName)) { - ClassInfo declaringClass = target.asField().declaringClass(); - throwables.add(new UnknownCacheNameException(declaringClass.name(), cacheName)); - } - break; - case METHOD: - /* - * This should only happen in CacheProducer. It'd be nice if we could forbid using @CacheName in any other - * class, but Arc throws an AmbiguousResolutionException before we get a chance to validate things here. - */ - break; - case METHOD_PARAMETER: - if (!names.contains(cacheName)) { - ClassInfo declaringClass = target.asMethodParameter().method().declaringClass(); - throwables.add(new UnknownCacheNameException(declaringClass.name(), cacheName)); - } - break; - default: - // This should never be thrown. - throw new DeploymentException("Unexpected @CacheName target: " + target.kind()); + // The @CacheName annotation from CacheProducer must be ignored. + if (qualifier.target().kind() == METHOD) { + /* + * This should only happen in CacheProducer. It'd be nice if we could forbid using @CacheName on a method in + * any other class, but Arc throws an AmbiguousResolutionException before we get a chance to validate things + * here. + */ + } else { + names.add(qualifier.value().asString()); } } + // Finally, additional cache names provided by other extensions must be added to the cache names collection. + for (AdditionalCacheNameBuildItem additionalCacheName : additionalCacheNames) { + names.add(additionalCacheName.getName()); + } + validationErrors.produce(new ValidationErrorBuildItem(throwables.toArray(new Throwable[0]))); cacheNames.produce(new CacheNamesBuildItem(names)); } diff --git a/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/exception/UnknownCacheNameException.java b/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/exception/UnknownCacheNameException.java deleted file mode 100644 index a1e91a78b4421..0000000000000 --- a/extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/exception/UnknownCacheNameException.java +++ /dev/null @@ -1,26 +0,0 @@ -package io.quarkus.cache.deployment.exception; - -import org.jboss.jandex.DotName; - -/** - * This exception is thrown at build time during the validation phase if a field, a constructor or a method parameter annotated - * with {@link io.quarkus.cache.CacheName @CacheName} is referencing an unknown cache name. A cache name is unknown if it is not - * used in any {@link io.quarkus.cache.CacheInvalidate @CacheInvalidate}, - * {@link io.quarkus.cache.CacheInvalidateAll @CacheInvalidateAll} or {@link io.quarkus.cache.CacheResult @CacheResult} - * annotation. - */ -@SuppressWarnings("serial") -public class UnknownCacheNameException extends RuntimeException { - - private final String cacheName; - - public UnknownCacheNameException(DotName className, String cacheName) { - super("A field or method parameter is annotated with a @CacheName annotation referencing an unknown cache name [class=" - + className + ", cacheName=" + cacheName + "]"); - this.cacheName = cacheName; - } - - public String getCacheName() { - return cacheName; - } -} diff --git a/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/deployment/DeploymentExceptionsTest.java b/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/deployment/DeploymentExceptionsTest.java index 4b026b31e7eb3..eb85a0da37dca 100644 --- a/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/deployment/DeploymentExceptionsTest.java +++ b/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/deployment/DeploymentExceptionsTest.java @@ -21,7 +21,6 @@ import io.quarkus.cache.CacheResult; import io.quarkus.cache.deployment.exception.ClassTargetException; import io.quarkus.cache.deployment.exception.PrivateMethodTargetException; -import io.quarkus.cache.deployment.exception.UnknownCacheNameException; import io.quarkus.cache.deployment.exception.VoidReturnTypeTargetException; import io.quarkus.test.QuarkusUnitTest; @@ -39,15 +38,12 @@ public class DeploymentExceptionsTest { .withApplicationRoot((jar) -> jar.addClasses(TestResource.class, TestBean.class)) .assertException(t -> { assertEquals(DeploymentException.class, t.getClass()); - assertEquals(10, t.getSuppressed().length); + assertEquals(7, t.getSuppressed().length); assertPrivateMethodTargetException(t, "shouldThrowPrivateMethodTargetException", 1); assertPrivateMethodTargetException(t, "shouldAlsoThrowPrivateMethodTargetException", 2); assertVoidReturnTypeTargetException(t, "showThrowVoidReturnTypeTargetException"); assertClassTargetException(t, TestResource.class, 1); assertClassTargetException(t, TestBean.class, 2); - assertUnknownCacheNameException(t, UNKNOWN_CACHE_1); - assertUnknownCacheNameException(t, UNKNOWN_CACHE_2); - assertUnknownCacheNameException(t, UNKNOWN_CACHE_3); }); private static void assertPrivateMethodTargetException(Throwable t, String expectedMethodName, long expectedCount) { @@ -65,11 +61,6 @@ private static void assertClassTargetException(Throwable t, Class expectedCla .filter(s -> expectedClassName.getName().equals(s.getClassName().toString())).count()); } - private static void assertUnknownCacheNameException(Throwable t, String expectedCacheName) { - assertEquals(1, filterSuppressed(t, UnknownCacheNameException.class) - .filter(s -> expectedCacheName.equals(s.getCacheName())).count()); - } - private static Stream filterSuppressed(Throwable t, Class filterClass) { return stream(t.getSuppressed()).filter(filterClass::isInstance).map(filterClass::cast); } diff --git a/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/CacheNamesTest.java b/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/CacheNamesTest.java index d32e03aba8909..ca592c53b6f5a 100644 --- a/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/CacheNamesTest.java +++ b/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/CacheNamesTest.java @@ -15,10 +15,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.cache.Cache; import io.quarkus.cache.CacheInvalidate; import io.quarkus.cache.CacheInvalidateAll; import io.quarkus.cache.CacheKey; import io.quarkus.cache.CacheManager; +import io.quarkus.cache.CacheName; import io.quarkus.cache.CacheResult; import io.quarkus.test.QuarkusUnitTest; @@ -27,6 +29,7 @@ public class CacheNamesTest { private static final String CACHE_NAME_1 = "test-cache-1"; private static final String CACHE_NAME_2 = "test-cache-2"; private static final String CACHE_NAME_3 = "test-cache-3"; + private static final String CACHE_NAME_4 = "test-cache-4"; @RegisterExtension static final QuarkusUnitTest TEST = new QuarkusUnitTest() @@ -35,15 +38,19 @@ public class CacheNamesTest { @Inject CacheManager cacheManager; + @CacheName(CACHE_NAME_4) + Cache cache; + @Test public void testCacheNamesCollection() { /* * The main goal of this test is to check that a cache with an empty name is not instantiated at build time because of - * the bindings with an empty `cacheName` parameter from the cache interceptors. + * the bindings with an empty `cacheName` parameter from the cache interceptors or because of the @CacheName annotation + * in CacheProducer. */ List cacheNames = new ArrayList<>(cacheManager.getCacheNames()); - assertEquals(3, cacheNames.size()); - assertTrue(cacheNames.containsAll(Arrays.asList(CACHE_NAME_1, CACHE_NAME_2, CACHE_NAME_3))); + assertEquals(4, cacheNames.size()); + assertTrue(cacheNames.containsAll(Arrays.asList(CACHE_NAME_1, CACHE_NAME_2, CACHE_NAME_3, CACHE_NAME_4))); } @ApplicationScoped diff --git a/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/ProgrammaticApiTest.java b/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/ProgrammaticApiTest.java index fc49f992ce6b3..1fd72297277f3 100644 --- a/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/ProgrammaticApiTest.java +++ b/extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/ProgrammaticApiTest.java @@ -7,6 +7,7 @@ import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Set; import javax.enterprise.context.Dependent; @@ -27,7 +28,8 @@ public class ProgrammaticApiTest { - private static final String CACHE_NAME = "test-cache"; + private static final String CACHE_NAME_1 = "test-cache-1"; + private static final String CACHE_NAME_2 = "test-cache-2"; private static final Object KEY_1 = new Object(); private static final Object KEY_2 = new Object(); @@ -40,16 +42,21 @@ public class ProgrammaticApiTest { @Inject CacheManager cacheManager; - @CacheName(CACHE_NAME) + @CacheName(CACHE_NAME_1) Cache cache; + @CacheName(CACHE_NAME_2) + Cache anotherCache; + @Test public void testInjection() { - assertTrue(cacheManager.getCacheNames().contains(CACHE_NAME)); + assertTrue(cacheManager.getCacheNames().containsAll(List.of(CACHE_NAME_1, CACHE_NAME_2))); assertSame(CaffeineCacheImpl.class, cache.getClass()); - assertSame(cache, cacheManager.getCache(CACHE_NAME).get()); + assertSame(CaffeineCacheImpl.class, anotherCache.getClass()); + assertSame(cache, cacheManager.getCache(CACHE_NAME_1).get()); assertSame(cache, cachedService.getConstructorInjectedCache()); assertSame(cache, cachedService.getMethodInjectedCache()); + assertNotSame(cache, anotherCache); } @Test @@ -146,7 +153,7 @@ static class CachedService { Cache constructorInjectedCache; Cache methodInjectedCache; - public CachedService(@CacheName(CACHE_NAME) Cache cache) { + public CachedService(@CacheName(CACHE_NAME_1) Cache cache) { constructorInjectedCache = cache; } @@ -159,20 +166,20 @@ public Cache getMethodInjectedCache() { } @Inject - public void setMethodInjectedCache(@CacheName(CACHE_NAME) Cache cache) { + public void setMethodInjectedCache(@CacheName(CACHE_NAME_1) Cache cache) { methodInjectedCache = cache; } - @CacheResult(cacheName = CACHE_NAME) + @CacheResult(cacheName = CACHE_NAME_1) public String cachedMethod(Object key) { return new String(); } - @CacheInvalidate(cacheName = CACHE_NAME) + @CacheInvalidate(cacheName = CACHE_NAME_1) public void invalidate(Object key) { } - @CacheInvalidateAll(cacheName = CACHE_NAME) + @CacheInvalidateAll(cacheName = CACHE_NAME_1) public void invalidateAll() { } }