Hibernate error on cohort + cohortMembership update for openmrs platform version 2.1.*

Development
#1

While working on group resource implementation, I came across a fascinating hibernate issue.

ca.uhn.fhir.rest.server.exceptions.InternalErrorException: Failed to call access method: 
org.springframework.orm.hibernate4.HibernateSystemException: A collection with cascade="all- 
delete-orphan" was no longer referenced by the owning entity instance: 
org.openmrs.Cohort.memberships; nested exception is org.hibernate.HibernateException: A 
collection with cascade="all-delete-orphan" was no longer referenced by the owning entity instance: 
org.openmrs.Cohort.memberships

Hibernate gives enough info (stack trace) about the error to identify what’s going on. Hibernate is complaining of not being able to track the change for the child collection object cohortMembership while it is getting set in the parent object cohort. Hibernate requires that parent object owns a child collection object completely.

To fix this, we need to modify the following code;

Old code: openmrs-core/Cohort.java at master · openmrs/openmrs-core · GitHub

public void setCohortMemberships(Collection<CohortMembership> members) {
  this.memberships = members;
}

New code

public void setCohortMemberships(Collection<CohortMembership> members) {
  this.memberships.addAll(members);
}

It should fix the issue, though I haven’t tried yet. So this requires modifying openmrs core. For now, I don’t want to do that. So is there any workaround or easy fix for this that does not require me modifying openmrs core?

cc: @dev5 @dev4 @dkayiwa @ibacher

1 Like
#2

So one quick thing to note is that the proposed code isn’t really equivalent to the previous code. To replace setCohortMemberships() with something like that we’d need:

public void setCohortMemberships(Collection<CohortMembership> members) {
  this.memberships.clear();
  this.memberships.addAll(members);
}

I bring this up because it helps me crystallise the problem which is that we need to modify the collection without changing the underlying collection identity. That leads me to think that instead of calling setCohortMemberships() (and maybe this shouldn’t be part of the Cohort public API), we should just use something like:

cohort.getCohortMemberships().addAll(members);
#3

I should say that I’m not recommending this replacement. We’d actually want something more sophisticated.

#4

Doesn’t work the same problem.

	if (group.hasMember()) {
		//Create new copy of existing cohortMemberships
		Set<CohortMembership> memberships = new HashSet<>(existingCohort.getMemberships());
		group.getMember().forEach(member -> memberships.add(groupMemberTranslator.toOpenmrsType(member)));
		existingCohort.setMemberships(memberships);
	}

I had this implementation, which I think there are similar

#5

The implementation is similar, but it has the same problem. When Hibernate is managing a collection, the object has Hibernate-specific metadata. When you swap that out for another collection (which is what this.memberships = <new collection> does), we get the error. In other words, things will generally break if we call setCohortMemberships() on a Cohort that has been saved to the database.

So, using your code as an example, we probably need something closer to this:

if (group.hasMember()) {
    Set<CohortMembership> newMemberships = group.getMember().stream().map(groupMemberTranslator::toOpenmrsType).collect(Collectors.toSet());
    existingCohort.getMemberships().removeIf(m -> !newMemberships.contains(m));
    existingCohort.getMemberships.addAll(newMemberships);
}

This should work for cohorts that haven’t yet been persisted as well as those that have.

#6

I was convinced that this would work but ended up with this; @ibacher?

java.lang.UnsupportedOperationException
at java.util.AbstractList.add(AbstractList.java:148)
at java.util.AbstractList.add(AbstractList.java:108)
at java.util.AbstractCollection.addAll(AbstractCollection.java:344)
#7

Hmmmm… yeah I’ve really got no idea what’s going on there. Any chance there’s some code that reproduces this issue you can share?

#8

@ibacher Just add your version of implementation here openmrs-module-fhir2/GroupTranslatorImpl_2_1.java at master · openmrs/openmrs-module-fhir2 · GitHub

It’s already in the master branch, then try running the respective tests.

#9

@corneliouzbett Looking at the code in context, would this work:

if (group.hasMember()) {
    group.getMember().stream()
        .map(groupMemberTranslator::toOpenmrsType)
        .forEach(existingCohort::addMembership);
}