Mark, Thanks so much for this investigating. It was a perfect addition to a plate of turkey and a glass of Australian shiraz. :-)
At the end, when you say "it is odd that dateCreated is being changed", you mean that the dateCreated *reference* has changed, not the value, right? Presumably because hibernate is doing some cloning or similar trickery under the hood? -Burke On Thu, Nov 24, 2011 at 9:39 AM, Mark Goodrich <[email protected]> wrote: > No, even after flushing the session, the assert still fails. > > There's only one intercepted method in the AuditableInterceptor... the > onDirtyFlush. The interceptor methods are called on an object-by-object > basis. I added a dummy onSave method to the interceptor and debugged and > found the following: > > 1 )After the first saveEncounter in the editNewEncounter method: > The onSave method is called three times, once for the Encounter, and once > for each of the two Obs added > The onDirtyFlush method is not called > > 2) After the second saveEncounter in the editNewEncounter method: > The onDirtyFlush method is called four times, twice for each of the two > edited Obs > The onSave method is not callled > > 3) After the only saveEncounter in the editExistingEncounter method: > The onDirtyFlush method is called six times, twice for the Encounter, and > twice for the two edited Obs > The onSave method is not called > > I looked a little deeper into #3 to see exactly why the onDirtyFlush > method was being called to hopefully determine why it wasn't being called > in case #2. > > First, during the saveEncounter in the editExisingEncounter method, > onDirtyFlush is called with the Encounter, because three encounter fields > have changing: > encounterDate and dateCreated are both changing from an existing timestamp > object to a new timestamp object > voidReason is changing from and empty string to null > > Next, onDirtyFlush is called for each of the two Obs, with the following > fields changing: > obsDatetime and dateCreated are both changing from an existing timestamp > object to a new timestamp object > the valueNumeric is updated, as expected > the personId is updated from one Integer object to another Integer object > (though with the same integer value, thankfully!) > > Next, onDirtyFlush is called again with the Encounter. Now, six fields > have changed... the three fields mentioned previous (encounterDate, > dateCreated, and voidReason) all have the same previous and current values > as when the last dirty flush was called, and three new fields have changed: > voided from null to false > dateChanged from null to a timestamp > changedBy from null to the current user > (So it looks like this caused by the voided value change, or it is a > recursive call caused by the changes made in the AuditableInterceptor > itself.) > > Then, finally, onDirtyFlush is called again with the two obs, and checking > the current previous states show the exact same changes as the first call. > > I don't know if I can come to any conclusions here except that it is odd > that dateCreated is being changed. One tangential point is that when obs > are updated this way (saved via a cascade from an encounter) they are not > voided and recreated, but I believe this is a known issue I entered a > ticket about at some point? > > I hope I've provided you all with some fun Thanksgiving reading. > > Mark > > > > ________________________________________ > From: [email protected] [[email protected]] On Behalf Of Ben Wolfe [ > [email protected]] > Sent: Thursday, November 24, 2011 7:40 AM > To: [email protected] > Subject: Re: [OPENMRS-DEV] Auditable Interceptor functionality > > Do you know which method in the interceptor is called on that first save? > > Is the dateChanged set at the end of the transaction? If you flush the > session right before your failing assert, does the assert then pass? > > Ben > > On Thu, Nov 24, 2011 at 12:22 AM, Mark Goodrich <[email protected]<mailto: > [email protected]>> wrote: > Sorry I missed the design call on hibernate interceptors today, but, > coincidentally, I was investigating an issue with the Auditable interceptor > that I uncovered when running the HFE unit tests against 1.8. Or, rather, > something that may be an issue. > > Take the following unit test, which creates a new encounter, saves it, > updates the obs associated with it, and saves it again. Using the old save > handlers, the dateChanged of the encounter would be set upon the second > save. But, with the hibernate interceptor, this is not the case and so > this test will fail: > > public void editingNewEncounter_shouldUpdateDateCreated() throws Exception > { > EncounterService es = Context.getEncounterService(); > > // First, create a new Encounter > Encounter enc = new Encounter(); > enc.setLocation(new Location(1)); > enc.setEncounterType(new EncounterType(1)); > enc.setEncounterDatetime(new Date()); > enc.setPatient(new Patient(3)); > enc.setProvider(new Person(1)); > > // Now add a couple obs to it > Obs newObs = new Obs(); > newObs.setConcept(new Concept(1)); > newObs.setValueNumeric(50d); > enc.addObs(newObs); > > Obs anotherNewObs = new Obs(); > anotherNewObs.setConcept(new Concept(1)); > anotherNewObs.setValueNumeric(100d); > enc.addObs(anotherNewObs); > > // now save the encounter > es.saveEncounter(enc); > > // flush the session, just to be safe > Context.flushSession(); > > // confirm that the encounter has been assigned an id, but > the date changed is still null > assertNotNull(enc.getId()); > assertNull(enc.getDateChanged()); > > // confirm tha the obs have been added to the encounter > assertEquals(2, enc.getAllObs().size()); > > // Now edit the obs and resave the encounter > for (Obs o : enc.getAllObs()) { > o.setValueNumeric(75d); > } > > // resave the encounter > es.saveEncounter(enc); > > // FAILS: confirm that the date changed has been set > assertNotNull(enc.getDateChanged()); > } > > Note, however, that if I load an existing encounter and update the obs, > the date changed will be set: > > public void editingExistingEncounter_shouldUpdateDateCreated() throws > Exception { > EncounterService es = Context.getEncounterService(); > > // First, load an existing Encounter > Encounter enc = > Context.getEncounterService().getEncounter(3); > > // confirm that the date changed is null > assertNull(enc.getDateChanged()); > > // Now edit the obs and resave the encounter > for (Obs o : enc.getAllObs()) { > o.setValueNumeric(75d); > } > > // resave the encounter > es.saveEncounter(enc); > > // PASSES: confirm that the date changed has been set > assertNotNull(enc.getDateChanged()); > } > > In the first case the encounter is never passed to the interceptor > onDirtyFlush method... I would assume because the Encounter is still in > the new/save state? Is this a problem? This is a contrived case, and it > seems to make sense that if an encounter is saved twice in rapid succession > it's date changed isn't set--but this isn't the behavior I would have > expected, and it is different from how the old save handlers worked. > > Thoughts? > Mark > > > > _________________________________________ > > 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 > > _________________________________________ > > 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] > > _________________________________________ 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]
