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

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

Juergen,

In fact w/regards to your refactoring of the BeanFactoryLocator and 
related classes, you had only taken out the inner class 
BeanFactoryReference from the JndiBeanFactoryLocator variants (and added 
close() calls on the release()), and there was no problem with that, 
since those classes never used a reference count. You actually left 
alone the no-op inner class variants in 
SingletonBeanFactoryLocator/ContextSingletonBeanFactoryLoctor which did 
nothing on the release() call.

But since it needed to be done anyways though, I have gone ahead and 
finished the code in those classes to actually decide whether to call 
close() or not, on release(), based on the reference count that is kept..

Regards,
Colin


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
>>
>>    
>>