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.

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

Skip caching Multi and log WARN message at build time #16706

Merged
merged 1 commit into from
Jan 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.quarkus.cache.runtime.CacheInvalidateInterceptor;
import io.quarkus.cache.runtime.CacheKeyParameterPositions;
import io.quarkus.cache.runtime.CacheResultInterceptor;
import io.smallrye.mutiny.Multi;

public class CacheDeploymentConstants {

Expand All @@ -37,6 +38,9 @@ public class CacheDeploymentConstants {
public static final DotName REGISTER_REST_CLIENT = DotName
.createSimple("org.eclipse.microprofile.rest.client.inject.RegisterRestClient");

// Mutiny.
public static final DotName MULTI = dotName(Multi.class);

// Annotations parameters.
public static final String CACHE_NAME_PARAM = "cacheName";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static io.quarkus.cache.deployment.CacheDeploymentConstants.INTERCEPTORS;
import static io.quarkus.cache.deployment.CacheDeploymentConstants.INTERCEPTOR_BINDINGS;
import static io.quarkus.cache.deployment.CacheDeploymentConstants.INTERCEPTOR_BINDING_CONTAINERS;
import static io.quarkus.cache.deployment.CacheDeploymentConstants.MULTI;
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;
Expand All @@ -33,6 +34,7 @@
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.Type;
import org.jboss.logging.Logger;

import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
import io.quarkus.arc.deployment.AutoInjectAnnotationBuildItem;
Expand Down Expand Up @@ -65,6 +67,8 @@

class CacheProcessor {

private static final Logger LOGGER = Logger.getLogger(CacheProcessor.class);

@BuildStep
FeatureBuildItem feature() {
return new FeatureBuildItem(Feature.CACHE);
Expand Down Expand Up @@ -184,8 +188,13 @@ private List<Throwable> validateInterceptorBindingTarget(AnnotationInstance bind
if (Modifier.isPrivate(methodInfo.flags())) {
throwables.add(new PrivateMethodTargetException(methodInfo, binding.name()));
}
if (CACHE_RESULT.equals(binding.name()) && methodInfo.returnType().kind() == Type.Kind.VOID) {
throwables.add(new VoidReturnTypeTargetException(methodInfo));
if (CACHE_RESULT.equals(binding.name())) {
if (methodInfo.returnType().kind() == Type.Kind.VOID) {
throwables.add(new VoidReturnTypeTargetException(methodInfo));
} else if (MULTI.equals(methodInfo.returnType().name())) {
LOGGER.warnf("@CacheResult is not currently supported on a method returning %s [class=%s, method=%s]",
MULTI, methodInfo.declaringClass().name(), methodInfo.name());
}
}
break;
default:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package io.quarkus.cache.test.runtime;

import static org.junit.jupiter.api.Assertions.assertEquals;

import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.cache.CacheResult;
import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.mutiny.Multi;

/**
* Tests the {@link CacheResult} annotation on methods returning {@link Multi}.
*/
public class MultiValueTest {

@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class).addClass(CachedService.class));

@Inject
CachedService cachedService;

@Test
public void test() {

/*
* io.smallrye.mutiny.Multi values returned by methods annotated with @CacheResult should never be cached.
* Let's check that the cached method from this test is executed on each call.
*/
Multi<String> multi1 = cachedService.cachedMethod();
assertEquals(1, cachedService.getInvocations());
Multi<String> multi2 = cachedService.cachedMethod();
assertEquals(2, cachedService.getInvocations());

/*
* io.smallrye.mutiny.Uni emitted items are cached by a callback when the Unis are resolved.
* We need to make sure this isn't the case for Multi values.
*/
multi1.collect().asList().await().indefinitely();
cachedService.cachedMethod();
assertEquals(3, cachedService.getInvocations());
}

@ApplicationScoped
static class CachedService {

private int invocations;

@CacheResult(cacheName = "test-cache")
public Multi<String> cachedMethod() {
invocations++;
return Multi.createFrom().items("We", "are", "not", "cached!");
}

public int getInvocations() {
return invocations;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import io.quarkus.cache.CacheException;
import io.quarkus.cache.CacheResult;
import io.smallrye.mutiny.Multi;
import io.smallrye.mutiny.TimeoutException;
import io.smallrye.mutiny.Uni;

Expand All @@ -25,6 +26,14 @@ public class CacheResultInterceptor extends CacheInterceptor {

@AroundInvoke
public Object intercept(InvocationContext invocationContext) throws Throwable {
/*
* io.smallrye.mutiny.Multi values are never cached.
* There's already a WARN log entry at build time so we don't need to log anything at run time.
*/
if (Multi.class.isAssignableFrom(invocationContext.getMethod().getReturnType())) {
return invocationContext.proceed();
}

CacheInterceptionContext<CacheResult> interceptionContext = getInterceptionContext(invocationContext,
CacheResult.class, true);

Expand Down