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