[jsscripting] Reimplement timer creation method of `ScriptExecution` (#13695)

* [jsscripting] Refactor ThreadsafeTimers to create futures inline instead of in an extra methods
* [jsscripting] Introduce utility class for providing easy access to script services
* [jsscripting] Reimplement timer creation methods from ScriptExecution for thread-safety
* [jsscripting] Add missing JavaDoc for reimplement timer creation methods
* [jsscripting] Remove the future from the map when setTimeout expires
* [jsscripting] Rename `GraalJSScriptServiceUtil` to `JSScriptServiceUtil`
* [jsscripting] Remove the `createTimerWithArgument` method
* [jsscripting] Replace the OSGi workaround of `JSScriptServiceUtil` with an injection mechanism
* [jsscripting] Use constructor to inject `JSScriptServiceUtil` into `GraalJSScriptEngineFactory`
* [jsscripting] Minor improvements by @J-N-K (#1)
* [jsscripting] Minor changes related to last commit to keep flexibility of `JSRuntimeFeatures`
* [jsscripting] Upgrade openhab-js to v2.1.1
* [jsscripting] Remove unused code

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Co-authored-by: Jan N. Klug <github@klug.nrw>

bundles/org.openhab.automation.jsscripting/README.md

@@ -172,7 +172,7 @@ When a script is unloaded, all created timers and intervals are automatically ca
 
 #### SetTimeout
 
-The global [`setTimeout()`](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout) method sets a timer which executes a function or specified piece of code once the timer expires.
+The global [`setTimeout()`](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout) method sets a timer which executes a function once the timer expires.
 `setTimeout()` returns a `timeoutId` (a positive integer value) which identifies the timer created.
 
 ```javascript
@@ -185,7 +185,7 @@ The global [`clearTimeout(timeoutId)`](https://developer.mozilla.org/en-US/docs/
 
 #### SetInterval
 
-The global [`setInterval()`](https://developer.mozilla.org/en-US/docs/Web/API/setInterval) method repeatedly calls a function or executes a code snippet, with a fixed time delay between each call.
+The global [`setInterval()`](https://developer.mozilla.org/en-US/docs/Web/API/setInterval) method repeatedly calls a function, with a fixed time delay between each call.
 `setInterval()` returns an `intervalId` (a positive integer value) which identifies the interval created.
 
 ```javascript
@@ -510,13 +510,57 @@ Replace `<url>` with the request url.
 
 #### ScriptExecution Actions
 
-The `ScriptExecution` actions provide the `callScript(string scriptName)` method, which calls a script located at the `$OH_CONF/scripts` folder.
+The `ScriptExecution` actions provide the `callScript(string scriptName)` method, which calls a script located at the `$OH_CONF/scripts` folder, as well as the `createTimer` method.
 
-Please note that `actions.ScriptExecution` also provides access to methods for creating timers, but it is NOT recommended to create timers using that raw Java API!
+You can also create timers using the [native JS methods for timer creation](#timers), your choice depends on the versatility you need.
-Usage of those timer creation methods can lead to failing timers.
+Sometimes, using `setTimer` is much faster and easier, but other times, you need the versatility that `createTimer` provides.
-Instead of those, use the [native JS methods for timer creation](#timers).
 
-See [openhab-js : actions.ScriptExecution](https://openhab.github.io/openhab-js/actions.html#.ScriptExecution) for complete documentation.
+##### `createTimer`
+
+```javascript
+actions.ScriptExecution.createTimer(time.ZonedDateTime instant, function callback);
+
+actions.ScriptExecution.createTimer(string identifier, time.ZonedDateTime instant, function callback);
+```
+
+`createTimer` accepts the following arguments:
+
+- `string` identifier (optional): Identifies the timer by a string, used e.g. for logging errors that occur during the callback execution.
+- [`time.ZonedDateTime`](#timetozdt) instant: Point in time when the callback should be executed.
+- `function` callback: Callback function to execute when the timer expires.
+
+`createTimer` returns an openHAB Timer, that provides the following methods:
+
+- `cancel()`: Cancels the timer. ⇒ `boolean`: true, if cancellation was successful
+- `getExecutionTime()`: The scheduled execution time or null if timer was cancelled. ⇒ `time.ZonedDateTime` or `null`
+- `isActive()`: Whether the scheduled execution is yet to happen. ⇒ `boolean`
+- `isCancelled()`: Whether the timer has been cancelled. ⇒ `boolean`
+- `hasTerminated()`: Whether the scheduled execution has already terminated. ⇒ `boolean`
+- `reschedule(time.ZonedDateTime)`: Reschedules a timer to a new starting time. This can also be called after a timer has terminated, which will result in another execution of the same code. ⇒ `boolean`: true, if rescheduling was successful
+
+
+```javascript
+var now = time.ZonedDateTime.now();
+
+// Function to run when the timer goes off.
+function timerOver () {
+  console.info('The timer expired.');
+}
+
+// Create the Timer.
+var myTimer = actions.ScriptExecution.createTimer('My Timer', now.plusSeconds(10), timerOver);
+
+// Cancel the timer.
+myTimer.cancel();
+
+// Check whether the timer is active. Returns true if the timer is active and will be executed as scheduled.
+var active = myTimer.isActive();
+
+// Reschedule the timer.
+myTimer.reschedule(now.plusSeconds(5));
+```
+
+See [openhab-js : actions.ScriptExecution](https://openhab.github.io/openhab-js/actions.ScriptExecution.html) for complete documentation.
 
 #### Semantics Actions
 

bundles/org.openhab.automation.jsscripting/pom.xml

@@ -25,7 +25,7 @@
     <graal.version>22.0.0.2</graal.version> <!-- DO NOT UPGRADE: 22.0.0.2 is the latest version working on armv7l / OpenJDK 11.0.16 -->
     <asm.version>6.2.1</asm.version>
     <oh.version>${project.version}</oh.version>
-    <ohjs.version>openhab@2.1.0</ohjs.version>
+    <ohjs.version>openhab@2.1.1</ohjs.version>
   </properties>
 
   <build>

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java

@@ -21,11 +21,11 @@ import javax.script.ScriptEngine;
 
 import org.openhab.core.automation.module.script.ScriptEngineFactory;
 import org.openhab.core.config.core.ConfigurableService;
-import org.osgi.framework.BundleContext;
 import org.osgi.framework.Constants;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Modified;
+import org.osgi.service.component.annotations.Reference;
 
 /**
  * An implementation of {@link ScriptEngineFactory} with customizations for GraalJS ScriptEngines.
@@ -42,6 +42,14 @@ public final class GraalJSScriptEngineFactory implements ScriptEngineFactory {
     private boolean injectionEnabled = true;
 
     public static final String MIME_TYPE = "application/javascript;version=ECMAScript-2021";
+    private final JSScriptServiceUtil jsScriptServiceUtil;
+
+    @Activate
+    public GraalJSScriptEngineFactory(final @Reference JSScriptServiceUtil jsScriptServiceUtil,
+            Map<String, Object> config) {
+        this.jsScriptServiceUtil = jsScriptServiceUtil;
+        modified(config);
+    }
 
     @Override
     public List<String> getScriptTypes() {
@@ -71,12 +79,7 @@ public final class GraalJSScriptEngineFactory implements ScriptEngineFactory {
     @Override
     public ScriptEngine createScriptEngine(String scriptType) {
         return new DebuggingGraalScriptEngine<>(
-                new OpenhabGraalJSScriptEngine(injectionEnabled ? INJECTION_CODE : null));
+                new OpenhabGraalJSScriptEngine(injectionEnabled ? INJECTION_CODE : null, jsScriptServiceUtil));
-    }
-
-    @Activate
-    protected void activate(BundleContext context, Map<String, ?> config) {
-        modified(config);
     }
 
     @Modified

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSRuntimeFeatures.java

@@ -31,8 +31,9 @@ public class JSRuntimeFeatures {
     private final Map<String, Object> features = new HashMap<>();
     public final ThreadsafeTimers threadsafeTimers;
 
-    JSRuntimeFeatures(Object lock) {
+    JSRuntimeFeatures(Object lock, JSScriptServiceUtil jsScriptServiceUtil) {
-        this.threadsafeTimers = new ThreadsafeTimers(lock);
+        this.threadsafeTimers = new ThreadsafeTimers(lock, jsScriptServiceUtil.getScriptExecution(),
+                jsScriptServiceUtil.getScheduler());
 
         features.put("ThreadsafeTimers", threadsafeTimers);
     }

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java

@@ -0,0 +1,50 @@
+/**
+ * Copyright (c) 2010-2022 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.automation.jsscripting.internal;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.openhab.core.automation.module.script.action.ScriptExecution;
+import org.openhab.core.scheduler.Scheduler;
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+
+/**
+ * OSGi utility service for providing easy access to script services.
+ *
+ * @author Florian Hotze - Initial contribution
+ */
+@Component(immediate = true, service = JSScriptServiceUtil.class)
+@NonNullByDefault
+public class JSScriptServiceUtil {
+    private final Scheduler scheduler;
+    private final ScriptExecution scriptExecution;
+
+    @Activate
+    public JSScriptServiceUtil(final @Reference Scheduler scheduler, final @Reference ScriptExecution scriptExecution) {
+        this.scheduler = scheduler;
+        this.scriptExecution = scriptExecution;
+    }
+
+    public Scheduler getScheduler() {
+        return scheduler;
+    }
+
+    public ScriptExecution getScriptExecution() {
+        return scriptExecution;
+    }
+
+    public JSRuntimeFeatures getJSRuntimeFeatures(Object lock) {
+        return new JSRuntimeFeatures(lock, this);
+    }
+}

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java

@@ -71,22 +71,23 @@ public class OpenhabGraalJSScriptEngine
 
     // shared lock object for synchronization of multi-thread access
     private final Object lock = new Object();
-    private final JSRuntimeFeatures jsRuntimeFeatures = new JSRuntimeFeatures(lock);
+    private final JSRuntimeFeatures jsRuntimeFeatures;
 
     // these fields start as null because they are populated on first use
     private String engineIdentifier;
     private Consumer<String> scriptDependencyListener;
 
     private boolean initialized = false;
-    private String globalScript;
+    private final String globalScript;
 
     /**
      * Creates an implementation of ScriptEngine (& Invocable), wrapping the contained engine, that tracks the script
      * lifecycle and provides hooks for scripts to do so too.
      */
-    public OpenhabGraalJSScriptEngine(@Nullable String injectionCode) {
+    public OpenhabGraalJSScriptEngine(@Nullable String injectionCode, JSScriptServiceUtil jsScriptServiceUtil) {
         super(null); // delegate depends on fields not yet initialised, so we cannot set it immediately
         this.globalScript = GLOBAL_REQUIRE + (injectionCode != null ? injectionCode : "");
+        this.jsRuntimeFeatures = jsScriptServiceUtil.getJSRuntimeFeatures(lock);
 
         LOGGER.debug("Initializing GraalJS script engine...");
 

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java

@@ -13,6 +13,7 @@
 package org.openhab.automation.jsscripting.internal.threading;
 
 import java.time.Duration;
+import java.time.Instant;
 import java.time.ZonedDateTime;
 import java.time.temporal.Temporal;
 import java.util.Map;
@@ -20,7 +21,8 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.eclipse.jdt.annotation.Nullable;
-import org.openhab.core.model.script.ScriptServiceUtil;
+import org.openhab.core.automation.module.script.action.ScriptExecution;
+import org.openhab.core.automation.module.script.action.Timer;
 import org.openhab.core.scheduler.ScheduledCompletableFuture;
 import org.openhab.core.scheduler.Scheduler;
 import org.openhab.core.scheduler.SchedulerTemporalAdjuster;
@@ -29,20 +31,22 @@ import org.openhab.core.scheduler.SchedulerTemporalAdjuster;
  * A polyfill implementation of NodeJS timer functionality (<code>setTimeout()</code>, <code>setInterval()</code> and
  * the cancel methods) which controls multithreaded execution access to the single-threaded GraalJS contexts.
  *
- * @author Florian Hotze - Initial contribution
+ * @author Florian Hotze - Initial contribution; Reimplementation to conform standard JS setTimeout and setInterval;
- * @author Florian Hotze - Reimplementation to conform standard JS setTimeout and setInterval
+ *         Threadsafe reimplementation of the timer creation methods of {@link ScriptExecution}
  */
 public class ThreadsafeTimers {
     private final Object lock;
     private final Scheduler scheduler;
+    private final ScriptExecution scriptExecution;
     // Mapping of positive, non-zero integer values (used as timeoutID or intervalID) and the Scheduler
     private final Map<Long, ScheduledCompletableFuture<Object>> idSchedulerMapping = new ConcurrentHashMap<>();
     private AtomicLong lastId = new AtomicLong();
     private String identifier = "noIdentifier";
 
-    public ThreadsafeTimers(Object lock) {
+    public ThreadsafeTimers(Object lock, ScriptExecution scriptExecution, Scheduler scheduler) {
         this.lock = lock;
-        this.scheduler = ScriptServiceUtil.getScheduler();
+        this.scheduler = scheduler;
+        this.scriptExecution = scriptExecution;
     }
 
     /**
@@ -55,19 +59,30 @@ public class ThreadsafeTimers {
     }
 
     /**
-     * Schedules a callback to run at a given time.
+     * Schedules a block of code for later execution.
      *
-     * @param id timerId to append to the identifier base for naming the scheduled job
+     * @param instant the point in time when the code should be executed
-     * @param zdt time to schedule the job
+     * @param closure the code block to execute
-     * @param callback function to run at the given time
+     * @return a handle to the created timer, so that it can be canceled or rescheduled
-     * @return a {@link ScheduledCompletableFuture}
      */
-    private ScheduledCompletableFuture<Object> createFuture(long id, ZonedDateTime zdt, Runnable callback) {
+    public Timer createTimer(ZonedDateTime instant, Runnable closure) {
-        return scheduler.schedule(() -> {
+        return createTimer(identifier, instant, closure);
-            synchronized (lock) {
-                callback.run();
     }
-        }, identifier + ".timeout." + id, zdt.toInstant());
+
+    /**
+     * 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
+     */
+    public Timer createTimer(@Nullable String identifier, ZonedDateTime instant, Runnable closure) {
+        return scriptExecution.createTimer(identifier, instant, () -> {
+            synchronized (lock) {
+                closure.run();
+            }
+        });
     }
 
     /**
@@ -95,8 +110,12 @@ public class ThreadsafeTimers {
      */
     public long setTimeout(Runnable callback, Long delay, Object... args) {
         long id = lastId.incrementAndGet();
-        ScheduledCompletableFuture<Object> future = createFuture(id, ZonedDateTime.now().plusNanos(delay * 1000000),
+        ScheduledCompletableFuture<Object> future = scheduler.schedule(() -> {
-                callback);
+            synchronized (lock) {
+                callback.run();
+                idSchedulerMapping.remove(id);
+            }
+        }, identifier + ".timeout." + id, Instant.now().plusMillis(delay));
         idSchedulerMapping.put(id, future);
         return id;
     }
@@ -115,22 +134,6 @@ public class ThreadsafeTimers {
         }
     }
 
-    /**
-     * Schedules a callback to run in a loop with a given delay between the executions.
-     *
-     * @param id timerId to append to the identifier base for naming the scheduled job
-     * @param delay time in milliseconds that the timer should delay in between executions of the callback
-     * @param callback function to run
-     */
-    private void createLoopingFuture(long id, Long delay, Runnable callback) {
-        ScheduledCompletableFuture<Object> future = scheduler.schedule(() -> {
-            synchronized (lock) {
-                callback.run();
-            }
-        }, identifier + ".interval." + id, new LoopingAdjuster(Duration.ofMillis(delay)));
-        idSchedulerMapping.put(id, future);
-    }
-
     /**
      * <a href="https://developer.mozilla.org/en-US/docs/Web/API/setInterval"><code>setInterval()</code></a> polyfill.
      * Repeatedly calls a function with a fixed time delay between each call.
@@ -156,7 +159,12 @@ public class ThreadsafeTimers {
      */
     public long setInterval(Runnable callback, Long delay, Object... args) {
         long id = lastId.incrementAndGet();
-        createLoopingFuture(id, delay, callback);
+        ScheduledCompletableFuture<Object> future = scheduler.schedule(() -> {
+            synchronized (lock) {
+                callback.run();
+            }
+        }, identifier + ".interval." + id, new LoopingAdjuster(Duration.ofMillis(delay)));
+        idSchedulerMapping.put(id, future);
         return id;
     }