Skip to content

Commit

Permalink
Merge pull request #22885 from gwenneg/issue-22538-cachename-cache-cr…
Browse files Browse the repository at this point in the history 
…eation

Allow cache creation from CacheName annotation
  • Loading branch information
@gwenneg
gwenneg authored Jan 14, 2022
2 parents 92be529 + b524539 commit 4bd4e90
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 84 deletions.
54 changes: 18 additions & 36 deletions extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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));
Expand All @@ -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));
}
Expand Down
26 changes: 0 additions & 26 deletions ...oyment/src/main/java/io/quarkus/cache/deployment/exception/UnknownCacheNameException.java
View file

This file was deleted.

11 changes: 1 addition & 10 deletions ...e/deployment/src/test/java/io/quarkus/cache/test/deployment/DeploymentExceptionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand All @@ -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 <T extends RuntimeException> Stream<T> filterSuppressed(Throwable t, Class<T> filterClass) {
return stream(t.getSuppressed()).filter(filterClass::isInstance).map(filterClass::cast);
}
Expand Down
13 changes: 10 additions & 3 deletions extensions/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/CacheNamesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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()
Expand All @@ -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<String> 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
Expand Down
25 changes: 16 additions & 9 deletions ...ons/cache/deployment/src/test/java/io/quarkus/cache/test/runtime/ProgrammaticApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();

Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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() {
}
}
Expand Down

0 comments on commit 4bd4e90

Please sign in to comment.