rmannibucau commented on a change in pull request #28:
URL: https://github.com/apache/openwebbeans/pull/28#discussion_r439825961
##########
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:
About "All the spurious cases fail this way and nothing has to be logged
or anything.", 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. So
question is: do we document how to solve that (custom qualifier making it not
resolvable by cdi for ex) or create the skip API i thought about originally.
Tempted to say both work for me after some thoughts, so I will let you decide
;).
About the speed up: factually it is not negligible so questions are:
1. Which does win: cache impl vs runtime overhead (tempted to say cache impl
is trivial and its lifecycle trivial to handle so it is worth avoid twice the
same instantiation)
2. What about @Dependent instances (should it be the same instance by test
or a new one per injection point)
3. Do we care cause generally people will just inject most beans in the
class (fields) and use parameter injections only for specific needs (maybe to
get new instances as mentionned in 2).
So overall - and thanks to 3 - I tend to think your impl can be good enough
to start (for next release) and we can gather feedback and refine it if needed.
However, it should be explicit in the doc how parameter injection is done,
how cdi param injection can be skipped in favor of another junit extension and
that field injection can be preferred for method stability (IDE support) +
reuse accross instances (@testinstance). So likely a note in
http://svn.apache.org/repos/asf/openwebbeans/cms-site/trunk/content/openwebbeans-junit5.mdtext
would solve most of my concerns.
----------------------------------------------------------------
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:
[email protected]
