Skip to content

Commit

Permalink
ArC - fix an issue with bridge methods and generics
Browse files Browse the repository at this point in the history
  • Loading branch information
@mkouba
mkouba committed Mar 24, 2021
1 parent d4828e6 commit 3d46f22
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 48 deletions.
35 changes: 35 additions & 0 deletions ...pendent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.function.Consumer;
Expand Down Expand Up @@ -78,6 +79,7 @@ public class BeanDeployment {
private final List<ObserverInfo> observers;

final BeanResolverImpl beanResolver;
private final ConcurrentMap<DotName, Set<DotName>> nameToAssignables;

private final InterceptorResolver interceptorResolver;

Expand Down Expand Up @@ -180,6 +182,7 @@ public class BeanDeployment {
this.beans = new CopyOnWriteArrayList<>();
this.observers = new CopyOnWriteArrayList<>();

this.nameToAssignables = new ConcurrentHashMap<>();
this.beanResolver = new BeanResolverImpl(this);
this.interceptorResolver = new InterceptorResolver(this);
this.transformUnproxyableClasses = builder.transformUnproxyableClasses;
Expand Down Expand Up @@ -1164,6 +1167,38 @@ public Set<String> getQualifierNonbindingMembers(DotName name) {
return qualifierNonbindingMembers.getOrDefault(name, Collections.emptySet());
}

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 nameToAssignables.computeIfAbsent(type1.name(), this::getAssignables).contains(type2.name());
}

private Set<DotName> getAssignables(DotName name) {
Set<DotName> assignables = new HashSet<>();
for (ClassInfo subclass : beanArchiveIndex.getAllKnownSubclasses(name)) {
assignables.add(subclass.name());
}
for (ClassInfo implementor : beanArchiveIndex.getAllKnownImplementors(name)) {
assignables.add(implementor.name());
}
if (applicationIndex != null) {
for (ClassInfo subclass : applicationIndex.getAllKnownSubclasses(name)) {
assignables.add(subclass.name());
}
for (ClassInfo implementor : applicationIndex.getAllKnownImplementors(name)) {
assignables.add(implementor.name());
}
}
return assignables;
}

private static class ValidationContextImpl implements ValidationContext {

private final BuildContext buildContext;
Expand Down
44 changes: 2 additions & 42 deletions ...ndent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanResolverImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,33 +28,10 @@ class BeanResolverImpl implements BeanResolver {

private final BeanDeployment beanDeployment;

private final ConcurrentMap<DotName, Set<DotName>> assignableFromMap;

private final Function<DotName, Set<DotName>> assignableFromMapFunction;

private final Map<TypeAndQualifiers, List<BeanInfo>> resolved;

BeanResolverImpl(BeanDeployment beanDeployment) {
this.beanDeployment = beanDeployment;
this.assignableFromMap = new ConcurrentHashMap<>();
this.assignableFromMapFunction = name -> {
Set<DotName> 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<>();
}

Expand Down Expand Up @@ -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.isAssignableFrom(bound, requiredParameter)) {
return false;
}
}
Expand All @@ -271,27 +244,14 @@ boolean boundsMatch(List<Type> bounds, List<Type> stricterBounds) {
stricterBounds = getUppermostBounds(stricterBounds);
for (Type bound : bounds) {
for (Type stricterBound : stricterBounds) {
if (!isAssignableFrom(bound, stricterBound)) {
if (!beanDeployment.isAssignableFrom(bound, stricterBound)) {
return false;
}
}
}
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);
}
Expand Down
14 changes: 9 additions & 5 deletions independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment beanDeploym
List<AnnotationInstance> classLevelBindings, Consumer<BytecodeTransformer> bytecodeTransformerConsumer,
boolean transformUnproxyableClasses) {
return addInterceptedMethodCandidates(beanDeployment, classInfo, candidates, classLevelBindings,
bytecodeTransformerConsumer, transformUnproxyableClasses, new SubclassSkipPredicate(), false);
bytecodeTransformerConsumer, transformUnproxyableClasses, new SubclassSkipPredicate(beanDeployment), false);
}

static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo,
Expand Down Expand Up @@ -396,10 +396,15 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str
*/
static class SubclassSkipPredicate implements Predicate<MethodInfo> {

private final BeanDeployment beanDeployment;
private ClassInfo clazz;
private List<MethodInfo> regularMethods;
private Set<MethodInfo> bridgeMethods = new HashSet<>();

public SubclassSkipPredicate(BeanDeployment beanDeployment) {
this.beanDeployment = beanDeployment;
}

void startProcessing(ClassInfo clazz) {
this.clazz = clazz;
this.regularMethods = new ArrayList<>();
Expand Down Expand Up @@ -459,8 +464,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 (beanDeployment.isAssignableFrom(bridgeParam, param)) {
continue;
} else {
paramsNotMatching = true;
Expand All @@ -477,8 +481,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 beanDeployment.isAssignableFrom(bridge.returnType(), declaredMethod.returnType());
}
}
return true;
Expand Down
2 changes: 1 addition & 1 deletion ...s/src/test/java/io/quarkus/arc/test/interceptors/bridge/BridgeMethodInterceptionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void testHierarchyWithInterface() {
assertEquals(1, counter.get());
}

static class Base<T> {
static class Base<T extends Comparable<T>> {

String echo(T payload) {
return payload.toString().toUpperCase();
Expand Down

0 comments on commit 3d46f22

Please sign in to comment.