diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/AssignabilityCheck.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/AssignabilityCheck.java new file mode 100644 index 00000000000000..5d5599203827c1 --- /dev/null +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/AssignabilityCheck.java @@ -0,0 +1,51 @@ +package io.quarkus.arc.processor; + +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.CompositeIndex; +import org.jboss.jandex.DotName; +import org.jboss.jandex.IndexView; +import org.jboss.jandex.Type; + +final class AssignabilityCheck { + + private final ConcurrentMap> cache; + private final IndexView index; + + public AssignabilityCheck(IndexView beanArchiveIndex, IndexView applicationIndex) { + this.cache = new ConcurrentHashMap<>(); + this.index = applicationIndex != null ? CompositeIndex.create(beanArchiveIndex, applicationIndex) : beanArchiveIndex; + } + + boolean isAssignableFrom(Type type1, Type type2) { + // java.lang.Object is assignable from any type + if (type1.name().equals(DotNames.OBJECT)) { + return true; + } + // type1 is the same as type2 + if (type1.name().equals(type2.name())) { + return true; + } + // type1 is a superclass + return getAssignables(type1.name()).contains(type2.name()); + } + + Set getAssignables(DotName name) { + return cache.computeIfAbsent(name, this::findAssignables); + } + + private Set findAssignables(DotName name) { + Set assignables = new HashSet<>(); + for (ClassInfo subclass : index.getAllKnownSubclasses(name)) { + assignables.add(subclass.name()); + } + for (ClassInfo implementor : index.getAllKnownImplementors(name)) { + assignables.add(implementor.name()); + } + return assignables; + } + +} diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java index c42aa1a1bb37a4..272e3aca031bdd 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java @@ -78,6 +78,7 @@ public class BeanDeployment { private final List observers; final BeanResolverImpl beanResolver; + private final AssignabilityCheck assignabilityCheck; private final InterceptorResolver interceptorResolver; @@ -180,6 +181,7 @@ public class BeanDeployment { this.beans = new CopyOnWriteArrayList<>(); this.observers = new CopyOnWriteArrayList<>(); + this.assignabilityCheck = new AssignabilityCheck(beanArchiveIndex, applicationIndex); this.beanResolver = new BeanResolverImpl(this); this.interceptorResolver = new InterceptorResolver(this); this.transformUnproxyableClasses = builder.transformUnproxyableClasses; @@ -417,6 +419,10 @@ public BeanResolver getBeanResolver() { return beanResolver; } + public AssignabilityCheck getAssignabilityCheck() { + return assignabilityCheck; + } + boolean hasApplicationIndex() { return applicationIndex != null; } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanResolverImpl.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanResolverImpl.java index 90d60969b1d270..a085f58197861a 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanResolverImpl.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanResolverImpl.java @@ -16,13 +16,9 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.function.Function; import javax.enterprise.inject.AmbiguousResolutionException; import org.jboss.jandex.AnnotationInstance; -import org.jboss.jandex.ClassInfo; import org.jboss.jandex.ClassType; -import org.jboss.jandex.DotName; import org.jboss.jandex.Type; import org.jboss.jandex.Type.Kind; import org.jboss.jandex.TypeVariable; @@ -32,33 +28,10 @@ class BeanResolverImpl implements BeanResolver { private final BeanDeployment beanDeployment; - private final ConcurrentMap> assignableFromMap; - - private final Function> assignableFromMapFunction; - private final Map> resolved; BeanResolverImpl(BeanDeployment beanDeployment) { this.beanDeployment = beanDeployment; - this.assignableFromMap = new ConcurrentHashMap<>(); - this.assignableFromMapFunction = name -> { - Set assignables = new HashSet<>(); - for (ClassInfo subclass : beanDeployment.getBeanArchiveIndex().getAllKnownSubclasses(name)) { - assignables.add(subclass.name()); - } - for (ClassInfo implementor : beanDeployment.getBeanArchiveIndex().getAllKnownImplementors(name)) { - assignables.add(implementor.name()); - } - if (beanDeployment.hasApplicationIndex()) { - for (ClassInfo subclass : beanDeployment.getApplicationIndex().getAllKnownSubclasses(name)) { - assignables.add(subclass.name()); - } - for (ClassInfo implementor : beanDeployment.getApplicationIndex().getAllKnownImplementors(name)) { - assignables.add(implementor.name()); - } - } - return assignables; - }; this.resolved = new ConcurrentHashMap<>(); } @@ -248,7 +221,7 @@ boolean parametersMatch(WildcardType requiredParameter, TypeVariable beanParamet boolean parametersMatch(Type requiredParameter, TypeVariable beanParameter) { for (Type bound : getUppermostTypeVariableBounds(beanParameter)) { - if (!isAssignableFrom(bound, requiredParameter)) { + if (!beanDeployment.getAssignabilityCheck().isAssignableFrom(bound, requiredParameter)) { return false; } } @@ -271,7 +244,7 @@ boolean boundsMatch(List bounds, List stricterBounds) { stricterBounds = getUppermostBounds(stricterBounds); for (Type bound : bounds) { for (Type stricterBound : stricterBounds) { - if (!isAssignableFrom(bound, stricterBound)) { + if (!beanDeployment.getAssignabilityCheck().isAssignableFrom(bound, stricterBound)) { return false; } } @@ -279,19 +252,6 @@ boolean boundsMatch(List bounds, List stricterBounds) { return true; } - boolean isAssignableFrom(Type type1, Type type2) { - // java.lang.Object is assignable from any type - if (type1.name().equals(DotNames.OBJECT)) { - return true; - } - // type1 is the same as type2 - if (type1.name().equals(type2.name())) { - return true; - } - // type1 is a superclass - return assignableFromMap.computeIfAbsent(type1.name(), assignableFromMapFunction).contains(type2.name()); - } - boolean lowerBoundsOfWildcardMatch(Type parameter, WildcardType requiredParameter) { return lowerBoundsOfWildcardMatch(singletonList(parameter), requiredParameter); } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java index d747ece6a63f7c..94fe82c64c05f4 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java @@ -143,7 +143,8 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym List classLevelBindings, Consumer bytecodeTransformerConsumer, boolean transformUnproxyableClasses) { return addInterceptedMethodCandidates(beanDeployment, classInfo, candidates, classLevelBindings, - bytecodeTransformerConsumer, transformUnproxyableClasses, new SubclassSkipPredicate(), false); + bytecodeTransformerConsumer, transformUnproxyableClasses, + new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom), false); } static Set addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo, @@ -396,10 +397,15 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str */ static class SubclassSkipPredicate implements Predicate { + private final BiFunction assignableFromFun; private ClassInfo clazz; private List regularMethods; private Set bridgeMethods = new HashSet<>(); + public SubclassSkipPredicate(BiFunction assignableFromFun) { + this.assignableFromFun = assignableFromFun; + } + void startProcessing(ClassInfo clazz) { this.clazz = clazz; this.regularMethods = new ArrayList<>(); @@ -459,8 +465,7 @@ private boolean hasImplementation(MethodInfo bridge) { for (int i = 0; i < bridgeParams.size(); i++) { Type bridgeParam = bridgeParams.get(i); Type param = params.get(i); - if (param.name().equals(bridgeParam.name()) - || bridgeParam.name().equals(DotNames.OBJECT)) { + if (assignableFromFun.apply(bridgeParam, param)) { continue; } else { paramsNotMatching = true; @@ -477,8 +482,8 @@ private boolean hasImplementation(MethodInfo bridge) { // both cases are a match return true; } else { - // as a last resort, we simply check equality of return Type - return bridge.returnType().name().equals(declaredMethod.returnType().name()); + // as a last resort, we simply check assignability of the return type + return assignableFromFun.apply(bridge.returnType(), declaredMethod.returnType()); } } return true; diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java new file mode 100644 index 00000000000000..c7b0cb88cd9391 --- /dev/null +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java @@ -0,0 +1,80 @@ +package io.quarkus.arc.processor; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.quarkus.arc.processor.Methods.SubclassSkipPredicate; +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; +import javax.enterprise.context.ApplicationScoped; +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.DotName; +import org.jboss.jandex.IndexView; +import org.jboss.jandex.MethodInfo; +import org.junit.jupiter.api.Test; + +public class SubclassSkipPredicateTest { + + @Test + public void testPredicate() throws IOException { + IndexView index = Basics.index(Base.class, Submarine.class, Long.class, Number.class); + AssignabilityCheck assignabilityCheck = new AssignabilityCheck(index, null); + SubclassSkipPredicate predicate = new SubclassSkipPredicate(assignabilityCheck::isAssignableFrom); + + ClassInfo submarineClass = index.getClassByName(DotName.createSimple(Submarine.class.getName())); + predicate.startProcessing(submarineClass); + + List echos = submarineClass.methods().stream().filter(m -> m.name().equals("echo")) + .collect(Collectors.toList()); + assertEquals(2, echos.size()); + assertPredicate(echos, predicate); + + List getNames = submarineClass.methods().stream().filter(m -> m.name().equals("getName")) + .collect(Collectors.toList()); + assertEquals(2, getNames.size()); + assertPredicate(getNames, predicate); + + predicate.methodsProcessed(); + } + + private void assertPredicate(List methods, SubclassSkipPredicate predicate) { + for (MethodInfo method : methods) { + if (Methods.isBridge(method)) { + // Bridge method with impl + assertTrue(predicate.test(method)); + } else { + assertFalse(predicate.test(method)); + } + } + } + + static class Base { + + String echo(T payload) { + return payload.toString().toUpperCase(); + } + + T getName() { + return null; + } + + } + + @ApplicationScoped + static class Submarine extends Base { + + @Override + String echo(Long payload) { + return payload.toString(); + } + + @Override + Long getName() { + return 10l; + } + + } + +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bridge/BridgeMethodInterceptionTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bridge/BridgeMethodInterceptionTest.java index a36f16860f924f..4d893c1ca96ffc 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bridge/BridgeMethodInterceptionTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/bridge/BridgeMethodInterceptionTest.java @@ -3,21 +3,23 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import io.quarkus.arc.Arc; -import io.quarkus.arc.ArcContainer; -import io.quarkus.arc.test.ArcTestContainer; -import io.quarkus.arc.test.interceptors.Counter; import javax.enterprise.context.ApplicationScoped; import javax.inject.Singleton; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.test.ArcTestContainer; +import io.quarkus.arc.test.interceptors.Counter; + public class BridgeMethodInterceptionTest { @RegisterExtension public ArcTestContainer container = new ArcTestContainer(Base.class, Submarine.class, Ubot.class, Ponorka.class, Counter.class, Simple.class, SimpleInterceptor.class, ExampleApi.class, ExampleResource.class, - AbstractResource.class); + AbstractResource.class, NextBase.class, NextSubmarine.class); @Test public void testInterception() { @@ -54,6 +56,17 @@ public void testInterception() { assertEquals("TRUE", basePonorka.echo(true)); assertNull(basePonorka.getName()); assertEquals(4, counter.get()); + + counter.reset(); + NextSubmarine nextSubmarine = container.instance(NextSubmarine.class).get(); + assertEquals("foo", nextSubmarine.echo("foo")); + assertEquals(NextSubmarine.class.getSimpleName(), nextSubmarine.getName()); + assertEquals(2, counter.get()); + // Now let's invoke the bridge method... + NextBase nextBase = nextSubmarine; + assertEquals("foo", nextBase.echo("foo")); + assertEquals(NextSubmarine.class.getSimpleName(), nextBase.getName()); + assertEquals(4, counter.get()); } @Test @@ -123,4 +136,33 @@ static class Ponorka extends Base { } + static class NextBase> { + + String echo(T payload) { + return payload.toString().toUpperCase(); + } + + T getName() { + return null; + } + + } + + @ApplicationScoped + static class NextSubmarine extends NextBase { + + @Simple + @Override + String echo(String payload) { + return payload.toString(); + } + + @Simple + @Override + String getName() { + return NextSubmarine.class.getSimpleName(); + } + + } + }