You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by David Jencks <dj...@apache.org> on 2007/03/12 16:43:23 UTC
Proposal for annotation processing
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
Re: Proposal for annotation processing
Posted by David Jencks <da...@yahoo.com>.
On Mar 13, 2007, at 3:42 PM, Bernd Bohmann wrote:
> Hello David,
>
> should every LifecycleProvider handle the difference between none
> scoped beans and other scoped beans. This is not a clean interface.
As I mentioned in another post I think the spec intends that
postConstruct methods be called on all managed beans, based on the
spec language you quote
thanks
david jencks
>
> 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 <da...@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
Re: Proposal for annotation processing
Posted by Mathias Brökelmann <mb...@googlemail.com>.
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 <be...@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 <da...@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
Re: Proposal for annotation processing
Posted by Bernd Bohmann <be...@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 <da...@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
>
>
Re: Proposal for annotation processing
Posted by David Jencks <da...@yahoo.com>.
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 <da...@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
Re: Proposal for annotation processing
Posted by Bernd Bohmann <be...@atanion.com>.
David Jencks wrote:
>
> I get to set the instance I want to use, and you get to use discovery?
> I also don't have to worry about whether the discovery framework is
> actually thread safe.
Ok, but I think it's thread safe.
I will apply your changes.
Regards
Bernd
Re: Proposal for annotation processing
Posted by David Jencks <da...@yahoo.com>.
On Mar 13, 2007, at 4:39 PM, Bernd Bohmann wrote:
> Hello David,
>
> comments inline
>
> David Jencks wrote:
>> On Mar 13, 2007, at 3:38 PM, Bernd Bohmann wrote:
>> I read this to mean that the jsf implementation is prohibited from
>> calling posConstruct methdods after putting the bean in scope, but
>> that it is required to call postConstruct on all annotated beans.
>> Do you have some evidence that the spec intends that postConstruct
>> not be called on any managed beans?
>> I asked in my first post on this subject whether a beans scope was
>> determined by its class or whether some instances of a class could
>> be of one scope and some of another. If the scope is determined
>> by its class then my original idea, before seeing the spec
>> language you quote above, was the the annotation processor would
>> ignore PostConstruct annotations on managed beans of scope none.
>> Now that I've seen the spec language, I'm quite sure the intent is
>> that posconstruct be called on all managed beans.
>>
>
> Ok, you are right I understand the spec in the wrong way. I will
> change the interface to your proposal.
Thanks!
>
>
>>>
>>> What is wrong with the discovery?
>>> In geronimo you can set the ProcessorFactory with a System Property.
>> Which do you consider easier to understand and more likely to work
>> in all environments?
>> private Thing thing;
>> public void setThing(Thing thing) {
>> this.thing = thing;
>> }
>> or
>> http://jakarta.apache.org/commons/discovery/apidocs/org/apache/
>> commons/discovery/tools/DiscoverSingleton.html
>
> Can we combine your proposal and the DiscoverSingleton?
>
> What about this:
>
> http://svn.apache.org/repos/asf/myfaces/core/branches/jsf12/impl/
> src/main/java/org/apache/myfaces/config/annotation/
> AnnotationProcessorFactory.java
>
>
I don't see how that relates to my request that I be allowed to set
the instance of the factory in code .... not the class, not
properties, but the actual instance itself.
What's wrong with my proposal in https://issues.apache.org/jira/
secure/attachment/12353202/MYFACES-1559-3.patch
public abstract class LifecycleProviderFactory {
protected static final String FACTORY_DEFAULT =
DefaultLifecycleProviderFactory.class.getName();
private static volatile LifecycleProviderFactory INSTANCE;
public static LifecycleProviderFactory
getLifecycleProviderFactory()
{
LifecycleProviderFactory instance = INSTANCE;
if (instance != null) {
return instance;
}
return (LifecycleProviderFactory) DiscoverSingleton.find
(LifecycleProviderFactory.class, FACTORY_DEFAULT);
}
public static void setLifecycleProviderFactory
(LifecycleProviderFactory instance) {
INSTANCE = instance;
}
public abstract LifecycleProvider getLifecycleProvider
(ExternalContext externalContext);
public abstract void release();
}
I get to set the instance I want to use, and you get to use
discovery? I also don't have to worry about whether the discovery
framework is actually thread safe.
thanks
david jencks
>
>> I've studied the docs for a bit and have no confidence that I
>> could could set it up properly in less than a day and even less
>> confidence that it wouldn't be accidently subverted by a user app.
>> thanks
>> david jencks
>
Re: Proposal for annotation processing
Posted by Bernd Bohmann <be...@atanion.com>.
Hello David,
comments inline
David Jencks wrote:
>
> On Mar 13, 2007, at 3:38 PM, Bernd Bohmann wrote:
>
> I read this to mean that the jsf implementation is prohibited from
> calling posConstruct methdods after putting the bean in scope, but that
> it is required to call postConstruct on all annotated beans.
>
> Do you have some evidence that the spec intends that postConstruct not
> be called on any managed beans?
>
> I asked in my first post on this subject whether a beans scope was
> determined by its class or whether some instances of a class could be of
> one scope and some of another. If the scope is determined by its class
> then my original idea, before seeing the spec language you quote above,
> was the the annotation processor would ignore PostConstruct annotations
> on managed beans of scope none. Now that I've seen the spec language,
> I'm quite sure the intent is that posconstruct be called on all managed
> beans.
>
Ok, you are right I understand the spec in the wrong way. I will change
the interface to your proposal.
>>
>> What is wrong with the discovery?
>> In geronimo you can set the ProcessorFactory with a System Property.
>
> Which do you consider easier to understand and more likely to work in
> all environments?
>
> private Thing thing;
>
> public void setThing(Thing thing) {
> this.thing = thing;
> }
>
> or
>
> http://jakarta.apache.org/commons/discovery/apidocs/org/apache/commons/discovery/tools/DiscoverSingleton.html
>
Can we combine your proposal and the DiscoverSingleton?
What about this:
http://svn.apache.org/repos/asf/myfaces/core/branches/jsf12/impl/src/main/java/org/apache/myfaces/config/annotation/AnnotationProcessorFactory.java
>
> I've studied the docs for a bit and have no confidence that I could
> could set it up properly in less than a day and even less confidence
> that it wouldn't be accidently subverted by a user app.
>
> thanks
> david jencks
>
Re: Proposal for annotation processing
Posted by David Jencks <da...@yahoo.com>.
On Mar 13, 2007, at 3:38 PM, Bernd Bohmann wrote:
> Hello,
>
> how can the simple interface handle managed beans declared to be in
> none scope without knowledge of the scope.
>
> See Section 5.4.1 of the jsf spec:
>
> Methods on managed beans declared to be in request, session, or
> application scope, annotated with @PostConstruct, must be called by
> the JSF implementation after resource injection is performed (if
> any) but before the bean is placed into scope.
I read this to mean that the jsf implementation is prohibited from
calling posConstruct methdods after putting the bean in scope, but
that it is required to call postConstruct on all annotated beans.
Do you have some evidence that the spec intends that postConstruct
not be called on any managed beans?
I asked in my first post on this subject whether a beans scope was
determined by its class or whether some instances of a class could be
of one scope and some of another. If the scope is determined by its
class then my original idea, before seeing the spec language you
quote above, was the the annotation processor would ignore
PostConstruct annotations on managed beans of scope none. Now that
I've seen the spec language, I'm quite sure the intent is that
posconstruct be called on all managed beans.
>
> What is wrong with the discovery?
> In geronimo you can set the ProcessorFactory with a System Property.
Which do you consider easier to understand and more likely to work in
all environments?
private Thing thing;
public void setThing(Thing thing) {
this.thing = thing;
}
or
http://jakarta.apache.org/commons/discovery/apidocs/org/apache/
commons/discovery/tools/DiscoverSingleton.html
I've studied the docs for a bit and have no confidence that I could
could set it up properly in less than a day and even less confidence
that it wouldn't be accidently subverted by a user app.
thanks
david jencks
> Regards
>
> Bernd
>
>
> 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 <da...@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
>>> >
>>>
>>>
Re: Proposal for annotation processing
Posted by Bernd Bohmann <be...@atanion.com>.
Hello,
how can the simple interface handle managed beans declared to be in none
scope without knowledge of the scope.
See Section 5.4.1 of the jsf spec:
Methods on managed beans declared to be in request, session, or
application scope, annotated with @PostConstruct, must be called by the
JSF implementation after resource injection is performed (if any) but
before the bean is placed into scope.
What is wrong with the discovery?
In geronimo you can set the ProcessorFactory with a System Property.
Regards
Bernd
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 <da...@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
>> >
>>
>>
>
>
Re: Proposal for annotation processing
Posted by Mathias Brökelmann <mb...@googlemail.com>.
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 <da...@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
Re: Proposal for annotation processing
Posted by David Jencks <da...@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
>
Re: Proposal for annotation processing
Posted by Bernd Bohmann <be...@atanion.com>.
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
>
>
>
>
>
>
Fwd: Proposal for annotation processing (MyFaces)
Posted by David Jencks <da...@yahoo.com>.
I've started talking about my idea for how to do annotation
processing in myfaces on the myfaces dev list.
thanks
david jencks
Begin forwarded message:
> From: David Jencks <dj...@apache.org>
> Date: March 12, 2007 11:43:23 AM EDT
> To: MyFaces Development <de...@myfaces.apache.org>
> Subject: Proposal for annotation processing
> Reply-To: "MyFaces Development" <de...@myfaces.apache.org>
>
> 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
>
>
>
>
>