Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow cache creation from CacheName annotation #22885

Merged
merged 1 commit into from
Jan 14, 2022
Merged

Allow cache creation from CacheName annotation #22885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 18 additions & 36 deletions extensions/cache/deployment/src/main/java/io/quarkus/cache/deployment/CacheProcessor.java
View file
Open in desktop
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
Open in desktop

This file was deleted.

11 changes: 1 addition & 10 deletions ...e/deployment/src/test/java/io/quarkus/cache/test/deployment/DeploymentExceptionsTest.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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