Search for patient by identifier and type question

We recently upgraded to openmrs 1.12 from 1.7. We are using PatientService.getPatients(String name, String identifier, List identifierTypes, boolean matchIdentifierExactly); to find patients that match the given identifier and types. The patient object that is returned from this method no longer has the full set of identifiers for the patient. Instead, it has only a single identifier in the set. As a test, I tried calling PatientService.getPatient(Integer patientId). This method also returned the patient with only a single identifier in the set. My patient has 3 identifiers stored in the system. Is there a different method that I should be using? Thanks, David Ely Software Developer Children’s Health Services Research IU School of Medicine Phone: 317-278-1642 Email:

This is a known issue, see TRUNK-5089

Does anyone know of a work around for this issue?

The Hibernate mapping of the collection is fine:

		<set name="identifiers" lazy="true" cascade="all-delete-orphan"
			table="patient_identifier" inverse="true" sort="natural">
			<key not-null="true" column="patient_id" />
			<one-to-many class="PatientIdentifier" />

If you perform select * from patient_identifier where patient_id = … it returns 3 rows ?

I believe I know what’s the problem, Patient initializes the identifiers collection with a TreeSet:

public Set getIdentifiers() { if (identifiers == null) { identifiers = new TreeSet(); } return this.identifiers; }

And PatientIdentier implements compareTo but not equals (I believe FindBugs detects this case):

public int compareTo(PatientIdentifier other) { DefaultComparator piDefaultComparator = new DefaultComparator(); return, other); }

According to the Treeset API:

Note that the ordering maintained by a set (whether or not an explicit comparator is provided) must be consistent with equals if it is to correctly implement the Set interface. (See Comparable or Comparator for a precise definition of consistent with equals.) This is so because the Set interface is defined in terms of the equals operation, but a TreeSet instance performs all element comparisons using its compareTo (or compare) method, so two elements that are deemed equal by this method are, from the standpoint of the set, equal. The behavior of a set is well-defined even if its ordering is inconsistent with equals; it just fails to obey the general contract of the Set interface.

I think we should remove all the Deprecated compareTo methods from the persistents ASAP to avoid strange behaviours. The DefaultComparator should be used only if sorting is needed.

BTW all of them are annotated with @SuppressWarnings(“squid:S1210”) - a Sonar Qube rule.

PatientService.getPatient(Integer patientId) only appears to work if the method in TRUNK-5089 has not been called first.

As I mentioned in TRUNK-5089, this is because PatientSearchCritera was changed in 1.11 to perform a left join to the patient_identifiers table meaning that only the matched patient identifier(s) is/are returned and the others are not. So this bug shows up for any PatientService service method that delegates to PatientSearchCriteria and searches on identifier only or identifier and name.

There is a work around for developers depending on what you’re doing, when I created the above ticket I was running into an issue where duplicate identifiers were getting added to the same patient since the existing identifier wasn’t getting loaded. Given that the affected client code was looking up a single Patient by their identifier by calling one of the affected search methods, the fix was to evict and reload the matched patient from the database before adding a new identifier to them as seen here. I think this bug might not be in later versions where we switched to lucene based patient searches.

Just so you know @david5780 is correct, any patient that gets matched when you call one of the affected methods first, any subsequent lookups for them whether by id within the same session will return the same record with partial ids loaded because of hibernate caching.

Yes, that is exactly what I was seeing. Thank you for taking the time to look into this :relaxed:

@david5780 would the work around I suggested above work for you before this is fixed in core?

The work around you suggested should work for us, but we haven’t had a chance to test it yet.