Uploaded image for project: 'OpenMRS Core'
  1. OpenMRS Core
  2. TRUNK-229

Unit of work - transaction bounary is not correct - Patient.form maybe more places

    XMLWordPrintable

    Details

    • Complexity:
      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.

        Attachments

          Activity

            People

            Assignee:
            dkayiwa Daniel Kayiwa
            Reporter:
            mharrison Marc Harrison [X] (Inactive)
            Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: