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: us...@infra.apache.org