|
From: Colin S. <col...@ex...> - 2004-07-26 12:16:48
|
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 >> >> |