Re: [Springframework-developer] Revised BeanFactoryLocator and EJB support classes

[Colin S. <col...@ex...>]

Yes, the Jndi variant's behaviour is ok.  I'll modify the keyed 
singleton variants later this week to use the reference count.

jürgen höller [werk3AT] wrote:

>Colin,
> 
>Oops, I seem to have gotten that wrong then for the SingletonBeanFactoryLocator: Please change it back to a more reasonable implementation before 1.0 final. 
> 
>However, the JndiBeanFactoryLocator doesn't have a reference count; it creates the factory on each locator call. Isn't it be appropriate here to call BeanFactory.destroySingletons respectively ApplicationContext.close on release?
> 
>Juergen
> 
>
>________________________________
>
>Von: spr...@li... im Auftrag von Colin Sampaleanu
>Gesendet: Sa 14.02.2004 20:06
>An: spr...@li...
>Betreff: Re: [Springframework-developer] Revised BeanFactoryLocator and EJB support classes
>
>
>
>I've got no problem with these changes in terms of making the names
>consistent. The new handling of the BeanFactoryReference is wrong
>though. There was a reason why it was an inner class before, as per the
>comment:
>
>            return new BeanFactoryReference() {
>                public BeanFactory getFactory() {
>                    return retval;
>                }
>                public void release() throws FatalBeanException {
>                    // Currently does nothing.
>                    // An ideal implementation would use reference
>counting data to release owning
>                    // container when no more BeanFactories within it
>are used, however depending on
>                    // the usage scenario, this could also cause thrashing.
>                }
>            };
>
>Now it was somewhat of a copout not to do anything on the release; my
>original intent when I checked this stuff in was that we would have some
>discussion on the best handling for the release call, and then it would
>get implemented, probably to just use the reference count in its outer
>class to decide whether to release or not, but we've all been pretty
>busy and that didn't happen.
>
>So you can not just call
>   ((ConfigurableBeanFactory) this.beanFactory).destroySingletons()
>and
>   ((ConfigurableApplicationContext) this.applicationContext).close();
>as your new implementation does in the new separate implementations of
>BeanFactoryReference. It should probably stay an inner class, and only
>call the destroy or close, respectively, if the reference count on the
>keyed singleton beanfactory or context goes down to zero.
>:
>This will work absolutely fine for people using it like I am, where it
>is used to obtain the parent for the web application context(s), and
>then only released when the web application context gets unloaded.
>
>The other situation, where people do not have one get and release
>wrapping all other gets and releases, is more problematic, since if they
>do sequential gets and releases they will get a sort of thrashing as
>stuff gets continuously loaded and unloaded. What these people will have
>to do is themselves force an initial load without a release, at their
>app startup.
>
>As for the default ejbRemove not calling unloadBeanFactory (like the
>comment for unloadBeanFActory says is supposed to happen), that appears
>to be an oversight and something that was in there since Nov. or Dec.
>Thanks for catching that. The old old code never did any unloading at
>all. Then in Nov./Dec. I added the unloadBeanFactory method, but
>apparently forgot to call it.
>
>Regards,
>Colin
>
>
>jürgen höller [werk3AT] wrote:
>
>  
>
>>Agreed, the RC1 API should be considered as final as possible. However, the BeanFactoryLocator was a brand-new RC1 feature, added pretty much last minute there, so I guess it's arguable to refine this for 1.0 final - particularly if it just affects advanced users that diverge from the default EJB support configuration.
>>
>>    
>>
>>From my point of view, I'm as happy as can be with the current state of the framework. What I would like to see included in 1.0 final nevertheless is (backward-compatible) support for more exception categories in the SQLException translator, as suggested by Thomas, and possibly a convenient option to set a transaction rollback-only no matter if driven by declarative or programmatic demarcation, as suggested by Colin and Alef.
>  
>
>>BTW, I'll send a mail regarding the Spring roadmap shortly.
>>
>>Juergen
>>
>>
>>________________________________
>>
>>Von: spr...@li... im Auftrag von Rod Johnson
>>Gesendet: Sa 14.02.2004 18:42
>>An: spr...@li...
>>Betreff: Re: [Springframework-developer] Revised BeanFactoryLocator and EJB support classes
>>
>>
>>
>>I agree with these changes, but I think we should try to avoid API changes
>>in general from now to 1.0 final. With RC1 we are committing to a final API.
>>Also, I'd rather we don't have enough changes that we need an RC2.
>>
>>Regards,
>>Rod
>>
>>----- Original Message -----
>>From: "jürgen höller [werk3AT]" <jue...@we...>
>>To: <spr...@li...>
>>Sent: Saturday, February 14, 2004 5:22 PM
>>Subject: [Springframework-developer] Revised BeanFactoryLocator and EJB
>>support classes
>>
>>
>>Colin, Rod, everyone,
>>
>>I revised the BeanFactoryLocator and EJB support classes yesterday, mainly
>>to align the naming of the implementation classes with Spring's general
>>naming patterns. For example, the ApplicationContext-specific classes are
>>now called "ContextJndiBeanFactoryLocator" and
>>"ContextSingletonBeanFactoryLocator". I've also factored out the
>>BeanFactoryReference implementations for newly created BeanFactories into
>>separate classes, making them invoke
>>"ConfigurableBeanFactory.destroySingletons" respectively
>>"ConfigurableApplicationContext.close" on release.
>>
>>I've adapted the EJB support classes accordingly and, on the occasion, moved
>>the logger instance variable from AbstractEnterpriseBean to
>>AbstractStatelessSessionBean and AbstractMessageDriverBean. Someone
>>complained on the mailing list a while ago that removing and setting the
>>logger instance for SFSBs is a nuisance, and I agree - the subclass should
>>hold its own *static* logger instance there. Of course, this doesn't apply
>>to SLSBs and MDBs, thus the change.
>>
>>I've also noted that AbstractEnterpriseBean's "ejbRemove" implementation did
>>*not* invoke BeanFactoryLocator.release; is there any rationale for this?
>>For the time being, I've made it invoke release, as I consider it important
>>to destroy resource singletons like a local SessionFactory or
>>PersistenceManager on BeanFactory respectively ApplicationContext shutdown.
>>
>>I hope you don't mind the name changes. My goal is to keep class and method
>>naming as consistent as possible within the Spring codebase; something many
>>other open source projects to not respect at all.
>>
>>Juergen
>>    
>>