Skip to content
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
@@ -1,6 +1,5 @@
package io.quarkus.deployment.builditem;

import java.util.Collections;
import java.util.Set;

import io.quarkus.builder.item.MultiBuildItem;
Expand All @@ -18,7 +17,7 @@ public AdditionalIndexedClassesBuildItem(String... classesToIndex) {
}

public AdditionalIndexedClassesBuildItem(String classToIndex) {
this.classesToIndex = Collections.singleton(classToIndex);
this.classesToIndex = Set.of(classToIndex);
}

public Set<String> getClassesToIndex() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static io.smallrye.config.ConfigMappings.ConfigClass.configClass;
import static org.jboss.jandex.AnnotationTarget.Kind.CLASS;

import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -222,7 +221,7 @@ private static Set<Type> collectTypes(CombinedIndexBuildItem combinedIndex, Clas
DotName configIfaceName = DotName.createSimple(configClass.getName());
ClassInfo configIfaceInfo = index.getClassByName(configIfaceName);
if ((configIfaceInfo == null) || configIfaceInfo.interfaceNames().isEmpty()) {
return Collections.singleton(Type.create(configIfaceName, Type.Kind.CLASS));
return Set.of(Type.create(configIfaceName, Type.Kind.CLASS));
}

Set<DotName> allIfaces = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,14 +409,14 @@ void validateConfigMappingsInjectionPoints(
}

if (arcConfig.shouldEnableBeanRemoval()) {
Set<String> unremovableClassNames = unremovableBeans.stream()
Set<DotName> unremovableClassNames = unremovableBeans.stream()
.map(UnremovableBeanBuildItem::getClassNames)
.flatMap(Collection::stream)
.collect(toSet());

for (ConfigClassBuildItem configClass : configMappingTypes.values()) {
if (configClass.getConfigClass().isAnnotationPresent(Unremovable.class)
|| unremovableClassNames.contains(configClass.getName().toString())) {
|| unremovableClassNames.contains(configClass.getName())) {
toRegister.add(new ConfigMappingBuildItem(configClass.getConfigClass(), configClass.getPrefix()));
}
}
Comment on lines -412 to 422
Copy link
Member Author

Choose a reason for hiding this comment

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

@mkouba I would appreciate if you could have a look at this as it looks extremely odd. I don't think it should be done this way: it's not actually applying the predicates but relying on the class names being set, which won't work if you have a custom predicate.
I can understand it given you don't have the BeanInfo but I wonder if there could be a better way to achieve the same goal by using ArC metadata?

It's a separate work though, I just kept the logic as is for now.

It was added by @radcortez here: 564d112 .

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not correct indeed.

Given the fact that this logic is executed after the validation phase (unused beans are already removed if needed) I think that the correct solution would be to attempt to find a bean for a given config mapping class and if it's present then register the ConfigMappingBuildItem.

@radcortez Something like this should IMO work:

for (ConfigClassBuildItem configClass : configMappingTypes.values()) {
   if (!validationPhase.getContext().beans().withBeanClass(configClass.getConfigClass()).isEmpty()) {
      toRegister.add(new ConfigMappingBuildItem(configClass.getConfigClass(), configClass.getPrefix()));
   }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, when we clean this up, let's get rid of the classNames field in UnremovableBuildItem. And I think we could also drop the unnecessary public constructors that were added in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

And I think we could also drop the unnecessary public constructors

We should deprecate them first...

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ UnremovableBeanBuildItem unremovableBeans() {
@Override
public boolean test(BeanInfo bean) {
if (bean.isClassBean()) {
return bean.getTarget().get().asClass().annotationsMap().containsKey(SHUTDOWN_NAME);
return bean.getTarget().get().asClass().hasAnnotation(SHUTDOWN_NAME);
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ UnremovableBeanBuildItem unremovableBeans() {
@Override
public boolean test(BeanInfo bean) {
if (bean.isClassBean()) {
return bean.getTarget().get().asClass().annotationsMap().containsKey(STARTUP_NAME);
return bean.getTarget().get().asClass().hasAnnotation(STARTUP_NAME);
} else if (bean.isProducerMethod()) {
return !getAnnotations(Kind.METHOD, STARTUP_NAME, bean.getTarget().get().asMethod().annotations())
.isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget.Kind;
import org.jboss.jandex.DotName;
import org.jboss.jandex.Type;

import io.quarkus.arc.processor.Annotations;
import io.quarkus.arc.processor.BeanInfo;
import io.quarkus.builder.item.MultiBuildItem;
import io.smallrye.common.annotation.CheckReturnValue;
Expand Down Expand Up @@ -48,16 +48,16 @@
public final class UnremovableBeanBuildItem extends MultiBuildItem {

private final Predicate<BeanInfo> predicate;
private final Set<String> classNames;
private final Set<DotName> classNames;

public UnremovableBeanBuildItem(Predicate<BeanInfo> predicate) {
this.predicate = predicate;
this.classNames = Collections.emptySet();
this.classNames = Set.of();
}

public UnremovableBeanBuildItem(BeanClassNameExclusion predicate) {
this.predicate = predicate;
this.classNames = Collections.singleton(predicate.className);
this.classNames = Set.of(predicate.className);
}

public UnremovableBeanBuildItem(BeanClassNamesExclusion predicate) {
Expand All @@ -67,19 +67,19 @@ public UnremovableBeanBuildItem(BeanClassNamesExclusion predicate) {

public UnremovableBeanBuildItem(BeanTypeExclusion predicate) {
this.predicate = predicate;
this.classNames = Collections.singleton(predicate.dotName.toString());
this.classNames = Set.of(predicate.dotName);
}

public UnremovableBeanBuildItem(BeanTypesExclusion predicate) {
this.predicate = predicate;
this.classNames = predicate.dotNames.stream().map(DotName::toString).collect(Collectors.toSet());
this.classNames = predicate.dotNames;
}

public Predicate<BeanInfo> getPredicate() {
return predicate;
}

public Set<String> getClassNames() {
public Set<DotName> getClassNames() {
return classNames;
}

Expand Down Expand Up @@ -123,7 +123,7 @@ public static UnremovableBeanBuildItem beanTypes(DotName... typeNames) {
/**
* Match beans which have any of the specified type names in its set of bean types.
*
* @param typeNames
* @param types
* @return a new build item
*/
@CheckReturnValue
Expand Down Expand Up @@ -161,7 +161,7 @@ public static UnremovableBeanBuildItem beanClassAnnotation(DotName annotationNam
* <p>
* The annotations can be declared on the class, and every nested element of the class (fields, types, methods, etc).
*
* @param annotationName
* @param nameStartsWith
* @return a new build item
*/
@CheckReturnValue
Expand All @@ -181,7 +181,7 @@ public static UnremovableBeanBuildItem targetWithAnnotation(DotName annotationNa
@Override
public boolean test(BeanInfo bean) {
if (bean.isClassBean()) {
return Annotations.contains(bean.getTarget().get().asClass().declaredAnnotations(), annotationName);
return bean.getTarget().get().asClass().hasDeclaredAnnotation(annotationName);
} else if (bean.isProducerMethod()) {
return !getAnnotations(Kind.METHOD, annotationName, bean.getTarget().get().asMethod().annotations())
.isEmpty();
Expand All @@ -196,15 +196,15 @@ public boolean test(BeanInfo bean) {

public static class BeanClassNameExclusion implements Predicate<BeanInfo> {

private final String className;
private final DotName className;

public BeanClassNameExclusion(String className) {
this.className = Objects.requireNonNull(className);
this.className = DotName.createSimple(Objects.requireNonNull(className));
}

@Override
public boolean test(BeanInfo bean) {
return bean.getBeanClass().toString().equals(className);
return bean.getBeanClass().equals(className);
}

@Override
Expand All @@ -216,15 +216,16 @@ public String toString() {

public static class BeanClassNamesExclusion implements Predicate<BeanInfo> {

private final Set<String> classNames;
private final Set<DotName> classNames;

public BeanClassNamesExclusion(Set<String> classNames) {
this.classNames = Objects.requireNonNull(classNames);
this.classNames = Objects.requireNonNull(classNames).stream().map(DotName::createSimple)
.collect(Collectors.toCollection(HashSet::new));
}

@Override
public boolean test(BeanInfo bean) {
return classNames.contains(bean.getBeanClass().toString());
return classNames.contains(bean.getBeanClass());
}

@Override
Expand All @@ -244,7 +245,13 @@ public BeanTypeExclusion(DotName dotName) {

@Override
public boolean test(BeanInfo bean) {
return bean.getTypes().stream().anyMatch(t -> dotName.equals(t.name()));
for (Type type : bean.getTypes()) {
if (dotName.equals(type.name())) {
return true;
}
}

return false;
}

@Override
Expand All @@ -264,7 +271,13 @@ public BeanTypesExclusion(Set<DotName> dotNames) {

@Override
public boolean test(BeanInfo bean) {
return bean.getTypes().stream().anyMatch(t -> dotNames.contains(t.name()));
for (Type type : bean.getTypes()) {
if (dotNames.contains(type.name())) {
return true;
}
}

return false;
}

@Override
Expand Down Expand Up @@ -293,10 +306,10 @@ public BeanClassAnnotationExclusion(DotName name) {
@Override
public boolean test(BeanInfo bean) {
if (bean.isClassBean()) {
Map<DotName, List<AnnotationInstance>> annotations = bean.getTarget().get().asClass().annotationsMap();
if (name != null) {
return annotations.containsKey(name);
return bean.getTarget().get().asClass().hasAnnotation(name);
} else {
Map<DotName, List<AnnotationInstance>> annotations = bean.getTarget().get().asClass().annotationsMap();
return annotations.keySet().stream().anyMatch(a -> a.toString().startsWith(nameStartsWith));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,17 +723,17 @@ Collection<AnnotationInstance> extractInterceptorBindings(AnnotationInstance ann
private static Collection<AnnotationInstance> extractAnnotations(AnnotationInstance annotation,
Map<DotName, ClassInfo> singulars, Map<DotName, ClassInfo> repeatables) {
if (!annotation.runtimeVisible()) {
return Collections.emptyList();
return List.of();
}
DotName annotationName = annotation.name();
if (singulars.get(annotationName) != null) {
return Collections.singleton(annotation);
return Set.of(annotation);
} else if (repeatables.get(annotationName) != null) {
// repeatable, we need to extract actual annotations
return Annotations.onlyRuntimeVisible(Arrays.asList(annotation.value().asNestedArray()));
} else {
// neither singular nor repeatable, return empty collection
return Collections.emptyList();
return List.of();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public Set<BeanInfo> resolveBeans(Type requiredType, Set<AnnotationInstance> req
List<BeanInfo> beans = findMatching(typeAndQualifiers);
Set<BeanInfo> ret;
if (beans.isEmpty()) {
ret = Collections.emptySet();
ret = Set.of();
} else if (beans.size() == 1) {
ret = Collections.singleton(beans.get(0));
ret = Set.of(beans.get(0));
} else {
ret = new HashSet<>(beans);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.ListIterator;
Expand Down Expand Up @@ -134,7 +133,7 @@ static InjectionPointInfo fromSyntheticInjectionPoint(TypeAndQualifiers typeAndQ
InjectionPointInfo(Type requiredType, Set<AnnotationInstance> requiredQualifiers, InjectionPointKind kind,
AnnotationTarget target, AnnotationTarget methodParameterTarget, boolean isTransientReference, boolean isDelegate) {
this(new TypeAndQualifiers(requiredType, requiredQualifiers.isEmpty()
? Collections.singleton(AnnotationInstance.create(DotNames.DEFAULT, null, Collections.emptyList()))
? Set.of(AnnotationInstance.create(DotNames.DEFAULT, null, List.of()))
: requiredQualifiers),
kind, target, methodParameterTarget, isTransientReference, isDelegate);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ static InterceptorInfo createInterceptor(ClassInfo interceptorClass, BeanDeploym
checkInterceptorFieldsAndMethods(interceptorClass, beanDeployment);

return new InterceptorInfo(interceptorClass, beanDeployment,
bindings.size() == 1 ? Collections.singleton(bindings.iterator().next())
bindings.size() == 1 ? Set.of(bindings.iterator().next())
: Collections.unmodifiableSet(bindings),
Injection.forBean(interceptorClass, null, beanDeployment, transformer, Injection.BeanType.INTERCEPTOR),
priority);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -333,8 +334,10 @@ static Set<AnnotationInstance> mergeMethodAndClassLevelBindings(Collection<Annot

static class MethodKey {

private static final DotName[] NO_PARAMS = new DotName[0];

final String name;
final List<DotName> params;
final DotName[] params;
final DotName returnType;
final MethodInfo method; // this is intentionally ignored for equals/hashCode
private final int hashCode;
Expand All @@ -343,18 +346,14 @@ public MethodKey(MethodInfo method) {
this.method = Objects.requireNonNull(method, "Method must not be null");
this.name = method.name();
this.returnType = method.returnType().name();
this.params = switch (method.parametersCount()) {
case 0 -> List.of();
case 1 -> List.of(method.parameterTypes().get(0).name());
case 2 -> List.of(method.parameterTypes().get(0).name(), method.parameterTypes().get(1).name());
default -> {
List<DotName> ret = new ArrayList<>(method.parametersCount());
for (Type parameterType : method.parameterTypes()) {
ret.add(parameterType.name());
}
yield ret;
if (method.parametersCount() == 0) {
this.params = NO_PARAMS;
} else {
this.params = new DotName[method.parametersCount()];
for (int i = 0; i < method.parametersCount(); i++) {
this.params[i] = method.parameterType(i).name();
}
};
}

// the Map can be resized several times so it's worth caching the hashCode
this.hashCode = buildHashCode(this.name, this.params, this.returnType);
Expand All @@ -364,11 +363,10 @@ public MethodKey(MethodInfo method) {
public boolean equals(Object o) {
if (this == o)
return true;
if (!(o instanceof MethodKey))
if (!(o instanceof MethodKey methodKey))
return false;
MethodKey methodKey = (MethodKey) o;
return Objects.equals(name, methodKey.name)
&& Objects.equals(params, methodKey.params)
&& Arrays.equals(params, methodKey.params)
&& Objects.equals(returnType, methodKey.returnType);
}

Expand All @@ -377,9 +375,9 @@ public int hashCode() {
return hashCode;
}

private static int buildHashCode(String name, List<DotName> params, DotName returnType) {
private static int buildHashCode(String name, DotName[] params, DotName returnType) {
int result = Objects.hashCode(name);
result = 31 * result + Objects.hashCode(params);
result = 31 * result + Arrays.hashCode(params);
result = 31 * result + Objects.hashCode(returnType);
return result;
}
Expand Down Expand Up @@ -477,8 +475,13 @@ public SubclassSkipPredicate(BiFunction<Type, Type, Boolean> assignableFromFun,
void startProcessing(ClassInfo clazz, ClassInfo originalClazz) {
this.clazz = clazz;
this.originalClazz = originalClazz;
this.regularMethods = new ArrayList<>();
for (MethodInfo method : clazz.methods()) {

List<MethodInfo> methodsInfos = clazz.methods();
// the list will sometimes be a bit larger than strictly necessary but it should be better sized than the default
// and the excluded methods are usually exceptions
// this way, we will avoid resizing the list
this.regularMethods = new ArrayList<>(methodsInfos.size());
for (MethodInfo method : methodsInfos) {
if (!Modifier.isAbstract(method.flags()) && !method.isSynthetic() && !isBridge(method)) {
regularMethods.add(method);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ static Set<BeanInfo> findRemovableBeans(BeanResolver beanResolver, Collection<Be

Set<BeanInfo> unusedProducers = new HashSet<>();
Set<BeanInfo> unusedButDeclaresProducer = new HashSet<>();
List<BeanInfo> producers = beans.stream().filter(BeanInfo::isProducer)
.collect(Collectors.toList());
List<BeanInfo> producers = beans.stream().filter(BeanInfo::isProducer).toList();
// Collect all:
// - injected beans; skip delegate injection points and injection points that resolve to a built-in bean
// - Instance<> injection points
Expand Down Expand Up @@ -84,7 +83,7 @@ static Set<BeanInfo> findRemovableBeans(BeanResolver beanResolver, Collection<Be
// Custom exclusions
for (Predicate<BeanInfo> exclusion : allUnusedExclusions) {
if (exclusion.test(bean)) {
LOG.debugf("Unremovable - excluded by %s: %s", exclusion.toString(), bean);
LOG.debugf("Unremovable - excluded by %s: %s", exclusion, bean);
continue test;
}
}
Expand Down
Loading
Loading