Skip to content

Commit

Permalink
Bean that has a bound interceptor must not declare final methods
Browse files Browse the repository at this point in the history
- otherwise a deployment exception is thrown
- final methods declared on bean classes that require client proxies are
ignored though
  • Loading branch information
mkouba committed Nov 1, 2019
1 parent 9ed6438 commit ebf00ee
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import javax.enterprise.inject.spi.DefinitionException;
import javax.enterprise.inject.spi.DeploymentException;
import javax.enterprise.inject.spi.InterceptionType;
import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
Expand Down Expand Up @@ -405,8 +407,10 @@ void init(List<Throwable> errors) {
if (disposer != null) {
disposer.init(errors);
}
interceptedMethods.putAll(initInterceptedMethods());
lifecycleInterceptors.putAll(initLifecycleInterceptors());
interceptedMethods.putAll(initInterceptedMethods(errors));
if (errors.isEmpty()) {
lifecycleInterceptors.putAll(initLifecycleInterceptors());
}
}

protected String getType() {
Expand All @@ -421,7 +425,7 @@ protected String getType() {
}
}

private Map<MethodInfo, InterceptionInfo> initInterceptedMethods() {
private Map<MethodInfo, InterceptionInfo> initInterceptedMethods(List<Throwable> errors) {
if (!isInterceptor() && isClassBean()) {
Map<MethodInfo, InterceptionInfo> interceptedMethods = new HashMap<>();
Map<MethodKey, Set<AnnotationInstance>> candidates = new HashMap<>();
Expand All @@ -434,7 +438,14 @@ private Map<MethodInfo, InterceptionInfo> initInterceptedMethods() {
}
}

Methods.addInterceptedMethodCandidates(beanDeployment, target.get().asClass(), candidates, classLevelBindings);
Set<MethodInfo> finalMethods = Methods.addInterceptedMethodCandidates(beanDeployment, target.get().asClass(),
candidates, classLevelBindings);
if (!finalMethods.isEmpty()) {
errors.add(new DeploymentException(String.format(
"Bean %s has a bound interceptor and must not declare final methods:\n\t- %s", getBeanClass(),
finalMethods.stream().map(Object::toString).sorted().collect(Collectors.joining("\n\t- ")))));
return Collections.emptyMap();
}

for (Entry<MethodKey, Set<AnnotationInstance>> entry : candidates.entrySet()) {
List<InterceptorInfo> interceptors = beanDeployment.getInterceptorResolver()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ private static boolean skipForClientProxy(MethodInfo method) {
if (Modifier.isFinal(method.flags())) {
String className = method.declaringClass().name().toString();
if (!className.startsWith("java.")) {
LOGGER.warn(
String.format("Method %s.%s() is final, skipped during generation of the corresponding client proxy",
className, method.name()));
LOGGER.warn(String.format(
"Final method %s.%s() is ignored during proxy generation and should never be invoked upon the proxy instance!",
className, method.name()));
}
return true;
}
Expand All @@ -113,9 +113,10 @@ static boolean isObjectToString(MethodInfo method) {
return method.declaringClass().name().equals(DotNames.OBJECT) && method.name().equals(TO_STRING);
}

static void addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo,
static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo,
Map<MethodKey, Set<AnnotationInstance>> candidates,
List<AnnotationInstance> classLevelBindings) {
Set<MethodInfo> finalMethods = new HashSet<>();
for (MethodInfo method : classInfo.methods()) {
if (skipForSubclass(method)) {
continue;
Expand All @@ -134,13 +135,7 @@ static void addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassI
}
if (!merged.isEmpty()) {
if (Modifier.isFinal(method.flags())) {
String className = method.declaringClass().name().toString();
if (!className.startsWith("java.")) {
LOGGER.warn(
String.format(
"Method %s.%s() is final, skipped during generation of the corresponding intercepted subclass",
className, method.name()));
}
finalMethods.add(method);
} else {
candidates.computeIfAbsent(new Methods.MethodKey(method), key -> merged);
}
Expand All @@ -149,9 +144,11 @@ static void addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassI
if (classInfo.superClassType() != null) {
ClassInfo superClassInfo = getClassByName(beanDeployment.getIndex(), classInfo.superName());
if (superClassInfo != null) {
addInterceptedMethodCandidates(beanDeployment, superClassInfo, candidates, classLevelBindings);
finalMethods
.addAll(addInterceptedMethodCandidates(beanDeployment, superClassInfo, candidates, classLevelBindings));
}
}
return finalMethods;
}

private static boolean skipForSubclass(MethodInfo method) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.quarkus.arc.test.clientproxy.finalmethod;

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

import io.quarkus.arc.Arc;
import io.quarkus.arc.ClientProxy;
import io.quarkus.arc.test.ArcTestContainer;
import java.io.IOException;
import javax.annotation.PostConstruct;
import javax.enterprise.context.ApplicationScoped;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

public class FinalMethodIgnoredTest {

@RegisterExtension
public ArcTestContainer container = new ArcTestContainer(Moo.class);

@Test
public void testProducer() throws IOException {
Moo moo = Arc.container().instance(Moo.class).get();
assertTrue(moo instanceof ClientProxy);
assertEquals(0, moo.getVal());
assertEquals(10, ((Moo) ((ClientProxy) moo).arc_contextualInstance()).val);
}

@ApplicationScoped
static class Moo {

private int val;

@PostConstruct
void init() {
this.val = 10;
}

// will return 0 if invoked upon a client proxy
final int getVal() {
return val;
}

}

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

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import io.quarkus.arc.test.ArcTestContainer;
import io.quarkus.arc.test.interceptors.Simple;
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.InvocationContext;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

public class FinalInterceptedMethodTest {

@RegisterExtension
public ArcTestContainer container = ArcTestContainer.builder().beanClasses(Simple.class, SimpleBean.class,
SimpleInterceptor.class).shouldFail().build();

@Test
public void testFailure() {
Throwable t = container.getFailure();
assertNotNull(t);
assertTrue(t instanceof DeploymentException);
assertTrue(t.getMessage().contains("foo"));
assertTrue(t.getMessage().contains("bar"));
}

@Simple
@Singleton
static class SimpleBean {

final String foo() {
return "foo";
}

final void bar() {
}

}

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

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

}

0 comments on commit ebf00ee

Please sign in to comment.