Skip to content

Commit

Permalink
ArC: beans injected into All List injection points should be unremovable
Browse files Browse the repository at this point in the history
  • Loading branch information
mkouba committed Jun 8, 2023
1 parent f3bbc25 commit 89469c0
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ public ObserverRegistrationPhaseBuildItem registerSyntheticObservers(BeanRegistr
// Initialize the type -> bean map
beanRegistrationPhase.getBeanProcessor().getBeanDeployment().initBeanByTypeMap();

// Register a synthetic bean for each List<?> with qualifier @All
// Validate @All List<> injection points
List<InjectionPointInfo> listAll = beanRegistrationPhase.getInjectionPoints().stream()
.filter(this::isListAllInjectionPoint).collect(Collectors.toList());
for (InjectionPointInfo injectionPoint : listAll) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ private Set<DecoratorInfo> removeUnusedDecorators(Set<DecoratorInfo> removedDeco
}

private Set<BeanInfo> removeUnusedBeans(Set<BeanInfo> declaresObserver, List<Predicate<BeanInfo>> allUnusedExclusions) {
Set<BeanInfo> removableBeans = UnusedBeans.findRemovableBeans(this.beans, this.injectionPoints, declaresObserver,
Set<BeanInfo> removableBeans = UnusedBeans.findRemovableBeans(beanResolver, this.beans, this.injectionPoints,
declaresObserver,
allUnusedExclusions);
if (!removableBeans.isEmpty()) {
this.beans.removeAll(removableBeans);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public enum BuiltinBean {
BuiltinBean::validateEventMetadata, DotNames.EVENT_METADATA),
LIST(BuiltinBean::generateListBytecode,
(ip, names) -> cdiAndRawTypeMatches(ip, DotNames.LIST) && ip.getRequiredQualifier(DotNames.ALL) != null,
DotNames.LIST),
BuiltinBean::validateList, DotNames.LIST),
;

private final DotName[] rawTypeDotNames;
Expand Down Expand Up @@ -443,6 +443,20 @@ private static void validateInstance(InjectionTargetInfo injectionTarget, Inject
}
}

private static void validateList(InjectionTargetInfo injectionTarget, InjectionPointInfo injectionPoint,
Consumer<Throwable> errors) {
if (injectionPoint.getType().kind() != Kind.PARAMETERIZED_TYPE) {
errors.accept(
new DefinitionException("An injection point of raw type is defined: " + injectionPoint.getTargetInfo()));
} else if (injectionPoint.getRequiredType().kind() == Kind.WILDCARD_TYPE) {
errors.accept(
new DefinitionException("Wildcard is not a legal type argument for: " + injectionPoint.getTargetInfo()));
} else if (injectionPoint.getRequiredType().kind() == Kind.TYPE_VARIABLE) {
errors.accept(new DefinitionException(
"Type variable is not a legal type argument for: " + injectionPoint.getTargetInfo()));
}
}

private static void validateInjectionPoint(InjectionTargetInfo injectionTarget, InjectionPointInfo injectionPoint,
Consumer<Throwable> errors) {
if (injectionTarget.kind() != TargetKind.BEAN || !BuiltinScope.DEPENDENT.is(injectionTarget.asBean().getScope())) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,69 @@
package io.quarkus.arc.processor;

import static java.util.function.Predicate.not;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.Type;
import org.jboss.logging.Logger;

import io.quarkus.arc.processor.InjectionPointInfo.TypeAndQualifiers;

final class UnusedBeans {

private static final Logger LOG = Logger.getLogger(UnusedBeans.class);

private UnusedBeans() {
}

static Set<BeanInfo> findRemovableBeans(Collection<BeanInfo> beans, Collection<InjectionPointInfo> injectionPoints,
Set<BeanInfo> declaresObserver, List<Predicate<BeanInfo>> allUnusedExclusions) {
static Set<BeanInfo> findRemovableBeans(BeanResolver beanResolver, Collection<BeanInfo> beans,
Iterable<InjectionPointInfo> injectionPoints, Set<BeanInfo> declaresObserver,
List<Predicate<BeanInfo>> allUnusedExclusions) {
Set<BeanInfo> removableBeans = new HashSet<>();

Set<BeanInfo> unusedProducers = new HashSet<>();
Set<BeanInfo> unusedButDeclaresProducer = new HashSet<>();
List<BeanInfo> producers = beans.stream().filter(BeanInfo::isProducer)
.collect(Collectors.toList());
List<InjectionPointInfo> instanceInjectionPoints = injectionPoints.stream()
.filter(InjectionPointInfo::isProgrammaticLookup)
.collect(Collectors.toList());
// Collect all injected beans; skip delegate injection points and injection points that resolve to a built-in bean
Set<BeanInfo> injected = injectionPoints.stream()
.filter(not(InjectionPointInfo::isDelegate).and(InjectionPointInfo::hasResolvedBean))
.map(InjectionPointInfo::getResolvedBean)
.collect(Collectors.toSet());
// Collect all:
// - injected beans; skip delegate injection points and injection points that resolve to a built-in bean
// - Instance<> injection points
// - @All List<> injection points
Set<BeanInfo> injected = new HashSet<>();
List<InjectionPointInfo> instanceInjectionPoints = new ArrayList<>();
List<TypeAndQualifiers> listAllInjectionPoints = new ArrayList<>();

for (InjectionPointInfo injectionPoint : injectionPoints) {
if (injectionPoint.isProgrammaticLookup()) {
instanceInjectionPoints.add(injectionPoint);
} else if (!injectionPoint.isDelegate()) {
BeanInfo resolved = injectionPoint.getResolvedBean();
if (resolved != null) {
injected.add(resolved);
} else {
BuiltinBean builtin = BuiltinBean.resolve(injectionPoint);
if (builtin == BuiltinBean.LIST) {
Type requiredType = injectionPoint.getType().asParameterizedType().arguments().get(0);
Set<AnnotationInstance> qualifiers = new HashSet<>(injectionPoint.getRequiredQualifiers());
for (Iterator<AnnotationInstance> it = qualifiers.iterator(); it.hasNext();) {
AnnotationInstance qualifier = it.next();
if (qualifier.name().equals(DotNames.ALL)) {
it.remove();
}
}
listAllInjectionPoints.add(new TypeAndQualifiers(requiredType, qualifiers));
}
}
}
}
Set<BeanInfo> declaresProducer = producers.stream().map(BeanInfo::getDeclaringBean).collect(Collectors.toSet());

// Beans - first pass to find unused beans that do not declare a producer
Expand Down Expand Up @@ -70,12 +98,20 @@ static Set<BeanInfo> findRemovableBeans(Collection<BeanInfo> beans, Collection<I
// Instance<Foo>
for (InjectionPointInfo injectionPoint : instanceInjectionPoints) {
if (Beans.hasQualifiers(bean, injectionPoint.getRequiredQualifiers())
&& bean.getDeployment().getBeanResolver().matchesType(bean,
&& beanResolver.matchesType(bean,
injectionPoint.getType().asParameterizedType().arguments().get(0))) {
LOG.debugf("Unremovable - programmatic lookup: %s", bean);
continue test;
}
}
// @All List<Foo>
for (TypeAndQualifiers tq : listAllInjectionPoints) {
if (Beans.hasQualifiers(bean, tq.qualifiers)
&& beanResolver.matchesType(bean, tq.type)) {
LOG.debugf("Unremovable - @All List: %s", bean);
continue test;
}
}
// Declares a producer - see also second pass
if (declaresProducer.contains(bean)) {
unusedButDeclaresProducer.add(bean);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public class ListAllTest {
public ArcTestContainer container = new ArcTestContainer(Service.class, ServiceAlpha.class, ServiceBravo.class,
MyQualifier.class, Foo.class);

@SuppressWarnings("serial")
@Test
public void testSelectAll() {
verifyHandleInjection(Arc.container().listAll(Service.class), Object.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.List;

import jakarta.annotation.PostConstruct;
import jakarta.annotation.Priority;
Expand All @@ -22,6 +23,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.arc.All;
import io.quarkus.arc.Arc;
import io.quarkus.arc.ArcContainer;
import io.quarkus.arc.impl.ArcContainerImpl;
Expand All @@ -32,10 +34,9 @@ public class RemoveUnusedBeansTest extends RemoveUnusedComponentsTest {
@RegisterExtension
public ArcTestContainer container = ArcTestContainer.builder()
.beanClasses(HasObserver.class, Foo.class, FooAlternative.class, HasName.class, UnusedProducers.class,
InjectedViaInstance.class, InjectedViaInstanceWithWildcard.class,
InjectedViaProvider.class, Excluded.class,
UsedProducers.class,
UnusedProducerButInjected.class, UsedViaInstanceWithUnusedProducer.class, UsesBeanViaInstance.class)
InjectedViaInstance.class, InjectedViaInstanceWithWildcard.class, InjectedViaProvider.class, Excluded.class,
UsedProducers.class, UnusedProducerButInjected.class, UsedViaInstanceWithUnusedProducer.class,
UsesBeanViaInstance.class, UsedViaAllList.class)
.removeUnusedBeans(true)
.addRemovalExclusion(b -> b.getBeanClass().toString().equals(Excluded.class.getName()))
.build();
Expand Down Expand Up @@ -67,6 +68,7 @@ public void testRemoval() {
assertFalse(ArcContainerImpl.instance().getRemovedBeans().isEmpty());
assertNotPresent(UnusedBean.class);
assertNotPresent(OnlyInjectedInUnusedBean.class);
assertPresent(UsedViaAllList.class);
}

@Dependent
Expand Down Expand Up @@ -197,6 +199,10 @@ static class UsesBeanViaInstance {

@Inject
Instance<UsedViaInstanceWithUnusedProducer> instance;

@Inject
@All
List<UsedViaAllList> list;
}

@Singleton
Expand All @@ -212,4 +218,9 @@ static class OnlyInjectedInUnusedBean {

}

@Singleton
static class UsedViaAllList {

}

}

0 comments on commit 89469c0

Please sign in to comment.