From 83dca3d10d19b224ade38fce22d0e8199d2e0ae4 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Mon, 20 Jul 2026 00:07:28 +0800 Subject: [PATCH 1/9] O3-5765: Automatically clear queue entries on a schedule --- .../module/queue/QueueModuleConstants.java | 4 + .../queue/tasks/AutoCloseQueueEntryTask.java | 168 ++++++++++++++++++ .../module/queue/tasks/QueueTaskExecutor.java | 3 +- .../tasks/AutoCloseQueueEntryTaskTest.java | 147 +++++++++++++++ omod/src/main/resources/config.xml | 10 ++ 5 files changed, 331 insertions(+), 1 deletion(-) create mode 100644 api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java create mode 100644 api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java diff --git a/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java b/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java index 9019f48a..12d16456 100644 --- a/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java +++ b/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java @@ -20,4 +20,8 @@ public class QueueModuleConstants { public final static String QUEUE_SORT_WEIGHT_GENERATOR = "queue.sortWeightGenerator"; public final static String EXISTING_VALUE_SORT_WEIGHT_GENERATOR = "existingValueSortWeightGenerator"; + + public final static String AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME = "queue.autoCloseQueueEntriesAtTime"; + + public final static String AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES = "queue.autoCloseQueueEntriesForQueues"; } diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java new file mode 100644 index 00000000..b240ba31 --- /dev/null +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -0,0 +1,168 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.module.queue.tasks; + +import static org.openmrs.module.queue.QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME; +import static org.openmrs.module.queue.QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Calendar; +import java.util.Date; +import java.util.List; + +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.openmrs.api.context.Context; +import org.openmrs.module.queue.api.QueueServicesWrapper; +import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; +import org.openmrs.module.queue.model.Queue; +import org.openmrs.module.queue.model.QueueEntry; + +/** + * This ends all active queue entries in the configured queues once per day at a configured time of + * day (default: end of day). The set of queues to clear and the time to clear them are both + * controlled by global properties. Only entries that were already active at the configured time are + * ended, so patients added later in the day are left in the queue. + */ +@Slf4j +public class AutoCloseQueueEntryTask implements Runnable { + + private static final String TIME_FORMAT = "HH:mm"; + + private static volatile boolean currentlyExecuting = false; + + @Override + public void run() { + if (currentlyExecuting) { + log.debug("AutoCloseQueueEntryTask is still executing, not running again"); + return; + } + log.debug("Executing AutoCloseQueueEntryTask"); + try { + currentlyExecuting = true; + + String configuredTime = getConfiguredCloseTime(); + if (StringUtils.isBlank(configuredTime)) { + log.debug("No auto-close time configured, not clearing any queue entries"); + return; + } + + Date now = now(); + Date closeTime = getCloseTimeForToday(configuredTime.trim(), now); + if (closeTime == null) { + return; + } + if (now.before(closeTime)) { + log.debug("Current time is before the configured auto-close time {}, nothing to do", configuredTime); + return; + } + + List queues = getQueuesToClear(); + if (queues != null && queues.isEmpty()) { + log.debug("No queues configured for auto-close, nothing to do"); + return; + } + + QueueEntrySearchCriteria criteria = new QueueEntrySearchCriteria(); + criteria.setIsEnded(Boolean.FALSE); + criteria.setStartedOnOrBefore(closeTime); + criteria.setQueues(queues); + + List queueEntries = getQueueEntries(criteria); + log.debug("There are {} queue entries to auto-close", queueEntries.size()); + for (QueueEntry queueEntry : queueEntries) { + try { + queueEntry.setEndedAt(now); + saveQueueEntry(queueEntry); + log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); + } + catch (Exception e) { + log.warn("Unable to auto-close queue entry {}", queueEntry.getQueueEntryId(), e); + } + } + } + finally { + currentlyExecuting = false; + } + } + + /** + * Parses the configured HH:mm time and returns the corresponding instant on the same day as the + * given reference date. Returns null if the configured value cannot be parsed. + */ + protected Date getCloseTimeForToday(String configuredTime, Date referenceDate) { + try { + SimpleDateFormat format = new SimpleDateFormat(TIME_FORMAT); + format.setLenient(false); + Calendar parsed = Calendar.getInstance(); + parsed.setTime(format.parse(configuredTime)); + + Calendar closeTime = Calendar.getInstance(); + closeTime.setTime(referenceDate); + closeTime.set(Calendar.HOUR_OF_DAY, parsed.get(Calendar.HOUR_OF_DAY)); + closeTime.set(Calendar.MINUTE, parsed.get(Calendar.MINUTE)); + closeTime.set(Calendar.SECOND, 0); + closeTime.set(Calendar.MILLISECOND, 0); + return closeTime.getTime(); + } + catch (ParseException e) { + log.warn("Invalid value '{}' for global property {}, expected format {}", configuredTime, + AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME, TIME_FORMAT); + return null; + } + } + + /** + * @return the configured time of day (HH:mm) at which to clear queue entries, or blank/null if + * auto-clearing is disabled + */ + protected String getConfiguredCloseTime() { + return getServices().getAdministrationService().getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME); + } + + /** + * @return the queues whose entries should be cleared, or null to clear entries in all queues + */ + protected List getQueuesToClear() { + String configuredQueues = getServices().getAdministrationService() + .getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES); + if (StringUtils.isBlank(configuredQueues)) { + return null; + } + return getServices().getQueues(configuredQueues.split(",")); + } + + /** + * @param criteria the criteria identifying the queue entries to end + * @return the queue entries matching the given criteria + */ + protected List getQueueEntries(QueueEntrySearchCriteria criteria) { + return getServices().getQueueEntryService().getQueueEntries(criteria); + } + + /** + * @param queueEntry the QueueEntry to save + */ + protected void saveQueueEntry(QueueEntry queueEntry) { + getServices().getQueueEntryService().saveQueueEntry(queueEntry); + } + + /** + * @return the current time; overridable to allow deterministic testing + */ + protected Date now() { + return new Date(); + } + + protected QueueServicesWrapper getServices() { + return Context.getRegisteredComponent("queue.QueueServicesWrapper", QueueServicesWrapper.class); + } +} diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java b/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java index ab244a75..7d995dba 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java @@ -28,7 +28,8 @@ public class QueueTaskExecutor extends ScheduledExecutorFactoryBean { private static final long ONE_MINUTE = ONE_SECOND * 60; public QueueTaskExecutor() { - setScheduledExecutorTasks(task(ONE_MINUTE, ONE_MINUTE, AutoCloseVisitQueueEntryTask.class)); + setScheduledExecutorTasks(task(ONE_MINUTE, ONE_MINUTE, AutoCloseVisitQueueEntryTask.class), + task(ONE_MINUTE, ONE_MINUTE, AutoCloseQueueEntryTask.class)); } private ScheduledExecutorTask task(long delay, long period, Class runnable) { diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java new file mode 100644 index 00000000..daf4f254 --- /dev/null +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -0,0 +1,147 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.module.queue.tasks; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; + +import java.text.DateFormat; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Before; +import org.junit.Test; +import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; +import org.openmrs.module.queue.model.Queue; +import org.openmrs.module.queue.model.QueueEntry; + +public class AutoCloseQueueEntryTaskTest { + + final List queueEntries = new ArrayList<>(); + + private String configuredTime; + + private List configuredQueues; + + private Date now; + + class TestAutoCloseQueueEntryTask extends AutoCloseQueueEntryTask { + + @Override + protected String getConfiguredCloseTime() { + return configuredTime; + } + + @Override + protected List getQueuesToClear() { + return configuredQueues; + } + + @Override + protected Date now() { + return now; + } + + @Override + protected List getQueueEntries(QueueEntrySearchCriteria criteria) { + // Emulate the DB-level filtering that getQueueEntries would normally perform + return queueEntries.stream().filter(e -> e.getEndedAt() == null) + .filter(e -> e.getStartedAt() == null || !e.getStartedAt().after(criteria.getStartedOnOrBefore())) + .filter(e -> criteria.getQueues() == null || criteria.getQueues().contains(e.getQueue())) + .collect(Collectors.toList()); + } + + @Override + protected void saveQueueEntry(QueueEntry queueEntry) { + // Do nothing + } + } + + @Before + public void setup() throws Exception { + queueEntries.clear(); + configuredQueues = null; + now = getDate("2020-01-01 23:59"); + } + + @Test + public void shouldDoNothingWhenTimeIsBlank() throws Exception { + configuredTime = ""; + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), nullValue()); + } + + @Test + public void shouldNotClearBeforeConfiguredTime() throws Exception { + configuredTime = "23:59"; + now = getDate("2020-01-01 17:00"); + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), nullValue()); + } + + @Test + public void shouldClearActiveEntriesAtOrAfterConfiguredTime() throws Exception { + configuredTime = "23:59"; + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), equalTo(now)); + } + + @Test + public void shouldNotClearEntriesStartedAfterConfiguredTime() throws Exception { + configuredTime = "18:00"; + now = getDate("2020-01-01 18:30"); + QueueEntry beforeCloseTime = queueEntryStartedAt("2020-01-01 09:00", null); + QueueEntry afterCloseTime = queueEntryStartedAt("2020-01-01 18:15", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(beforeCloseTime.getEndedAt(), notNullValue()); + assertThat(afterCloseTime.getEndedAt(), nullValue()); + } + + @Test + public void shouldOnlyClearConfiguredQueues() throws Exception { + configuredTime = "23:59"; + Queue queueA = new Queue(); + Queue queueB = new Queue(); + configuredQueues = new ArrayList<>(); + configuredQueues.add(queueA); + + QueueEntry inQueueA = queueEntryStartedAt("2020-01-01 09:00", queueA); + QueueEntry inQueueB = queueEntryStartedAt("2020-01-01 09:00", queueB); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(inQueueA.getEndedAt(), notNullValue()); + assertThat(inQueueB.getEndedAt(), nullValue()); + } + + private QueueEntry queueEntryStartedAt(String startedAt, Queue queue) throws Exception { + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setStartedAt(getDate(startedAt)); + queueEntry.setQueue(queue); + queueEntries.add(queueEntry); + return queueEntry; + } + + Date getDate(String dateStr) throws Exception { + DateFormat df = new SimpleDateFormat("yyyy-MM-dd HH:mm"); + return df.parse(dateStr); + } +} diff --git a/omod/src/main/resources/config.xml b/omod/src/main/resources/config.xml index db9cd7e6..ad38ad60 100644 --- a/omod/src/main/resources/config.xml +++ b/omod/src/main/resources/config.xml @@ -55,5 +55,15 @@ The bean name of a registered component that provides an algorithm to set a queue entry sort weight when saved + + ${project.parent.artifactId}.autoCloseQueueEntriesAtTime + 23:59 + Time of day (HH:mm, server local time) at which active queue entries are automatically ended each day. Leave blank to disable automatic clearing. + + + ${project.parent.artifactId}.autoCloseQueueEntriesForQueues + + Comma-separated list of queue uuids whose entries are automatically ended at the configured time. Leave blank to clear entries in all queues. + From fb2a13d808f9ecbe32086ef9774b79a2a043b9e2 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Fri, 24 Jul 2026 21:15:03 +0800 Subject: [PATCH 2/9] O3-5765: Address review feedback on scheduled queue clearing --- README.md | 26 ++++++++++ .../module/queue/QueueModuleConstants.java | 4 +- .../queue/tasks/AutoCloseQueueEntryTask.java | 50 +++++++++++++------ .../tasks/AutoCloseQueueEntryTaskTest.java | 9 ++++ 4 files changed, 73 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index a2d66a12..377c1ea6 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,32 @@ A typical set of priorities might include: `Normal` and `Emergency` This provides a means to configure the sort weight generator that maintains the primary ordering of Queue Entries on a particular Queue. By default, the `existingValueSortWeightGenerator` will be utilized. +#### queue.autoCloseQueueEntriesAtTime + +**Default Value:** `23:59` + +**Required?** False + +**Description:** +The time of day, in `HH:mm` 24-hour format and the server's local time, at which active queue entries are +automatically ended each day. A scheduled task ends every queue entry that was still active as of this time; entries +started later in the day are left untouched. Leave this property blank to disable automatic clearing entirely. + +**⚠️ Note for existing deployments:** Because this ships with a default of `23:59`, deployments that upgrade to this +version will begin automatically clearing queue entries at end of day as soon as they pick up the new version. Blank +this property (or restrict it via `queue.autoCloseQueueEntriesForQueues`) if that is not the desired behavior. + +#### queue.autoCloseQueueEntriesForQueues + +**Default Value:** None (empty) + +**Required?** False + +**Description:** +A comma-separated list of queue uuids whose entries are automatically ended at the time configured by +`queue.autoCloseQueueEntriesAtTime`. Leave this property blank to clear entries in **all** queues. Unknown uuids are +logged and skipped rather than aborting the task. + ### Sort Weight Generators As described above in Global Property configuration, one can configure the specific algorithm to use to generate and diff --git a/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java b/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java index 12d16456..bfa434ad 100644 --- a/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java +++ b/api/src/main/java/org/openmrs/module/queue/QueueModuleConstants.java @@ -21,7 +21,7 @@ public class QueueModuleConstants { public final static String EXISTING_VALUE_SORT_WEIGHT_GENERATOR = "existingValueSortWeightGenerator"; - public final static String AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME = "queue.autoCloseQueueEntriesAtTime"; + public static final String AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME = "queue.autoCloseQueueEntriesAtTime"; - public final static String AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES = "queue.autoCloseQueueEntriesForQueues"; + public static final String AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES = "queue.autoCloseQueueEntriesForQueues"; } diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java index b240ba31..a050e536 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -14,9 +14,11 @@ import java.text.ParseException; import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.Calendar; import java.util.Date; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; @@ -37,18 +39,16 @@ public class AutoCloseQueueEntryTask implements Runnable { private static final String TIME_FORMAT = "HH:mm"; - private static volatile boolean currentlyExecuting = false; + private static final AtomicBoolean currentlyExecuting = new AtomicBoolean(false); @Override public void run() { - if (currentlyExecuting) { + if (!currentlyExecuting.compareAndSet(false, true)) { log.debug("AutoCloseQueueEntryTask is still executing, not running again"); return; } log.debug("Executing AutoCloseQueueEntryTask"); try { - currentlyExecuting = true; - String configuredTime = getConfiguredCloseTime(); if (StringUtils.isBlank(configuredTime)) { log.debug("No auto-close time configured, not clearing any queue entries"); @@ -79,18 +79,26 @@ public void run() { List queueEntries = getQueueEntries(criteria); log.debug("There are {} queue entries to auto-close", queueEntries.size()); for (QueueEntry queueEntry : queueEntries) { - try { - queueEntry.setEndedAt(now); - saveQueueEntry(queueEntry); - log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); - } - catch (Exception e) { - log.warn("Unable to auto-close queue entry {}", queueEntry.getQueueEntryId(), e); - } + closeQueueEntry(queueEntry, now); } } + catch (Exception e) { + log.error("AutoCloseQueueEntryTask failed to complete", e); + } finally { - currentlyExecuting = false; + currentlyExecuting.set(false); + } + } + + private void closeQueueEntry(QueueEntry queueEntry, Date endedAt) { + try { + queueEntry.setEndedAt(endedAt); + saveQueueEntry(queueEntry); + log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); + } + catch (Exception e) { + Context.evictFromSession(queueEntry); + log.warn("Unable to auto-close queue entry {}", queueEntry.getQueueEntryId(), e); } } @@ -137,7 +145,21 @@ protected List getQueuesToClear() { if (StringUtils.isBlank(configuredQueues)) { return null; } - return getServices().getQueues(configuredQueues.split(",")); + List queues = new ArrayList<>(); + for (String queueRef : configuredQueues.split(",")) { + String trimmed = queueRef.trim(); + if (StringUtils.isBlank(trimmed)) { + continue; + } + try { + queues.add(getServices().getQueue(trimmed)); + } + catch (IllegalArgumentException e) { + log.warn("Ignoring unknown queue '{}' configured in global property {}", trimmed, + AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES); + } + } + return queues; } /** diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java index daf4f254..020d5740 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -85,6 +85,15 @@ public void shouldDoNothingWhenTimeIsBlank() throws Exception { assertThat(queueEntry.getEndedAt(), nullValue()); } + @Test + public void shouldDoNothingWhenTimeIsUnparseable() throws Exception { + configuredTime = "nonsense"; + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), nullValue()); + } + @Test public void shouldNotClearBeforeConfiguredTime() throws Exception { configuredTime = "23:59"; From 896f05dce0cdc992886d572d5d051bc6fa49dffe Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Mon, 27 Jul 2026 15:19:39 +0800 Subject: [PATCH 3/9] O3-5765: Default auto-close off and catch up missed runs --- README.md | 20 ++-- .../queue/tasks/AutoCloseQueueEntryTask.java | 62 ++++++----- .../tasks/AutoCloseQueueEntryTaskTest.java | 102 +++++++++++++++++- omod/src/main/resources/config.xml | 4 +- 4 files changed, 151 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 377c1ea6..032b6b32 100644 --- a/README.md +++ b/README.md @@ -71,18 +71,26 @@ a particular Queue. By default, the `existingValueSortWeightGenerator` will be #### queue.autoCloseQueueEntriesAtTime -**Default Value:** `23:59` +**Default Value:** None (empty), which disables automatic clearing **Required?** False **Description:** The time of day, in `HH:mm` 24-hour format and the server's local time, at which active queue entries are automatically ended each day. A scheduled task ends every queue entry that was still active as of this time; entries -started later in the day are left untouched. Leave this property blank to disable automatic clearing entirely. - -**⚠️ Note for existing deployments:** Because this ships with a default of `23:59`, deployments that upgrade to this -version will begin automatically clearing queue entries at end of day as soon as they pick up the new version. Blank -this property (or restrict it via `queue.autoCloseQueueEntriesForQueues`) if that is not the desired behavior. +started later in the day are left untouched. + +This property is blank by default, so no clearing happens until an implementer sets it. An ordinary outpatient clinic +that wants its queues emptied at end of day would set `23:59`. Leave it blank if queue entries are used for anything +that should survive overnight — tracking where inpatients currently are within a multi-day visit, for instance, or a +queue of patients to follow up with over the coming week. Use `queue.autoCloseQueueEntriesForQueues` to enable +clearing for some queues but not others. + +The task works from the most recent occurrence of the configured time rather than from the moment it happens to run, +so if it does not get a chance to run at that time (a restart or a maintenance window, say) the next run catches up +instead of leaving the queues uncleared until the following day. One consequence: the first run after this property +is set ends anything still open from before the most recent occurrence of the configured time, rather than waiting +for the next one. #### queue.autoCloseQueueEntriesForQueues diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java index a050e536..44574821 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -12,7 +12,7 @@ import static org.openmrs.module.queue.QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME; import static org.openmrs.module.queue.QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES; -import java.text.ParseException; +import java.text.ParsePosition; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Calendar; @@ -22,6 +22,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; +import org.openmrs.api.ValidationException; import org.openmrs.api.context.Context; import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; @@ -30,9 +31,10 @@ /** * This ends all active queue entries in the configured queues once per day at a configured time of - * day (default: end of day). The set of queues to clear and the time to clear them are both - * controlled by global properties. Only entries that were already active at the configured time are - * ended, so patients added later in the day are left in the queue. + * day. The set of queues to clear and the time to clear them are both controlled by global + * properties, and nothing is cleared unless a time has been configured. Each run works from the + * most recent occurrence of that time rather than from the current tick, so patients added since + * then are left in the queue and a run missed at the configured time is caught up by the next one. */ @Slf4j public class AutoCloseQueueEntryTask implements Runnable { @@ -56,14 +58,10 @@ public void run() { } Date now = now(); - Date closeTime = getCloseTimeForToday(configuredTime.trim(), now); + Date closeTime = getMostRecentCloseTime(configuredTime.trim(), now); if (closeTime == null) { return; } - if (now.before(closeTime)) { - log.debug("Current time is before the configured auto-close time {}, nothing to do", configuredTime); - return; - } List queues = getQueuesToClear(); if (queues != null && queues.isEmpty()) { @@ -72,7 +70,7 @@ public void run() { } QueueEntrySearchCriteria criteria = new QueueEntrySearchCriteria(); - criteria.setIsEnded(Boolean.FALSE); + criteria.setIsEnded(false); criteria.setStartedOnOrBefore(closeTime); criteria.setQueues(queues); @@ -96,6 +94,10 @@ private void closeQueueEntry(QueueEntry queueEntry, Date endedAt) { saveQueueEntry(queueEntry); log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); } + catch (ValidationException ve) { + Context.evictFromSession(queueEntry); + log.warn("Unable to auto-close queue entry {}: {}", queueEntry.getQueueEntryId(), ve.getMessage()); + } catch (Exception e) { Context.evictFromSession(queueEntry); log.warn("Unable to auto-close queue entry {}", queueEntry.getQueueEntryId(), e); @@ -103,29 +105,33 @@ private void closeQueueEntry(QueueEntry queueEntry, Date endedAt) { } /** - * Parses the configured HH:mm time and returns the corresponding instant on the same day as the - * given reference date. Returns null if the configured value cannot be parsed. + * Parses the configured HH:mm time and returns the most recent instant at which that time of day + * occurred: today's occurrence if it has already passed, otherwise yesterday's. Returns null if the + * configured value cannot be parsed. */ - protected Date getCloseTimeForToday(String configuredTime, Date referenceDate) { - try { - SimpleDateFormat format = new SimpleDateFormat(TIME_FORMAT); - format.setLenient(false); - Calendar parsed = Calendar.getInstance(); - parsed.setTime(format.parse(configuredTime)); - - Calendar closeTime = Calendar.getInstance(); - closeTime.setTime(referenceDate); - closeTime.set(Calendar.HOUR_OF_DAY, parsed.get(Calendar.HOUR_OF_DAY)); - closeTime.set(Calendar.MINUTE, parsed.get(Calendar.MINUTE)); - closeTime.set(Calendar.SECOND, 0); - closeTime.set(Calendar.MILLISECOND, 0); - return closeTime.getTime(); - } - catch (ParseException e) { + protected Date getMostRecentCloseTime(String configuredTime, Date referenceDate) { + SimpleDateFormat format = new SimpleDateFormat(TIME_FORMAT); + format.setLenient(false); + ParsePosition position = new ParsePosition(0); + Date parsedTime = format.parse(configuredTime, position); + if (parsedTime == null || position.getIndex() != configuredTime.length()) { log.warn("Invalid value '{}' for global property {}, expected format {}", configuredTime, AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME, TIME_FORMAT); return null; } + Calendar parsed = Calendar.getInstance(); + parsed.setTime(parsedTime); + + Calendar closeTime = Calendar.getInstance(); + closeTime.setTime(referenceDate); + closeTime.set(Calendar.HOUR_OF_DAY, parsed.get(Calendar.HOUR_OF_DAY)); + closeTime.set(Calendar.MINUTE, parsed.get(Calendar.MINUTE)); + closeTime.set(Calendar.SECOND, 0); + closeTime.set(Calendar.MILLISECOND, 0); + if (closeTime.getTime().after(referenceDate)) { + closeTime.add(Calendar.DATE, -1); + } + return closeTime.getTime(); } /** diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java index 020d5740..3dafe164 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -10,9 +10,14 @@ package org.openmrs.module.queue.tasks; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.openmrs.module.queue.QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES; import java.text.DateFormat; import java.text.SimpleDateFormat; @@ -23,6 +28,8 @@ import org.junit.Before; import org.junit.Test; +import org.openmrs.api.AdministrationService; +import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; import org.openmrs.module.queue.model.Queue; import org.openmrs.module.queue.model.QueueEntry; @@ -57,7 +64,8 @@ protected Date now() { @Override protected List getQueueEntries(QueueEntrySearchCriteria criteria) { // Emulate the DB-level filtering that getQueueEntries would normally perform - return queueEntries.stream().filter(e -> e.getEndedAt() == null) + return queueEntries.stream() + .filter(e -> criteria.getIsEnded() == null || criteria.getIsEnded().equals(e.getEndedAt() != null)) .filter(e -> e.getStartedAt() == null || !e.getStartedAt().after(criteria.getStartedOnOrBefore())) .filter(e -> criteria.getQueues() == null || criteria.getQueues().contains(e.getQueue())) .collect(Collectors.toList()); @@ -94,6 +102,15 @@ public void shouldDoNothingWhenTimeIsUnparseable() throws Exception { assertThat(queueEntry.getEndedAt(), nullValue()); } + @Test + public void shouldDoNothingWhenTimeHasTrailingCharacters() throws Exception { + configuredTime = "11:00 PM"; + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), nullValue()); + } + @Test public void shouldNotClearBeforeConfiguredTime() throws Exception { configuredTime = "23:59"; @@ -113,6 +130,37 @@ public void shouldClearActiveEntriesAtOrAfterConfiguredTime() throws Exception { assertThat(queueEntry.getEndedAt(), equalTo(now)); } + @Test + public void shouldClearEntriesWhenTheConfiguredTimeWasMissed() throws Exception { + configuredTime = "23:59"; + now = getDate("2020-01-02 08:00"); + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), equalTo(now)); + } + + @Test + public void shouldNotClearEntriesStartedAfterTheMostRecentCloseTime() throws Exception { + configuredTime = "23:59"; + now = getDate("2020-01-02 08:00"); + QueueEntry queueEntry = queueEntryStartedAt("2020-01-02 07:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), nullValue()); + } + + @Test + public void shouldNotRewriteEntriesThatAreAlreadyEnded() throws Exception { + configuredTime = "23:59"; + Date alreadyEndedAt = getDate("2020-01-01 10:00"); + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + queueEntry.setEndedAt(alreadyEndedAt); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), equalTo(alreadyEndedAt)); + } + @Test public void shouldNotClearEntriesStartedAfterConfiguredTime() throws Exception { configuredTime = "18:00"; @@ -141,6 +189,58 @@ public void shouldOnlyClearConfiguredQueues() throws Exception { assertThat(inQueueB.getEndedAt(), nullValue()); } + @Test + public void getQueuesToClearShouldReturnNullWhenNoQueuesAreConfigured() { + assertThat(taskForConfiguredQueues(" ").getQueuesToClear(), nullValue()); + } + + @Test + public void getQueuesToClearShouldResolveConfiguredUuids() { + Queue queueA = new Queue(); + Queue queueB = new Queue(); + AutoCloseQueueEntryTask task = taskForConfiguredQueues(" uuid-a , ,uuid-b,"); + when(task.getServices().getQueue("uuid-a")).thenReturn(queueA); + when(task.getServices().getQueue("uuid-b")).thenReturn(queueB); + + assertThat(task.getQueuesToClear(), contains(queueA, queueB)); + } + + @Test + public void getQueuesToClearShouldSkipUnknownUuids() { + Queue queueA = new Queue(); + AutoCloseQueueEntryTask task = taskForConfiguredQueues("uuid-a,not-a-queue"); + when(task.getServices().getQueue("uuid-a")).thenReturn(queueA); + when(task.getServices().getQueue("not-a-queue")).thenThrow(new IllegalArgumentException()); + + assertThat(task.getQueuesToClear(), contains(queueA)); + } + + @Test + public void getQueuesToClearShouldReturnEmptyListWhenNoConfiguredUuidResolves() { + AutoCloseQueueEntryTask task = taskForConfiguredQueues("not-a-queue"); + when(task.getServices().getQueue("not-a-queue")).thenThrow(new IllegalArgumentException()); + + assertThat(task.getQueuesToClear(), empty()); + } + + /** + * @return a task whose services report the given value for the configured queues global property, + * so that the real {@link AutoCloseQueueEntryTask#getQueuesToClear()} is exercised + */ + private AutoCloseQueueEntryTask taskForConfiguredQueues(String configuredQueueUuids) { + QueueServicesWrapper services = mock(QueueServicesWrapper.class); + AdministrationService administrationService = mock(AdministrationService.class); + when(services.getAdministrationService()).thenReturn(administrationService); + when(administrationService.getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES)).thenReturn(configuredQueueUuids); + return new AutoCloseQueueEntryTask() { + + @Override + protected QueueServicesWrapper getServices() { + return services; + } + }; + } + private QueueEntry queueEntryStartedAt(String startedAt, Queue queue) throws Exception { QueueEntry queueEntry = new QueueEntry(); queueEntry.setStartedAt(getDate(startedAt)); diff --git a/omod/src/main/resources/config.xml b/omod/src/main/resources/config.xml index ad38ad60..c788a351 100644 --- a/omod/src/main/resources/config.xml +++ b/omod/src/main/resources/config.xml @@ -57,8 +57,8 @@ ${project.parent.artifactId}.autoCloseQueueEntriesAtTime - 23:59 - Time of day (HH:mm, server local time) at which active queue entries are automatically ended each day. Leave blank to disable automatic clearing. + + Time of day (HH:mm, server local time) at which active queue entries are automatically ended each day. Blank by default, which disables automatic clearing; an outpatient clinic that wants its queues cleared at end of day would set this to 23:59. ${project.parent.artifactId}.autoCloseQueueEntriesForQueues From 4ac5619b5b8d427232b4933783559f7df9a3652d Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Tue, 28 Jul 2026 14:02:29 +0800 Subject: [PATCH 4/9] O3-5765: Cover the evict-and-continue path --- .../queue/tasks/AutoCloseQueueEntryTask.java | 11 ++++- .../tasks/AutoCloseQueueEntryTaskTest.java | 46 ++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java index 44574821..451b4737 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -95,11 +95,11 @@ private void closeQueueEntry(QueueEntry queueEntry, Date endedAt) { log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); } catch (ValidationException ve) { - Context.evictFromSession(queueEntry); + evictFromSession(queueEntry); log.warn("Unable to auto-close queue entry {}: {}", queueEntry.getQueueEntryId(), ve.getMessage()); } catch (Exception e) { - Context.evictFromSession(queueEntry); + evictFromSession(queueEntry); log.warn("Unable to auto-close queue entry {}", queueEntry.getQueueEntryId(), e); } } @@ -183,6 +183,13 @@ protected void saveQueueEntry(QueueEntry queueEntry) { getServices().getQueueEntryService().saveQueueEntry(queueEntry); } + /** + * @param queueEntry the QueueEntry to evict from the current Hibernate session + */ + protected void evictFromSession(QueueEntry queueEntry) { + Context.evictFromSession(queueEntry); + } + /** * @return the current time; overridable to allow deterministic testing */ diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java index 3dafe164..b86ec684 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -28,7 +28,9 @@ import org.junit.Before; import org.junit.Test; +import org.openmrs.api.APIException; import org.openmrs.api.AdministrationService; +import org.openmrs.api.ValidationException; import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; import org.openmrs.module.queue.model.Queue; @@ -44,6 +46,12 @@ public class AutoCloseQueueEntryTaskTest { private Date now; + private final List evictedFromSession = new ArrayList<>(); + + private QueueEntry saveFailsFor; + + private RuntimeException saveFailure; + class TestAutoCloseQueueEntryTask extends AutoCloseQueueEntryTask { @Override @@ -73,14 +81,24 @@ protected List getQueueEntries(QueueEntrySearchCriteria criteria) { @Override protected void saveQueueEntry(QueueEntry queueEntry) { - // Do nothing + if (queueEntry == saveFailsFor) { + throw saveFailure; + } + } + + @Override + protected void evictFromSession(QueueEntry queueEntry) { + evictedFromSession.add(queueEntry); } } @Before public void setup() throws Exception { queueEntries.clear(); + evictedFromSession.clear(); configuredQueues = null; + saveFailsFor = null; + saveFailure = null; now = getDate("2020-01-01 23:59"); } @@ -189,6 +207,32 @@ public void shouldOnlyClearConfiguredQueues() throws Exception { assertThat(inQueueB.getEndedAt(), nullValue()); } + @Test + public void shouldEvictAndContinueWhenValidationRejectsAnEntry() throws Exception { + configuredTime = "23:59"; + QueueEntry rejected = queueEntryStartedAt("2020-01-01 09:00", null); + QueueEntry saved = queueEntryStartedAt("2020-01-01 10:00", null); + saveFailsFor = rejected; + saveFailure = new ValidationException("endedAt is after the visit stop date"); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(evictedFromSession, contains(rejected)); + assertThat(saved.getEndedAt(), equalTo(now)); + } + + @Test + public void shouldEvictAndContinueWhenSavingAnEntryFails() throws Exception { + configuredTime = "23:59"; + QueueEntry failed = queueEntryStartedAt("2020-01-01 09:00", null); + QueueEntry saved = queueEntryStartedAt("2020-01-01 10:00", null); + saveFailsFor = failed; + saveFailure = new APIException("could not save"); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(evictedFromSession, contains(failed)); + assertThat(saved.getEndedAt(), equalTo(now)); + } + @Test public void getQueuesToClearShouldReturnNullWhenNoQueuesAreConfigured() { assertThat(taskForConfiguredQueues(" ").getQueuesToClear(), nullValue()); From bac1bc21e9aaf94f50c7509ab0b008662b497443 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Tue, 28 Jul 2026 20:26:08 +0800 Subject: [PATCH 5/9] O3-5765: Record the close time as endedAt --- .../queue/tasks/AutoCloseQueueEntryTask.java | 9 ++++-- .../tasks/AutoCloseQueueEntryTaskTest.java | 28 ++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java index 451b4737..da8caa7e 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -77,7 +77,7 @@ public void run() { List queueEntries = getQueueEntries(criteria); log.debug("There are {} queue entries to auto-close", queueEntries.size()); for (QueueEntry queueEntry : queueEntries) { - closeQueueEntry(queueEntry, now); + closeQueueEntry(queueEntry, closeTime); } } catch (Exception e) { @@ -88,8 +88,13 @@ public void run() { } } - private void closeQueueEntry(QueueEntry queueEntry, Date endedAt) { + private void closeQueueEntry(QueueEntry queueEntry, Date closeTime) { try { + Date endedAt = closeTime; + Date startedAt = queueEntry.getStartedAt(); + if (startedAt != null && !endedAt.after(startedAt)) { + endedAt = new Date(startedAt.getTime() + 1000L); + } queueEntry.setEndedAt(endedAt); saveQueueEntry(queueEntry); log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java index b86ec684..8eb8c4a1 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -52,6 +52,8 @@ public class AutoCloseQueueEntryTaskTest { private RuntimeException saveFailure; + private RuntimeException getQueueEntriesFailure; + class TestAutoCloseQueueEntryTask extends AutoCloseQueueEntryTask { @Override @@ -71,6 +73,9 @@ protected Date now() { @Override protected List getQueueEntries(QueueEntrySearchCriteria criteria) { + if (getQueueEntriesFailure != null) { + throw getQueueEntriesFailure; + } // Emulate the DB-level filtering that getQueueEntries would normally perform return queueEntries.stream() .filter(e -> criteria.getIsEnded() == null || criteria.getIsEnded().equals(e.getEndedAt() != null)) @@ -99,6 +104,7 @@ public void setup() throws Exception { configuredQueues = null; saveFailsFor = null; saveFailure = null; + getQueueEntriesFailure = null; now = getDate("2020-01-01 23:59"); } @@ -155,7 +161,27 @@ public void shouldClearEntriesWhenTheConfiguredTimeWasMissed() throws Exception QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); new TestAutoCloseQueueEntryTask().run(); - assertThat(queueEntry.getEndedAt(), equalTo(now)); + assertThat(queueEntry.getEndedAt(), equalTo(getDate("2020-01-01 23:59"))); + } + + @Test + public void shouldEndEntriesStartedAtTheCloseTimeOneSecondLater() throws Exception { + configuredTime = "18:00"; + now = getDate("2020-01-01 18:30"); + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 18:00", null); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), equalTo(new Date(getDate("2020-01-01 18:00").getTime() + 1000L))); + } + + @Test + public void shouldNotPropagateWhenFetchingQueueEntriesFails() throws Exception { + configuredTime = "23:59"; + QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); + getQueueEntriesFailure = new APIException("could not query queue entries"); + + new TestAutoCloseQueueEntryTask().run(); + assertThat(queueEntry.getEndedAt(), nullValue()); } @Test From be54eb8ef2b95fa8eadb55ed355f812dd05740cc Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Sat, 1 Aug 2026 18:03:04 +0800 Subject: [PATCH 6/9] O3-5765: Schedule the queue tasks with core's scheduler Both tasks are now AbstractTasks registered as TaskDefinitions when the module starts, following the pattern the reference application and chartsearchai activators use. That retires QueueTaskExecutor and QueueTimerTask along with the daemon token plumbing, since core runs scheduled tasks as the daemon user itself, and replaces the two hand rolled re-entrancy flags with the isExecuting guard AbstractTask provides. Implementers get both tasks on the Manage Scheduler page, where the interval can be changed or a task stopped. Nothing here overrides that afterwards: the scheduler starts tasks with startOnStartup at server startup and restores them across a module being started or stopped, so registration only happens once. The definition is scheduled as it is created, which is the one case the scheduler does not cover, of this module being installed into a running server. Services stay behind Context lookups because a task built by TaskFactory is not a Spring bean, which is what core's own AutoCloseVisitsTask does. --- README.md | 3 + .../module/queue/QueueModuleActivator.java | 56 ++++++++++++--- .../queue/tasks/AutoCloseQueueEntryTask.java | 18 +++-- .../tasks/AutoCloseVisitQueueEntryTask.java | 13 ++-- .../module/queue/tasks/QueueTaskExecutor.java | 43 ----------- .../module/queue/tasks/QueueTimerTask.java | 62 ---------------- .../tasks/AutoCloseQueueEntryTaskTest.java | 33 ++++----- .../AutoCloseVisitQueueEntryTaskTest.java | 6 +- .../queue/QueueModuleActivatorTest.java | 71 +++++++++++++++++++ 9 files changed, 154 insertions(+), 151 deletions(-) delete mode 100644 api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java delete mode 100644 api/src/main/java/org/openmrs/module/queue/tasks/QueueTimerTask.java create mode 100644 integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java diff --git a/README.md b/README.md index 032b6b32..336d59fd 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,9 @@ that should survive overnight — tracking where inpatients currently are within queue of patients to follow up with over the coming week. Use `queue.autoCloseQueueEntriesForQueues` to enable clearing for some queues but not others. +The task is registered with the scheduler as `Queue Module - Auto Close Queue Entries` and appears on the Manage +Scheduler page, where its interval can be changed or the task stopped altogether. + The task works from the most recent occurrence of the configured time rather than from the moment it happens to run, so if it does not get a chance to run at that time (a restart or a maintenance window, say) the next run catches up instead of leaving the queues uncleared until the following day. One consequence: the first run after this property diff --git a/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java b/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java index 493b5777..380ae16a 100644 --- a/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java +++ b/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java @@ -9,27 +9,67 @@ */ package org.openmrs.module.queue; +import java.util.Date; + import lombok.extern.slf4j.Slf4j; +import org.openmrs.api.context.Context; import org.openmrs.module.BaseModuleActivator; -import org.openmrs.module.DaemonToken; -import org.openmrs.module.DaemonTokenAware; -import org.openmrs.module.queue.tasks.QueueTimerTask; +import org.openmrs.module.queue.tasks.AutoCloseQueueEntryTask; +import org.openmrs.module.queue.tasks.AutoCloseVisitQueueEntryTask; +import org.openmrs.scheduler.SchedulerService; +import org.openmrs.scheduler.Task; +import org.openmrs.scheduler.TaskDefinition; /** * This class contains the logic that is run every time this module is either started or shutdown */ @Slf4j -public class QueueModuleActivator extends BaseModuleActivator implements DaemonTokenAware { +public class QueueModuleActivator extends BaseModuleActivator { + + private static final String AUTO_CLOSE_VISIT_QUEUE_ENTRY_TASK = "Queue Module - Auto Close Visit Queue Entries"; + + private static final String AUTO_CLOSE_QUEUE_ENTRY_TASK = "Queue Module - Auto Close Queue Entries"; + + private static final long REPEAT_INTERVAL_SECONDS = 60L; @Override public void started() { super.started(); log.info("Queue Module Started"); - QueueTimerTask.setEnabled(true); + registerTask(AutoCloseVisitQueueEntryTask.class, AUTO_CLOSE_VISIT_QUEUE_ENTRY_TASK, + "Ends queue entries whose visit has been stopped"); + registerTask(AutoCloseQueueEntryTask.class, AUTO_CLOSE_QUEUE_ENTRY_TASK, + "Ends active queue entries at the time of day configured in " + + QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME); } - @Override - public void setDaemonToken(DaemonToken daemonToken) { - QueueTimerTask.setDaemonToken(daemonToken); + /** + * Creates the task definition the first time this module starts, and does nothing thereafter, so + * that an interval change or a stop made from the Manage Scheduler page is left alone. The + * scheduler starts the task at server startup and restores it across a module being started or + * stopped; starting it here covers only the case it cannot, of this module being installed into a + * running server. + */ + private void registerTask(Class taskClass, String name, String description) { + SchedulerService schedulerService = Context.getSchedulerService(); + if (schedulerService.getTaskByName(name) != null) { + log.debug("Scheduled task {} is registered already", name); + return; + } + TaskDefinition taskDefinition = new TaskDefinition(); + taskDefinition.setName(name); + taskDefinition.setDescription(description); + taskDefinition.setTaskClass(taskClass.getName()); + taskDefinition.setStartTime(new Date()); + taskDefinition.setRepeatInterval(REPEAT_INTERVAL_SECONDS); + taskDefinition.setStartOnStartup(true); + try { + schedulerService.saveTaskDefinition(taskDefinition); + schedulerService.scheduleIfNotRunning(taskDefinition); + log.info("Registered scheduled task {}", name); + } + catch (Exception e) { + log.error("Unable to register task {}", name, e); + } } } diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java index da8caa7e..bcf5a30c 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -18,7 +18,6 @@ import java.util.Calendar; import java.util.Date; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; @@ -28,6 +27,7 @@ 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.scheduler.tasks.AbstractTask; /** * This ends all active queue entries in the configured queues once per day at a configured time of @@ -37,19 +37,18 @@ * then are left in the queue and a run missed at the configured time is caught up by the next one. */ @Slf4j -public class AutoCloseQueueEntryTask implements Runnable { +public class AutoCloseQueueEntryTask extends AbstractTask { private static final String TIME_FORMAT = "HH:mm"; - private static final AtomicBoolean currentlyExecuting = new AtomicBoolean(false); - @Override - public void run() { - if (!currentlyExecuting.compareAndSet(false, true)) { + public void execute() { + if (isExecuting) { log.debug("AutoCloseQueueEntryTask is still executing, not running again"); return; } log.debug("Executing AutoCloseQueueEntryTask"); + startExecuting(); try { String configuredTime = getConfiguredCloseTime(); if (StringUtils.isBlank(configuredTime)) { @@ -84,7 +83,7 @@ public void run() { log.error("AutoCloseQueueEntryTask failed to complete", e); } finally { - currentlyExecuting.set(false); + stopExecuting(); } } @@ -144,15 +143,14 @@ protected Date getMostRecentCloseTime(String configuredTime, Date referenceDate) * auto-clearing is disabled */ protected String getConfiguredCloseTime() { - return getServices().getAdministrationService().getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME); + return getServices().getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME); } /** * @return the queues whose entries should be cleared, or null to clear entries in all queues */ protected List getQueuesToClear() { - String configuredQueues = getServices().getAdministrationService() - .getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES); + String configuredQueues = getServices().getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES); if (StringUtils.isBlank(configuredQueues)) { return null; } diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java index 2de1c446..c37e8727 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java @@ -19,6 +19,7 @@ import org.openmrs.module.queue.api.QueueEntryService; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; import org.openmrs.module.queue.model.QueueEntry; +import org.openmrs.scheduler.tasks.AbstractTask; /** * This iterates over all active VisitQueueEntries If the Visit associated with any of these has @@ -26,19 +27,17 @@ * datetime as the Visit was stopped. */ @Slf4j -public class AutoCloseVisitQueueEntryTask implements Runnable { - - private static volatile boolean currentlyExecuting = false; +public class AutoCloseVisitQueueEntryTask extends AbstractTask { @Override - public void run() { - if (currentlyExecuting) { + public void execute() { + if (isExecuting) { log.debug("AutoCloseVisitQueueEntryTask is still executing, not running again"); return; } log.debug("Executing AutoCloseVisitQueueEntryTask"); + startExecuting(); try { - currentlyExecuting = true; List queueEntries = getActiveVisitQueueEntries(); log.debug("There are {} active visit queue entries", queueEntries.size()); for (QueueEntry queueEntry : queueEntries) { @@ -62,7 +61,7 @@ public void run() { } } finally { - currentlyExecuting = false; + stopExecuting(); } } diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java b/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java deleted file mode 100644 index 7d995dba..00000000 --- a/api/src/main/java/org/openmrs/module/queue/tasks/QueueTaskExecutor.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * This Source Code Form is subject to the terms of the Mozilla Public License, - * v. 2.0. If a copy of the MPL was not distributed with this file, You can - * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under - * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. - * - * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS - * graphic logo is a trademark of OpenMRS Inc. - */ -package org.openmrs.module.queue.tasks; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.scheduling.concurrent.ScheduledExecutorFactoryBean; -import org.springframework.scheduling.concurrent.ScheduledExecutorTask; -import org.springframework.stereotype.Component; - -/** - * Executor that is responsible for scheduling and running the scheduled tasks - */ -@Component -public class QueueTaskExecutor extends ScheduledExecutorFactoryBean { - - private final Logger log = LoggerFactory.getLogger(getClass()); - - private static final long ONE_SECOND = 1000; - - private static final long ONE_MINUTE = ONE_SECOND * 60; - - public QueueTaskExecutor() { - setScheduledExecutorTasks(task(ONE_MINUTE, ONE_MINUTE, AutoCloseVisitQueueEntryTask.class), - task(ONE_MINUTE, ONE_MINUTE, AutoCloseQueueEntryTask.class)); - } - - private ScheduledExecutorTask task(long delay, long period, Class runnable) { - log.info("Scheduling task " + runnable.getSimpleName() + " with delay " + delay + " and period " + period); - ScheduledExecutorTask task = new ScheduledExecutorTask(); - task.setDelay(delay); - task.setPeriod(period); - task.setRunnable(new QueueTimerTask(runnable)); - return task; - } -} diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/QueueTimerTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/QueueTimerTask.java deleted file mode 100644 index 3325be6f..00000000 --- a/api/src/main/java/org/openmrs/module/queue/tasks/QueueTimerTask.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * This Source Code Form is subject to the terms of the Mozilla Public License, - * v. 2.0. If a copy of the MPL was not distributed with this file, You can - * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under - * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. - * - * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS - * graphic logo is a trademark of OpenMRS Inc. - */ -package org.openmrs.module.queue.tasks; - -import java.util.TimerTask; - -import org.openmrs.api.context.Daemon; -import org.openmrs.module.DaemonToken; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Timer task implementation that utilises a daemon thread to execute a runnable - */ -public class QueueTimerTask extends TimerTask { - - private final Logger log = LoggerFactory.getLogger(getClass()); - - private static DaemonToken daemonToken; - - private static boolean enabled = false; - - private final Class taskClass; - - public QueueTimerTask(Class taskClass) { - this.taskClass = taskClass; - } - - /** - * @see TimerTask#run() - */ - @Override - public final void run() { - if (daemonToken != null && enabled) { - try { - log.debug("Running task: {}", taskClass.getSimpleName()); - Runnable taskInstance = taskClass.getDeclaredConstructor().newInstance(); - Daemon.runInDaemonThreadWithoutResult(taskInstance, daemonToken); - } - catch (Exception e) { - log.error("An error occurred while running scheduled task {}", taskClass.getSimpleName(), e); - } - } else { - log.debug("Not running scheduled task. enabled = {}", enabled); - } - } - - public static void setEnabled(boolean enabled) { - QueueTimerTask.enabled = enabled; - } - - public static void setDaemonToken(DaemonToken daemonToken) { - QueueTimerTask.daemonToken = daemonToken; - } -} diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java index 8eb8c4a1..a3d0dfde 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -29,7 +29,6 @@ import org.junit.Before; import org.junit.Test; import org.openmrs.api.APIException; -import org.openmrs.api.AdministrationService; import org.openmrs.api.ValidationException; import org.openmrs.module.queue.api.QueueServicesWrapper; import org.openmrs.module.queue.api.search.QueueEntrySearchCriteria; @@ -113,7 +112,7 @@ public void shouldDoNothingWhenTimeIsBlank() throws Exception { configuredTime = ""; QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), nullValue()); } @@ -122,7 +121,7 @@ public void shouldDoNothingWhenTimeIsUnparseable() throws Exception { configuredTime = "nonsense"; QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), nullValue()); } @@ -131,7 +130,7 @@ public void shouldDoNothingWhenTimeHasTrailingCharacters() throws Exception { configuredTime = "11:00 PM"; QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), nullValue()); } @@ -141,7 +140,7 @@ public void shouldNotClearBeforeConfiguredTime() throws Exception { now = getDate("2020-01-01 17:00"); QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), nullValue()); } @@ -150,7 +149,7 @@ public void shouldClearActiveEntriesAtOrAfterConfiguredTime() throws Exception { configuredTime = "23:59"; QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), equalTo(now)); } @@ -160,7 +159,7 @@ public void shouldClearEntriesWhenTheConfiguredTimeWasMissed() throws Exception now = getDate("2020-01-02 08:00"); QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), equalTo(getDate("2020-01-01 23:59"))); } @@ -170,7 +169,7 @@ public void shouldEndEntriesStartedAtTheCloseTimeOneSecondLater() throws Excepti now = getDate("2020-01-01 18:30"); QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 18:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), equalTo(new Date(getDate("2020-01-01 18:00").getTime() + 1000L))); } @@ -180,7 +179,7 @@ public void shouldNotPropagateWhenFetchingQueueEntriesFails() throws Exception { QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); getQueueEntriesFailure = new APIException("could not query queue entries"); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), nullValue()); } @@ -190,7 +189,7 @@ public void shouldNotClearEntriesStartedAfterTheMostRecentCloseTime() throws Exc now = getDate("2020-01-02 08:00"); QueueEntry queueEntry = queueEntryStartedAt("2020-01-02 07:00", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), nullValue()); } @@ -201,7 +200,7 @@ public void shouldNotRewriteEntriesThatAreAlreadyEnded() throws Exception { QueueEntry queueEntry = queueEntryStartedAt("2020-01-01 09:00", null); queueEntry.setEndedAt(alreadyEndedAt); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(queueEntry.getEndedAt(), equalTo(alreadyEndedAt)); } @@ -212,7 +211,7 @@ public void shouldNotClearEntriesStartedAfterConfiguredTime() throws Exception { QueueEntry beforeCloseTime = queueEntryStartedAt("2020-01-01 09:00", null); QueueEntry afterCloseTime = queueEntryStartedAt("2020-01-01 18:15", null); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(beforeCloseTime.getEndedAt(), notNullValue()); assertThat(afterCloseTime.getEndedAt(), nullValue()); } @@ -228,7 +227,7 @@ public void shouldOnlyClearConfiguredQueues() throws Exception { QueueEntry inQueueA = queueEntryStartedAt("2020-01-01 09:00", queueA); QueueEntry inQueueB = queueEntryStartedAt("2020-01-01 09:00", queueB); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(inQueueA.getEndedAt(), notNullValue()); assertThat(inQueueB.getEndedAt(), nullValue()); } @@ -241,7 +240,7 @@ public void shouldEvictAndContinueWhenValidationRejectsAnEntry() throws Exceptio saveFailsFor = rejected; saveFailure = new ValidationException("endedAt is after the visit stop date"); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(evictedFromSession, contains(rejected)); assertThat(saved.getEndedAt(), equalTo(now)); } @@ -254,7 +253,7 @@ public void shouldEvictAndContinueWhenSavingAnEntryFails() throws Exception { saveFailsFor = failed; saveFailure = new APIException("could not save"); - new TestAutoCloseQueueEntryTask().run(); + new TestAutoCloseQueueEntryTask().execute(); assertThat(evictedFromSession, contains(failed)); assertThat(saved.getEndedAt(), equalTo(now)); } @@ -299,9 +298,7 @@ public void getQueuesToClearShouldReturnEmptyListWhenNoConfiguredUuidResolves() */ private AutoCloseQueueEntryTask taskForConfiguredQueues(String configuredQueueUuids) { QueueServicesWrapper services = mock(QueueServicesWrapper.class); - AdministrationService administrationService = mock(AdministrationService.class); - when(services.getAdministrationService()).thenReturn(administrationService); - when(administrationService.getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES)).thenReturn(configuredQueueUuids); + when(services.getGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_FOR_QUEUES)).thenReturn(configuredQueueUuids); return new AutoCloseQueueEntryTask() { @Override diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java index 22aeb283..98dcbb29 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java @@ -59,17 +59,17 @@ public void shouldAutoCloseVisitQueueEntriesIfVisitIsClosed() throws Exception { queueEntries.add(queueEntry2); TestAutoCloseVisitEntryTask task = new TestAutoCloseVisitEntryTask(); - task.run(); + task.execute(); assertThat(queueEntry1.getEndedAt(), nullValue()); assertThat(queueEntry2.getEndedAt(), nullValue()); visit1.setStopDatetime(getDate("2020-01-01 23:15")); - task.run(); + task.execute(); assertThat(queueEntry1.getEndedAt(), equalTo(visit1.getStopDatetime())); assertThat(queueEntry2.getEndedAt(), nullValue()); visit2.setStopDatetime(getDate("2021-01-05 11:30")); - task.run(); + task.execute(); assertThat(queueEntry1.getEndedAt(), equalTo(visit1.getStopDatetime())); assertThat(queueEntry2.getEndedAt(), equalTo(visit2.getStopDatetime())); } diff --git a/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java b/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java new file mode 100644 index 00000000..5fb7ca2a --- /dev/null +++ b/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java @@ -0,0 +1,71 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.module.queue; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import org.junit.Before; +import org.junit.Test; +import org.openmrs.api.context.Context; +import org.openmrs.module.queue.tasks.AutoCloseQueueEntryTask; +import org.openmrs.module.queue.tasks.AutoCloseVisitQueueEntryTask; +import org.openmrs.scheduler.SchedulerException; +import org.openmrs.scheduler.TaskDefinition; +import org.openmrs.test.BaseModuleContextSensitiveTest; +import org.springframework.test.context.ContextConfiguration; + +/** + * The scheduled tasks are registered at module startup and instantiated by the scheduler from their + * class name, which no other test covers. + */ +@ContextConfiguration(classes = SpringTestConfiguration.class, inheritLocations = false) +public class QueueModuleActivatorTest extends BaseModuleContextSensitiveTest { + + private static final String AUTO_CLOSE_VISIT_QUEUE_ENTRY_TASK = "Queue Module - Auto Close Visit Queue Entries"; + + private static final String AUTO_CLOSE_QUEUE_ENTRY_TASK = "Queue Module - Auto Close Queue Entries"; + + @Before + public void setup() { + new QueueModuleActivator().started(); + } + + @Test + public void shouldRegisterAndStartBothTasks() { + assertThat(taskDefinition(AUTO_CLOSE_VISIT_QUEUE_ENTRY_TASK).getTaskClass(), + equalTo(AutoCloseVisitQueueEntryTask.class.getName())); + assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getTaskClass(), + equalTo(AutoCloseQueueEntryTask.class.getName())); + assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getStarted(), equalTo(true)); + } + + @Test + public void shouldNotRegisterATaskThatIsRegisteredAlready() { + Integer id = taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getId(); + + new QueueModuleActivator().started(); + + assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getId(), equalTo(id)); + } + + @Test + public void shouldNotRestartATaskThatHasBeenStopped() throws SchedulerException { + Context.getSchedulerService().shutdownTask(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK)); + + new QueueModuleActivator().started(); + + assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getStarted(), equalTo(false)); + } + + private TaskDefinition taskDefinition(String name) { + return Context.getSchedulerService().getTaskByName(name); + } +} From 590b1a67fb25f4a35a979a4ceed27c741be15daa Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Sat, 1 Aug 2026 18:18:02 +0800 Subject: [PATCH 7/9] O3-5765: Run the auto-close task against the real services The unit tests stub the services the task talks through, so nothing checked that the search criteria filter the way they assume, that the save survives validation, or that endedAt reaches the database. This builds the task the way the scheduler does, from the class name on a TaskDefinition, and reads the entry back from the database rather than from the session the task used. --- ...utoCloseQueueEntryTaskIntegrationTest.java | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java diff --git a/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java b/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java new file mode 100644 index 00000000..b77b7693 --- /dev/null +++ b/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java @@ -0,0 +1,149 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.module.queue.tasks; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.openmrs.module.queue.QueueModuleConstants.AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME; + +import java.text.SimpleDateFormat; +import java.util.Arrays; +import java.util.Date; +import java.util.List; + +import org.apache.commons.lang3.time.DateUtils; +import org.junit.Before; +import org.junit.Test; +import org.openmrs.Concept; +import org.openmrs.Patient; +import org.openmrs.api.ConceptService; +import org.openmrs.api.PatientService; +import org.openmrs.api.context.Context; +import org.openmrs.module.queue.SpringTestConfiguration; +import org.openmrs.module.queue.api.QueueEntryService; +import org.openmrs.module.queue.api.QueueService; +import org.openmrs.module.queue.model.Queue; +import org.openmrs.module.queue.model.QueueEntry; +import org.openmrs.scheduler.Task; +import org.openmrs.scheduler.TaskDefinition; +import org.openmrs.scheduler.TaskFactory; +import org.openmrs.test.BaseModuleContextSensitiveTest; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.test.context.ContextConfiguration; + +/** + * Runs the task the way the scheduler does and checks the entry it ends reaches the database. The + * unit tests stub the services the task talks through, so this is what covers the search criteria + * filtering as they assume and the save surviving validation. + */ +@ContextConfiguration(classes = SpringTestConfiguration.class, inheritLocations = false) +public class AutoCloseQueueEntryTaskIntegrationTest extends BaseModuleContextSensitiveTest { + + private static final List INITIAL_DATASET_XML = Arrays.asList( + "org/openmrs/module/queue/api/dao/QueueDaoTest_locationInitialDataset.xml", + "org/openmrs/module/queue/api/dao/QueueEntryDaoTest_conceptsInitialDataset.xml", + "org/openmrs/module/queue/api/dao/QueueEntryDaoTest_patientInitialDataset.xml", + "org/openmrs/module/queue/api/dao/VisitQueueEntryDaoTest_visitInitialDataset.xml", + "org/openmrs/module/queue/api/dao/QueueDaoTest_initialDataset.xml", + "org/openmrs/module/queue/api/dao/QueueEntryDaoTest_initialDataset.xml", + "org/openmrs/module/queue/validators/QueueEntryValidatorTest_globalPropertyInitialDataset.xml"); + + private static final String PATIENT_UUID = "90b38324-e2fd-4feb-95b7-9e9a2a8876fg"; + + private static final String STATUS_CONCEPT_UUID = "56b910bd-298c-4ecf-a632-661ae2f7865y"; + + private static final String PRIORITY_CONCEPT_UUID = "90b910bd-298c-4ecf-a632-661ae2f446op"; + + private static final String TEST_QUEUE_UUID = "5ob8gj90-9090-4kbc-80dc-2e5d30252bb3"; + + @Autowired + @Qualifier("queue.QueueEntryService") + private QueueEntryService queueEntryService; + + @Autowired + @Qualifier("queue.QueueService") + private QueueService queueService; + + @Autowired + private PatientService patientService; + + @Autowired + private ConceptService conceptService; + + @Before + public void setup() { + INITIAL_DATASET_XML.forEach(this::executeDataSet); + } + + @Test + public void shouldEndAnActiveQueueEntryOnceTheCloseTimeHasPassed() throws Exception { + Integer queueEntryId = activeQueueEntryStartedHoursAgo(3); + setCloseTimeToHoursAgo(1); + + task().execute(); + + assertThat(reloadedEndedAt(queueEntryId), notNullValue()); + } + + @Test + public void shouldLeaveQueueEntriesAloneWhenNoCloseTimeIsConfigured() throws Exception { + Integer queueEntryId = activeQueueEntryStartedHoursAgo(3); + Context.getAdministrationService().setGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME, ""); + + task().execute(); + + assertThat(reloadedEndedAt(queueEntryId), nullValue()); + } + + /** + * @return the task built the way the scheduler builds it, from the class name on the definition + */ + private Task task() throws Exception { + TaskDefinition taskDefinition = new TaskDefinition(); + taskDefinition.setName("Auto Close Queue Entries"); + taskDefinition.setTaskClass(AutoCloseQueueEntryTask.class.getName()); + taskDefinition.setRepeatInterval(60L); + Task task = TaskFactory.getInstance().createInstance(taskDefinition); + task.initialize(taskDefinition); + return task; + } + + private void setCloseTimeToHoursAgo(int hours) { + Date closeTime = DateUtils.addHours(new Date(), -hours); + Context.getAdministrationService().setGlobalProperty(AUTO_CLOSE_QUEUE_ENTRIES_AT_TIME, + new SimpleDateFormat("HH:mm").format(closeTime)); + } + + private Integer activeQueueEntryStartedHoursAgo(int hours) { + Queue queue = queueService.getQueueByUuid(TEST_QUEUE_UUID).orElse(null); + Patient patient = patientService.getPatientByUuid(PATIENT_UUID); + Concept status = conceptService.getConceptByUuid(STATUS_CONCEPT_UUID); + Concept priority = conceptService.getConceptByUuid(PRIORITY_CONCEPT_UUID); + + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setQueue(queue); + queueEntry.setPatient(patient); + queueEntry.setStatus(status); + queueEntry.setPriority(priority); + queueEntry.setStartedAt(DateUtils.addHours(new Date(), -hours)); + return queueEntryService.saveQueueEntry(queueEntry).getQueueEntryId(); + } + + /** + * @return the endedAt read back from the database rather than from the session the task used + */ + private Date reloadedEndedAt(Integer queueEntryId) { + Context.flushSession(); + Context.clearSession(); + return queueEntryService.getQueueEntryById(queueEntryId).get().getEndedAt(); + } +} From 69f4917745b4731a0727edb3eaf1c7ca532cbc8e Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Sun, 23 Aug 2026 01:02:00 +0800 Subject: [PATCH 8/9] O3-5765: Evict rejected entries in the visit-close task --- .../tasks/AutoCloseVisitQueueEntryTask.java | 9 +++ .../AutoCloseVisitQueueEntryTaskTest.java | 62 ++++++++++++++++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java index c37e8727..9df022a2 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java @@ -53,9 +53,11 @@ public void execute() { } } catch (ValidationException ve) { + evictFromSession(queueEntry); log.warn("Unable to auto-close queue entry {}: {}", queueEntry.getQueueEntryId(), ve.getMessage()); } catch (Exception e) { + evictFromSession(queueEntry); log.warn("Unable to auto-close queue entry {}", queueEntry.getQueueEntryId(), e); } } @@ -81,4 +83,11 @@ protected List getActiveVisitQueueEntries() { protected void saveQueueEntry(QueueEntry queueEntry) { Context.getService(QueueEntryService.class).saveQueueEntry(queueEntry); } + + /** + * @param queueEntry the QueueEntry to evict from the current Hibernate session + */ + protected void evictFromSession(QueueEntry queueEntry) { + Context.evictFromSession(queueEntry); + } } diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java index 98dcbb29..eaecc4df 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java @@ -10,6 +10,7 @@ package org.openmrs.module.queue.tasks; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -20,14 +21,23 @@ import java.util.List; import java.util.stream.Collectors; +import org.junit.Before; import org.junit.Test; import org.openmrs.Visit; +import org.openmrs.api.APIException; +import org.openmrs.api.ValidationException; import org.openmrs.module.queue.model.QueueEntry; public class AutoCloseVisitQueueEntryTaskTest { final List queueEntries = new ArrayList<>(); + private final List evictedFromSession = new ArrayList<>(); + + private QueueEntry saveFailsFor; + + private RuntimeException saveFailure; + class TestAutoCloseVisitEntryTask extends AutoCloseVisitQueueEntryTask { @Override @@ -37,8 +47,23 @@ protected List getActiveVisitQueueEntries() { @Override protected void saveQueueEntry(QueueEntry queueEntry) { - // Do nothing + if (queueEntry == saveFailsFor) { + throw saveFailure; + } } + + @Override + protected void evictFromSession(QueueEntry queueEntry) { + evictedFromSession.add(queueEntry); + } + } + + @Before + public void setup() { + queueEntries.clear(); + evictedFromSession.clear(); + saveFailsFor = null; + saveFailure = null; } @Test @@ -74,6 +99,41 @@ public void shouldAutoCloseVisitQueueEntriesIfVisitIsClosed() throws Exception { assertThat(queueEntry2.getEndedAt(), equalTo(visit2.getStopDatetime())); } + @Test + public void shouldEvictAndContinueWhenValidationRejectsAnEntry() throws Exception { + QueueEntry rejected = closedVisitQueueEntry("2020-01-01 10:00", "2020-01-01 09:00"); + QueueEntry saved = closedVisitQueueEntry("2020-01-01 10:00", "2020-01-01 23:15"); + saveFailsFor = rejected; + saveFailure = new ValidationException("endedAt is before startedAt"); + + new TestAutoCloseVisitEntryTask().execute(); + assertThat(evictedFromSession, contains(rejected)); + assertThat(saved.getEndedAt(), equalTo(saved.getVisit().getStopDatetime())); + } + + @Test + public void shouldEvictAndContinueWhenSavingAnEntryFails() throws Exception { + QueueEntry failed = closedVisitQueueEntry("2020-01-01 10:00", "2020-01-01 23:15"); + QueueEntry saved = closedVisitQueueEntry("2020-01-01 10:00", "2020-01-01 23:15"); + saveFailsFor = failed; + saveFailure = new APIException("could not save"); + + new TestAutoCloseVisitEntryTask().execute(); + assertThat(evictedFromSession, contains(failed)); + assertThat(saved.getEndedAt(), equalTo(saved.getVisit().getStopDatetime())); + } + + QueueEntry closedVisitQueueEntry(String startedAt, String visitStopDatetime) throws Exception { + Visit visit = new Visit(); + visit.setStartDatetime(getDate("2020-01-01 09:00")); + visit.setStopDatetime(getDate(visitStopDatetime)); + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setStartedAt(getDate(startedAt)); + queueEntry.setVisit(visit); + queueEntries.add(queueEntry); + return queueEntry; + } + Date getDate(String dateStr) throws Exception { DateFormat df = new SimpleDateFormat("yyyy-MM-dd HH:mm"); return df.parse(dateStr); From 542910fbceab184006d295dbafe18fd28c4604b1 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Tue, 25 Aug 2026 03:10:20 +0800 Subject: [PATCH 9/9] O3-5765: Address review feedback on the scheduled clear Close queue entries through a new QueueEntryService.closeQueueEntry, which reloads the entry and writes it with optimistic locking, so an entry transitioned between the query and its turn in the loop is left alone instead of being overwritten from a stale snapshot. Both tasks use it, and the unreachable closeActiveQueueEntries is removed. Also flush and clear the session every 250 entries so the first sweep after a close time is configured stays linear, wrap the whole task registration so a lookup failure cannot skip the second task, and tighten a couple of log messages. --- README.md | 3 +- .../module/queue/QueueModuleActivator.java | 24 ++--- .../module/queue/api/QueueEntryService.java | 14 ++- .../queue/api/impl/QueueEntryServiceImpl.java | 34 +++++--- .../queue/tasks/AutoCloseQueueEntryTask.java | 41 +++++++-- .../tasks/AutoCloseVisitQueueEntryTask.java | 18 ++-- .../queue/api/QueueEntryServiceTest.java | 87 +++++++++++++++++++ .../tasks/AutoCloseQueueEntryTaskTest.java | 67 +++++++++++++- .../AutoCloseVisitQueueEntryTaskTest.java | 26 +++++- .../queue/QueueModuleActivatorTest.java | 2 + ...utoCloseQueueEntryTaskIntegrationTest.java | 2 +- 11 files changed, 276 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 336d59fd..2eb7832c 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,8 @@ queue of patients to follow up with over the coming week. Use `queue.autoCloseQu clearing for some queues but not others. The task is registered with the scheduler as `Queue Module - Auto Close Queue Entries` and appears on the Manage -Scheduler page, where its interval can be changed or the task stopped altogether. +Scheduler page, where its interval can be changed or the task stopped until the next restart. Blanking this +property is what turns the clear off for good. The task works from the most recent occurrence of the configured time rather than from the moment it happens to run, so if it does not get a chance to run at that time (a restart or a maintenance window, say) the next run catches up diff --git a/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java b/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java index 380ae16a..cc611c89 100644 --- a/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java +++ b/api/src/main/java/org/openmrs/module/queue/QueueModuleActivator.java @@ -51,19 +51,19 @@ public void started() { * running server. */ private void registerTask(Class taskClass, String name, String description) { - SchedulerService schedulerService = Context.getSchedulerService(); - if (schedulerService.getTaskByName(name) != null) { - log.debug("Scheduled task {} is registered already", name); - return; - } - TaskDefinition taskDefinition = new TaskDefinition(); - taskDefinition.setName(name); - taskDefinition.setDescription(description); - taskDefinition.setTaskClass(taskClass.getName()); - taskDefinition.setStartTime(new Date()); - taskDefinition.setRepeatInterval(REPEAT_INTERVAL_SECONDS); - taskDefinition.setStartOnStartup(true); try { + SchedulerService schedulerService = Context.getSchedulerService(); + if (schedulerService.getTaskByName(name) != null) { + log.debug("Scheduled task {} is registered already", name); + return; + } + TaskDefinition taskDefinition = new TaskDefinition(); + taskDefinition.setName(name); + taskDefinition.setDescription(description); + taskDefinition.setTaskClass(taskClass.getName()); + taskDefinition.setStartTime(new Date()); + taskDefinition.setRepeatInterval(REPEAT_INTERVAL_SECONDS); + taskDefinition.setStartOnStartup(true); schedulerService.saveTaskDefinition(taskDefinition); schedulerService.scheduleIfNotRunning(taskDefinition); log.info("Registered scheduled task {}", name); diff --git a/api/src/main/java/org/openmrs/module/queue/api/QueueEntryService.java b/api/src/main/java/org/openmrs/module/queue/api/QueueEntryService.java index b28a818e..988712db 100644 --- a/api/src/main/java/org/openmrs/module/queue/api/QueueEntryService.java +++ b/api/src/main/java/org/openmrs/module/queue/api/QueueEntryService.java @@ -147,10 +147,18 @@ String generateVisitQueueNumber(@NotNull Location location, @NotNull Queue queue @NotNull VisitAttributeType visitAttributeType); /** - * Closes all active queue entries + * Ends the given queue entry at the given time. The entry is reloaded before it is written, so a + * queue entry that has been ended or otherwise modified since it was loaded is left alone rather + * than having the stale state written back over it. This is intended for the scheduled tasks, which + * work from a list of entries loaded before the first of them is saved. + * + * @param queueEntry the queue entry to end + * @param endedAt the time at which to end it + * @return true if the queue entry was ended, false if it had already ended or was modified by + * another transaction */ - @Authorized(PrivilegeConstants.MANAGE_QUEUE_ENTRIES) - void closeActiveQueueEntries(); + @Authorized({ PrivilegeConstants.MANAGE_QUEUE_ENTRIES }) + boolean closeQueueEntry(@NotNull QueueEntry queueEntry, @NotNull Date endedAt); /** * @return the instance of SortWeightGenerator that is configured via global property, or null if diff --git a/api/src/main/java/org/openmrs/module/queue/api/impl/QueueEntryServiceImpl.java b/api/src/main/java/org/openmrs/module/queue/api/impl/QueueEntryServiceImpl.java index 15c1c479..5e1563e7 100644 --- a/api/src/main/java/org/openmrs/module/queue/api/impl/QueueEntryServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/queue/api/impl/QueueEntryServiceImpl.java @@ -250,12 +250,31 @@ public String generateVisitQueueNumber(Location location, Queue queue, Visit vis return queueNumber; } + /** + * @see QueueEntryService#closeQueueEntry(QueueEntry, Date) + */ @Override - public void closeActiveQueueEntries() { - QueueEntrySearchCriteria criteria = new QueueEntrySearchCriteria(); - criteria.setIsEnded(Boolean.FALSE); - List queueEntries = getQueueEntries(criteria); - queueEntries.forEach(this::endQueueEntry); + public boolean closeQueueEntry(@NotNull QueueEntry queueEntry, @NotNull Date endedAt) { + if (queueEntry.getId() == null) { + throw new IllegalArgumentException("Cannot close a queue entry that has not been saved"); + } + + // Reload from database to check current state and guard against concurrent modifications + QueueEntry currentState = dao.get(queueEntry.getId()).orElse(null); + if (currentState == null) { + log.debug("Queue entry {} no longer exists, not closing it", queueEntry.getId()); + return false; + } + if (currentState.getVoided() || currentState.getEndedAt() != null) { + log.debug("Queue entry {} is already voided or ended, not closing it", queueEntry.getId()); + return false; + } + + // Capture the dateChanged for optimistic locking + Date expectedDateChanged = currentState.getDateChanged(); + + currentState.setEndedAt(endedAt); + return dao.updateIfUnmodified(currentState, expectedDateChanged); } @Override @@ -279,11 +298,6 @@ protected QueueEntryService getProxiedQueueEntryService() { return Context.getService(QueueEntryService.class); } - private void endQueueEntry(@NotNull QueueEntry queueEntry) { - queueEntry.setEndedAt(new Date()); - dao.createOrUpdate(queueEntry); - } - private static Date roundToSecond(Date date) { if (date == null) { return null; diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java index bcf5a30c..ef739f0a 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTask.java @@ -41,6 +41,13 @@ public class AutoCloseQueueEntryTask extends AbstractTask { private static final String TIME_FORMAT = "HH:mm"; + /** + * The first run after an implementer configures a close time can find a very large number of + * never-ended entries, so the session is flushed and cleared periodically to keep it from growing + * over the whole sweep, which would make every save dirty-check every entry loaded before it. + */ + private static final int FLUSH_BATCH_SIZE = 250; + @Override public void execute() { if (isExecuting) { @@ -64,7 +71,7 @@ public void execute() { List queues = getQueuesToClear(); if (queues != null && queues.isEmpty()) { - log.debug("No queues configured for auto-close, nothing to do"); + log.debug("None of the queues configured for auto-close could be resolved, nothing to do"); return; } @@ -75,8 +82,12 @@ public void execute() { List queueEntries = getQueueEntries(criteria); log.debug("There are {} queue entries to auto-close", queueEntries.size()); + int processed = 0; for (QueueEntry queueEntry : queueEntries) { closeQueueEntry(queueEntry, closeTime); + if (++processed % FLUSH_BATCH_SIZE == 0) { + flushAndClearSession(); + } } } catch (Exception e) { @@ -92,11 +103,16 @@ private void closeQueueEntry(QueueEntry queueEntry, Date closeTime) { Date endedAt = closeTime; Date startedAt = queueEntry.getStartedAt(); if (startedAt != null && !endedAt.after(startedAt)) { + // startedOnOrBefore is inclusive, so an entry started exactly at the close time is swept + // too, and QueueEntryValidator requires endedAt to be strictly after startedAt endedAt = new Date(startedAt.getTime() + 1000L); } - queueEntry.setEndedAt(endedAt); - saveQueueEntry(queueEntry); - log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); + if (endQueueEntry(queueEntry, endedAt)) { + log.info("Queue entry auto-closed on schedule: {}", queueEntry.getQueueEntryId()); + } else { + log.debug("Queue entry {} was ended or modified since it was loaded, leaving it alone", + queueEntry.getQueueEntryId()); + } } catch (ValidationException ve) { evictFromSession(queueEntry); @@ -180,10 +196,13 @@ protected List getQueueEntries(QueueEntrySearchCriteria criteria) { } /** - * @param queueEntry the QueueEntry to save + * @param queueEntry the QueueEntry to end + * @param endedAt the time at which to end it + * @return true if the queue entry was ended, false if it was ended or otherwise modified since it + * was loaded */ - protected void saveQueueEntry(QueueEntry queueEntry) { - getServices().getQueueEntryService().saveQueueEntry(queueEntry); + protected boolean endQueueEntry(QueueEntry queueEntry, Date endedAt) { + return getServices().getQueueEntryService().closeQueueEntry(queueEntry, endedAt); } /** @@ -193,6 +212,14 @@ protected void evictFromSession(QueueEntry queueEntry) { Context.evictFromSession(queueEntry); } + /** + * Flushes and clears the Hibernate session of the thread running this task + */ + protected void flushAndClearSession() { + Context.flushSession(); + Context.clearSession(); + } + /** * @return the current time; overridable to allow deterministic testing */ diff --git a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java index 9df022a2..2d06f485 100644 --- a/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java +++ b/api/src/main/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTask.java @@ -47,9 +47,12 @@ public void execute() { if (visitStopDatetime != null) { log.debug("Visit {} is closed at {}", visit.getVisitId(), visitStopDatetime); log.debug("Auto closing queue entry {}", queueEntry.getQueueEntryId()); - queueEntry.setEndedAt(visitStopDatetime); - saveQueueEntry(queueEntry); - log.info("Queue entry auto-closed following close of visit: {}", queueEntry.getQueueEntryId()); + if (endQueueEntry(queueEntry, visitStopDatetime)) { + log.info("Queue entry auto-closed following close of visit: {}", queueEntry.getQueueEntryId()); + } else { + log.debug("Queue entry {} was ended or modified since it was loaded, leaving it alone", + queueEntry.getQueueEntryId()); + } } } catch (ValidationException ve) { @@ -78,10 +81,13 @@ protected List getActiveVisitQueueEntries() { } /** - * @param queueEntry the QueueEntry to save + * @param queueEntry the QueueEntry to end + * @param endedAt the time at which to end it + * @return true if the queue entry was ended, false if it was ended or otherwise modified since it + * was loaded */ - protected void saveQueueEntry(QueueEntry queueEntry) { - Context.getService(QueueEntryService.class).saveQueueEntry(queueEntry); + protected boolean endQueueEntry(QueueEntry queueEntry, Date endedAt) { + return Context.getService(QueueEntryService.class).closeQueueEntry(queueEntry, endedAt); } /** diff --git a/api/src/test/java/org/openmrs/module/queue/api/QueueEntryServiceTest.java b/api/src/test/java/org/openmrs/module/queue/api/QueueEntryServiceTest.java index ac356c2b..dc148f06 100644 --- a/api/src/test/java/org/openmrs/module/queue/api/QueueEntryServiceTest.java +++ b/api/src/test/java/org/openmrs/module/queue/api/QueueEntryServiceTest.java @@ -14,6 +14,7 @@ import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; import java.util.Arrays; @@ -532,6 +533,92 @@ public void transitionShouldBumpTransitionDateWhenSameSecondAsStartedAt() { assertThat(queueEntry.getEndedAt().after(queueEntry.getStartedAt()), is(true)); } + @Test + public void shouldCloseQueueEntryAtTheGivenTime() { + Date startedAt = DateUtils.addHours(DateUtils.truncate(new Date(), Calendar.SECOND), -3); + Date endedAt = DateUtils.addHours(startedAt, 1); + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setQueueEntryId(1); + queueEntry.setStartedAt(startedAt); + when(dao.get(1)).thenReturn(Optional.of(queueEntry)); + when(dao.updateIfUnmodified(any(), any())).thenReturn(true); + + assertThat(queueEntryService.closeQueueEntry(queueEntry, endedAt), is(true)); + assertThat(queueEntry.getEndedAt(), equalTo(endedAt)); + } + + @Test + public void shouldCloseQueueEntryThroughTheStateLoadedFromTheDatabase() { + Date startedAt = DateUtils.addHours(DateUtils.truncate(new Date(), Calendar.SECOND), -3); + Date endedAt = DateUtils.addHours(startedAt, 1); + // The state the task is holding, loaded before any of the batch was saved + QueueEntry staleState = new QueueEntry(); + staleState.setQueueEntryId(1); + staleState.setStartedAt(startedAt); + // The state currently in the database, which is what should be written back + QueueEntry currentState = new QueueEntry(); + currentState.setQueueEntryId(1); + currentState.setStartedAt(startedAt); + when(dao.get(1)).thenReturn(Optional.of(currentState)); + when(dao.updateIfUnmodified(any(), any())).thenReturn(true); + + assertThat(queueEntryService.closeQueueEntry(staleState, endedAt), is(true)); + verify(dao).updateIfUnmodified(eq(currentState), any()); + assertThat(currentState.getEndedAt(), equalTo(endedAt)); + assertNull(staleState.getEndedAt()); + } + + @Test + public void shouldNotCloseQueueEntryThatHasAlreadyEnded() { + Date alreadyEndedAt = DateUtils.addHours(new Date(), -1); + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setQueueEntryId(1); + queueEntry.setEndedAt(alreadyEndedAt); + when(dao.get(1)).thenReturn(Optional.of(queueEntry)); + + assertThat(queueEntryService.closeQueueEntry(queueEntry, new Date()), is(false)); + assertThat(queueEntry.getEndedAt(), equalTo(alreadyEndedAt)); + verify(dao, never()).updateIfUnmodified(any(), any()); + } + + @Test + public void shouldNotCloseVoidedQueueEntry() { + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setQueueEntryId(1); + queueEntry.setVoided(true); + when(dao.get(1)).thenReturn(Optional.of(queueEntry)); + + assertThat(queueEntryService.closeQueueEntry(queueEntry, new Date()), is(false)); + assertNull(queueEntry.getEndedAt()); + verify(dao, never()).updateIfUnmodified(any(), any()); + } + + @Test + public void shouldNotCloseQueueEntryThatNoLongerExists() { + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setQueueEntryId(1); + when(dao.get(1)).thenReturn(Optional.empty()); + + assertThat(queueEntryService.closeQueueEntry(queueEntry, new Date()), is(false)); + verify(dao, never()).updateIfUnmodified(any(), any()); + } + + @Test + public void shouldNotCloseQueueEntryThatWasModifiedByAnotherTransaction() { + QueueEntry queueEntry = new QueueEntry(); + queueEntry.setQueueEntryId(1); + queueEntry.setStartedAt(DateUtils.addHours(new Date(), -3)); + when(dao.get(1)).thenReturn(Optional.of(queueEntry)); + when(dao.updateIfUnmodified(any(), any())).thenReturn(false); + + assertThat(queueEntryService.closeQueueEntry(queueEntry, new Date()), is(false)); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldThrowWhenClosingUnsavedQueueEntry() { + queueEntryService.closeQueueEntry(new QueueEntry(), new Date()); + } + @Test public void shouldGenerateVisitQueueNumber() { Visit visit = new Visit(); diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java index a3d0dfde..370fb064 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskTest.java @@ -51,8 +51,12 @@ public class AutoCloseQueueEntryTaskTest { private RuntimeException saveFailure; + private QueueEntry modifiedSinceLoading; + private RuntimeException getQueueEntriesFailure; + private int sessionFlushes; + class TestAutoCloseQueueEntryTask extends AutoCloseQueueEntryTask { @Override @@ -83,17 +87,32 @@ protected List getQueueEntries(QueueEntrySearchCriteria criteria) { .collect(Collectors.toList()); } + /** + * Emulates + * {@link org.openmrs.module.queue.api.QueueEntryService#closeQueueEntry(QueueEntry, Date)}, which + * ends the entry unless it has been modified since it was loaded + */ @Override - protected void saveQueueEntry(QueueEntry queueEntry) { + protected boolean endQueueEntry(QueueEntry queueEntry, Date endedAt) { if (queueEntry == saveFailsFor) { throw saveFailure; } + if (queueEntry == modifiedSinceLoading) { + return false; + } + queueEntry.setEndedAt(endedAt); + return true; } @Override protected void evictFromSession(QueueEntry queueEntry) { evictedFromSession.add(queueEntry); } + + @Override + protected void flushAndClearSession() { + sessionFlushes++; + } } @Before @@ -103,7 +122,9 @@ public void setup() throws Exception { configuredQueues = null; saveFailsFor = null; saveFailure = null; + modifiedSinceLoading = null; getQueueEntriesFailure = null; + sessionFlushes = 0; now = getDate("2020-01-01 23:59"); } @@ -173,6 +194,50 @@ public void shouldEndEntriesStartedAtTheCloseTimeOneSecondLater() throws Excepti assertThat(queueEntry.getEndedAt(), equalTo(new Date(getDate("2020-01-01 18:00").getTime() + 1000L))); } + @Test + public void shouldClearEntriesOnALaterRunOfTheSameTaskInstance() throws Exception { + configuredTime = "23:59"; + TestAutoCloseQueueEntryTask task = new TestAutoCloseQueueEntryTask(); + task.execute(); + + QueueEntry queueEntry = queueEntryStartedAt("2020-01-02 09:00", null); + now = getDate("2020-01-02 23:59"); + task.execute(); + assertThat(queueEntry.getEndedAt(), equalTo(now)); + } + + @Test + public void shouldLeaveEntriesModifiedSinceTheyWereLoadedAlone() throws Exception { + configuredTime = "23:59"; + QueueEntry transitionedInTheMeantime = queueEntryStartedAt("2020-01-01 09:00", null); + QueueEntry stillActive = queueEntryStartedAt("2020-01-01 10:00", null); + modifiedSinceLoading = transitionedInTheMeantime; + + new TestAutoCloseQueueEntryTask().execute(); + assertThat(transitionedInTheMeantime.getEndedAt(), nullValue()); + assertThat(stillActive.getEndedAt(), equalTo(now)); + } + + @Test + public void shouldFlushTheSessionPeriodicallyWhileClearingALargeNumberOfEntries() throws Exception { + configuredTime = "23:59"; + for (int i = 0; i < 501; i++) { + queueEntryStartedAt("2020-01-01 09:00", null); + } + + new TestAutoCloseQueueEntryTask().execute(); + assertThat(sessionFlushes, equalTo(2)); + } + + @Test + public void shouldNotFlushTheSessionWhenClearingAHandfulOfEntries() throws Exception { + configuredTime = "23:59"; + queueEntryStartedAt("2020-01-01 09:00", null); + + new TestAutoCloseQueueEntryTask().execute(); + assertThat(sessionFlushes, equalTo(0)); + } + @Test public void shouldNotPropagateWhenFetchingQueueEntriesFails() throws Exception { configuredTime = "23:59"; diff --git a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java index eaecc4df..7a29ccae 100644 --- a/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java +++ b/api/src/test/java/org/openmrs/module/queue/tasks/AutoCloseVisitQueueEntryTaskTest.java @@ -38,6 +38,8 @@ public class AutoCloseVisitQueueEntryTaskTest { private RuntimeException saveFailure; + private QueueEntry modifiedSinceLoading; + class TestAutoCloseVisitEntryTask extends AutoCloseVisitQueueEntryTask { @Override @@ -45,11 +47,21 @@ protected List getActiveVisitQueueEntries() { return queueEntries.stream().filter(e -> e.getEndedAt() == null).collect(Collectors.toList()); } + /** + * Emulates + * {@link org.openmrs.module.queue.api.QueueEntryService#closeQueueEntry(QueueEntry, Date)}, which + * ends the entry unless it has been modified since it was loaded + */ @Override - protected void saveQueueEntry(QueueEntry queueEntry) { + protected boolean endQueueEntry(QueueEntry queueEntry, Date endedAt) { if (queueEntry == saveFailsFor) { throw saveFailure; } + if (queueEntry == modifiedSinceLoading) { + return false; + } + queueEntry.setEndedAt(endedAt); + return true; } @Override @@ -64,6 +76,18 @@ public void setup() { evictedFromSession.clear(); saveFailsFor = null; saveFailure = null; + modifiedSinceLoading = null; + } + + @Test + public void shouldLeaveEntriesModifiedSinceTheyWereLoadedAlone() throws Exception { + QueueEntry transitionedInTheMeantime = closedVisitQueueEntry("2020-01-01 10:00", "2020-01-01 23:15"); + QueueEntry stillActive = closedVisitQueueEntry("2020-01-01 10:00", "2020-01-01 23:15"); + modifiedSinceLoading = transitionedInTheMeantime; + + new TestAutoCloseVisitEntryTask().execute(); + assertThat(transitionedInTheMeantime.getEndedAt(), nullValue()); + assertThat(stillActive.getEndedAt(), equalTo(stillActive.getVisit().getStopDatetime())); } @Test diff --git a/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java b/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java index 5fb7ca2a..73ffbac1 100644 --- a/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java +++ b/integration-tests/src/test/java/org/openmrs/module/queue/QueueModuleActivatorTest.java @@ -45,6 +45,8 @@ public void shouldRegisterAndStartBothTasks() { assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getTaskClass(), equalTo(AutoCloseQueueEntryTask.class.getName())); assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getStarted(), equalTo(true)); + assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getRepeatInterval(), equalTo(60L)); + assertThat(taskDefinition(AUTO_CLOSE_QUEUE_ENTRY_TASK).getStartOnStartup(), equalTo(true)); } @Test diff --git a/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java b/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java index b77b7693..c2945c29 100644 --- a/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java +++ b/integration-tests/src/test/java/org/openmrs/module/queue/tasks/AutoCloseQueueEntryTaskIntegrationTest.java @@ -43,7 +43,7 @@ /** * Runs the task the way the scheduler does and checks the entry it ends reaches the database. The * unit tests stub the services the task talks through, so this is what covers the search criteria - * filtering as they assume and the save surviving validation. + * filtering as they assume and the write reaching the row. */ @ContextConfiguration(classes = SpringTestConfiguration.class, inheritLocations = false) public class AutoCloseQueueEntryTaskIntegrationTest extends BaseModuleContextSensitiveTest {