Details
-
Bug
-
Status: Closed
-
Non-Essential
-
Resolution: Fixed
-
None
-
None
-
None
-
Undetermined
Description
Here's an example of OpenMRS not having the correct request/transaction unit of work boundaries. To reproduce, open a patient and click "Edit this Patient" to go to the long patient edit form. Change the patient IMB identifier (or any checked identifier so that it is invalid - in this case an IMB ID should have the form 00000000-C) to 1. Click Save. On the screen you'll see a message saying the identifier is invalid. Click "View Patient Dashboard". You'll notice that the identifier was saved enough though there was an error.
When you look into it you'll see that there's more than one transaction is committed. The first from PatientFormController.bindAndValidate and the second from PatientFormController.onSubmit. The error checking is done in "onSubmit" which is after the invalid data has been stored.
I think there's a few problems. First, a commit is triggered in the hibernate call stack from bindAndValidate (PatientIdentifierValidator.validateIdentifier(String, PatientIdentifierType) line: 122 - I have not looked why) . After the commit the "onSubmit" code validates the identifier and fails. Since, the transaction has already been committed it's to late to rollback. Even so, and the second problem, the error is handled by a catch (PatientIdentifierValidator.validate(Object, Errors) line: 59) and added to a list of errors but the code does not try to rollback (since hibernate does not guarantee if/when flushes happen, the transaction should be rolled back). The third problem is that I believe we should have 1 transaction per request.
This might be in other parts of OpenMRS that do error validation.