myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mathias Brökelmann" <mbroekelm...@googlemail.com>
Subject Re: Proposal for annotation processing
Date Tue, 13 Mar 2007 21:28:08 GMT
AFAIK the LifecycleProvider will not handle the scope of the beans.
After calling newInstance(..) the returned value will be placed by
myfaces into the right scope (after injecting the declared managed
bean properties in faces config).

2007/3/13, Bernd Bohmann <bernd.bohmann@atanion.com>:
> Hello David,
>
> should every LifecycleProvider handle the difference between none scoped
> beans and other scoped beans. This is not a clean interface.
>
> Regards
>
> Bernd
>
> David Jencks wrote:
> > I actually think my MYFACES-1559-2.patch is a better solution than the
> > current code + my MYFACES-1559-3.patch.  I don't think anyone will be
> > interested in implementing the new myfaces-specific AnnotationProcessor
> > interface: there are lots of projects that know about the former
> > AnnotationProcessor interface (tomcat, glassfish) and the new interface
> > makes it harder to integrate with them.  We have already seen some
> > complaints ("Error in 1.2").  I think having MyFaces use the proposed
> > LifecycleProvider interface and providing an adapter to the already used
> > AnnotationProcessor interface will cause less problems.
> >
> > thanks
> > david jencks
> >
> > On Mar 13, 2007, at 12:40 PM, Mathias Brökelmann wrote:
> >
> >> IMO the simple interface which David suggest is quite sufficient to
> >> solve the problem. I also think that discovery to look up the
> >> implementation is not a good way in a app server with a complex
> >> classloader hierarchy.
> >>
> >> 2007/3/13, David Jencks <david_jencks@yahoo.com>:
> >>> 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
> >>> >
> >>>
> >>>
> >>
> >>
> >> --Mathias
> >
> >
>


-- 
Mathias

Mime
View raw message