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

ArC - enable implementing all types of interception with single method #15633

Merged
merged 1 commit into from
Mar 12, 2021
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 @@ -46,6 +46,7 @@
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InterceptorBinding;
import javax.interceptor.InvocationContext;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;

Expand Down Expand Up @@ -101,6 +102,7 @@ public final class DotNames {
public static final DotName TRANSACTION_PHASE = create(TransactionPhase.class);
public static final DotName INITIALIZED = create(Initialized.class);
public static final DotName TRANSIENT_REFERENCE = create(TransientReference.class);
public static final DotName INVOCATION_CONTEXT = create(InvocationContext.class);

public static final DotName BOOLEAN = create(Boolean.class);
public static final DotName BYTE = create(Byte.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,9 @@ private void addIntercept(MethodCreator intercept, MethodInfo interceptorMethod,
Class<?> retType = null;
if (InterceptionType.AROUND_INVOKE.equals(interceptionType)) {
retType = Object.class;
} else if (InterceptionType.AROUND_CONSTRUCT.equals(interceptionType)) {
retType = interceptorMethod.returnType().kind().equals(Type.Kind.VOID) ? void.class : Object.class;
} else {
retType = void.class;
// @PostConstruct, @PreDestroy, @AroundConstruct
retType = interceptorMethod.returnType().kind().equals(Type.Kind.VOID) ? void.class : Object.class;
}
ResultHandle ret;
if (Modifier.isPrivate(interceptorMethod.flags())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,16 @@ public class InterceptorInfo extends BeanInfo implements Comparable<InterceptorI
continue;
}
if (method.hasAnnotation(DotNames.AROUND_INVOKE)) {
aroundInvokes.add(method);
} else if (method.hasAnnotation(DotNames.AROUND_CONSTRUCT)) {
// validate compliance with rules for AroundConstruct methods
if (!method.parameters().equals(Collections.singletonList(
Type.create(DotName.createSimple("javax.interceptor.InvocationContext"), Type.Kind.CLASS)))) {
throw new IllegalStateException(
"@AroundConstruct must have exactly one argument of type javax.interceptor.InvocationContext, but method "
+ method.asMethod() + " declared by " + method.declaringClass()
+ " violates this.");
}
if (!method.returnType().kind().equals(Type.Kind.VOID) &&
!method.returnType().name().equals(DotNames.OBJECT)) {
throw new IllegalStateException(
"Return type of @AroundConstruct method must be Object or void, but method "
+ method.asMethod() + " declared by " + method.declaringClass()
+ " violates this.");
}
aroundConstructs.add(method);
} else if (method.hasAnnotation(DotNames.POST_CONSTRUCT)) {
postConstructs.add(method);
} else if (method.hasAnnotation(DotNames.PRE_DESTROY)) {
preDestroys.add(method);
aroundInvokes.add(validateSignature(method));
}
if (method.hasAnnotation(DotNames.AROUND_CONSTRUCT)) {
aroundConstructs.add(validateSignature(method));
}
if (method.hasAnnotation(DotNames.POST_CONSTRUCT)) {
postConstructs.add(validateSignature(method));
}
if (method.hasAnnotation(DotNames.PRE_DESTROY)) {
preDestroys.add(validateSignature(method));
}
}

Expand All @@ -103,6 +91,22 @@ public class InterceptorInfo extends BeanInfo implements Comparable<InterceptorI
}
}

private MethodInfo validateSignature(MethodInfo method) {
List<Type> parameters = method.parameters();
if (parameters.size() != 1 || !parameters.get(0).name().equals(DotNames.INVOCATION_CONTEXT)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also allow ArcInvocationContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point but I don't think it would work right now... let me check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this would actually require some more work, because we also generate the interceptor beans that implement javax.enterprise.inject.spi.Interceptor.intercept(InterceptionType, T, InvocationContext) etc. We can create a feature request though. Currently, you would need to cast the InvocationContext inside the interceptor method. @manovotn WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, it just occurred to me when I saw the code :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #15660

throw new IllegalStateException(
"An interceptor method must accept exactly one parameter of type javax.interceptor.InvocationContext: "
+ method + " declared on " + method.declaringClass());
}
if (!method.returnType().kind().equals(Type.Kind.VOID) &&
!method.returnType().name().equals(DotNames.OBJECT)) {
throw new IllegalStateException(
"The return type of an interceptor method must be java.lang.Object or void: "
+ method + " declared on " + method.declaringClass());
}
return method;
}

public Set<AnnotationInstance> getBindings() {
return bindings;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package io.quarkus.arc.test.interceptors.mixed;

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

import io.quarkus.arc.Arc;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.arc.test.ArcTestContainer;
import java.util.concurrent.atomic.AtomicInteger;
import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
import javax.annotation.Priority;
import javax.enterprise.context.Dependent;
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 BusinessLifecycleInterceptorTest {

@RegisterExtension
public ArcTestContainer container = new ArcTestContainer(MyTransactional.class, SimpleBean.class,
SimpleInterceptor.class);

public static final AtomicInteger INTERCEPTORS_CALLED = new AtomicInteger();

@Test
public void testInterception() {
InstanceHandle<SimpleBean> instance = Arc.container().instance(SimpleBean.class);
SimpleBean simpleBean = instance.get();
assertEquals(1, INTERCEPTORS_CALLED.get());
assertEquals(2, simpleBean.ping());
assertEquals(2, INTERCEPTORS_CALLED.get());
}

@Dependent
@MyTransactional
static class SimpleBean {

int ping() {
return 0;
}

}

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

@PostConstruct
@PreDestroy
@AroundInvoke
Object intercept(InvocationContext ctx) throws Exception {
return INTERCEPTORS_CALLED.incrementAndGet();
}

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

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

import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import javax.interceptor.InterceptorBinding;

@Target({ TYPE, CONSTRUCTOR })
@Retention(RUNTIME)
@InterceptorBinding
public @interface MyTransactional {

}