|
From: Colin S. <col...@ex...> - 2004-07-26 14:21:22
|
Sure, I think that would work fine. (It's very much debatable, but I think allowCreate=false would probably have been a better default given that it would work in about 98% of deployment scenarios and catch errors with tx setup, but that's in no way an option to change to now as it would break a lot of people's existing deployments.) Colin jürgen höller [werk3AT] wrote: >Colin, > >I see your point. We could keep setSessionFactory as final, but let it delegate to a non-final protected createHibernateTemplate(SessionFactory) method that could be overridden in subclasses, even allowing to plug in custom HibernateTemplate subclasses. That's a quite clean and very flexible solution, IMO. I'd be happy to adapt HibernateDaoSupport, JdoDaoSupport and co accordingly. > >Let's also not forget that we're talking about a convenience base class here: For special needs, it's trivial to create a custom base class that does not derive from HibernateDaoSupport in the first place. We "just" need to cover 95% of use cases here. I agree that allowCreate=false is quite common, though, so we should cover that; also, setting the flushMode: createHibernateTemplate would cover both. > >Juergen > > >-----Original Message----- >From: spr...@li... >[mailto:spr...@li...]On Behalf >Of Colin Sampaleanu >Sent: Monday, July 26, 2004 3:16 PM >To: spr...@li... >Subject: Re: [Springframework-developer] HibernateDaoSupport > > >Juergen, > >Personally, I do feel the class has somewhat of a 'smell' as it is now, >although part of the reason is that setSessionFactory itself is doing >work instead of this stuff being done in the init method (I realize this >is to allow the thing to work without the init method ever being >called). The issue to me though is the fact that I have seen on a number >of occassions people get burned by the default allowCreate flag being >true. They mess up their tx settings, and have no tx, but don't realize >it because the sessions are still being created. If instead they were >using the flag with a false value (at absolutely no pain for most >situations when using HibernateTransactionManager or another tx manger >eith HibernateInterceptor), they automatically get an error when their >DAO is not running in a tx. So I feel it is important to have some >relatively easy way to override this, at least so that some team lead or >architect can enforce this. I think it's not really necessary to have >the default HibernateDaoSupport allow setting this by DAOs which derive >directly, it's not a big deal to ask people to subclass it to change the >handing of this, and just use that common user variant as their >superclass. So taking out the setters and getters is not a big deal. Now >with regards to allowing the default flag handling to be overriden by a >subclass though, allowing setSessionFactory to be overriden would work >fine, although arguably it's not much cleaner than still having the >field there (for the default value) which a subclass can override in a >constructor. If setSessionFactory were to be made non-final like this >then we need to document much more clearly (javadoc) that that method is >actually responsible for creating a template if none is set, in the >default impl. > >Regards, >Colin > > >jürgen höller [werk3AT] wrote: > > > >>Colin, >> >>I've just had a look at the new "defaultTemplateAllowCreateValue" flag in HibernateDaoSupport, and I'm frankly not happy with it. We have a special check whether the HibernateTemplate is self-created etc, indicating - for me at least - that something is wrong there. I see your use case, which is worth addressing, but I think the current approach is not ideal. >> >>This is not really how our XxxDaoSupport classes are meant to be used. In the case of HibernateDaoSupport: either pass in a SessionFactory *or* a preconfigured HibernateTemplate. If you want a default value for HibernateTemplate's "allowCreate" flag, why not set up a HibernateTemplate bean in your context, configure it with "allowCreate"=false and pass it to all your DAOs? This would work nicely with the old HibernateDaoSupport implementation, without any special checks... >> >>My two main concerns are 1. consistency, i.e. that JdoDaoSupport and the others would have to be coded in a similar fashion and 2. future extensions, i.e. what happens if there's a further flag on HibernateTemplate that you want to set via your DAO? Particularly in the latter case, I'm not keen on having that sort of "if-self-created-then" properties on the DAO: They confuse things, and just apply when passing in a SessionFactory (not when passing in a HibernateTemplate), which is not immediately obvious. >> >>To allow for programmatic setting of such HibernateTemplate properties, we could turn setSessionFactory non-final, allowing it to override with a version that builds a custom HibernateTemplate for that SessionFactory. That would be much cleaner, IMO, and easy to adapt in JdoDaoSupport and co. Our data access templates and DAO base classes are among the most popular parts of Spring, so it's important to indicate a clear strategy there. >> >>All things considered, I quite strongly believe that we should use to the old HibernateDaoSupport implementation again, just turning setSessionFactory non-final to allow for programmatic overriding. For context-driven configuration, let's recommend to pass in a preconfigured HibernateTemplate bean. Can I convince you of this? >> >>Unfortunately, I'm already in a bit of a hurry here: I'd like to - finally! - get 1.1 RC1 out tomorrow. >> >>Juergen >> >> >>________________________________ >> >>Von: spr...@li... im Auftrag von Colin Sampaleanu >>Gesendet: Do 15.07.2004 16:02 >>An: spr...@li... >>Betreff: Re: [Springframework-developer] HibernateDaoSupport >> >> >> >>I don't like that solution, because it will then also affect any >>HibernateTemplate that is passed in. The current code does not touch any >>template that is passed in. These changes do not want to modify that >>behaviour, but rather allow the default value for allowCreate on _new_ >>templates created to be specified. And I think there are two ways that >>people would want to play with the flag: either make their own subclass >>which sets the flag to false, and that becomes the superclass of all >>their DAOs, or alternately, do not use a subclass at all, but in some >>(or all) DAOs, specifically set the default value. >> >>Colin >> >>jürgen höller [werk3AT] wrote: >> >> >> >> >> >>>Colin, >>> >>>What about simply invoking >>> >>>getHibernateTemplate().setAllowCreate(false); >>> >>>in the initDao implementation of your subclass? >>> >>>Juergen >>> >>> >>>________________________________ >>> >>>Von: spr...@li... im Auftrag von Colin Sampaleanu >>>Gesendet: Do 15.07.2004 05:34 >>>An: spr...@li... >>>Betreff: [Springframework-developer] HibernateDaoSupport >>> >>> >>> >>>Juergen, >>> >>>The existing code in HibernateDaoSupport created a HibernateTemplate >>>with the allowCreate flag as true by default, and as the >>>setSessionFactory method where the template was created was final, there >>>was no way to override this. I have created a protected field called >>>defaultTemplateAllowCreateValue (true by default to maintain existing >>>behaviour), which a user HibernateDaoSupport subclass may override. >>>Additionally, I have added a setter for this default value, so a DAO >>>instance can be set to true or false specifically (without having to >>>subclass). This value does not affect any HibernateTemplate passed in >>>explicitly. >>> >>>Colin >>> >>> >>> |