I'm run into similar issues in the past as well... it is also a reminder that the assumption that an object has not been persisted because a save method has not been called made not always be a safe one.
#2 sounds like the best option to me. Hibernate interceptors would also seem to have the advantage of being more robust than Spring AOP because they could be made to trap, say, all saves of OpenmrsObjects, as opposed to relying on following method naming conventions like save*. Mark ________________________________________ From: [email protected] [[email protected]] On Behalf Of Darius Jazayeri [[email protected]] Sent: Tuesday, November 22, 2011 9:42 PM To: [email protected] Subject: Re: [OPENMRS-DEV] Design Call - Flushing Session I believe all that we want to move from the service layer to the DAO layer is the assignment of creator and dateCreated. And that sounds appropriate for the database layer. It's something we were already planning to do at some point, and I'm pretty sure it will solve our current issue. Let's discuss tomorrow. -Darius On Tue, Nov 22, 2011 at 6:10 PM, Burke Mamlin <[email protected]<mailto:[email protected]>> wrote: I assume this is one of the gotchas of using Hibernate magic – i.e., attached objects – where Hibernate is trying to do it's job by persisting changes as they happen and not necessarily waiting for a "save". FWIW, if business logic needs to be triggered by the database side (e.g., from Hibernate), I'd rather have the database layer invoking/triggering business logic within the service layer instead of pushing business logic into the database layer. -Burke On Tue, Nov 22, 2011 at 2:48 PM, Darius Jazayeri <[email protected]<mailto:djazayeri%[email protected]>> wrote: Here's a specific example of some new code we're trying to write, that is failing. def va = new VisitAttribute(); va.attributeType = auditDate; va.value = DateUtils.parse("2011-11-15"); // do not set dateCreated and creator, because the framework promises to do it def visit = getAnExistingVisit(); visit.addAttribute(va); ValidationUtils.validate(visit); // <-- THIS LINE FAILS UNEXPECTEDLY Context.getVisitService().saveVisit(visit); The highlighted line throws an unexpected exception (and not because validation fails). It makes a call to the API (to get all VisitAttributeTypes, so it can check that visit has between minOccurs and maxOccurs of each of them), and this API call triggers a hibernate flush. The hibernate flush tries to write visit and all its children to the DB even though we haven't called saveVisit yet. And that fails because the new attribute hasn't had dateCreated and creator set yet. (That will only happen in the next line, because of the AOP around the saveVisit method.) Further gory details: * Apparently, even though we've been setting hibernate's flush mode to MANUAL, Rafal has discovered that anytime we enter an @Transactional method, hibernate changes the flush mode to AUTOMATIC. * This "premature commit" has been going on forever. We haven't noticed it because if validation fails, and the transaction is rolled back, the premature commit is rolled back as well. The two options we have in mind for approaching this are: 1. Write a custom Hibernate transaction manager, that doesn't change the flush mode to AUTOMATIC 2. Move a bunch of logic currently in our AOP SaveHandlers into our relatively new Hibernate interceptors. (Rafal and I like option 2 better.) -Darius On Tue, Nov 22, 2011 at 11:15 AM, Rafal Korytkowski <[email protected]<mailto:[email protected]>> wrote: Hi, tomorrow I want us to discuss an issue which arouse in https://tickets.openmrs.org/browse/TRUNK-2588 . The thing is that although we try to setup Hibernate to work in the MANUAL flush mode the setting has never been respected. In the effect we experience flushes during transactions whenever Hibernate decides to do so. I do not think it is bad, but it does not work with our AOP save handlers, because things may be saved before handlers are triggered resulting in not-null or transient value exceptions. I have attached a patch to the ticket which fixes this behavior by setting the flush mode to COMMIT at the beginning of each transaction. As I commented on the ticket I do not feel it is the right approach and we should consider changing AOP save handlers to Hibernate interceptors. Opinions are welcome. -Rafal _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected]<mailto:[email protected]> with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]<mailto:[email protected]>?body=SIGNOFF%20openmrs-devel-l] ________________________________ Click here to unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> from OpenMRS Developers' mailing list ________________________________ Click here to unsubscribe<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l> from OpenMRS Developers' mailing list ________________________________ Click here to unsubscribe<mailto:[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]
