I'd vote to put @Transactional(readOnly=true, propagation=Propagation.SUPPORTS) at the class level of service layer impls and fine tune methods that aren't read only.
Propagation.SUPPORTS allows to eliminate the transaction overhead that Darius is hoping to achieve with the DAO example in a more general and nicer way. More info here [0]. There's not much difference between having transactional DAOs and Services. The latter is more widely used for the reason I gave, but it's a matter of taste. -Rafal [0] - http://www.ibm.com/developerworks/java/library/j-ts1/index.html On 24 February 2012 22:00, Darius Jazayeri <[email protected]> wrote: > Spring tries to use the existing transaction unless you say > @Transactional(propagation=Propagation.REQUIRES_NEW). > > I would *love* to get rid of the @Transactional annotations on our > services, and just put annotations on the methods that actually require > them. > > -Darius > > > On Fri, Feb 24, 2012 at 12:32 PM, Ben Wolfe <[email protected]> wrote: > >> FYI, here is the relevant ticket: >> https://tickets.openmrs.org/browse/TRUNK-208 >> >> I could live with it on the DAO if we also examine all serviceimpls for >> multiple calls to the DAO layer and add the annotation to the impl if it >> does more than one. >> >> Can we do away with the generic @Transactional on the class and force >> devs to add @Transactional on methods that need it? This should remove the >> need for @Transactional(readOnly=true) and limit the number of times its >> forgotten. Alternatively, we make the class level annotation readOnly=true. >> >> Are we also sure that if a transaction is started at the service layer >> that the DAO layer is using that same one instead of starting a new one? >> >> Ben >> >> >> On Fri, Feb 24, 2012 at 3:13 PM, Darius Jazayeri <[email protected] >> > wrote: >> >>> The reason I'd rather have them in the HibernateDAO is so we can write a >>> service method like this, without paying the transactional penalty: >>> >>> public MySettings getSettings() { >>> if (cached == null) { >>> cached = dao.getSettings(); // this method has an @Transactional >>> annotation >>> } >>> return cached; >>> } >>> >>> Basically I'm saying that the transactional annotation should be put in >>> the concrete DAO only where we actually hit a transactional datastore. >>> >>> If we're putting the annotations in the ServiceImpl I'd need to do >>> something like this: >>> >>> public MySettings getSettings() { >>> if (cached == null) { >>> cached = getSettingsFromDao(); >>> } >>> return cached; >>> } >>> >>> @Transactional(readOnly=true) >>> private MySettings getSettingsFromDao() { >>> return dao.getSettings(); >>> } >>> >>> It's not the end of the world to have to do that, but is there any >>> theoretical justification or best-practice recommendation that says we >>> should put annotations in the service layer when we believe that we're >>> calling a transactional DAO, rather than in the DAO layer when we know for >>> sure we're hitting the datastore? >>> >>> -Darius >>> >>> PS- Definitely if a ServiceImpl or a Controller method is going to make >>> multiple Service or DAO calls, it should explicitly mark the transaction >>> itself. But that's independent. >>> >>> >>> On Fri, Feb 24, 2012 at 10:08 AM, Ben Wolfe <[email protected]> wrote: >>> >>>> Yes, lets move these to the ServiceImpl. Do we have a ticket somewhere >>>> to move the rest of our transaction annotations from interface to impl? >>>> >>>> Ben >>>> >>>> >>>> On Fri, Feb 24, 2012 at 9:55 AM, Wyclif Luyima <[email protected]>wrote: >>>> >>>>> @Rafal, i agree with what you are saying, so we should move them from >>>>> HibernateVisitDao to VisitServiceImpl? >>>>> >>>>> Wyclif >>>>> >>>>> >>>>> >>>>> On Fri, Feb 24, 2012 at 5:15 AM, Rafal Korytkowski >>>>> <[email protected]>wrote: >>>>> >>>>>> I've found a way to have the advice around @Transactional so we don't >>>>>> need to change anything in Visits. >>>>>> >>>>>> Anyway, I don't think we should move @Transactional to the DAO layer. >>>>>> In general it's the service layer that defines units of work that should >>>>>> be >>>>>> transactional. If you have multiple calls to DAO in a service method, you >>>>>> will want to have @Transactional in the service method anyway. >>>>>> >>>>>> -Rafal >>>>>> >>>>>> >>>>>> >>>>>> On 24 February 2012 03:11, Darius Jazayeri <[email protected]>wrote: >>>>>> >>>>>>> Moving this to the dev list. >>>>>>> >>>>>>> Wyclif, I assume that you mean from HibernateVisitDAO to >>>>>>> VisitServiceImpl... >>>>>>> >>>>>>> So, in the long run, we need to move the transactional annotations >>>>>>> off of the service interface. I'd argue that they should go to the >>>>>>> concrete >>>>>>> DAO, and only on the methods that actually hit the database. The >>>>>>> alternative would be to move them to the ServiceImpl. >>>>>>> >>>>>>> That said, I'm fine if we want to move them from HibernateVisitDAO >>>>>>> up to the VisitService *interface* for consistency, so that we can >>>>>>> refactor *all* our services later. But what's the argument for >>>>>>> moving it from the HibernateDAO to the ServiceImpl? >>>>>>> >>>>>>> Ideally Rafal will quickly figure out how to move the recent advice >>>>>>> code he wrote so it's triggered by any @Transactional method anywhere, >>>>>>> and >>>>>>> this will be moot. >>>>>>> >>>>>>> -Darius >>>>>>> >>>>>>> >>>>>>> On Thu, Feb 23, 2012 at 5:58 PM, Wyclif Luyima >>>>>>> <[email protected]>wrote: >>>>>>> >>>>>>>> Hi guys, >>>>>>>> >>>>>>>> @Daniel, we need to move the @Transanctional annotations from >>>>>>>> HibernateVisitDao to ConceptServiceImpl before cutting the 1.9 RC >>>>>>>> >>>>>>>> Wyclif >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> ------------------------------ >>>>>>> Click here to >>>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>>> OpenMRS Developers' mailing list >>>>>> >>>>>> >>>>>> ------------------------------ >>>>>> Click here to >>>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>>> OpenMRS Developers' mailing list >>>>>> >>>>> >>>>> ------------------------------ >>>>> Click here to >>>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>>> OpenMRS Developers' mailing list >>>>> >>>> >>>> ------------------------------ >>>> Click here to >>>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>>> OpenMRS Developers' mailing list >>>> >>> >>> ------------------------------ >>> Click here to >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >>> OpenMRS Developers' mailing list >>> >> >> ------------------------------ >> Click here to >> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >> OpenMRS Developers' mailing list >> > > ------------------------------ > Click here to > unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from > OpenMRS Developers' mailing list > _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
