From 70b858298cedd630adb80afbb204add6cf8fffa3 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Wed, 3 Jun 2026 20:30:30 +0800 Subject: [PATCH 1/3] O3-5696: Fix ORM mapping, duplicate UUID query and null-patient display crash --- .../queue/api/dao/impl/AbstractBaseQueueDaoImpl.java | 1 - .../java/org/openmrs/module/queue/model/QueueEntry.java | 3 +-- .../module/queue/web/resources/QueueEntryResource.java | 9 +++++++-- .../queue/web/resources/QueueEntryResourceTest.java | 5 +++++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/queue/api/dao/impl/AbstractBaseQueueDaoImpl.java b/api/src/main/java/org/openmrs/module/queue/api/dao/impl/AbstractBaseQueueDaoImpl.java index a5319a09..0ff8edac 100644 --- a/api/src/main/java/org/openmrs/module/queue/api/dao/impl/AbstractBaseQueueDaoImpl.java +++ b/api/src/main/java/org/openmrs/module/queue/api/dao/impl/AbstractBaseQueueDaoImpl.java @@ -60,7 +60,6 @@ public Optional get(int id) { public Optional get(@NotNull String uuid) { Criteria criteria = getCurrentSession().createCriteria(getClazz()); includeVoidedObjects(criteria, false); - criteria.add(eq("uuid", uuid)).uniqueResult(); return Optional.ofNullable((Q) criteria.add(eq("uuid", uuid)).uniqueResult()); } diff --git a/api/src/main/java/org/openmrs/module/queue/model/QueueEntry.java b/api/src/main/java/org/openmrs/module/queue/model/QueueEntry.java index a2ab33be..b109d120 100644 --- a/api/src/main/java/org/openmrs/module/queue/model/QueueEntry.java +++ b/api/src/main/java/org/openmrs/module/queue/model/QueueEntry.java @@ -16,7 +16,6 @@ import javax.persistence.Id; import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; -import javax.persistence.OneToOne; import javax.persistence.Table; import java.util.Date; @@ -89,7 +88,7 @@ public class QueueEntry extends BaseChangeableOpenmrsData { //The queue the patient is coming from, if any. @ToString.Exclude - @OneToOne + @ManyToOne @JoinColumn(name = "queue_coming_from", referencedColumnName = "queue_id") private Queue queueComingFrom; diff --git a/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntryResource.java b/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntryResource.java index 8f63592b..e3e1ec90 100644 --- a/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntryResource.java +++ b/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntryResource.java @@ -25,6 +25,7 @@ import io.swagger.models.properties.StringProperty; import lombok.Setter; import lombok.extern.slf4j.Slf4j; +import org.openmrs.Patient; import org.openmrs.PersonName; import org.openmrs.api.context.Context; import org.openmrs.module.queue.api.QueueServicesWrapper; @@ -281,8 +282,12 @@ private void addSharedResourceDescriptionProperties(DelegatingResourceDescriptio @PropertyGetter("display") public String getDisplay(QueueEntry queueEntry) { - PersonName personName = queueEntry.getPatient().getPersonName(); - return (personName == null ? queueEntry.getPatient().toString() : personName.getFullName()); + Patient patient = queueEntry.getPatient(); + if (patient == null) { + return queueEntry.getUuid(); + } + PersonName personName = patient.getPersonName(); + return (personName == null ? patient.toString() : personName.getFullName()); } @PropertyGetter("previousQueueEntry") diff --git a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java index 4b51bcc4..9ca726ce 100644 --- a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java +++ b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java @@ -428,6 +428,11 @@ public void shouldSearchQueueEntriesByIncludeVoidedFalse() { assertThat(criteria.isIncludedVoided(), equalTo(false)); } + @Test + public void shouldReturnUuidForDisplayWhenPatientIsNull() { + assertThat(resource.getDisplay(queueEntry), is(QUEUE_ENTRY_UUID)); + } + @Test public void shouldInstantiateNewDelegate() { assertThat(getResource().newDelegate(), notNullValue()); From e0c6da7393733b41139b7a15a8a0250e9bf58060 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Fri, 31 Jul 2026 00:17:15 +0800 Subject: [PATCH 2/3] O3-5696: Apply the null-patient display guard to QueueEntrySubResource QueueEntrySubResource.getDisplay had the same unguarded patient dereference as QueueEntryResource, and served the same display property for the same entity from GET /ws/rest/v1/queue/{uuid}/entry. Guarding only one getter left that endpoint exposed to the same crash. --- .../queue/web/resources/QueueEntrySubResource.java | 10 +++++++--- .../queue/web/resources/QueueEntrySubResourceTest.java | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntrySubResource.java b/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntrySubResource.java index b101e5a4..7169c9a0 100644 --- a/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntrySubResource.java +++ b/omod/src/main/java/org/openmrs/module/queue/web/resources/QueueEntrySubResource.java @@ -20,6 +20,8 @@ import io.swagger.models.ModelImpl; import io.swagger.models.properties.*; import lombok.Setter; +import org.openmrs.Patient; +import org.openmrs.PersonName; import org.openmrs.api.context.Context; import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; @@ -232,10 +234,12 @@ public Model getGETModel(Representation rep) { @PropertyGetter("display") public String getDisplay(QueueEntry queueEntry) { //Display patient name - if (queueEntry.getPatient().getPerson().getPersonName() == null) { - return ""; + Patient patient = queueEntry.getPatient(); + if (patient == null) { + return queueEntry.getUuid(); } - return queueEntry.getPatient().getPerson().getPersonName().getFullName(); + PersonName personName = patient.getPersonName(); + return (personName == null ? patient.toString() : personName.getFullName()); } @Override diff --git a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java index bfe2b87a..59ca1add 100644 --- a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java +++ b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java @@ -143,6 +143,11 @@ public void shouldCreateNewResource() { assertThat(newlyCreatedObject.getUuid(), is(QUEUE_ENTRY_UUID)); } + @Test + public void shouldReturnUuidForDisplayWhenPatientIsNull() { + assertThat(getResource().getDisplay(queueEntry), is(QUEUE_ENTRY_UUID)); + } + @Test public void shouldInstantiateNewDelegate() { assertThat(getResource().newDelegate(), notNullValue()); From e79b6252bf5dfdd21cf320c29103ddd8fb3a0e95 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Mon, 17 Aug 2026 02:05:33 +0800 Subject: [PATCH 3/3] O3-5696: Cover the no-unvoided-name display branch Aligning QueueEntrySubResource.getDisplay with QueueEntryResource changed the all-names-voided branch from "" to patient.toString(), and nothing covered it. That branch is the one known to occur: it dates to 2c4f7f1 "Fix NPE for patients without unvoided names". --- .../queue/web/resources/QueueEntryResourceTest.java | 9 +++++++++ .../queue/web/resources/QueueEntrySubResourceTest.java | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java index 9ca726ce..85e9eab5 100644 --- a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java +++ b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntryResourceTest.java @@ -433,6 +433,15 @@ public void shouldReturnUuidForDisplayWhenPatientIsNull() { assertThat(resource.getDisplay(queueEntry), is(QUEUE_ENTRY_UUID)); } + @Test + public void shouldReturnPatientToStringForDisplayWhenPatientHasNoUnvoidedName() { + Patient patient = new Patient(); + patient.setPatientId(7); + queueEntry.setPatient(patient); + + assertThat(resource.getDisplay(queueEntry), is("Patient#7")); + } + @Test public void shouldInstantiateNewDelegate() { assertThat(getResource().newDelegate(), notNullValue()); diff --git a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java index 59ca1add..e3578e35 100644 --- a/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java +++ b/omod/src/test/java/org/openmrs/module/queue/web/resources/QueueEntrySubResourceTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.openmrs.Patient; import org.openmrs.api.ConceptService; import org.openmrs.api.LocationService; import org.openmrs.api.PatientService; @@ -148,6 +149,15 @@ public void shouldReturnUuidForDisplayWhenPatientIsNull() { assertThat(getResource().getDisplay(queueEntry), is(QUEUE_ENTRY_UUID)); } + @Test + public void shouldReturnPatientToStringForDisplayWhenPatientHasNoUnvoidedName() { + Patient patient = new Patient(); + patient.setPatientId(7); + when(queueEntry.getPatient()).thenReturn(patient); + + assertThat(getResource().getDisplay(queueEntry), is("Patient#7")); + } + @Test public void shouldInstantiateNewDelegate() { assertThat(getResource().newDelegate(), notNullValue());