Context: I am writing a module that archives a subset of patient data (specifically encounters, encounter providers, obs) by moving them to a different set of tables with the intent to reduce load on a large database for one of our clients. I have structured this as follows:
User selects a set of patients to archive
The module goes through the list of patients, archiving all encounters/enc providers/obs that don’t have other dependencies. The archival of each encounter is a transaction i.e. the encounter, linked encounter providers, and obs are either all archived or the entire encounter is skipped (i.e. transaction rolled back). A patient’s data can be partially archived i.e. it’s okay if some encounters are archived and others aren’t.
This is done via a controller receiving a list of patients from a JSP page, extracting their encounters, and iterating through the list of encounters, invoking a service method (ArchivalService() -> archiveEnounter() ) to archive each encounter. In line with what I understand is a Spring recommendation, I have annotated archiveEncounter() as @Transactional in the concrete class (ArchivalServiceImpl) for my service.
Problem I am facing: When the encounters can all be archived successfully, everything works fine. However, in the instance where I have, say, one patient with two encounters and for some reason (e.g. a dependency on Order) the first encounter can’t be archived, the first transaction appears to be rolling back just fine, but the attempt to archive the next encounter fails with a LazyInitializationException (LazyInitializationException - Pastebin) when accessing the EncounterProviders for the Encounter in question. I’m not sure I understand why one failed/rolled-back transaction is terminating the entire session. I’m probably missing either something really obvious or really arcane around Transaction Management in Spring/OpenMRS. Any and all help appreciated.
This type of error can be really painful and difficult to pin down, but we can try a few things.
First, looking at your service class, I see that it is annotated with @Service and the dao property is annotated with @Autowired. These are both unnecessary, as you have things wired up explicitly (as you should) in the moduleApplicationContext.xml. Autowiring withing services is a known performance killer. But more importantly, it’s possible that your @Service annotation is additive with your moduleApplicationContext configuration resulting in 2 instances of the service- one that is transactional (the one wired in the moduleApplicationContext) and one that is not (the one with the Annotation). I’d have to test to see for sure, but that seems plausible to me, as it is the proxy wiring that is configured in the moduleApplicationContext that adds the transactional behavior.
(I’d also similarly remove the @Component and @Autowired annogations from your DAO implementation, though I’m not sure these would have the same possible effects, the are still unnecessary and likely creating multiple instances of these beans).
Other things I see - I don’t think you should need any code inside your DAO to open the session. You should always have a session at this point.
One other thing I would try is to move your exception handling into your service layer. Do not allow exceptions to propogate out of your archiveEncounter service method, but rather catch the exception inside the method, log it, and then change your return type to return a boolean or other object that is able to communicate back to your calling code whether the archive operation was successful or not.
Other things you can try might be to, in your controller, get the list of encounter ids that you wish to archive, and the pass the encounter id into your service method rather than the encounter, and let your service (or dao) first fetch this from the session before operating on it.
There are other things that we might try, but these are a few of the things off the top of my head.
Thanks a lot for the super helpful response. I went through your list of solutions one and by one and, as it turns out, the last one i.e. passing Encounter ID’s (as opposed to Encounters) from Controller to Service and having the Service fetch the Encounter before operating on it solved my problem. This one had me completely stumped but (on hindsight) I think I see how having the fetch at the service layer avoids the Lazy Initialization issue. Really appreciate the help!