philippkunz commented on a change in pull request #28:
URL: https://github.com/apache/openwebbeans/pull/28#discussion_r439839380
##########
File path:
webbeans-junit5/src/main/java/org/apache/openwebbeans/junit5/internal/CdiExtension.java
##########
@@ -172,4 +182,76 @@ private void doClose(final SeContainer container)
}
});
}
+
+ private Cdi getCdiConfig(ExtensionContext extensionContext)
+ {
+ return AnnotationUtils.findAnnotation(extensionContext.getElement(),
Cdi.class).orElse(null);
+ }
+
+ private SeContainer getContainer()
+ {
+ if (testInstanceContainer != null)
+ {
+ return testInstanceContainer;
+ }
+ else
+ {
+ return reusableContainer;
+ }
+ }
+
+ @Override
+ public boolean supportsParameter(ParameterContext parameterContext,
ExtensionContext extensionContext)
+ throws ParameterResolutionException
+ {
+ final Cdi config =
extensionContext.getParent().map(this::getCdiConfig).orElse(null);
+ if (config == null || !config.injectParameters())
+ {
+ return false;
+ }
+
+ final SeContainer container = getContainer();
+ if (container == null)
+ {
+ return false;
+ }
+
+ Bean<?> bean = resolveParameterBean(container, parameterContext,
extensionContext);
+ return bean != null;
+ }
+
+ @Override
+ public Object resolveParameter(ParameterContext parameterContext,
ExtensionContext extensionContext)
+ throws ParameterResolutionException
+ {
+ final SeContainer container = getContainer();
+ if (container == null)
+ {
+ return false;
+ }
+
+ Bean<?> bean = resolveParameterBean(container, parameterContext,
extensionContext);
+ BeanManager beanManager = container.getBeanManager();
+ CreationalContext<?> creationalContext =
beanManager.createCreationalContext(bean);
+ creationalContexts.add(creationalContext);
+ return beanManager.getReference(bean,
parameterContext.getParameter().getType(), creationalContext);
+ }
+
+ private Bean<?> resolveParameterBean(SeContainer container,
ParameterContext parameterContext, ExtensionContext extensionContext)
+ {
+ BeanManager beanManager = container.getBeanManager();
+ Set<Bean<?>> beans = beanManager.getBeans(
+ parameterContext.getParameter().getType(),
+ getQualifiers(parameterContext.getParameter()));
+ return beanManager.resolve(beans);
Review comment:
I guess your statement is wrong:
> ambiguous parameter injection does not throw AmbiguousResolutionException
since it is not ambiguous for CDI but for junit and this one takes the first
extensions returning true in supportsParameter
Ambiguous in cdi bean throws an AmbiguousResolutionException and more than
one parameter resolver returning supportsParameter true makes JUnit throw
ParameterResolutionException
> About the speed up: avoid twice the same instantiation
The current version of the code already avoids that. See a more recent
version of the `ParameterResolutionTest` test near
`testThatParameterGetsInjectedSameScopedBeanInstance`.
> That said if you take the option to split the parameter injections in
another extension then it is fully fine to impl it this way since you will not
impact @cdi + field injections users which is what we should avoid with this
new feature IMHO.
I would not see how this PR would affect cdi field injection. It only adds a
parameter resolver and does not (should not as far I'm aware of or it would be
a bug). It could possibly make a test method parameter ambiguously resolved by
more than one parameter resolver but then that would not affect field injection
either would it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
