Hi all,
I'm currently facing a problem with the CDIScanner in TomEE. I noticed this
old JIRA https://issues.apache.org/jira/browse/TOMEE-1640
To avoid potential NCDF errors in CDI Extensions using reflection, we
introduced an eager check for NCDF errors while registering beans. But by
default the error is swallowed and the bean is not registered. So if you
have a bean with a dependency to a bean where the eager check failed, then
the exception is reported at the wrong place
Here is an example.
public class BeanA {
private final MyDep dep = new MyDep();
}
MyDep comes from a library not available in the application or in TomEE.
When scanning BeanA, our CDIScanner will load the class as follow
private Class load(final String className, final ClassLoader classLoader) {
try {
final Class<?> loadClass = classLoader.loadClass(className);
tryToMakeItFail(loadClass);
return loadClass;
} catch (final ClassNotFoundException e) {
return null;
} catch (final NoClassDefFoundError e) {
if (logDebug) {
Logger.getInstance(LogCategory.OPENEJB_CDI,
CdiScanner.class).warning(className + " -> " + e);
}
return null;
}
}
private void tryToMakeItFail(final Class<?> loadClass) { // we try to
avoid later NoClassDefFoundError
loadClass.getDeclaredFields();
loadClass.getDeclaredMethods();
}
This will trigger a NCDF because MyDep isn't available, and even though the
bean class is not used anymore or part of a feature under development and
not fully available.
The spec says
> Deployment problems are detected by the container at initialization time.
> If a deployment problem exists in a deployment, initialization will be
> aborted by the container.
>
> The container is permitted to define a non-portable mode, for use at
> development time, in which some definition errors and deployment problems
> do not cause application initialization to abort.
>
>
To me, tryToMakeItFail must be called only if the logDebug flag is true. If
not, then we should not eagerly try to load dependencies.
Also because now let's assume there is a
public class BeanB {
@Inject private BeanClassA bean;
}
The app will fail at deploy time because of an unsatisfied dependency. It
won't be able to inject BeanA into BeanB, because BeanA dependencies failed
to load and the bean is discarded without any logs.
That being said, I'm not sure what the default should be.
- Should we "just" do what's required in the spec and not eagerly try to
load dependencies and let the extension to "maybe" fail later
- Eagerly test but then log without having to set up a flag
- Eagerly test and fail deployment which might generate failures for
beans not used in the application
We are halfway between 1 and 2.
Hopefully it's clear.
What do you guys think?
I'd go for 2 but with a slightly more noticable error.
--
Jean-Louis Monteiro
http://twitter.com/jlouismonteiro
http://www.tomitribe.com
