Skip to content

Commit

Permalink
Merge pull request #22185 from geoand/#21260-followup
Browse files Browse the repository at this point in the history
Provide Arc configuration allowing to fail when interceptors used on private methods
  • Loading branch information
geoand authored Dec 15, 2021
2 parents c8212c7 + b13bff5 commit 39e2e8b
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ public class ArcConfig {
@ConfigItem(defaultValue = "true")
public boolean transformUnproxyableClasses;

/**
* If set to true, the build fails if a private method that is neither an observer nor a producer, is annotated with an
* interceptor
* binding.
* An example of this is the use of {@code Transactional} on a private method of a bean.
* If set to false, Quarkus simply logs a warning that the annotation will be ignored.
*/
@ConfigItem(defaultValue = "false")
public boolean failOnInterceptedPrivateMethod;

/**
* The default naming strategy for {@link ConfigProperties.NamingStrategy}. The allowed values are determined
* by that enum
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ public boolean test(BeanInfo bean) {
});
}
builder.setTransformUnproxyableClasses(arcConfig.transformUnproxyableClasses);
builder.setFailOnInterceptedPrivateMethod(arcConfig.failOnInterceptedPrivateMethod);
builder.setJtaCapabilities(capabilities.isPresent(Capability.TRANSACTIONS));
builder.setGenerateSources(BootstrapDebug.DEBUG_SOURCES_DIR != null);
builder.setAllowMocking(launchModeBuildItem.getLaunchMode() == LaunchMode.TEST);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package io.quarkus.arc.test.interceptor;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import javax.annotation.Priority;
import javax.enterprise.inject.spi.DeploymentException;
import javax.inject.Singleton;
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InterceptorBinding;
import javax.interceptor.InvocationContext;

import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class FailingPrivateInterceptedMethodTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(SimpleBean.class, SimpleInterceptor.class, Simple.class)
.addAsResource(new StringAsset("quarkus.arc.fail-on-intercepted-private-method=true"),
"application.properties"))
.setExpectedException(DeploymentException.class);

@Test
public void testDeploymentFailed() {
// This method should not be invoked
}

@Singleton
static class SimpleBean {

@Simple
private String foo() {
return "foo";
}

}

@Simple
@Priority(1)
@Interceptor
public static class SimpleInterceptor {

@AroundInvoke
public Object mySuperCoolAroundInvoke(InvocationContext ctx) throws Exception {
return "private" + ctx.proceed();
}
}

@Target({ TYPE, METHOD })
@Retention(RUNTIME)
@Documented
@InterceptorBinding
public @interface Simple {

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package io.quarkus.arc.test.interceptor;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import javax.annotation.Priority;
import javax.inject.Inject;
import javax.inject.Singleton;
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InterceptorBinding;
import javax.interceptor.InvocationContext;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class IgnoredPrivateInterceptedMethodTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(SimpleBean.class, SimpleInterceptor.class, Simple.class));

@Inject
SimpleBean simpleBean;

@Test
public void testBeanInvocation() {
assertEquals("foo", simpleBean.foo());
}

@Singleton
static class SimpleBean {

@Simple
private String foo() {
return "foo";
}

}

@Simple
@Priority(1)
@Interceptor
public static class SimpleInterceptor {

@AroundInvoke
public Object mySuperCoolAroundInvoke(InvocationContext ctx) throws Exception {
return "private" + ctx.proceed();
}
}

@Target({ TYPE, METHOD })
@Retention(RUNTIME)
@Documented
@InterceptorBinding
public @interface Simple {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ public class BeanDeployment {

final boolean transformUnproxyableClasses;

final boolean failOnInterceptedPrivateMethod;

private final boolean jtaCapabilities;

private final AlternativePriorities alternativePriorities;
Expand Down Expand Up @@ -190,6 +192,7 @@ public class BeanDeployment {
this.beanResolver = new BeanResolverImpl(this);
this.interceptorResolver = new InterceptorResolver(this);
this.transformUnproxyableClasses = builder.transformUnproxyableClasses;
this.failOnInterceptedPrivateMethod = builder.failOnInterceptedPrivateMethod;
this.jtaCapabilities = builder.jtaCapabilities;
this.alternativePriorities = builder.alternativePriorities;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public static Builder builder() {
private final boolean generateSources;
private final boolean allowMocking;
private final boolean transformUnproxyableClasses;
private final boolean failOnInterceptedPrivateMethod;
private final List<Function<BeanInfo, Consumer<BytecodeCreator>>> suppressConditionGenerators;

// This predicate is used to filter annotations for InjectionPoint metadata
Expand All @@ -86,6 +87,7 @@ private BeanProcessor(Builder builder) {
this.generateSources = builder.generateSources;
this.allowMocking = builder.allowMocking;
this.transformUnproxyableClasses = builder.transformUnproxyableClasses;
this.failOnInterceptedPrivateMethod = builder.failOnInterceptedPrivateMethod;
this.suppressConditionGenerators = builder.suppressConditionGenerators;

// Initialize all build processors
Expand Down Expand Up @@ -298,6 +300,7 @@ public static class Builder {
boolean generateSources;
boolean jtaCapabilities;
boolean transformUnproxyableClasses;
boolean failOnInterceptedPrivateMethod;
boolean allowMocking;

AlternativePriorities alternativePriorities;
Expand Down Expand Up @@ -329,6 +332,7 @@ public Builder() {
generateSources = false;
jtaCapabilities = false;
transformUnproxyableClasses = false;
failOnInterceptedPrivateMethod = false;
allowMocking = false;

excludeTypes = new ArrayList<>();
Expand Down Expand Up @@ -503,6 +507,14 @@ public Builder setTransformUnproxyableClasses(boolean value) {
return this;
}

/**
* If set to true, the build will fail if an annotation that would result in an interceptor being created (such as
* {@code @Transactional})
*/
public void setFailOnInterceptedPrivateMethod(boolean failOnInterceptedPrivateMethod) {
this.failOnInterceptedPrivateMethod = failOnInterceptedPrivateMethod;
}

/**
* If set to true the will generate source files of all generated classes for debug purposes. The generated source is
* not actually a source file but a textual representation of generated code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import javax.enterprise.inject.spi.DeploymentException;
import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
Expand Down Expand Up @@ -253,16 +254,24 @@ private static Set<AnnotationInstance> mergeBindings(BeanDeployment beanDeployme
}

if (Modifier.isPrivate(method.flags())
&& !Annotations.contains(methodAnnotations, DotNames.PRODUCES)
&& !Annotations.contains(methodAnnotations, DotNames.OBSERVES)
&& !Annotations.contains(methodAnnotations, DotNames.OBSERVES_ASYNC)) {
String message;
if (methodLevelBindings.size() == 1) {
LOGGER.warnf("%s will have no effect on method %s.%s() because the method is private",
message = String.format("%s will have no effect on method %s.%s() because the method is private",
methodLevelBindings.iterator().next(), classInfo.name(), method.name());
} else {
LOGGER.warnf("Annotations %s will have no effect on method %s.%s() because the method is private",
message = String.format(
"Annotations %s will have no effect on method %s.%s() because the method is private",
methodLevelBindings.stream().map(AnnotationInstance::toString).collect(Collectors.joining(",")),
classInfo.name(), method.name());
}
if (beanDeployment.failOnInterceptedPrivateMethod) {
throw new DeploymentException(message);
} else {
LOGGER.warn(message);
}
}
}
return merged;
Expand Down

0 comments on commit 39e2e8b

Please sign in to comment.