Matt Raible added a comment - 17/Oct/06 12:09 AM

Matt Raible added a comment - 17/Oct/06 12:09 AM FWIW, there's a prototype of the preceding implementation at http://static.appfuse.org/downloads/audit-1.6.zip.

Bryan Noll added a comment - 19/Oct/06 09:00 AM

Bryan Noll added a comment - 19/Oct/06 09:00 AM For a first cut of this patch, I'm going to avoid all the fancy 'FinderIntroductionAdvisor' business mentioned in that dev works article. It seems simpler to me to just write subclasses of BaseDaoHibernate that go ahead and simply implement finder methods there. The patch I provide will do it that way unless anyone has any huge objection to me doing it that way. Once that patch gets in, if there's demand for going the FinderIntroductionAdvisor route, we can re-visit. Please speak up if you feel strongly that the FinderIntroductionAdvisor stuff should just be implemented now.

Bryan Noll added a comment - 19/Oct/06 03:21 PM

Bryan Noll added a comment - 19/Oct/06 03:21 PM I've attached a zip instead of a patch because files have been deleted/renamed/added. To give you an idea of what to look at, here's the result of an svn stat: $ svn stat ? hibernate/src/test/java/org/appfuse/dao/UniversalDaoTest.java ! hibernate/src/test/java/org/appfuse/dao/GenericDaoTest.java M hibernate/src/test/java/org/appfuse/dao/UserDaoTest.java M hibernate/src/test/java/org/appfuse/dao/RoleDaoTest.java ? hibernate/src/main/java/org/appfuse/dao/hibernate/GenericDaoHibernate.java ? hibernate/src/main/java/org/appfuse/dao/hibernate/UniversalDaoHibernate.java M hibernate/src/main/java/org/appfuse/dao/hibernate/LookupDaoHibernate.java ! hibernate/src/main/java/org/appfuse/dao/hibernate/BaseDaoHibernate.java M hibernate/src/main/java/org/appfuse/dao/hibernate/UserDaoHibernate.java M hibernate/src/main/java/org/appfuse/dao/hibernate/RoleDaoHibernate.java M hibernate/src/main/resources/applicationContext-dao.xml M hibernate/pom.xml ? common/src/main/java/org/appfuse/dao/GenericDao.java ? common/src/main/java/org/appfuse/dao/UniversalDao.java M common/src/main/java/org/appfuse/dao/UserDao.java ! common/src/main/java/org/appfuse/dao/Dao.java M common/src/main/java/org/appfuse/dao/RoleDao.java M common/src/main/java/org/appfuse/dao/LookupDao.java M common/pom.xml M pom.xml M ibatis/pom.xml In a nutshell, GenericDao (implemention is GenericDaoHibernate) replaced Dao and BaseDaoHibernate respectively. Essentially, you can now do CRUD on any new domain model object without having to write a new Dao interface or implementation, without having to cast. You simply do this in Spring (see applicationContext-dao.xml in the appfuse-hibernate module).     <bean id="fooDao" class="org.appfuse.dao.hibernate.GenericDaoHibernate">         <constructor-arg value="org.appfuse.model.Foo"/>         <property name="sessionFactory" ref="sessionFactory"/>     </bean> The old concept of Dao and BaseDaoHibernate has been converted to objects called UniversalDao and UniversalDaoHibernate respectively, as I believe there is still value in having something like this around. An example of that value would be using one Dao instance to work with multiple types of domain objects, which you cannot do with the GenericDao. The difference is, the UniversalDao simply requires casting. I tried to be clear about the purposes and differences of these two Dao's in their respect javadoc comments. A couple things to note: - The name of the attachment is data.zip, and should be unzipped in the 'appfuse' directory. It will unzip as a directory named 'data'. - http://issues.appfuse.org/browse/APF-425 is the iBatis version of this same feature, which has not yet been implemented, so the /data/pom.xml file has simply commented out that module so it doesn't try to build at all when using the contents of this zip file. Once I implement the iBatis portion, the same general patterns will apply. - Only compile and test this thing from the 'appfuse/data' level of the directory structure, as none of the service stuff has been modified to deal with these new classes and method signatures yet. Please provide feedback surrounding the way I went about implementing the GenericDao and UniversalDao... letting me know if it works for you or if you hate it for some good reason. Thanks to Raible and Bob Johnson for helping me flush through some of these issues.

Darren Salomons added a comment - 22/Oct/06 08:51 PM

Darren Salomons added a comment - 22/Oct/06 08:51 PM I've noticed in the most reason spring javadoc(http://static.springframework.org/spring/docs/2.0.x/api/org/springframework/orm/hibernate3/HibernateTemplate.html) they have put this little note in there: "NOTE: As of Hibernate 3.0.1, transactional Hibernate access code can also be coded in plain Hibernate style. Hence, for newly started projects, consider adopting the standard Hibernate3 style of coding data access objects instead, based on SessionFactory.getCurrentSession(). (Spring's LocalSessionFactoryBean automatically supports Spring transaction management for the Hibernate3 getCurrentSession() method.)" I know HibernateDaoSupport/HibernateTemplate was a great way of converting hibernates checked exceptions to runtime exceptions. But now that hibernate 3 uses runtime exceptions is this little note Springs hint to maybe move way from using HibernateDaoSupport. I know there seemed to be some tension between the Spring and Hibernate developers about using that class. Just thought I'd bring it up since in your implmentation Bryan you are still using HiberateDaoSupport.

Bryan Noll added a comment - 31/Oct/06 11:04 AM

Bryan Noll added a comment - 31/Oct/06 11:04 AM Darren.... In response to your thoughts about not using HibernateDaoSupport... here are some comments from the Spring reference manual (http://static.springframework.org/spring/docs/2.0.x/spring-reference.pdf). Regarding the DAO style you're suggesting, from 12.2.5L <spring-ref> The main advantage of this DAO style is that it depends on Hibernate API only; no import of any Spring class is required. This is of course appealing from a non-invasiveness perspective, and might feel more natural to Hibernate developers. However, the DAO throws plain HibernateException (which is unchecked, so does not have to be declared or caught), which means that callers can only treat exceptions as generally fatal - unless they want to depend on Hibernate's own exception hierarchy. Catching specific causes such as an optimistic locking failure is not possible without tieing the caller to the implementation strategy. This tradeoff might be acceptable to applications that are strongly Hibernate-based and/or do not need any special exception treatment. ... In summary: DAOs can be implemented based on the plain Hibernate3 API, while still being able to participate in Spring-managed transactions. This might in particular appeal to people already familiar with Hibernate, feeling more natural to them. However, such DAOs will throw plain HibernateException; conversion to Spring's DataAccessException would have to happen explicitly (if desired). </spring-ref> So, to summarize, I see the pros and cons as follows: Pros of converting Dao's so that they don't extend HibernateDaoSupport: 1) No dependence on Spring code... aka... not importing anything from Spring 2) Dealing strictly with the Hibernate API, not having to learn the nuances of HibernateDaoSupport methods, even though they are fairly straightforward and simple Pros of leaving it the way it is: 1) More descriptive exceptions coming from the Spring exception framework 2) It works well, and we wouldn't have change any of the code in the DAO methods. In all honesty, I don't feel real strong about this one way or the other. I tend to lean toward leaving it the way it is, just because it already works well, and I'd rather not have to change anything if I don't have to. To add to that, I believe the idea has been kicked around of adding JPA support to appfuse, which would require the implementations of the Dao's to change some anyway (using the EntityManager API instead of the Hibernate-specific API). I guess this is my +1 to leave it the way it is, although not something I would push back on if enough people decided it was worthwhile to ditch HibernateDaoSupport.

Bryan Noll added a comment - 31/Oct/06 12:05 PM

Bryan Noll added a comment - 31/Oct/06 12:05 PM A patch that can be applied after the generic dao modifications in the zip file are made. This patch needs to be applied after the zip files from this bug and APF-425 are applied.