On 3/3/07, Mathias Brökelmann <[EMAIL PROTECTED]> wrote:
No code in myfaces (or any other apache project) is owned by someone. That is important for a healthy and moving community. Bernds submission was a little bit fast but sometimes a complex issue is better understand if we can look at the code - no matter if that is the solution we finally have.
I hope you did not get the idea that this was about territory here. How about this solution:
We define a myfaces specific interface of an AnnotationProcessor/InjectionProvider which will be used throughout the code in myfaces. The initialization code lookups an implementation for that interface. If there is nothing found (through context param, META-INF/service, etc.) we implement some fallback strategies to either looking for a InjectionProvider implementation for suns RI or if that also fails we try to lookup container specific implementations like tomcats AnnotationProcessor implementation. If all fails we supply a default implementation which only handles @PostConstruct and @PreDestroy annotations - I don't think that it is possible to inject any resource w/o knowing how to lookup these.
This sounds very similar to what Ryan just talked about in his blog. Bernd, if are happy w/ this and still interested, feel free to go forward with this. If you still want to, I ask that the default implementation uses the annotation name, rather than the annotation. This gives Tomcat 5.5/Facelets users fewer headaches. Do either of you have a problem w/ the context listener doing a classpath search for well known injection providers? I meet a lot of devs who find JSF too configuration heavy. Bernd if you don't get around to doing this, I will just end up doing it sometime next week. 2007/3/3, Bernd Bohmann <[EMAIL PROTECTED]>:
> Sorry, > I ask you if it's ok to add the code. > I get no answer then I asume it's ok to add my idea. > > It's only a different possible implementation. > If you don't like it you can remove it. > Is anything wrong with the code? > > Bernd > > > > Dennis Byrne wrote: > > Mathias, me and Paul were discussing the future of this feature. For the > > most part we were in agreement. I appreciate your input, but it did not > > gain traction w/ anyone. You then went ahead and committed a bunch of > > code. > > > > > > IMO, you have not been a good team member today Bernd. Please assign the > > issue to yourself. > > > > Thank you very much for your interest in getting us closer to a > > 1.2implementation. > > > > Dennis Byrne > > > > On 3/3/07, Bernd Bohmann <[EMAIL PROTECTED]> wrote: > >> > >> ??? Yes, I know it. > >> > >> Dennis Byrne wrote: > >> > Are you under the impression that our 1.2 branch didn't already handle > >> > @PostConstruct and @PreDestroy? Looks like the left hand doesn't know > >> what > >> > the right hand is doing here. > >> > > >> > Dennis Byrne > >> > > >> > On 3/3/07, Bernd Bohmann <[EMAIL PROTECTED]> wrote: > >> >> > >> >> Just added an implementation of the annotation handling based on the > >> >> tomcat implementation. > >> >> > >> >> Bernd > >> >> > >> >> Dennis Byrne wrote: > >> >> > I added a comment > >> >> > > >> >> > >> http://blogs.sun.com/rlubke/entry/jsf_ri_1_2_and#comment-1172947192023. > >> >> > > >> >> > Two more ideas. Ryan mentions applications using facelets, jsf > >> 1.2and > >> >> > servlet 2.4. Unless I'm missing something, there's one reason to > >> use > >> >> > annotation.getName().equals("javax.annotation.PostConstruct") rather > >> >> than > >> >> > annotation.class == PostConstruct.class. Also, the tomcat guys are > >> >> > processing annotations even when the method is marked private. > >> >> > > >> >> > Dennis Byrne > >> >> > > >> >> > On 3/3/07, Paul McMahan <[EMAIL PROTECTED]> wrote: > >> >> >> > >> >> >> OK, I think the RI also recently added the META-INF/services > >> >> >> technique. But I don't know how reliable this sun blog entry is: > >> >> >> http://tinyurl.com/yrrjs2 > >> >> >> > >> >> >> Best wishes, > >> >> >> Paul > >> >> >> > >> >> >> On 3/2/07, Mathias Brökelmann <[EMAIL PROTECTED]> wrote: > >> >> >> > The RI uses two ways to lookup the implementation of the vendor > >> >> >> > specific implementation of the InjectionProvider. They first try > >> to > >> >> >> > use a web context init param and if that is not configured they > >> >> simply > >> >> >> > use a system property. Both keyed by the class name of the > >> >> >> > InjectionProvider interface. > >> >> >> > > >> >> >> > 2007/3/2, Paul McMahan <[EMAIL PROTECTED]>: > >> >> >> > > I think Mathias' suggestion is right on. I haven't looked at > >> the > >> >> RI > >> >> >> > > code but I read somewhere that it passes the name of > >> >> >> > > InjectionProviders via entries in META-INF/services. If > >> MyFaces > >> >> used > >> >> >> > > a similar approach I think it should work OK for Geronimo and > >> just > >> >> >> > > about any other container that supports injection, And it > >> should > >> >> >> also > >> >> >> > > make MyFaces more interchangeable with the RI. > >> >> >> > > > >> >> >> > > Best wishes, > >> >> >> > > Paul > >> >> >> > > > >> >> >> > > On 3/2/07, Dennis Byrne <[EMAIL PROTECTED]> wrote: > >> >> >> > > > Similar to what Mathias mentioned? > >> >> >> > > > > >> >> >> > > > > >> >> http://issues.apache.org/jira/browse/MYFACES-1246#action_12475337 > >> >> >> > > > > >> >> >> > > > It's not much work (on our side) but it sounds pretty vendor > >> >> >> specific. > >> >> >> > > > Again, I don't have a better solution. Mathias writes "which > >> is > >> >> >> implemented > >> >> >> > > > by j2ee containers". I wonder if each container will end up > >> >> >> looking > >> >> >> for > >> >> >> > > > different interfaces. How would MyFaces find Geronimo's > >> >> >> implementation ? A > >> >> >> > > > context parameter? A for loop like this ... > >> >> >> > > > > >> >> >> > > > String[] providers = new > >> >> String[]{"org,apache.geronimo.DIProvider > >> >> ", > >> >> >> > > > "com.bea.DIProvider", "org.jboss.DIProvider"} > >> >> >> > > > > >> >> >> > > > for(String clazz : providers){ > >> >> >> > > > try{ > >> >> >> > > > return ClassUtils.load (clazz) > >> >> >> > > > }catch(){} > >> >> >> > > > } > >> >> >> > > > > >> >> >> > > > Dennis Byrne > >> >> >> > > > > >> >> >> > > > > >> >> >> > > > On 3/1/07, Paul McMahan <[EMAIL PROTECTED]> wrote: > >> >> >> > > > > OK, I think your interpretation of the spec makes sense and > >> >> >> there's > >> >> >> > > > > one particular aspect we should discuss a little further. > >> >> >> > > > > > >> >> >> > > > > The container doesn't know which classes are managed beans > >> so > >> >> it > >> >> >> > > > > wouldn't know when to intercept the instantiation > >> process to > >> >> >> perform > >> >> >> > > > > injection. What would you all think about MyFaces > >> >> providing an > >> >> >> > > > > interface that containers could implement to perform > >> injection > >> >> on > >> >> >> a > >> >> >> > > > > managed bean when MyFaces brings that bean into > >> service? This > >> >> >> would > >> >> >> > > > > allow MyFaces to maintain control of the timing while > >> allowing > >> >> >> the > >> >> >> > > > > container to scan for annotations and handle injection when > >> >> >> prompted > >> >> >> > > > > to do so. > >> >> >> > > > > > >> >> >> > > > > Best wishes, > >> >> >> > > > > Paul > >> >> >> > > > > > >> >> >> > > > > > >> >> >> > > > > On 2/26/07, Dennis Byrne <[EMAIL PROTECTED]> wrote: > >> >> >> > > > > > I know the specs can be vague sometimes, but I don't > >> >> interpret > >> >> >> the first > >> >> >> > > > > > paragraph of section 5.4 as meaning the JSF > >> >> implementation is > >> >> >> > > > responsible > >> >> >> > > > > > for @EJB, @Resources, etc. The JSF spec specifically > >> >> mentions > >> >> >> "the > >> >> >> > > > > > container to inject references". If Geronimo can > >> intercept > >> >> the > >> >> >> > > > > > instantiation of these classes in the classloader > >> (something > >> >> I > >> >> >> know > >> >> >> > > > nothing > >> >> >> > > > > > about right now), I think we are all good here. Wouldn't > >> >> >> MyFaces then > >> >> >> > > > be > >> >> >> > > > > > observing the requirements (in plain java) around > >> >> >> @PostConstruct > >> >> >> after > >> >> >> > > > > > Geronimo had injected the resources? > >> >> >> > > > > > > >> >> >> > > > > > I think the JSF impl is only responsible for > >> @PostConstruct > >> >> and > >> >> >> > > > @Predestroy. > >> >> >> > > > > > This makes sense because scoped (request, session, > >> >> >> application) > >> >> >> are the > >> >> >> > > > > > only candidates for this, and it would be more awkward to > >> do > >> >> >> that from > >> >> >> > > > the > >> >> >> > > > > > container. I say all of this in an open manner, so anyone > >> >> feel > >> >> >> free to > >> >> >> > > > > > discuss. > >> >> >> > > > > > > >> >> >> > > > > > You're point about javax.annotation being in the Servlet > >> >> 2.5is > >> >> >> taken. > >> >> >> > > > I > >> >> >> > > > > > totally missed that. > >> >> >> > > > > > > >> >> >> > > > > > > >> >> >> > > > > > Dennis Byrne > >> >> >> > > > > > > >> >> >> > > > > > On 2/26/07, Paul McMahan <[EMAIL PROTECTED]> wrote: > >> >> >> > > > > > > Actually by "dependency injection" I'm specifically > >> >> referring > >> >> >> to the > >> >> >> > > > > > > portion of the JSF spec that discusses injecting > >> resources > >> >> >> where > >> >> >> > > > > > > certain annotations are found in a managed bean. So, > >> >> while > >> >> >> scanning > >> >> >> > > > > > > for the @PreConstruct and @PostDestroy annotations > >> MyFaces > >> >> >> would also > >> >> >> > > > > > > scan for @EJB, @Resource, @WebServiceRef, etc and then > >> >> time > >> >> >> the > >> >> >> > > > > > > invocation of @PreConstruct and @PostDestroy to support > >> >> the > >> >> >> injection. > >> >> >> > > > > > > > >> >> >> > > > > > > Tomcat provides an example of how this can be done. > >> The > >> >> >> following > >> >> >> > > > > > > class scans for annotations when a web app starts: > >> >> >> > > > > > > > >> >> >> > > > > > > >> >> >> > > > > >> >> >> > >> >> > >> http://svn.apache.org/repos/asf/tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/WebAnnotationSet.java > >> > >> >> > >> >> >> > >> >> >> > > > > > > > >> >> >> > > > > > > And then this class handles the injection and calling > >> the > >> >> >> > > > > > > PostConstruct and PreDestroy methods when an servlet, > >> >> filter, > >> >> >> or > >> >> >> > > > > > > listener is brought into service: > >> >> >> > > > > > > > >> >> >> > > > > > > >> >> >> > > > > >> >> >> > >> >> > >> http://svn.apache.org/repos/asf/tomcat/tc6.0.x/trunk/java/org/apache/catalina/util/DefaultAnnotationProcessor.java > >> > >> >> > >> >> >> > >> >> >> > > > > > > > >> >> >> > > > > > > Would it make sense for MyFaces to follow a similar > >> >> approach > >> >> >> for > >> >> >> > > > > > > managed beans? Also, I'm curious why you're hoping to > >> >> avoid > >> >> >> importing > >> >> >> > > > > > > classes from javax.annotation classes since servlet > >> >> >> 2.5containers are > >> >> >> > > > > > > required to support them. Is this so that MyFaces > >> 1.2can > >> >> >> support > >> >> >> > > > > > > servlet 2.4 containers? > >> >> >> > > > > > > > >> >> >> > > > > > > Best wishes, > >> >> >> > > > > > > Paul > >> >> >> > > > > > > > >> >> >> > > > > > > On 2/26/07, Dennis Byrne <[EMAIL PROTECTED]> wrote: > >> >> >> > > > > > > > I ended up going with Servlet/Request/Context > >> attribute > >> >> >> listeners > >> >> >> > > > for > >> >> >> > > > > > > > @PreDestroy. This did not involve importing > >> >> >> > > > > > javax.annotation.PreDestroy, so > >> >> >> > > > > > > > people w/out application servers don't have to worry > >> >> about > >> >> >> > > > > > > > ClassDefNotFoundErrors. This also keeps us > >> compatible > >> >> with > >> >> >> the > >> >> >> > > > > > reference > >> >> >> > > > > > > > implementation in terms of timing, although I really > >> >> wish > >> >> >> they'd > >> >> >> > > > change > >> >> >> > > > > > the > >> >> >> > > > > > > > wording in the spec. > >> >> >> > > > > > > > > >> >> >> > > > > > > > Dennis Byrne > >> >> >> > > > > > > > > >> >> >> > > > > > > > > >> >> >> > > > > > > > On 2/26/07, Paul McMahan <[EMAIL PROTECTED]> > >> wrote: > >> >> >> > > > > > > > > Sorry if I'm behind on this discussion but what are > >> >> the > >> >> >> current > >> >> >> > > > > > > > > thoughts on how dependency injection will be > >> >> implemented > >> >> >> for > >> >> >> > > > managed > >> >> >> > > > > > > > > beans? The reason I'm curious is because > >> PreDestroy > >> >> and > >> >> >> > > > PostConstruct > >> >> >> > > > > > > > > annotations are used to deal with injected > >> resources, > >> >> so > >> >> >> from a > >> >> >> > > > timing > >> >> >> > > > > > > > > perspective it would be important to process all > >> these > >> >> >> annotations > >> >> >> > > > > > > > > accordingly. > >> >> >> > > > > > > > > > >> >> >> > > > > > > > > Best wishes, > >> >> >> > > > > > > > > Paul > >> >> >> > > > > > > > > > >> >> >> > > > > > > > > On 2/24/07, Dennis Byrne < [EMAIL PROTECTED]> > >> wrote: > >> >> >> > > > > > > > > > I'm weighing options about invoking @PreDestroy > >> >> methods > >> >> >> > > > > > (@PostConstruct > >> >> >> > > > > > > > is > >> >> >> > > > > > > > > > done BTW). I haven't made up my mind yet but > >> here's > >> >> >> where I'm > >> >> >> > > > at on > >> >> >> > > > > > > > this. > >> >> >> > > > > > > > > > > >> >> >> > > > > > > > > > As far as *when* this happens, the request is > >> easy, > >> >> in > >> >> >> > > > > > > > > > FacesServelet.service(). Session and app scope > >> are > >> >> more > >> >> >> > > > difficult > >> >> >> > > > > > > > decisions. > >> >> >> > > > > > > > > > A new > >> >> >> > > > > > HttpSessionActivationListener.attributeRemoved > >> >> >> > > > > > > > and a > >> >> >> > > > > > > > > > new > >> >> >> > > > > > ServletContextAttributeListener.attributeRemoved () > >> >> >> > > > > > > > seem > >> >> >> > > > > > > > > > like nice solutions, but this doesn't meet the > >> spec > >> >> >> requirements > >> >> >> > > > for > >> >> >> > > > > > > > 5.4.1. > >> >> >> > > > > > > > > > The methods have to be invoked *before* the bean > >> is > >> >> >> pulled from > >> >> >> > > > > > scope > >> >> >> > > > > > > > and > >> >> >> > > > > > > > > > the servlet API does not provide a > >> >> >> > > > > > > > > > > >> >> >> > > > > > > > > >> >> >> > > > > > > >> >> >> > > > ServletContextAttributeListener.attribute_WILL_BE_Removed () > >> >> >> > > > > > > > > > or a > >> >> >> > > > > > > > > > > >> >> >> > > > > > HttpSessionActivationListener.attribute_WILL_BE_Removed > >> >> >> > > > > > > > (). > >> >> >> > > > > > > > > > Also, I say *new* > >> ServletContextAttributeListener > >> >> and > >> >> >> because > >> >> >> > > > > > > > > > StartupServletContextListener (already in code > >> base) > >> >> >> implements > >> >> >> > > > > > > > > > ServletContextListener, not > >> >> >> > > > > > > > > > ServletContextAttributeListener. > >> >> >> > > > > > > > > > > >> >> >> > > > > > > > > > The other side of the problem is *how* to invoke > >> >> each > >> >> >> > > > @PreDestroy > >> >> >> > > > > > > > method, > >> >> >> > > > > > > > > > much easier. Iterating over the attributes at > >> each > >> >> >> scope level > >> >> >> > > > is > >> >> >> > > > > > > > trivial, > >> >> >> > > > > > > > > > and so is determining if the bean's class is a > >> >> managed > >> >> >> bean > >> >> >> > > > class. > >> >> >> > > > > > But > >> >> >> > > > > > > > this > >> >> >> > > > > > > > > > does not mean the *instance* was placed there by > >> the > >> >> >> JSF > >> >> >> > > > > > implementation. > >> >> >> > > > > > > > > > Using a list (at each level of scope) to track > >> >> managed > >> >> >> instances > >> >> >> > > > > > solves > >> >> >> > > > > > > > the > >> >> >> > > > > > > > > > problem, as long as you sync on the session (only > >> >> one > >> >> >> time per > >> >> >> > > > > > session) > >> >> >> > > > > > > > in > >> >> >> > > > > > > > > > order to avoid concurrency issues; it also means > >> >> three > >> >> >> more data > >> >> >> > > > > > > > structures > >> >> >> > > > > > > > > > in memory. > >> >> >> > > > > > > > > > > >> >> >> > > > > > > > > > -- > >> >> >> > > > > > > > > > Dennis Byrne > >> >> >> > > > > > > > > > >> >> >> > > > > > > > > >> >> >> > > > > > > > > >> >> >> > > > > > > > > >> >> >> > > > > > > > -- > >> >> >> > > > > > > > Dennis Byrne > >> >> >> > > > > > > > >> >> >> > > > > > > >> >> >> > > > > > > >> >> >> > > > > > > >> >> >> > > > > > -- > >> >> >> > > > > > Dennis Byrne > >> >> >> > > > > > >> >> >> > > > > >> >> >> > > > > >> >> >> > > > > >> >> >> > > > -- > >> >> >> > > > Dennis Byrne > >> >> >> > > > >> >> >> > > >> >> >> > > >> >> >> > -- > >> >> >> > Mathias > >> >> >> > > >> >> >> > >> >> > > >> >> > > >> >> > > >> >> > >> > > >> > > >> > > >> > > > > > > > -- Mathias
-- Dennis Byrne
