From bfb091a75fd7e8355510344a1a976ff850fc395d Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Sat, 22 Aug 2026 17:06:01 +0800 Subject: [PATCH 1/2] O3-5760: Accept timezone-qualified dates in queue entry searches --- .../module/queue/utils/QueueUtils.java | 6 +- .../QueueEntrySearchCriteriaParser.java | 22 +++++-- .../web/resources/QueueEntryResourceTest.java | 65 +++++++++++++++---- 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/queue/utils/QueueUtils.java b/api/src/main/java/org/openmrs/module/queue/utils/QueueUtils.java index 913798c3..d8ba3d44 100644 --- a/api/src/main/java/org/openmrs/module/queue/utils/QueueUtils.java +++ b/api/src/main/java/org/openmrs/module/queue/utils/QueueUtils.java @@ -26,12 +26,14 @@ public class QueueUtils { /** - * Utility method for parsing a date from a string into a Date. TODO: This will need review and - * testing related to handling of timezones and other date formats + * Utility method for parsing a date from a string into a Date * * @param dateVal the date value ot parse * @return the resulting date object + * @deprecated as of 3.1.0, as this only accepts yyyy-MM-dd HH:mm:ss in the server's default + * timezone. REST date parameters now use ConversionUtil.convert(String, Date.class). */ + @Deprecated public static Date parseDate(String dateVal) { try { return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").parse(dateVal); diff --git a/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java b/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java index 19f07f9c..aacec4ac 100644 --- a/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java +++ b/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java @@ -10,6 +10,7 @@ package org.openmrs.module.queue.web.resources.parser; import java.util.Arrays; +import java.util.Date; import java.util.List; import java.util.Map; @@ -19,7 +20,7 @@ import org.openmrs.Provider; import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; -import org.openmrs.module.queue.utils.QueueUtils; +import org.openmrs.module.webservices.rest.web.ConversionUtil; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -153,32 +154,32 @@ public QueueEntrySearchCriteria constructFromRequest(Map param } case SEARCH_PARAM_STARTED_ON_OR_AFTER: { String date = parameterMap.get(SEARCH_PARAM_STARTED_ON_OR_AFTER)[0]; - criteria.setStartedOnOrAfter(QueueUtils.parseDate(date)); + criteria.setStartedOnOrAfter(parseDate(date)); break; } case SEARCH_PARAM_STARTED_ON_OR_BEFORE: { String date = parameterMap.get(SEARCH_PARAM_STARTED_ON_OR_BEFORE)[0]; - criteria.setStartedOnOrBefore(QueueUtils.parseDate(date)); + criteria.setStartedOnOrBefore(parseDate(date)); break; } case SEARCH_PARAM_STARTED_ON: { String date = parameterMap.get(SEARCH_PARAM_STARTED_ON)[0]; - criteria.setStartedOn(QueueUtils.parseDate(date)); + criteria.setStartedOn(parseDate(date)); break; } case SEARCH_PARAM_ENDED_ON_OR_AFTER: { String date = parameterMap.get(SEARCH_PARAM_ENDED_ON_OR_AFTER)[0]; - criteria.setEndedOnOrAfter(QueueUtils.parseDate(date)); + criteria.setEndedOnOrAfter(parseDate(date)); break; } case SEARCH_PARAM_ENDED_ON_OR_BEFORE: { String date = parameterMap.get(SEARCH_PARAM_ENDED_ON_OR_BEFORE)[0]; - criteria.setEndedOnOrBefore(QueueUtils.parseDate(date)); + criteria.setEndedOnOrBefore(parseDate(date)); break; } case SEARCH_PARAM_ENDED_ON: { String date = parameterMap.get(SEARCH_PARAM_ENDED_ON)[0]; - criteria.setEndedOn(QueueUtils.parseDate(date)); + criteria.setEndedOn(parseDate(date)); break; } case SEARCH_PARAM_IS_ENDED: { @@ -197,6 +198,13 @@ public QueueEntrySearchCriteria constructFromRequest(Map param return criteria; } + private Date parseDate(String value) { + if (StringUtils.isBlank(value)) { + return null; + } + return (Date) ConversionUtil.convert(value, Date.class); + } + private Boolean parseBoolean(String value) { if (StringUtils.isBlank(value)) { return null; 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..936365b8 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 @@ -16,6 +16,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; @@ -41,8 +42,12 @@ import javax.servlet.http.HttpServletRequest; +import java.time.Instant; import java.util.Arrays; +import java.util.Calendar; import java.util.Collections; +import java.util.Date; +import java.util.GregorianCalendar; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -63,6 +68,7 @@ import org.openmrs.api.LocationService; import org.openmrs.api.PatientService; import org.openmrs.api.context.Context; +import org.openmrs.messagesource.MessageSourceService; import org.openmrs.module.queue.api.QueueEntryService; import org.openmrs.module.queue.api.QueueRoomService; import org.openmrs.module.queue.api.QueueService; @@ -71,20 +77,26 @@ import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; import org.openmrs.module.queue.model.Queue; import org.openmrs.module.queue.model.QueueEntry; -import org.openmrs.module.queue.utils.QueueUtils; import org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser; import org.openmrs.module.webservices.rest.web.RequestContext; import org.openmrs.module.webservices.rest.web.RestUtil; +import org.openmrs.module.webservices.rest.web.api.RestService; import org.openmrs.module.webservices.rest.web.representation.CustomRepresentation; import org.openmrs.module.webservices.rest.web.representation.DefaultRepresentation; import org.openmrs.module.webservices.rest.web.representation.FullRepresentation; import org.openmrs.module.webservices.rest.web.representation.RefRepresentation; +import org.openmrs.module.webservices.rest.web.response.ConversionException; @ExtendWith(MockitoExtension.class) public class QueueEntryResourceTest extends BaseQueueResourceTest { private static final String QUEUE_ENTRY_UUID = "6hje567a-fca0-11e5-9e59-08002719a7"; + // An instant expressed with an offset, so the expectation holds whatever timezone the tests run in + private static final String DATE_WITH_OFFSET = "2023-09-10T16:42:13.000+05:30"; + + private static final Date EXPECTED_DATE = Date.from(Instant.parse("2023-09-10T11:12:13Z")); + private QueueEntryResource resource; private QueueEntry queueEntry; @@ -136,6 +148,11 @@ public void prepareMocks() { //By pass authentication getContext().when(Context::isAuthenticated).thenReturn(true); + // ConversionUtil looks for a Date converter before parsing; finding none raises an APIException + // that it swallows, and building that exception localizes its message. Both are needed. + getContext().when(() -> Context.getService(RestService.class)).thenReturn(mock(RestService.class)); + getContext().when(Context::getMessageSourceService).thenReturn(mock(MessageSourceService.class)); + getContext().when(() -> Context.getRegisteredComponents(QueueServicesWrapper.class)) .thenReturn(Collections.singletonList(queueServicesWrapper)); @@ -354,42 +371,64 @@ public void shouldSearchQueueEntriesByQueueComingFrom() { @Test public void shouldSearchQueueEntriesByStartedOnOrAfter() { - String dateStr = "2023-09-10 11:12:13"; - parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { dateStr }); + parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { DATE_WITH_OFFSET }); resource.doSearch(requestContext); verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); - assertThat(criteria.getStartedOnOrAfter(), equalTo(QueueUtils.parseDate(dateStr))); + assertThat(criteria.getStartedOnOrAfter(), equalTo(EXPECTED_DATE)); } @Test public void shouldSearchQueueEntriesByStartedOnOrBefore() { - String dateStr = "2023-09-10 11:12:13"; - parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_BEFORE, new String[] { dateStr }); + parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_BEFORE, new String[] { DATE_WITH_OFFSET }); resource.doSearch(requestContext); verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); - assertThat(criteria.getStartedOnOrBefore(), equalTo(QueueUtils.parseDate(dateStr))); + assertThat(criteria.getStartedOnOrBefore(), equalTo(EXPECTED_DATE)); } @Test public void shouldSearchQueueEntriesByEndedOnOrAfter() { - String dateStr = "2023-09-10 11:12:13"; - parameterMap.put(SEARCH_PARAM_ENDED_ON_OR_AFTER, new String[] { dateStr }); + parameterMap.put(SEARCH_PARAM_ENDED_ON_OR_AFTER, new String[] { DATE_WITH_OFFSET }); resource.doSearch(requestContext); verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); - assertThat(criteria.getEndedOnOrAfter(), equalTo(QueueUtils.parseDate(dateStr))); + assertThat(criteria.getEndedOnOrAfter(), equalTo(EXPECTED_DATE)); } @Test public void shouldSearchQueueEntriesByEndedOnOrBefore() { - String dateStr = "2023-09-10 11:12:13"; - parameterMap.put(SEARCH_PARAM_ENDED_ON_OR_BEFORE, new String[] { dateStr }); + parameterMap.put(SEARCH_PARAM_ENDED_ON_OR_BEFORE, new String[] { DATE_WITH_OFFSET }); + resource.doSearch(requestContext); + verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); + QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); + assertThat(criteria.getEndedOnOrBefore(), equalTo(EXPECTED_DATE)); + } + + @Test + public void shouldSearchQueueEntriesByDateWithAUtcDesignator() { + // The shape Date.toISOString() produces, which is what the O3 service queues app sends + parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { "2023-09-10T11:12:13.000Z" }); resource.doSearch(requestContext); verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); - assertThat(criteria.getEndedOnOrBefore(), equalTo(QueueUtils.parseDate(dateStr))); + assertThat(criteria.getStartedOnOrAfter(), equalTo(EXPECTED_DATE)); + } + + @Test + public void shouldSearchQueueEntriesByDateWithoutATimezone() { + parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { "2023-09-10 11:12:13" }); + resource.doSearch(requestContext); + verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); + QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); + Date expected = new GregorianCalendar(2023, Calendar.SEPTEMBER, 10, 11, 12, 13).getTime(); + assertThat(criteria.getStartedOnOrAfter(), equalTo(expected)); + } + + @Test + public void shouldFailToSearchQueueEntriesByAnUnparseableDate() { + parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { "not-a-date" }); + assertThrows(ConversionException.class, () -> resource.doSearch(requestContext)); } @Test From d6a6850c0f1779d769fc856814a55fa3a93ffb4b Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Tue, 25 Aug 2026 02:57:11 +0800 Subject: [PATCH 2/2] O3-5760: fail on blank dates and name the offending parameter - A blank date param (e.g. ?startedOnOrAfter=) now raises a 400 rather than silently dropping the filter and returning everything. - parseDate takes the parameter name and rethrows ConversionException with it, so a request carrying several date params says which one failed. - Cover startedOn and endedOn, which moved to the new parser too, plus the blank case next to the unparseable one. --- .../QueueEntrySearchCriteriaParser.java | 24 ++++++++------ .../web/resources/QueueEntryResourceTest.java | 31 ++++++++++++++++++- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java b/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java index aacec4ac..68b5e1ac 100644 --- a/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java +++ b/omod/src/main/java/org/openmrs/module/queue/web/resources/parser/QueueEntrySearchCriteriaParser.java @@ -21,6 +21,7 @@ import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; import org.openmrs.module.webservices.rest.web.ConversionUtil; +import org.openmrs.module.webservices.rest.web.response.ConversionException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -154,32 +155,32 @@ public QueueEntrySearchCriteria constructFromRequest(Map param } case SEARCH_PARAM_STARTED_ON_OR_AFTER: { String date = parameterMap.get(SEARCH_PARAM_STARTED_ON_OR_AFTER)[0]; - criteria.setStartedOnOrAfter(parseDate(date)); + criteria.setStartedOnOrAfter(parseDate(parameterName, date)); break; } case SEARCH_PARAM_STARTED_ON_OR_BEFORE: { String date = parameterMap.get(SEARCH_PARAM_STARTED_ON_OR_BEFORE)[0]; - criteria.setStartedOnOrBefore(parseDate(date)); + criteria.setStartedOnOrBefore(parseDate(parameterName, date)); break; } case SEARCH_PARAM_STARTED_ON: { String date = parameterMap.get(SEARCH_PARAM_STARTED_ON)[0]; - criteria.setStartedOn(parseDate(date)); + criteria.setStartedOn(parseDate(parameterName, date)); break; } case SEARCH_PARAM_ENDED_ON_OR_AFTER: { String date = parameterMap.get(SEARCH_PARAM_ENDED_ON_OR_AFTER)[0]; - criteria.setEndedOnOrAfter(parseDate(date)); + criteria.setEndedOnOrAfter(parseDate(parameterName, date)); break; } case SEARCH_PARAM_ENDED_ON_OR_BEFORE: { String date = parameterMap.get(SEARCH_PARAM_ENDED_ON_OR_BEFORE)[0]; - criteria.setEndedOnOrBefore(parseDate(date)); + criteria.setEndedOnOrBefore(parseDate(parameterName, date)); break; } case SEARCH_PARAM_ENDED_ON: { String date = parameterMap.get(SEARCH_PARAM_ENDED_ON)[0]; - criteria.setEndedOn(parseDate(date)); + criteria.setEndedOn(parseDate(parameterName, date)); break; } case SEARCH_PARAM_IS_ENDED: { @@ -198,11 +199,16 @@ public QueueEntrySearchCriteria constructFromRequest(Map param return criteria; } - private Date parseDate(String value) { + private Date parseDate(String parameterName, String value) { if (StringUtils.isBlank(value)) { - return null; + throw new ConversionException("The " + parameterName + " parameter is blank"); + } + try { + return (Date) ConversionUtil.convert(value, Date.class); + } + catch (ConversionException e) { + throw new ConversionException("Unable to parse the " + parameterName + " parameter: " + value, e); } - return (Date) ConversionUtil.convert(value, Date.class); } private Boolean parseBoolean(String value) { 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 936365b8..efdd9781 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 @@ -11,6 +11,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; @@ -22,6 +23,7 @@ import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_ENDED_ON; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_ENDED_ON_OR_AFTER; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_ENDED_ON_OR_BEFORE; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_HAS_VISIT; @@ -35,6 +37,7 @@ import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_QUEUE; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_QUEUE_COMING_FROM; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_SERVICE; +import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_STARTED_ON; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_STARTED_ON_OR_AFTER; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_STARTED_ON_OR_BEFORE; import static org.openmrs.module.queue.web.resources.parser.QueueEntrySearchCriteriaParser.SEARCH_PARAM_STATUS; @@ -387,6 +390,15 @@ public void shouldSearchQueueEntriesByStartedOnOrBefore() { assertThat(criteria.getStartedOnOrBefore(), equalTo(EXPECTED_DATE)); } + @Test + public void shouldSearchQueueEntriesByStartedOn() { + parameterMap.put(SEARCH_PARAM_STARTED_ON, new String[] { DATE_WITH_OFFSET }); + resource.doSearch(requestContext); + verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); + QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); + assertThat(criteria.getStartedOn(), equalTo(EXPECTED_DATE)); + } + @Test public void shouldSearchQueueEntriesByEndedOnOrAfter() { parameterMap.put(SEARCH_PARAM_ENDED_ON_OR_AFTER, new String[] { DATE_WITH_OFFSET }); @@ -405,6 +417,15 @@ public void shouldSearchQueueEntriesByEndedOnOrBefore() { assertThat(criteria.getEndedOnOrBefore(), equalTo(EXPECTED_DATE)); } + @Test + public void shouldSearchQueueEntriesByEndedOn() { + parameterMap.put(SEARCH_PARAM_ENDED_ON, new String[] { DATE_WITH_OFFSET }); + resource.doSearch(requestContext); + verify(queueEntryService).getQueueEntries(queueEntryArgumentCaptor.capture()); + QueueEntrySearchCriteria criteria = queueEntryArgumentCaptor.getValue(); + assertThat(criteria.getEndedOn(), equalTo(EXPECTED_DATE)); + } + @Test public void shouldSearchQueueEntriesByDateWithAUtcDesignator() { // The shape Date.toISOString() produces, which is what the O3 service queues app sends @@ -428,7 +449,15 @@ public void shouldSearchQueueEntriesByDateWithoutATimezone() { @Test public void shouldFailToSearchQueueEntriesByAnUnparseableDate() { parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { "not-a-date" }); - assertThrows(ConversionException.class, () -> resource.doSearch(requestContext)); + ConversionException e = assertThrows(ConversionException.class, () -> resource.doSearch(requestContext)); + assertThat(e.getMessage(), containsString(SEARCH_PARAM_STARTED_ON_OR_AFTER)); + } + + @Test + public void shouldFailToSearchQueueEntriesByABlankDate() { + parameterMap.put(SEARCH_PARAM_STARTED_ON_OR_AFTER, new String[] { "" }); + ConversionException e = assertThrows(ConversionException.class, () -> resource.doSearch(requestContext)); + assertThat(e.getMessage(), containsString(SEARCH_PARAM_STARTED_ON_OR_AFTER)); } @Test