So I’ve been helping the ICRC troubleshoot an issue that’s been arising recently from the Angular form engine when a form with diagnoses is modified, however, the issues we’ve run into are only loosely related to that.
1. Handling Lists / Sets
First, we notice that whenever data in the form of a List or Set on the backend is submitted as an update, it is additive, meaning that the data in the request is always appended to the list. This means, for example, that if I submit an Encounter like this at create:
{
// ...snip
"encounterProviders": [{ "provider": "1234", "role": "Encounter Provider" }]
// ...snip
}
And then, when updating the encounter, I submit the same thing:
{
// ...snip
"encounterProviders": [{ "provider": "1234", "role": "Encounter Provider" }]
// ...snip
}
I end up with two EncounterProvider entries, assigning the same provider the same role in the same encounter.
This arises, in part, because BaseDelegatingResource#unchangedValue() doesn’t have any special handling for Lists or Sets, and particularly Lists or Sets of objects. This also means that there’s no way of working around the duplication by setting the UUID of encounterProviders entries if it already exists to avoid generating this duplication, because:
{
// ...snip
"encounterProviders": [{ "uuid": "8a413042-7024-44e7-8db6-da3e10496018" ,"provider": "1234", "role": "Encounter Provider" }]
// ...snip
}
Will still be treated as a new entry, now with a duplicate UUID. It seems like the unchangedValue() method should have some handling for lists and sets of OpenmrsObjects, though this would likely be a breaking change at this point.
2. Flushing Lists / Sets
Since the List / Set is never treated as unchanged here, it means that we always end up calling BaseDelegatingResource#setProperty() with the value of the list. This, in turn, calls the #setPropertyWhichMayBeAHibernateCollection() function, which, if it gets a collection, will load the existing collection, call clear() and then addAll() to add the newly added items.
This results in Hibernate adding a new item to the persistent collection that needs to be flushed. However, because the main object isn’t flushed from the Hibernate session, it means that sub-properties of the main object may not be properly filled out. Concretely, for example, on creation, we accept encounterProviders as shown above:
{
// ...snip
"encounterProviders": [{ "provider": "1234", "role": "Encounter Provider" }]
// ...snip
}
Even though this entry is missing required properties on the actual EncounterProvider object (specifically, a link to the containing encounter). In the normal course of things, this is handled by Hibernate’s automatic cascade when the encounter is saved, but if the Hibernate context is flushed before the encounter is saved, Hibernate ends up trying to create a new row in the encounter_providers table without an encounter_id field.
Unfortunately, this happens in the REST module because the encounter diagnoses are processed after the Encounter provider and can result in a call to getDiagnosisByUuid(), which starts a new Hibernate transaction and flushes the pending new Encounter Provider.
I’m not sure what the resolution here should be, so I’m raising this Talk post to discuss how we should resolve this.
CC: @ICRC
