From bdea9cc3174c2e0b30f0a91bf5e51ecfc892e6a0 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Mon, 11 Apr 2022 16:24:50 +0200 Subject: [PATCH] Add ability to name timers / scheduled jobs Signed-off-by: Jan N. Klug --- .../model/script/actions/ScriptExecution.java | 35 ++++++++++++++-- .../script/internal/actions/TimerImpl.java | 8 +++- bundles/org.openhab.core/pom.xml | 9 +++++ .../scheduler/DelegatedSchedulerImpl.java | 7 ++++ .../internal/scheduler/SchedulerImpl.java | 40 ++++++++++++------- .../org/openhab/core/scheduler/Scheduler.java | 13 ++++++ .../internal/scheduler/SchedulerImplTest.java | 29 ++++++++++++-- 7 files changed, 119 insertions(+), 22 deletions(-) diff --git a/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions/ScriptExecution.java b/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions/ScriptExecution.java index 3a7d6e4a0a3..be837c5dcb8 100644 --- a/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions/ScriptExecution.java +++ b/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions/ScriptExecution.java @@ -14,6 +14,7 @@ import java.time.ZonedDateTime; +import org.eclipse.jdt.annotation.Nullable; import org.eclipse.xtext.xbase.XExpression; import org.eclipse.xtext.xbase.lib.Procedures.Procedure0; import org.eclipse.xtext.xbase.lib.Procedures.Procedure1; @@ -76,11 +77,25 @@ public static Object callScript(String scriptName) throws ScriptExecutionExcepti * @throws ScriptExecutionException if an error occurs during the execution */ public static Timer createTimer(ZonedDateTime instant, Procedure0 closure) { + return createTimer(null, instant, closure); + } + + /** + * Schedules a block of code for later execution. + * + * @param identifier an optional identifier + * @param instant the point in time when the code should be executed + * @param closure the code block to execute + * + * @return a handle to the created timer, so that it can be canceled or rescheduled + * @throws ScriptExecutionException if an error occurs during the execution + */ + public static Timer createTimer(@Nullable String identifier, ZonedDateTime instant, Procedure0 closure) { Scheduler scheduler = ScriptServiceUtil.getScheduler(); return new TimerImpl(scheduler, instant, () -> { closure.apply(); - }); + }, identifier); } /** @@ -94,11 +109,25 @@ public static Timer createTimer(ZonedDateTime instant, Procedure0 closure) { * @throws ScriptExecutionException if an error occurs during the execution */ public static Timer createTimerWithArgument(ZonedDateTime instant, Object arg1, Procedure1 closure) { + return createTimerWithArgument(null, instant, arg1, closure); + } + + /** + * Schedules a block of code (with argument) for later execution + * + * @param identifier an optional identifier + * @param instant the point in time when the code should be executed + * @param arg1 the argument to pass to the code block + * @param closure the code block to execute + * + * @return a handle to the created timer, so that it can be canceled or rescheduled + * @throws ScriptExecutionException if an error occurs during the execution + */ + public static Timer createTimerWithArgument(@Nullable String identifier, ZonedDateTime instant, Object arg1, Procedure1 closure) { Scheduler scheduler = ScriptServiceUtil.getScheduler(); return new TimerImpl(scheduler, instant, () -> { closure.apply(arg1); - }); + }, identifier); } - } diff --git a/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/internal/actions/TimerImpl.java b/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/internal/actions/TimerImpl.java index 6fd6092bf90..f6910ba0fea 100644 --- a/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/internal/actions/TimerImpl.java +++ b/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/internal/actions/TimerImpl.java @@ -33,14 +33,20 @@ public class TimerImpl implements Timer { private final Scheduler scheduler; private final ZonedDateTime startTime; private final SchedulerRunnable runnable; + private final @Nullable String identifier; private ScheduledCompletableFuture future; public TimerImpl(Scheduler scheduler, ZonedDateTime startTime, SchedulerRunnable runnable) { + this(scheduler, startTime, runnable, null); + } + + public TimerImpl(Scheduler scheduler, ZonedDateTime startTime, SchedulerRunnable runnable, @Nullable String identifier) { this.scheduler = scheduler; this.startTime = startTime; this.runnable = runnable; + this.identifier = identifier; - future = scheduler.schedule(runnable, startTime.toInstant()); + future = scheduler.schedule(runnable, identifier, startTime.toInstant()); } @Override diff --git a/bundles/org.openhab.core/pom.xml b/bundles/org.openhab.core/pom.xml index 278650ae977..810e4b6aa1f 100644 --- a/bundles/org.openhab.core/pom.xml +++ b/bundles/org.openhab.core/pom.xml @@ -14,6 +14,15 @@ openHAB Core :: Bundles :: Core + + + ch.qos.logback + logback-classic + 1.2.3 + test + + + diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/DelegatedSchedulerImpl.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/DelegatedSchedulerImpl.java index 765c5c59789..798a7f4cf5f 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/DelegatedSchedulerImpl.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/DelegatedSchedulerImpl.java @@ -22,6 +22,7 @@ import java.util.concurrent.CompletableFuture; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.scheduler.ScheduledCompletableFuture; import org.openhab.core.scheduler.Scheduler; import org.openhab.core.scheduler.SchedulerRunnable; @@ -96,6 +97,12 @@ public ScheduledCompletableFuture schedule(SchedulerRunnable runnable, Te return add(delegate.schedule(runnable, temporalAdjuster)); } + @Override + public ScheduledCompletableFuture schedule(SchedulerRunnable runnable, @Nullable String identifier, + TemporalAdjuster temporalAdjuster) { + return add(delegate.schedule(runnable, identifier, temporalAdjuster)); + } + private ScheduledCompletableFuture add(ScheduledCompletableFuture t) { synchronized (scheduledJobs) { scheduledJobs.add(t); diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/SchedulerImpl.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/SchedulerImpl.java index 34065858430..2f15fc80a9a 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/SchedulerImpl.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/scheduler/SchedulerImpl.java @@ -19,6 +19,7 @@ import java.time.temporal.ChronoUnit; import java.time.temporal.Temporal; import java.time.temporal.TemporalAdjuster; +import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; @@ -67,7 +68,7 @@ public ScheduledCompletableFuture after(Duration duration) { @Override public ScheduledCompletableFuture after(Callable callable, Duration duration) { - return afterInternal(new ScheduledCompletableFutureOnce<>(duration), callable); + return afterInternal(new ScheduledCompletableFutureOnce<>(null, duration), callable); } private ScheduledCompletableFutureOnce afterInternal(ScheduledCompletableFutureOnce deferred, @@ -89,7 +90,8 @@ private ScheduledCompletableFutureOnce afterInternal(ScheduledCompletable } catch (InterruptedException e) { Thread.currentThread().interrupt(); } catch (Exception e) { - logger.warn("Scheduled job failed and stopped", e); + logger.warn("Scheduled job '{}' failed and stopped", + Objects.requireNonNullElse(deferred.identifier, ""), e); deferred.completeExceptionally(e); } }, duration, TimeUnit.MILLISECONDS); @@ -114,7 +116,7 @@ public ScheduledCompletableFuture before(CompletableFuture promise, Du runnable.run(); } }; - final ScheduledCompletableFutureOnce wrappedPromise = new ScheduledCompletableFutureOnce<>(timeout); + final ScheduledCompletableFutureOnce wrappedPromise = new ScheduledCompletableFutureOnce<>(null, timeout); Callable callable = () -> { wrappedPromise.completeExceptionally(new TimeoutException()); return null; @@ -144,7 +146,7 @@ public ScheduledCompletableFuture at(Instant instant) { @Override public ScheduledCompletableFuture at(Callable callable, Instant instant) { return atInternal( - new ScheduledCompletableFutureOnce<>(ZonedDateTime.ofInstant(instant, ZoneId.systemDefault())), + new ScheduledCompletableFutureOnce<>(null, ZonedDateTime.ofInstant(instant, ZoneId.systemDefault())), callable); } @@ -155,17 +157,22 @@ private ScheduledCompletableFuture atInternal(ScheduledCompletableFutureO @Override public ScheduledCompletableFuture schedule(SchedulerRunnable runnable, TemporalAdjuster temporalAdjuster) { - final ScheduledCompletableFutureRecurring schedule = new ScheduledCompletableFutureRecurring<>( - ZonedDateTime.now()); + return schedule(runnable, null, temporalAdjuster); + } - schedule(schedule, runnable, temporalAdjuster); + @Override + public ScheduledCompletableFuture schedule(SchedulerRunnable runnable, @Nullable String identifier, + TemporalAdjuster temporalAdjuster) { + final ScheduledCompletableFutureRecurring schedule = new ScheduledCompletableFutureRecurring<>(identifier, + ZonedDateTime.now()); + schedule(schedule, runnable, identifier, temporalAdjuster); return schedule; } private void schedule(ScheduledCompletableFutureRecurring recurringSchedule, SchedulerRunnable runnable, - TemporalAdjuster temporalAdjuster) { + @Nullable String identifier, TemporalAdjuster temporalAdjuster) { final Temporal newTime = recurringSchedule.getScheduledTime().with(temporalAdjuster); - final ScheduledCompletableFutureOnce deferred = new ScheduledCompletableFutureOnce<>( + final ScheduledCompletableFutureOnce deferred = new ScheduledCompletableFutureOnce<>(identifier, ZonedDateTime.from(newTime)); deferred.thenAccept(v -> { @@ -173,7 +180,7 @@ private void schedule(ScheduledCompletableFutureRecurring recurringSchedu final SchedulerTemporalAdjuster schedulerTemporalAdjuster = (SchedulerTemporalAdjuster) temporalAdjuster; if (!schedulerTemporalAdjuster.isDone(newTime)) { - schedule(recurringSchedule, runnable, temporalAdjuster); + schedule(recurringSchedule, runnable, identifier, temporalAdjuster); return; } } @@ -195,9 +202,10 @@ private void schedule(ScheduledCompletableFutureRecurring recurringSchedu */ private static class ScheduledCompletableFutureRecurring extends ScheduledCompletableFutureOnce { private @Nullable volatile ScheduledCompletableFuture scheduledPromise; + private @Nullable String identifier; - public ScheduledCompletableFutureRecurring(ZonedDateTime scheduledTime) { - super(scheduledTime); + public ScheduledCompletableFutureRecurring(@Nullable String identifier, ZonedDateTime scheduledTime) { + super(identifier, scheduledTime); exceptionally(e -> { synchronized (this) { if (e instanceof CancellationException) { @@ -245,12 +253,14 @@ public ZonedDateTime getScheduledTime() { private static class ScheduledCompletableFutureOnce extends CompletableFuture implements ScheduledCompletableFuture { private ZonedDateTime scheduledTime; + private @Nullable String identifier; - public ScheduledCompletableFutureOnce(Duration duration) { - this(ZonedDateTime.now().plusNanos(duration.toNanos())); + public ScheduledCompletableFutureOnce(@Nullable String identifier, Duration duration) { + this(identifier, ZonedDateTime.now().plusNanos(duration.toNanos())); } - public ScheduledCompletableFutureOnce(ZonedDateTime scheduledTime) { + public ScheduledCompletableFutureOnce(@Nullable String identifier, ZonedDateTime scheduledTime) { + this.identifier = identifier; this.scheduledTime = scheduledTime; } diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/Scheduler.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/Scheduler.java index 5eb4d3d9edc..d4ad91f2b2e 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/Scheduler.java +++ b/bundles/org.openhab.core/src/main/java/org/openhab/core/scheduler/Scheduler.java @@ -114,4 +114,17 @@ public interface Scheduler { * @return A {@link ScheduledCompletableFuture} */ ScheduledCompletableFuture schedule(SchedulerRunnable callable, TemporalAdjuster temporalAdjuster); + + /** + * Schedules the callable once or repeating using the temporalAdjuster to determine the + * time the callable should run. Runs until the job is cancelled or if the temporalAdjuster + * method {@link SchedulerTemporalAdjuster#isDone()) returns true. + * + * @param callable Provides the result + * @param an optional identifier for this job + * @param temporalAdjuster the temperalAdjuster to return the time the callable should run + * @return A {@link ScheduledCompletableFuture} + */ + ScheduledCompletableFuture schedule(SchedulerRunnable callable, @Nullable String identifier, + TemporalAdjuster temporalAdjuster); } diff --git a/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/SchedulerImplTest.java b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/SchedulerImplTest.java index 71081de36bd..74cc9ceae2c 100644 --- a/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/SchedulerImplTest.java +++ b/bundles/org.openhab.core/src/test/java/org/openhab/core/internal/scheduler/SchedulerImplTest.java @@ -42,6 +42,12 @@ import org.openhab.core.scheduler.ScheduledCompletableFuture; import org.openhab.core.scheduler.SchedulerRunnable; import org.openhab.core.scheduler.SchedulerTemporalAdjuster; +import org.slf4j.LoggerFactory; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; /** * Test class for {@link SchedulerImpl}. @@ -222,24 +228,41 @@ public void testScheduleCancel() throws InterruptedException { @Test @Timeout(value = 15, unit = TimeUnit.SECONDS) public void testScheduleException() throws InterruptedException { + // add logging interceptor + Logger logger = (Logger) LoggerFactory.getLogger(SchedulerImpl.class); + logger.setLevel(Level.DEBUG); + ListAppender listAppender = new ListAppender<>(); + listAppender.start(); + logger.addAppender(listAppender); + Semaphore s = new Semaphore(0); TestSchedulerWithCounter temporalAdjuster = new TestSchedulerWithCounter(); SchedulerRunnable runnable = () -> { - // Pass a exception not very likely thrown by the scheduler it self to avoid missing real exceptions. + // Pass an exception not very likely thrown by the scheduler itself to avoid missing real exceptions. throw new FileNotFoundException("testBeforeTimeoutException"); }; - ScheduledCompletableFuture<@Nullable Void> schedule = scheduler.schedule(runnable, temporalAdjuster); + ScheduledCompletableFuture<@Nullable Void> schedule = scheduler.schedule(runnable, "myScheduledJob", + temporalAdjuster); schedule.getPromise().exceptionally(e -> { if (e instanceof FileNotFoundException) { s.release(); } return null; }); + s.acquire(1); - Thread.sleep(1000); // wait a little longer to see if not more are scheduled. + Thread.sleep(300); // wait a little longer to see if not more are scheduled. + + logger.detachAppender(listAppender); + assertEquals(0, s.availablePermits(), "Scheduler should not have released more after cancel"); assertEquals(0, temporalAdjuster.getCount(), "Scheduler should have run 0 time"); + + assertEquals(1, listAppender.list.size()); + ILoggingEvent loggingEvent = listAppender.list.get(0); + assertEquals(Level.WARN, loggingEvent.getLevel()); + assertEquals("Scheduled job 'myScheduledJob' failed and stopped", loggingEvent.getFormattedMessage()); } @Test