Skip to content

Commit

Permalink
ArC - load removed beans lazily
Browse files Browse the repository at this point in the history
- only if Instance#get(), InjectableInstance#getHandle() and ArcContainer.instance() is called
- added InjectableInstance.orElse() and InjectableInstance.orNull()
- resolves quarkusio#18345
  • Loading branch information
mkouba authored and igorregis committed Oct 16, 2022
1 parent d769a65 commit 17b9747
Show file tree
Hide file tree
Showing 11 changed files with 140 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ AdditionalBeanBuildItem quarkusApplication(CombinedIndexBuildItem combinedIndex)
List<String> quarkusApplications = new ArrayList<>();
for (ClassInfo quarkusApplication : combinedIndex.getIndex()
.getAllKnownImplementors(DotName.createSimple(QuarkusApplication.class.getName()))) {
if (quarkusApplication.classAnnotation(DotNames.DECORATOR) == null) {
if (quarkusApplication.declaredAnnotation(DotNames.DECORATOR) == null) {
quarkusApplications.add(quarkusApplication.name().toString());
}
}
Expand Down Expand Up @@ -470,7 +470,7 @@ public ObserverRegistrationPhaseBuildItem registerSyntheticObservers(BeanRegistr
} else {
declaringClass = injectionPoint.getTarget().asMethod().declaringClass();
}
if (declaringClass.classAnnotation(DotNames.KOTLIN_METADATA_ANNOTATION) != null) {
if (declaringClass.declaredAnnotation(DotNames.KOTLIN_METADATA_ANNOTATION) != null) {
validationErrors.produce(new ValidationErrorBuildItem(
new DefinitionException(
"kotlin.collections.List cannot be used together with the @All qualifier, please use MutableList or java.util.List instead: "
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package io.quarkus.arc.test.unused;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.logging.Formatter;
import java.util.logging.LogRecord;

import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.inject.UnsatisfiedResolutionException;
import javax.enterprise.inject.spi.CDI;

import org.jboss.logmanager.formatters.PatternFormatter;
Expand All @@ -31,7 +32,7 @@ public class ArcLookupProblemDetectedTest {
Formatter fmt = new PatternFormatter("%m");
String message = fmt.format(warning);
assertTrue(message.contains(
"Stack frame: io.quarkus.arc.test.unused.ArcLookupProblemDetectedTest.testWarning"),
"Stack frame: io.quarkus.arc.test.unused.ArcLookupProblemDetectedTest"),
message);
assertTrue(message.contains(
"Required type: class io.quarkus.arc.test.unused.ArcLookupProblemDetectedTest$Alpha"),
Expand All @@ -41,7 +42,7 @@ public class ArcLookupProblemDetectedTest {
@Test
public void testWarning() {
// Note that the warning is only displayed once, subsequent calls use a cached result
assertFalse(CDI.current().select(Alpha.class).isResolvable());
assertThrows(UnsatisfiedResolutionException.class, () -> CDI.current().select(Alpha.class).get());
}

// unused bean, will be removed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ default <T> T instance(Class<T> type, Annotation... qualifiers) {
}

/**
* Note that if there are multiple sub classes of the given type this will return the exact match. This means
* that this can be used to directly instantiate superclasses of other beans without causing problems. This behavior differs
* to standard CDI rules where an ambiguous dependency would exist.
*
* @param type
* @param qualifiers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ public void accept(Closeable closeable) {
shutdownContext.addShutdownTask(new ShutdownContext.CloseRunnable(closeable));
}
};
CurrentIdentityAssociation currentIdentityAssociation = Arc.container().instance(CurrentIdentityAssociation.class)
.get();
CurrentIdentityAssociation currentIdentityAssociation = Arc.container().select(CurrentIdentityAssociation.class)
.orNull();
ClassLoader tccl = Thread.currentThread().getContextClassLoader();
if (contextFactory == null) {
contextFactory = new RequestContextFactory() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,7 @@ public void visit(int version, int access, String name, String signature,
mv.visitInsn(Opcodes.RETURN);
mv.visitMaxs(1, 1);
mv.visitEnd();
LOGGER.debugf("Added a no-args constructor to bean class: ", className);
LOGGER.debugf("Added a no-args constructor to bean class: %s", className);
}
};
}
Expand All @@ -935,7 +935,7 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str
if (name.equals(Methods.INIT)) {
access = access & (~Opcodes.ACC_PRIVATE);
LOGGER.debugf(
"Changed visibility of a private no-args constructor to package-private: ",
"Changed visibility of a private no-args constructor to package-private: %s",
className);
}
return super.visitMethod(access, name, descriptor, signature, exceptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static java.util.stream.Collectors.toList;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;

import io.quarkus.arc.Arc;
import io.quarkus.arc.Components;
Expand All @@ -15,6 +16,7 @@
import io.quarkus.gizmo.ClassCreator;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.gizmo.FieldDescriptor;
import io.quarkus.gizmo.FunctionCreator;
import io.quarkus.gizmo.MethodCreator;
import io.quarkus.gizmo.MethodDescriptor;
import io.quarkus.gizmo.ResultHandle;
Expand Down Expand Up @@ -122,12 +124,18 @@ Collection<Resource> generate(String name, BeanDeployment beanDeployment, Map<Be
beanIdToBeanHandle);

// Break removed beans processing into multiple addRemovedBeans() methods
ResultHandle removedBeansHandle = getComponents.newInstance(MethodDescriptor.ofConstructor(ArrayList.class));
ResultHandle typeCacheHandle = getComponents.newInstance(MethodDescriptor.ofConstructor(HashMap.class));
FunctionCreator removedBeansSupplier = getComponents.createFunction(Supplier.class);
BytecodeCreator removedBeansSupplierBytecode = removedBeansSupplier.getBytecode();
ResultHandle removedBeansList = removedBeansSupplierBytecode
.newInstance(MethodDescriptor.ofConstructor(ArrayList.class));
ResultHandle typeCacheHandle = removedBeansSupplierBytecode.newInstance(MethodDescriptor.ofConstructor(HashMap.class));
if (detectUnusedFalsePositives) {
processRemovedBeans(componentsProvider, getComponents, removedBeansHandle, typeCacheHandle, beanDeployment,
// Generate static addRemovedBeans() methods
processRemovedBeans(componentsProvider, removedBeansSupplierBytecode, removedBeansList, typeCacheHandle,
beanDeployment,
classOutput);
}
removedBeansSupplierBytecode.returnValue(removedBeansList);

// All qualifiers
ResultHandle qualifiers = getComponents.newInstance(MethodDescriptor.ofConstructor(HashSet.class));
Expand All @@ -150,8 +158,8 @@ Collection<Resource> generate(String name, BeanDeployment beanDeployment, Map<Be

ResultHandle componentsHandle = getComponents.newInstance(
MethodDescriptor.ofConstructor(Components.class, Collection.class, Collection.class, Collection.class,
Map.class, Collection.class, Map.class, Set.class),
beansHandle, observersHandle, contextsHandle, transitiveBindingsHandle, removedBeansHandle,
Map.class, Supplier.class, Map.class, Set.class),
beansHandle, observersHandle, contextsHandle, transitiveBindingsHandle, removedBeansSupplier.getInstance(),
qualifiersNonbindingMembers, qualifiers);
getComponents.returnValue(componentsHandle);

Expand All @@ -161,8 +169,11 @@ Collection<Resource> generate(String name, BeanDeployment beanDeployment, Map<Be
List<Resource> resources = new ArrayList<>();
for (Resource resource : classOutput.getResources()) {
resources.add(resource);
resources.add(ResourceImpl.serviceProvider(ComponentsProvider.class.getName(),
(resource.getName().replace('/', '.')).getBytes(StandardCharsets.UTF_8), null));
if (resource.getName().endsWith(COMPONENTS_PROVIDER_SUFFIX)) {
// We need to filter out nested classes and functions
resources.add(ResourceImpl.serviceProvider(ComponentsProvider.class.getName(),
(resource.getName().replace('/', '.')).getBytes(StandardCharsets.UTF_8), null));
}
}
return resources;
}
Expand Down Expand Up @@ -266,10 +277,10 @@ private void processObservers(ClassCreator componentsProvider, MethodCreator get
}
}

private void processRemovedBeans(ClassCreator componentsProvider, MethodCreator getComponents,
private void processRemovedBeans(ClassCreator componentsProvider, BytecodeCreator targetMethod,
ResultHandle removedBeansHandle, ResultHandle typeCacheHandle, BeanDeployment beanDeployment,
ClassOutput classOutput) {
try (RemovedBeanAdder removedBeanAdder = new RemovedBeanAdder(componentsProvider, getComponents, removedBeansHandle,
try (RemovedBeanAdder removedBeanAdder = new RemovedBeanAdder(componentsProvider, targetMethod, removedBeansHandle,
typeCacheHandle, classOutput)) {
for (BeanInfo remnovedBean : beanDeployment.getRemovedBeans()) {
removedBeanAdder.addComponent(remnovedBean);
Expand Down Expand Up @@ -388,10 +399,10 @@ MethodCreator newAddMethod() {

@Override
void invokeAddMethod() {
getComponentsMethod.invokeVirtualMethod(
targetMethod.invokeVirtualMethod(
MethodDescriptor.ofMethod(componentsProvider.getClassName(),
addMethod.getMethodDescriptor().getName(), void.class, Map.class, List.class),
getComponentsMethod.getThis(), beanIdToBeanHandle, observersHandle);
targetMethod.getThis(), beanIdToBeanHandle, observersHandle);
}

@Override
Expand Down Expand Up @@ -435,9 +446,9 @@ class RemovedBeanAdder extends ComponentAdder<BeanInfo> {

private final MapTypeCache typeCache;

public RemovedBeanAdder(ClassCreator componentsProvider, MethodCreator getComponentsMethod,
public RemovedBeanAdder(ClassCreator componentsProvider, BytecodeCreator targetMethod,
ResultHandle removedBeansHandle, ResultHandle typeCacheHandle, ClassOutput classOutput) {
super(getComponentsMethod, componentsProvider);
super(targetMethod, componentsProvider);
this.removedBeansHandle = removedBeansHandle;
this.typeCacheHandle = typeCacheHandle;
this.sharedQualifers = new HashMap<>();
Expand All @@ -454,10 +465,10 @@ MethodCreator newAddMethod() {
// Clear the shared maps for each addRemovedBeansX() method
sharedQualifers.clear();

// private void addRemovedBeans1(List removedBeans, List typeCache)
// static void addRemovedBeans1(List removedBeans, List typeCache)
MethodCreator addMethod = componentsProvider
.getMethodCreator(ADD_REMOVED_BEANS + group++, void.class, List.class, Map.class)
.setModifiers(ACC_PRIVATE);
.setModifiers(ACC_STATIC);
// Get the TCCL - we will use it later
ResultHandle currentThread = addMethod
.invokeStaticMethod(MethodDescriptors.THREAD_CURRENT_THREAD);
Expand All @@ -470,10 +481,11 @@ MethodCreator newAddMethod() {

@Override
void invokeAddMethod() {
getComponentsMethod.invokeVirtualMethod(
// Static methods are invoked from within the generated supplier
targetMethod.invokeStaticMethod(
MethodDescriptor.ofMethod(componentsProvider.getClassName(),
addMethod.getMethodDescriptor().getName(), void.class, List.class, Map.class),
getComponentsMethod.getThis(), removedBeansHandle, typeCacheHandle);
removedBeansHandle, typeCacheHandle);
}

@Override
Expand Down Expand Up @@ -613,10 +625,10 @@ MethodCreator newAddMethod() {

@Override
void invokeAddMethod() {
getComponentsMethod.invokeVirtualMethod(
targetMethod.invokeVirtualMethod(
MethodDescriptor.ofMethod(componentsProvider.getClassName(),
addMethod.getMethodDescriptor().getName(), void.class, Map.class),
getComponentsMethod.getThis(), beanIdToBeanHandle);
targetMethod.getThis(), beanIdToBeanHandle);
}

@Override
Expand Down Expand Up @@ -708,12 +720,12 @@ static abstract class ComponentAdder<T extends InjectionTargetInfo> implements A
protected int group;
private int componentsAdded;
protected MethodCreator addMethod;
protected final MethodCreator getComponentsMethod;
protected final BytecodeCreator targetMethod;
protected final ClassCreator componentsProvider;

public ComponentAdder(MethodCreator getComponentsMethod, ClassCreator componentsProvider) {
public ComponentAdder(BytecodeCreator getComponentsMethod, ClassCreator componentsProvider) {
this.group = 1;
this.getComponentsMethod = getComponentsMethod;
this.targetMethod = getComponentsMethod;
this.componentsProvider = componentsProvider;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ public interface ArcContainer {
* Returns a supplier that can be used to create new instances, or null if no matching bean can be found.
*
* Note that if there are multiple sub classes of the given type this will return the exact match. This means
* that this can be used to directly instantiate superclasses of other beans without causing problems.
* that this can be used to directly instantiate superclasses of other beans without causing problems. This behavior differs
* to standard CDI rules where an ambiguous dependency would exist.
*
* see https://github.com/quarkusio/quarkus/issues/3369
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
import java.util.Collection;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

public final class Components {

private final Collection<InjectableBean<?>> beans;
private final Collection<RemovedBean> removedBeans;
private final Supplier<Collection<RemovedBean>> removedBeans;
private final Collection<InjectableObserverMethod<?>> observers;
private final Collection<InjectableContext> contexts;
private final Map<Class<? extends Annotation>, Set<Annotation>> transitiveInterceptorBindings;
Expand All @@ -18,7 +19,8 @@ public final class Components {
public Components(Collection<InjectableBean<?>> beans, Collection<InjectableObserverMethod<?>> observers,
Collection<InjectableContext> contexts,
Map<Class<? extends Annotation>, Set<Annotation>> transitiveInterceptorBindings,
Collection<RemovedBean> removedBeans, Map<String, Set<String>> qualifierNonbindingMembers, Set<String> qualifiers) {
Supplier<Collection<RemovedBean>> removedBeans, Map<String, Set<String>> qualifierNonbindingMembers,
Set<String> qualifiers) {
this.beans = beans;
this.observers = observers;
this.contexts = contexts;
Expand All @@ -44,7 +46,7 @@ public Map<Class<? extends Annotation>, Set<Annotation>> getTransitiveIntercepto
return transitiveInterceptorBindings;
}

public Collection<RemovedBean> getRemovedBeans() {
public Supplier<Collection<RemovedBean>> getRemovedBeans() {
return removedBeans;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,26 @@ public interface InjectableInstance<T> extends Instance<T> {
@Override
Iterator<T> iterator();

/**
* If there is exactly one bean that matches the required type and qualifiers, returns the instance, otherwise returns
* {@code other}.
*
* @param other
* @return the bean instance or the other value
*/
default T orElse(T other) {
return isResolvable() ? get() : other;
}

/**
* If there is exactly one bean that matches the required type and qualifiers, returns the instance, otherwise returns
* {@code null}.
*
* @param other
* @return the bean instance or {@code null}
*/
default T orNull() {
return orElse(null);
}

}
Loading

0 comments on commit 17b9747

Please sign in to comment.