Hello Bernd,
Thanks for looking into this. I think geronimo might be able to work
with the changes you have made, but I don't think it would be a good
idea in the current form. I have two suggestions.
1. Please make use of the discovery mechanism optional. Geronimo
controls the startup order of all these components and it is much
simpler to simply create and install the instances of the components
at he appropriate time during startup rather than hiding the
information about what class (not instance) will be used somewhere in
the structure of the classloading hierarchy and then trying to fish
it out again through some procedure that takes at least a full page
to explain. I feel strongly enough about this that I would rather
use setAccessible on private members of the discovery classes to
install components than to use it as it is intended. Also, in its
current form geronimo is getting exceptions from the discovery code
on shutdown and its possible that the changes have significantly
slowed startup.
2. While I think adding a newInstance method to AnnotationProcessor
is a step forward, I would still prefer that myfaces use an interface
like the one I suggested with only
newInstance
and
destroyInstance
methods, and provided an adapter to the AnnotationProcessor
interface. If geronimo implements the AnnotationProcessor interface
itself, we will do all the work implied by the newInstance,
processAnnotations, and postConstruct methods in the newInstance
method. While right now this won't cause any problems according to
my analysis of the MyFaces code currently using the
AnnotationProcessor interface, it makes it very easy for future
developers to insert required functionality between MyFaces calls to
these methods. If these calls to newInstance, processAnnotations,
and postConstruct are all in an adapter class if is very easy to find
and compensate for any changes in this area.
I've attached a further patch MYFACES-1559-3.patch with suggestions
for these changes to the jira issue.
i'll mention that I think it might simplify the structure, ease of
understanding, and speed of myfaces if the LifecycleProvider
instances used by ManagedBeanFactory and the listeners were supplied
to their constructors and held in final variables. This would
require that there were instances of ManagedBeanFactory and the
listeners for each application deployed. Despite my interest in this
I don't have time or sufficient understanding of myfaces to try to
implement it at this time. This is similar to my desire to have
geronimo directly set the LifecycleProviderFactory/
AnnotationProcessorFactory rather than having the factory examine its
environment for something that might work.
many thanks!
david jencks
On Mar 12, 2007, at 6:06 PM, Bernd Bohmann wrote:
Hello David,
we can move the AnnotationProcessor to the package
org.apache.myfaces or an other package and add the method
Object newInstance(String className);
to the interface.
(I like the idea for possible constructor dependency injection)
And we should lookup the AnnotationProcessorFactory with
JDK1.3-style service discovery.
http://jakarta.apache.org/commons/discovery/apidocs/org/apache/
commons/discovery/tools/DiscoverSingleton.html
With this service discovery you can add your
ApplicationIndexedAnnotationPrcessorFactory
Just commited my changes based on your idea.
Regards
Bernd
David Jencks wrote:
There's been a lot of discussion about annotation processing in a
long thread http://www.nabble.com/%40PreDestroy%2C-Servlet-API%2C-
tf3284592.html#a9136472 The current state of the code is that
managed objects are created by MyFaces code, and then fed to an
annotation processor using an interface like:
public interface AnnotationProcessor {
public void postConstruct(Object instance);
public void preDestroy(Object instance);
public void processAnnotations(Object instance);
}
(Exceptions removed for clarity)
I have been implementing annotation support in the geronimo app
client container and the geronimo-jetty6 integration and studying
the openejb3 and native jetty annotation support and am starting
to look at annotation support in a geronimo-myfaces integration,
and have some ideas about how I'd like to handle geronimo
injecting dependencies into jsf managed beans.
I'd like to propose that MyFaces use an interface like this for
dealing with managed object construction, dependency injection,
and lifecycle methods:
public interface LifecycleProvider {
Object newInstance(String className);
void destroyInstance(Object o);
}
This would fit in well with how annotation processing/dependency
injection is done in the rest of geronimo. It also would let the
container in which MyFaces is running supply additional features
such as supporting constructor dependency injection.
To go into what is probably blindingly obvious detail, this would
be a MyFaces interface and the container in which MyFaces is
running would supply an object implementing this interface for
each application.
It's more or less trivial to write an adapter between this
interface and the AnnotationProcessor interface currently in use,
for integration with containers that want to supply an
AnnotationProcessor.
So far I've thought of two issues, IMO one minor and the other
requiring more thought (at least on my part :-). Also I'm not at
all familiar with the jsf spec so it's entirely possible I'm
proposing details that can't be implemented.
1. The current code looks for ManagedBeanBuilder.NONE in between
injecting dependencies and calling postConstruct. I don't think
this is appropriate: I think whatever is handling the annotations
should know not to call postConstruct for this class. If however
the same class can be used in several different scopes the
newInstance method could take the ManagedBean as parameter instead
of the class name.
2. I'm proposing that the container inject an instance of
LifecycleProvider for each jsf application. This leads to the
question, injects into what? One simple possibility is a
singleton LifecycleProviderFactory that indexes LifecycleProvider
by application classloader. However I wonder if there is a way to
more directly inject the LifecycleProvider into the parts of
MyFaces that actually need to use it rather than making these
components go fishing for the one they need. The kind of lazy
initialization currently in wide use requires a lot more
synchronization than it currently has to work reliably. I would
prefer to use constructor dependency injection to final fields to
avoid this kind of problem.
I've opened a jira issue to hold code samples related to this
proposal, and attached some initial implementations of these ideas
for discussion. Right now these new classes aren't hooked up to
MyFaces, although I plan to work on that next.
Initial classes:
A core/impl/src/main/java/org/apache/myfaces/config/
annotation/LifecycleProviderFactory.java abstract class for
singleton factory.
A core/impl/src/main/java/org/apache/myfaces/config/
annotation/ApplicationIndexedLifecycleProviderFactory.java a
LifecycleProviderFactory that expects to be populated by an
external framework, with one LifecycleProvider instance per
application classloader.
A core/impl/src/main/java/org/apache/myfaces/config/
annotation/LifecycleProviderToAnnotationProcessorAdapter.java an
adapter between the LifecycleProvider implementation I'm proposing
and the existing AnnotationProcessor interface currently in use.
This basically relies on there being only one AnnotationProcessor
shared between all applications. This matches the current
implementation but I think it is unsatisfactory in general.
A core/impl/src/main/java/org/apache/myfaces/config/
annotation/LifecycleProvider.java Proposed interface for MyFaces
to plug in external services that handle annotations, object
construction, etc.
A core/impl/src/main/java/org/apache/myfaces/config/
annotation/AnnotationProcessorLifecycleProviderFactory.java a
LifecycleProviderFactory that uses the
LifecycleProviderToAnnotationProcessorAdapter.
The jira issue is https://issues.apache.org/jira/browse/MYFACES-1559
Comments? Flames?
many thanks,
david jencks
