From abc6042cf490ab1976611d6c85024a037e3fb80c Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 20 Nov 2022 18:10:14 +0100 Subject: [PATCH 1/8] Refactor script dependency tracking Signed-off-by: Jan N. Klug --- .../loader/DefaultScriptFileWatcher.java | 6 +- .../internal/loader/ScriptLibraryWatcher.java | 16 +- .../loader/collection/BidiSetBag.java | 90 +++++---- .../AbstractScriptDependencyTracker.java | 113 +++++++++++ ...er.java => AbstractScriptFileWatcher.java} | 32 ++-- .../rulesupport/loader/DependencyTracker.java | 108 ----------- .../AbstractScriptDependencyTrackerTest.java | 152 +++++++++++++++ ...ava => AbstractScriptFileWatcherTest.java} | 175 +++++++++--------- .../pom.xml | 6 + .../script/ScriptDependencyTracker.java | 56 ++++++ .../module/script/ScriptEngineFactory.java | 10 + .../module/script/ScriptEngineManager.java | 10 - .../internal/ScriptEngineManagerImpl.java | 18 +- .../internal/ScriptEngineManagerImplTest.java | 120 ++++++++++++ 14 files changed, 636 insertions(+), 276 deletions(-) create mode 100644 bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java rename bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/{ScriptFileWatcher.java => AbstractScriptFileWatcher.java} (91%) delete mode 100644 bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/DependencyTracker.java create mode 100644 bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java rename bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/{ScriptFileWatcherTest.java => AbstractScriptFileWatcherTest.java} (59%) create mode 100644 bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptDependencyTracker.java create mode 100644 bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/DefaultScriptFileWatcher.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/DefaultScriptFileWatcher.java index 1a540cda0dd..2291a1a9e43 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/DefaultScriptFileWatcher.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/DefaultScriptFileWatcher.java @@ -16,7 +16,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.automation.module.script.ScriptEngineManager; -import org.openhab.core.automation.module.script.rulesupport.loader.ScriptFileWatcher; +import org.openhab.core.automation.module.script.rulesupport.loader.AbstractScriptFileWatcher; import org.openhab.core.service.ReadyService; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; @@ -31,14 +31,14 @@ */ @NonNullByDefault @Component(immediate = true) -public class DefaultScriptFileWatcher extends ScriptFileWatcher { +public class DefaultScriptFileWatcher extends AbstractScriptFileWatcher { private static final String FILE_DIRECTORY = "automation" + File.separator + "jsr223"; @Activate public DefaultScriptFileWatcher(final @Reference ScriptEngineManager manager, final @Reference ReadyService readyService) { - super(manager, null, readyService, FILE_DIRECTORY); + super(manager, readyService, FILE_DIRECTORY); } @Activate diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java index f3ebb550926..d47ec6e27ba 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java @@ -17,6 +17,7 @@ import java.io.File; import java.nio.file.Path; import java.nio.file.WatchEvent; +import java.util.function.Consumer; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -28,10 +29,15 @@ * @author Jonathan Gilbert - Initial contribution */ @NonNullByDefault -public abstract class ScriptLibraryWatcher extends AbstractWatchService { +public class ScriptLibraryWatcher extends AbstractWatchService { - public ScriptLibraryWatcher(final String libraryPath) { + // package private for testing + final Consumer listener; + + public ScriptLibraryWatcher(final String libraryPath, Consumer listener) { super(libraryPath); + + this.listener = listener; } @Override @@ -49,14 +55,12 @@ protected void processWatchEvent(WatchEvent watchEvent, WatchEvent.Kind ki File file = path.toFile(); if (!file.isHidden()) { if (kind.equals(ENTRY_DELETE)) { - this.updateFile(file.getPath()); + listener.accept(file.getPath()); } if (file.canRead() && (kind.equals(ENTRY_CREATE) || kind.equals(ENTRY_MODIFY))) { - this.updateFile(file.getPath()); + listener.accept(file.getPath()); } } } - - protected abstract void updateFile(String filePath); } diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/collection/BidiSetBag.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/collection/BidiSetBag.java index fe76f589baa..e8f217875e3 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/collection/BidiSetBag.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/collection/BidiSetBag.java @@ -17,6 +17,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.ReentrantReadWriteLock; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -25,61 +26,84 @@ * Provides optimized lookup of values for a key, as well as keys referencing a value. * * @author Jonathan Gilbert - Initial contribution + * @author Jan N. Klug - Make implementation thread-safe * @param Type of Key * @param Type of Value */ @NonNullByDefault public class BidiSetBag { - private Map> keyToValues = new HashMap<>(); - private Map> valueToKeys = new HashMap<>(); + + private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); + private final Map> keyToValues = new HashMap<>(); + private final Map> valueToKeys = new HashMap<>(); public void put(K key, V value) { - addElement(keyToValues, key, value); - addElement(valueToKeys, value, key); + lock.writeLock().lock(); + try { + keyToValues.computeIfAbsent(key, k -> new HashSet<>()).add(value); + valueToKeys.computeIfAbsent(value, v -> new HashSet<>()).add(key); + } finally { + lock.writeLock().unlock(); + } } public Set getValues(K key) { - Set existing = keyToValues.get(key); - return existing == null ? Collections.emptySet() : Collections.unmodifiableSet(existing); + lock.readLock().lock(); + try { + Set values = keyToValues.getOrDefault(key, Set.of()); + return Collections.unmodifiableSet(values); + } finally { + lock.readLock().unlock(); + } } public Set getKeys(V value) { - Set existing = valueToKeys.get(value); - return existing == null ? Collections.emptySet() : Collections.unmodifiableSet(existing); + lock.readLock().lock(); + try { + Set keys = valueToKeys.getOrDefault(value, Set.of()); + return Collections.unmodifiableSet(keys); + } finally { + lock.readLock().unlock(); + } } public Set removeKey(K key) { - Set values = keyToValues.remove(key); - if (values != null) { - for (V value : values) { - valueToKeys.computeIfPresent(value, (k, v) -> { - v.remove(key); - return v; - }); + lock.writeLock().lock(); + try { + Set values = keyToValues.remove(key); + if (values != null) { + for (V value : values) { + valueToKeys.computeIfPresent(value, (k, v) -> { + v.remove(key); + return v; + }); + } + return values; + } else { + return Set.of(); } - return values; - } else { - return Collections.emptySet(); + } finally { + lock.writeLock().unlock(); } } public Set removeValue(V value) { - Set keys = valueToKeys.remove(value); - if (keys != null) { - for (K key : keys) { - keyToValues.computeIfPresent(key, (k, v) -> { - v.remove(value); - return v; - }); + lock.writeLock().lock(); + try { + Set keys = valueToKeys.remove(value); + if (keys != null) { + for (K key : keys) { + keyToValues.computeIfPresent(key, (k, v) -> { + v.remove(value); + return v; + }); + } + return keys; + } else { + return Set.of(); } - return keys; - } else { - return Collections.emptySet(); + } finally { + lock.writeLock().unlock(); } } - - private static void addElement(Map> map, T key, U value) { - Set elements = map.compute(key, (k, l) -> l == null ? new HashSet<>() : l); - elements.add(value); - } } diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java new file mode 100644 index 00000000000..22180f8c1a9 --- /dev/null +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java @@ -0,0 +1,113 @@ +/** + * 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.core.automation.module.script.rulesupport.loader; + +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; +import org.openhab.core.automation.module.script.rulesupport.internal.loader.ScriptLibraryWatcher; +import org.openhab.core.automation.module.script.rulesupport.internal.loader.collection.BidiSetBag; +import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.osgi.service.component.annotations.ReferencePolicy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * The {@link AbstractScriptDependencyTracker} tracks dependencies between scripts and reloads dependees + * It needs to be sub-classed for each {@link org.openhab.core.automation.module.script.ScriptEngineFactory} + * that wants to support dependency tracking + * + * @author Jonathan Gilbert - Initial contribution + */ +@NonNullByDefault +public abstract class AbstractScriptDependencyTracker implements ScriptDependencyTracker { + private final Logger logger = LoggerFactory.getLogger(AbstractScriptDependencyTracker.class); + + protected final String libraryPath; + + private final Set dependencyChangeListeners = ConcurrentHashMap.newKeySet(); + + private final BidiSetBag scriptToLibs = new BidiSetBag<>(); + private @Nullable ScriptLibraryWatcher scriptLibraryWatcher; + + public AbstractScriptDependencyTracker(final String libraryPath) { + this.libraryPath = libraryPath; + } + + public void activate() { + ScriptLibraryWatcher scriptLibraryWatcher = createScriptLibraryWatcher(); + scriptLibraryWatcher.activate(); + this.scriptLibraryWatcher = scriptLibraryWatcher; + } + + public void deactivate() { + ScriptLibraryWatcher scriptLibraryWatcher = this.scriptLibraryWatcher; + if (scriptLibraryWatcher != null) { + scriptLibraryWatcher.deactivate(); + } + } + + protected ScriptLibraryWatcher createScriptLibraryWatcher() { + return new ScriptLibraryWatcher(libraryPath, this::dependencyChanged); + } + + protected void dependencyChanged(String dependency) { + Set scripts = new HashSet<>(scriptToLibs.getKeys(dependency)); // take a copy as it will change as we + AbstractScriptDependencyTracker.this.logger.debug("Library {} changed; reimporting {} scripts...", libraryPath, + scripts.size()); + for (String scriptUrl : scripts) { + for (ScriptDependencyTracker.Listener listener : dependencyChangeListeners) { + try { + listener.onDependencyChange(scriptUrl); + } catch (Exception e) { + logger.warn("Failed to notify tracker of dependency change: {}: {}", e.getClass(), e.getMessage()); + } + } + } + } + + @Override + public Consumer getTracker(String scriptId) { + return dependencyPath -> startTracking(scriptId, dependencyPath); + } + + @Override + public void removeTracking(String scriptId) { + scriptToLibs.removeKey(scriptId); + } + + protected void startTracking(String scriptId, String libPath) { + scriptToLibs.put(scriptId, libPath); + } + + /** + * Add a dependency change listener + * + * Since this is done via service injection and OSGi annotations are not inherited it is required that sub-classes expose this method with proper annotation + * + * @param listener the dependency change listener + */ + public void addChangeTracker(ScriptDependencyTracker.Listener listener) { + dependencyChangeListeners.add(listener); + } + + public void removeChangeTracker(ScriptDependencyTracker.Listener listener) { + dependencyChangeListeners.remove(listener); + } +} diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcher.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java similarity index 91% rename from bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcher.java rename to bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java index 7e7488d1155..a7ad7a0a387 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcher.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java @@ -39,6 +39,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.OpenHAB; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; import org.openhab.core.automation.module.script.ScriptEngineContainer; import org.openhab.core.automation.module.script.ScriptEngineManager; import org.openhab.core.common.NamedThreadFactory; @@ -51,23 +52,24 @@ import org.slf4j.LoggerFactory; /** - * The {@link ScriptFileWatcher} watches a directory for files. If a new/modified file is detected, the script - * is read and passed to the {@link ScriptEngineManager}. + * The {@link AbstractScriptFileWatcher} is default implementation for watching a directory for files. If a new/modified + * file is detected, the script + * is read and passed to the {@link ScriptEngineManager}. It needs to be sub-classed for actual use. * * @author Simon Merschjohann - Initial contribution * @author Kai Kreuzer - improved logging and removed thread pool * @author Jonathan Gilbert - added dependency tracking & per-script start levels; made reusable + * @author Jan N. Klug - Refactored dependy tracking to script engine factories */ @NonNullByDefault -public class ScriptFileWatcher extends AbstractWatchService implements ReadyService.ReadyTracker, - DependencyTracker.DependencyChangeListener, ScriptEngineManager.FactoryChangeListener { +public abstract class AbstractScriptFileWatcher extends AbstractWatchService implements ReadyService.ReadyTracker, + ScriptDependencyTracker.Listener, ScriptEngineManager.FactoryChangeListener { private static final long RECHECK_INTERVAL = 20; - private final Logger logger = LoggerFactory.getLogger(ScriptFileWatcher.class); + private final Logger logger = LoggerFactory.getLogger(AbstractScriptFileWatcher.class); private final ScriptEngineManager manager; - private final Optional dependencyTracker; private final ReadyService readyService; private @Nullable ScheduledExecutorService scheduler; @@ -78,11 +80,10 @@ public class ScriptFileWatcher extends AbstractWatchService implements ReadyServ private volatile int currentStartLevel = 0; - public ScriptFileWatcher(final ScriptEngineManager manager, @Nullable final DependencyTracker dependencyTracker, - final ReadyService readyService, final String fileDirectory) { + public AbstractScriptFileWatcher(final ScriptEngineManager manager, final ReadyService readyService, + final String fileDirectory) { super(OpenHAB.getConfigFolder() + File.separator + fileDirectory); this.manager = manager; - this.dependencyTracker = Optional.ofNullable(dependencyTracker); this.readyService = readyService; this.executorFactory = () -> Executors .newSingleThreadScheduledExecutor(new NamedThreadFactory("scriptwatcher")); @@ -206,7 +207,7 @@ protected void processWatchEvent(@Nullable WatchEvent event, @Nullable Kind tracker.removeScript(scriptIdentifier)); + manager.removeEngine(scriptIdentifier); loaded.remove(ref); } @@ -248,8 +249,7 @@ protected boolean createAndLoad(ScriptFileReference ref) { if (container != null) { container.getScriptEngine().put(ScriptEngine.FILENAME, fileName); - manager.loadScript(container.getIdentifier(), reader, dependency -> dependencyTracker - .ifPresent((it) -> it.addLibForScript(scriptIdentifier, dependency))); + manager.loadScript(container.getIdentifier(), reader); logger.debug("Script loaded: {}", fileName); return true; @@ -318,12 +318,12 @@ private synchronized void onStartLevelChanged(int newLevel) { } @Override - public void onDependencyChange(@Nullable String scriptPath) { - logger.debug("Reimporting {}...", scriptPath); + public void onDependencyChange(@Nullable String scriptId) { + logger.debug("Reimporting {}...", scriptId); try { - importFileWhenReady(new ScriptFileReference(new URL(scriptPath))); + importFileWhenReady(new ScriptFileReference(new URL(scriptId))); } catch (MalformedURLException e) { - logger.warn("Failed to reimport {} as it cannot be parsed as a URL", scriptPath); + logger.warn("Failed to reimport {} as it cannot be parsed as a URL", scriptId); } } diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/DependencyTracker.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/DependencyTracker.java deleted file mode 100644 index d840e50dbdc..00000000000 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/DependencyTracker.java +++ /dev/null @@ -1,108 +0,0 @@ -/** - * 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.core.automation.module.script.rulesupport.loader; - -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; - -import org.openhab.core.automation.module.script.rulesupport.internal.loader.ScriptLibraryWatcher; -import org.openhab.core.automation.module.script.rulesupport.internal.loader.collection.BidiSetBag; -import org.osgi.service.component.annotations.Reference; -import org.osgi.service.component.annotations.ReferenceCardinality; -import org.osgi.service.component.annotations.ReferencePolicy; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Tracks dependencies between scripts and reloads dependees - * - * @author Jonathan Gilbert - Initial contribution - */ -public class DependencyTracker { - - public String libraryPath; - - private final Logger logger = LoggerFactory.getLogger(DependencyTracker.class); - - private final Set dependencyChangeListeners = ConcurrentHashMap.newKeySet(); - - private final BidiSetBag scriptToLibs = new BidiSetBag<>(); - private ScriptLibraryWatcher scriptLibraryWatcher; - - public DependencyTracker(final String libraryPath) { - this.libraryPath = libraryPath; - } - - public void activate() { - createScriptLibraryWatcher(); - scriptLibraryWatcher.activate(); - } - - public void deactivate() { - scriptLibraryWatcher.deactivate(); - } - - private void createScriptLibraryWatcher() { - scriptLibraryWatcher = new ScriptLibraryWatcher(libraryPath) { - @Override - protected void updateFile(String libraryPath) { - Set scripts; - synchronized (scriptToLibs) { - scripts = new HashSet<>(scriptToLibs.getKeys(libraryPath)); // take a copy as it will change as we - // reimport - } - DependencyTracker.this.logger.debug("Library {} changed; reimporting {} scripts...", libraryPath, - scripts.size()); - for (String scriptUrl : scripts) { - reimportScript(scriptUrl); - } - } - }; - } - - public void addLibForScript(String scriptPath, String libPath) { - synchronized (scriptToLibs) { - scriptToLibs.put(scriptPath, libPath); - } - } - - public void removeScript(String scriptPath) { - synchronized (scriptToLibs) { - scriptToLibs.removeKey(scriptPath); - } - } - - @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC, unbind = "removeChangeTracker") - public void addChangeTracker(DependencyChangeListener listener) { - dependencyChangeListeners.add(listener); - } - - public void removeChangeTracker(DependencyChangeListener listener) { - dependencyChangeListeners.remove(listener); - } - - public void reimportScript(String scriptPath) { - for (DependencyChangeListener listener : dependencyChangeListeners) { - try { - listener.onDependencyChange(scriptPath); - } catch (Exception e) { - logger.warn("Failed to notify tracker of dependency change: {}: {}", e.getClass(), e.getMessage()); - } - } - } - - public interface DependencyChangeListener { - void onDependencyChange(String scriptPath); - } -} diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java new file mode 100644 index 00000000000..c77f1cb9e0d --- /dev/null +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java @@ -0,0 +1,152 @@ +/** + * 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.core.automation.module.script.rulesupport.loader; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import java.nio.file.Path; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; +import org.openhab.core.automation.module.script.rulesupport.internal.loader.ScriptLibraryWatcher; + +/** + * The {@link AbstractScriptDependencyTrackerTest} contains tests for the {@link AbstractScriptDependencyTracker} + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +public class AbstractScriptDependencyTrackerTest { + + private static final String WATCH_DIR = "test"; + + private @Nullable ScriptLibraryWatcher scriptLibraryWatcher; + + private @NonNullByDefault({}) AbstractScriptDependencyTracker scriptDependencyTracker; + + @BeforeEach + public void setup() { + scriptDependencyTracker = new AbstractScriptDependencyTracker(WATCH_DIR) { + + @Override + protected ScriptLibraryWatcher createScriptLibraryWatcher() { + ScriptLibraryWatcher scriptLibraryWatcher = Mockito.spy(super.createScriptLibraryWatcher()); + AbstractScriptDependencyTrackerTest.this.scriptLibraryWatcher = scriptLibraryWatcher; + return scriptLibraryWatcher; + } + + @Override + public void dependencyChanged(String dependency) { + super.dependencyChanged(dependency); + } + }; + scriptDependencyTracker.activate(); + } + + @AfterEach + public void tearDown() { + scriptDependencyTracker.deactivate(); + } + + @Test + public void testScriptLibraryWatcherIsCreatedAndActivated() { + assertThat(scriptLibraryWatcher, is(notNullValue())); + + assertThat(scriptLibraryWatcher.getSourcePath(), is(Path.of(WATCH_DIR))); + + verify(scriptLibraryWatcher).activate(); + } + + @Test + public void testScriptLibraryWatchersIsDeactivatedOnShutdown() { + scriptDependencyTracker.deactivate(); + + verify(scriptLibraryWatcher).deactivate(); + } + + @Test + public void testDependencyChangeIsForwardedToMultipleListeners() { + ScriptDependencyTracker.Listener listener1 = mock(ScriptDependencyTracker.Listener.class); + ScriptDependencyTracker.Listener listener2 = mock(ScriptDependencyTracker.Listener.class); + + scriptDependencyTracker.addChangeTracker(listener1); + scriptDependencyTracker.addChangeTracker(listener2); + + scriptDependencyTracker.startTracking("scriptId", "depPath"); + scriptDependencyTracker.dependencyChanged("depPath"); + + verify(listener1).onDependencyChange(eq("scriptId")); + verify(listener2).onDependencyChange(eq("scriptId")); + verifyNoMoreInteractions(listener1); + verifyNoMoreInteractions(listener2); + } + + @Test + public void testDependencyChangeIsForwardedForMultipleScriptIds() { + ScriptDependencyTracker.Listener listener = mock(ScriptDependencyTracker.Listener.class); + + scriptDependencyTracker.addChangeTracker(listener); + + scriptDependencyTracker.startTracking("scriptId1", "depPath"); + scriptDependencyTracker.startTracking("scriptId2", "depPath"); + + scriptDependencyTracker.dependencyChanged("depPath"); + + verify(listener).onDependencyChange(eq("scriptId1")); + verify(listener).onDependencyChange(eq("scriptId2")); + verifyNoMoreInteractions(listener); + } + + @Test + public void testDependencyChangeIsForwardedForMultipleDependencies() { + ScriptDependencyTracker.Listener listener = mock(ScriptDependencyTracker.Listener.class); + + scriptDependencyTracker.addChangeTracker(listener); + + scriptDependencyTracker.startTracking("scriptId", "depPath1"); + scriptDependencyTracker.startTracking("scriptId", "depPath2"); + + scriptDependencyTracker.dependencyChanged("depPath1"); + scriptDependencyTracker.dependencyChanged("depPath2"); + + verify(listener, times(2)).onDependencyChange(eq("scriptId")); + verifyNoMoreInteractions(listener); + } + + @Test + public void testDependencyChangeIsForwardedForCorrectDependencies() { + ScriptDependencyTracker.Listener listener = mock(ScriptDependencyTracker.Listener.class); + + scriptDependencyTracker.addChangeTracker(listener); + + scriptDependencyTracker.startTracking("scriptId1", "depPath1"); + scriptDependencyTracker.startTracking("scriptId2", "depPath2"); + + scriptDependencyTracker.dependencyChanged("depPath1"); + + verify(listener).onDependencyChange(eq("scriptId1")); + verifyNoMoreInteractions(listener); + } +} diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcherTest.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java similarity index 59% rename from bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcherTest.java rename to bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java index 664211454f3..ed5e94ec4dc 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/ScriptFileWatcherTest.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java @@ -29,11 +29,17 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; -import org.openhab.core.automation.module.script.ScriptDependencyListener; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; import org.openhab.core.automation.module.script.ScriptEngineContainer; +import org.openhab.core.automation.module.script.ScriptEngineFactory; import org.openhab.core.automation.module.script.ScriptEngineManager; import org.openhab.core.automation.module.script.rulesupport.internal.loader.DelegatingScheduledExecutorService; import org.openhab.core.service.ReadyMarker; @@ -47,22 +53,26 @@ * @author Jonathan Gilbert - initial contribution */ @NonNullByDefault -class ScriptFileWatcherTest { +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class AbstractScriptFileWatcherTest { - private @NonNullByDefault({}) ScriptFileWatcher scriptFileWatcher; - private @NonNullByDefault({}) ScriptEngineManager scriptEngineManager; - private @NonNullByDefault({}) DependencyTracker dependencyTracker; + private @NonNullByDefault({}) AbstractScriptFileWatcher scriptFileWatcher; + + @Mock + private @NonNullByDefault({}) ScriptEngineManager scriptEngineManagerMock; + @Mock + private @NonNullByDefault({}) ScriptDependencyTracker scriptDependencyTrackerMock; + @Mock private @NonNullByDefault({}) ReadyService readyService; protected @NonNullByDefault({}) @TempDir Path tempScriptDir; @BeforeEach public void setUp() { - scriptEngineManager = mock(ScriptEngineManager.class); - dependencyTracker = mock(DependencyTracker.class); - readyService = mock(ReadyService.class); - scriptFileWatcher = new ScriptFileWatcher(scriptEngineManager, dependencyTracker, readyService, - "automation" + File.separator + "jsr223"); + scriptFileWatcher = new AbstractScriptFileWatcher(scriptEngineManagerMock, readyService, + "automation" + File.separator + "jsr223") { + }; scriptFileWatcher.activate(); } @@ -96,10 +106,10 @@ void updateStartLevel(int level) { @Test public void testLoadOneDefaultFileAlreadyStarted() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); updateStartLevel(100); @@ -107,33 +117,34 @@ public void testLoadOneDefaultFileAlreadyStarted() { scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p); - verify(scriptEngineManager, timeout(10000)).createScriptEngine("js", p.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000)).createScriptEngine("js", p.toFile().toURI().toString()); } @Test public void testLoadOneDefaultFileWaitUntilStarted() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); Path p = getFile("script.js"); scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); // verify is called when the start level increases updateStartLevel(100); - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p.toFile().toURI().toString()); } @Test public void testLoadOneCustomFileWaitUntilStarted() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); updateStartLevel(50); @@ -141,19 +152,20 @@ public void testLoadOneCustomFileWaitUntilStarted() { scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); // verify is called when the start level increases updateStartLevel(100); - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p.toFile().toURI().toString()); } @Test public void testLoadTwoCustomFilesDifferentStartLevels() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); Path p1 = getFile("script.sl70.js"); scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p1); @@ -161,29 +173,31 @@ public void testLoadTwoCustomFilesDifferentStartLevels() { scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p2); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); updateStartLevel(40); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); updateStartLevel(60); - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p2.toFile().toURI().toString()); - verify(scriptEngineManager, never()).createScriptEngine(anyString(), eq(p1.toFile().toURI().toString())); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p2.toFile().toURI().toString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), eq(p1.toFile().toURI().toString())); updateStartLevel(80); - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p1.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p1.toFile().toURI().toString()); } @Test public void testLoadTwoCustomFilesAlternativePatternDifferentStartLevels() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); Path p1 = getFile("sl70/script.js"); scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p1); @@ -191,21 +205,23 @@ public void testLoadTwoCustomFilesAlternativePatternDifferentStartLevels() { scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p2); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); updateStartLevel(40); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); updateStartLevel(60); - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p2.toFile().toURI().toString()); - verify(scriptEngineManager, never()).createScriptEngine(anyString(), eq(p1.toFile().toURI().toString())); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p2.toFile().toURI().toString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), eq(p1.toFile().toURI().toString())); updateStartLevel(80); - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p1.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p1.toFile().toURI().toString()); } @Test @@ -216,10 +232,10 @@ public void testLoadOneDefaultFileDelayedSupport() { ArgumentCaptor scheduledTask = ArgumentCaptor.forClass(Runnable.class); scriptFileWatcher.setExecutorFactory(() -> scheduledExecutorService); - when(scriptEngineManager.isSupported("js")).thenReturn(false); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(false); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); updateStartLevel(100); verify(scheduledExecutorService).scheduleWithFixedDelay(scheduledTask.capture(), anyLong(), anyLong(), any()); @@ -228,24 +244,25 @@ public void testLoadOneDefaultFileDelayedSupport() { scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p); // verify not yet called - verify(scriptEngineManager, never()).createScriptEngine(anyString(), anyString()); + verify(scriptEngineManagerMock, never()).createScriptEngine(anyString(), anyString()); // add support is added for .js files - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); // update (in current thread) scheduledTask.getValue().run(); // verify script has now been processed - verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", p.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p.toFile().toURI().toString()); } @Test public void testOrderingWithinSingleStartLevel() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); Path p64 = getFile("script.sl64.js"); scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p64); @@ -256,23 +273,23 @@ public void testOrderingWithinSingleStartLevel() { updateStartLevel(70); - InOrder inOrder = inOrder(scriptEngineManager); + InOrder inOrder = inOrder(scriptEngineManagerMock); - inOrder.verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", p64.toFile().toURI().toString()); - inOrder.verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", p65.toFile().toURI().toString()); - inOrder.verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", p66.toFile().toURI().toString()); } @Test public void testOrderingStartlevelFolders() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); Path p50 = getFile("a_script.js"); scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p50); @@ -283,22 +300,25 @@ public void testOrderingStartlevelFolders() { updateStartLevel(70); - InOrder inOrder = inOrder(scriptEngineManager); + InOrder inOrder = inOrder(scriptEngineManagerMock); - inOrder.verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", p30.toFile().toURI().toString()); - inOrder.verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", p40.toFile().toURI().toString()); - inOrder.verify(scriptEngineManager, timeout(10000).times(1)).createScriptEngine("js", + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", p50.toFile().toURI().toString()); } @Test public void testReloadActiveWhenDependencyChanged() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + ScriptEngineFactory scriptEngineFactoryMock = mock(ScriptEngineFactory.class); + when(scriptEngineFactoryMock.getDependencyTracker()).thenReturn(scriptDependencyTrackerMock); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineContainer.getFactory()).thenReturn(scriptEngineFactoryMock); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); updateStartLevel(100); @@ -308,15 +328,16 @@ public void testReloadActiveWhenDependencyChanged() { scriptFileWatcher.onDependencyChange(p.toFile().toURI().toString()); - verify(scriptEngineManager, timeout(10000).times(2)).createScriptEngine("js", p.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(2)).createScriptEngine("js", + p.toFile().toURI().toString()); } @Test public void testNotReloadInactiveWhenDependencyChanged() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); Path p = getFile("script.js"); @@ -324,44 +345,15 @@ public void testNotReloadInactiveWhenDependencyChanged() { scriptFileWatcher.onDependencyChange(p.toFile().toURI().toString()); - verify(scriptEngineManager, never()).createScriptEngine("js", p.toFile().toURI().toString()); - } - - @Test - public void testRegisterDependency() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); - ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); - when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); - - updateStartLevel(100); - - Path p = getFile("script.js"); - - when(scriptEngineContainer.getIdentifier()).thenReturn(p.toFile().toURI().toString()); - - // capture and run the listener - ArgumentCaptor listener = ArgumentCaptor.forClass(ScriptDependencyListener.class); - - scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p); - - verify(scriptEngineManager).loadScript(eq(p.toFile().toURI().toString()), any(), listener.capture()); - - listener.getValue().accept("test"); - - // verify the dependency was tracked - verify(dependencyTracker).addLibForScript(p.toFile().toURI().toString(), "test"); + verify(scriptEngineManagerMock, never()).createScriptEngine("js", p.toFile().toURI().toString()); } - /** - * @see https://github.com/openhab/openhab-core/issues/2246 - */ @Test public void testRemoveBeforeReAdd() { - when(scriptEngineManager.isSupported("js")).thenReturn(true); + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); - when(scriptEngineManager.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); updateStartLevel(100); @@ -370,7 +362,8 @@ public void testRemoveBeforeReAdd() { scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p); scriptFileWatcher.processWatchEvent(null, ENTRY_MODIFY, p); - verify(scriptEngineManager).removeEngine(p.toFile().toURI().toString()); - verify(scriptEngineManager, timeout(10000).times(2)).createScriptEngine("js", p.toFile().toURI().toString()); + verify(scriptEngineManagerMock).removeEngine(p.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000).times(2)).createScriptEngine("js", + p.toFile().toURI().toString()); } } diff --git a/bundles/org.openhab.core.automation.module.script/pom.xml b/bundles/org.openhab.core.automation.module.script/pom.xml index bfdfea949ea..ea137aa528b 100644 --- a/bundles/org.openhab.core.automation.module.script/pom.xml +++ b/bundles/org.openhab.core.automation.module.script/pom.xml @@ -25,6 +25,12 @@ org.openhab.core.transform ${project.version} + + org.openhab.core.bundles + org.openhab.core.test + ${project.version} + test + diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptDependencyTracker.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptDependencyTracker.java new file mode 100644 index 00000000000..136d0389e46 --- /dev/null +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptDependencyTracker.java @@ -0,0 +1,56 @@ +/** + * 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.core.automation.module.script; + +import java.util.function.Consumer; + +import org.eclipse.jdt.annotation.NonNullByDefault; + +/** + * The {@link ScriptDependencyTracker} is an interface that script dependency trackers can implement to allow automatic + * re-loading if scripts on dependency changes + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +public interface ScriptDependencyTracker { + + /** + * Get the tracker for a given script identifier + * + * @param scriptId the unique id of the script + * @return a {@link Consumer} that accepts a the path to a dependency + */ + Consumer getTracker(String scriptId); + + /** + * Remove all tracking data for a given scipt identifier + * + * @param scriptId the uniwue id of the script + */ + void removeTracking(String scriptId); + + /** + * The {@link ScriptDependencyTracker.Listener} is an interface that needs to be implemented by listeners that want + * to be notified about a dependency change + */ + interface Listener { + + /** + * Called by the dependency tracker when a registered dependency changes + * + * @param scriptId the identifier of the script whose dependency changed + */ + void onDependencyChange(String scriptId); + } +} diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineFactory.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineFactory.java index fe216a175d6..4475b4a8d11 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineFactory.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineFactory.java @@ -74,4 +74,14 @@ public interface ScriptEngineFactory { */ @Nullable ScriptEngine createScriptEngine(String scriptType); + + /** + * This method returns a {@link ScriptDependencyTracker} if it is available + * + * @return a {@link ScriptDependencyTracker} or null if dependency tracking is not supported for + * {@link ScriptEngine}s created by this factory + */ + default @Nullable ScriptDependencyTracker getDependencyTracker() { + return null; + } } diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineManager.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineManager.java index ca778487da2..7339bcefeca 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineManager.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/ScriptEngineManager.java @@ -45,16 +45,6 @@ public interface ScriptEngineManager { */ void loadScript(String engineIdentifier, InputStreamReader scriptData); - /** - * Loads a script and initializes its scope variables - * - * @param engineIdentifier the unique identifier for the ScriptEngine (script file path or UUID) - * @param scriptData the content of the script - * @param scriptDependencyListener listener to be notified of script dependencies - */ - void loadScript(String engineIdentifier, InputStreamReader scriptData, - ScriptDependencyListener scriptDependencyListener); - /** * Unloads the ScriptEngine loaded with the engineIdentifier * diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java index 9f0f91d23fc..504b50fb9d5 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java @@ -31,7 +31,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; -import org.openhab.core.automation.module.script.ScriptDependencyListener; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; import org.openhab.core.automation.module.script.ScriptEngineContainer; import org.openhab.core.automation.module.script.ScriptEngineFactory; import org.openhab.core.automation.module.script.ScriptEngineManager; @@ -169,20 +169,16 @@ private boolean isCustomFactory(ScriptEngineFactory engineFactory) { @Override public void loadScript(String engineIdentifier, InputStreamReader scriptData) { - loadScript(engineIdentifier, scriptData, null); - } - - @Override - public void loadScript(String engineIdentifier, InputStreamReader scriptData, - @Nullable ScriptDependencyListener dependencyListener) { ScriptEngineContainer container = loadedScriptEngineInstances.get(engineIdentifier); if (container == null) { logger.error("Could not load script, as no ScriptEngine has been created"); } else { ScriptEngine engine = container.getScriptEngine(); - if (dependencyListener != null) { - addAttributeToScriptContext(engine, CONTEXT_KEY_DEPENDENCY_LISTENER, dependencyListener); + ScriptDependencyTracker tracker = container.getFactory().getDependencyTracker(); + if (tracker != null) { + addAttributeToScriptContext(engine, CONTEXT_KEY_DEPENDENCY_LISTENER, + tracker.getTracker(engineIdentifier)); } try { @@ -207,6 +203,10 @@ public void loadScript(String engineIdentifier, InputStreamReader scriptData, public void removeEngine(String engineIdentifier) { ScriptEngineContainer container = loadedScriptEngineInstances.remove(engineIdentifier); if (container != null) { + ScriptDependencyTracker tracker = container.getFactory().getDependencyTracker(); + if (tracker != null) { + tracker.removeTracking(engineIdentifier); + } ScriptEngine scriptEngine = container.getScriptEngine(); if (scriptEngine instanceof Invocable) { Invocable inv = (Invocable) scriptEngine; diff --git a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java new file mode 100644 index 00000000000..36852a08865 --- /dev/null +++ b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java @@ -0,0 +1,120 @@ +/** + * 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.core.automation.module.script.internal; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.openhab.core.automation.module.script.ScriptEngineFactory.CONTEXT_KEY_DEPENDENCY_LISTENER; + +import java.io.ByteArrayInputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.function.Consumer; + +import javax.script.ScriptContext; +import javax.script.ScriptEngine; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; +import org.openhab.core.automation.module.script.ScriptEngineFactory; +import org.openhab.core.test.java.JavaTest; + +/** + * The {@link ScriptEngineManagerImplTest} is a test class for the {@link ScriptEngineManagerImpl} + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class ScriptEngineManagerImplTest extends JavaTest { + + private static final String SUPPORTED_SCRIPT_TYPE = "supported"; + + @Mock + private @NonNullByDefault({}) ScriptExtensionManager scriptExtensionManagerMock; + + @Mock + private @NonNullByDefault({}) ScriptEngineFactory scriptEngineFactoryMock; + + @Mock + private @NonNullByDefault({}) ScriptEngine scriptEngineMock; + + @Mock + private @NonNullByDefault({}) javax.script.ScriptEngineFactory internalScriptEngineFactoryMock; + + @Mock + private @NonNullByDefault({}) ScriptDependencyTracker scriptDependencyTrackerMock; + + @Mock + private @NonNullByDefault({}) ScriptContext scriptContextMock; + + @Mock + private @NonNullByDefault({}) Consumer dependencyListenerMock; + + private @NonNullByDefault({}) ScriptEngineManagerImpl scriptEngineManager; + + @BeforeEach + public void setup() { + when(scriptEngineMock.getFactory()).thenReturn(internalScriptEngineFactoryMock); + when(scriptEngineMock.getContext()).thenReturn(scriptContextMock); + + when(scriptEngineFactoryMock.getScriptTypes()).thenReturn(List.of(SUPPORTED_SCRIPT_TYPE)); + when(scriptEngineFactoryMock.createScriptEngine(SUPPORTED_SCRIPT_TYPE)).thenReturn(scriptEngineMock); + when(scriptEngineFactoryMock.getDependencyTracker()).thenReturn(scriptDependencyTrackerMock); + when(scriptDependencyTrackerMock.getTracker(any())).thenReturn(dependencyListenerMock); + + scriptEngineManager = new ScriptEngineManagerImpl(scriptExtensionManagerMock); + scriptEngineManager.addScriptEngineFactory(scriptEngineFactoryMock); + } + + @Test + public void testDependencyListenerIsProperlyHandled() { + String engineIdentifier = "testIdentifier"; + String scriptContent = "testContent"; + + InputStreamReader scriptContentReader = new InputStreamReader( + new ByteArrayInputStream(scriptContent.getBytes(StandardCharsets.UTF_8))); + + scriptEngineManager.createScriptEngine(SUPPORTED_SCRIPT_TYPE, engineIdentifier); + scriptEngineManager.loadScript(engineIdentifier, scriptContentReader); + + // verify the dependency tracker is requested + verify(scriptDependencyTrackerMock).getTracker(eq(engineIdentifier)); + + // verify dependency tracker is set in the context + ArgumentCaptor captor = ArgumentCaptor.forClass(Object.class); + verify(scriptContextMock).setAttribute(eq(CONTEXT_KEY_DEPENDENCY_LISTENER), captor.capture(), + eq(ScriptContext.ENGINE_SCOPE)); + + Object captured = captor.getValue(); + assertThat(captured, is(dependencyListenerMock)); + + // verify tracking is stopped when script engine is removed + scriptEngineManager.removeEngine(engineIdentifier); + verify(scriptDependencyTrackerMock).removeTracking(eq(engineIdentifier)); + } +} From 5d0c71da128a47f6bc647fa61500c0bdd0efeefc Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 20 Nov 2022 19:02:50 +0100 Subject: [PATCH 2/8] fix spotless Signed-off-by: Jan N. Klug --- .../rulesupport/loader/AbstractScriptDependencyTracker.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java index 22180f8c1a9..b799a8e23e3 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java @@ -22,9 +22,6 @@ import org.openhab.core.automation.module.script.ScriptDependencyTracker; import org.openhab.core.automation.module.script.rulesupport.internal.loader.ScriptLibraryWatcher; import org.openhab.core.automation.module.script.rulesupport.internal.loader.collection.BidiSetBag; -import org.osgi.service.component.annotations.Reference; -import org.osgi.service.component.annotations.ReferenceCardinality; -import org.osgi.service.component.annotations.ReferencePolicy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -99,7 +96,8 @@ protected void startTracking(String scriptId, String libPath) { /** * Add a dependency change listener * - * Since this is done via service injection and OSGi annotations are not inherited it is required that sub-classes expose this method with proper annotation + * Since this is done via service injection and OSGi annotations are not inherited it is required that sub-classes + * expose this method with proper annotation * * @param listener the dependency change listener */ From 758600f932cf2d30c3fc4f5bab043174f2b22f3f Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 20 Nov 2022 20:53:22 +0100 Subject: [PATCH 3/8] enable tracking for UI scripts Signed-off-by: Jan N. Klug --- .../loader/AbstractScriptFileWatcher.java | 9 ++++--- .../internal/ScriptEngineManagerImpl.java | 12 ++++----- .../factory/ScriptModuleHandlerFactory.java | 25 ++++++++++++++++--- .../handler/AbstractScriptModuleHandler.java | 18 +++++++++++++ .../internal/handler/ScriptActionHandler.java | 15 ++++++++++- 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java index a7ad7a0a387..1bec9d49f8d 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java @@ -321,10 +321,13 @@ private synchronized void onStartLevelChanged(int newLevel) { public void onDependencyChange(@Nullable String scriptId) { logger.debug("Reimporting {}...", scriptId); try { - importFileWhenReady(new ScriptFileReference(new URL(scriptId))); - } catch (MalformedURLException e) { - logger.warn("Failed to reimport {} as it cannot be parsed as a URL", scriptId); + ScriptFileReference scriptFileReference = new ScriptFileReference(new URL(scriptId)); + if (loaded.contains(scriptFileReference)) { + importFileWhenReady(scriptFileReference); + } + } catch (MalformedURLException ignored) { } + logger.debug("Ignoring dependency change for {} as it is no file or not loaded by this file watcher", scriptId); } @Override diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java index 504b50fb9d5..f75c2bc270d 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java @@ -155,6 +155,12 @@ private boolean isCustomFactory(ScriptEngineFactory engineFactory) { addAttributeToScriptContext(engine, CONTEXT_KEY_ENGINE_IDENTIFIER, engineIdentifier); addAttributeToScriptContext(engine, CONTEXT_KEY_EXTENSION_ACCESSOR, scriptExtensionManager); + + ScriptDependencyTracker tracker = engineFactory.getDependencyTracker(); + if (tracker != null) { + addAttributeToScriptContext(engine, CONTEXT_KEY_DEPENDENCY_LISTENER, + tracker.getTracker(engineIdentifier)); + } } else { logger.error("ScriptEngine for language '{}' could not be created for identifier: {}", scriptType, engineIdentifier); @@ -175,12 +181,6 @@ public void loadScript(String engineIdentifier, InputStreamReader scriptData) { } else { ScriptEngine engine = container.getScriptEngine(); - ScriptDependencyTracker tracker = container.getFactory().getDependencyTracker(); - if (tracker != null) { - addAttributeToScriptContext(engine, CONTEXT_KEY_DEPENDENCY_LISTENER, - tracker.getTracker(engineIdentifier)); - } - try { engine.eval(scriptData); if (engine instanceof Invocable) { diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java index 2d80383e4ff..d662846b657 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java @@ -14,6 +14,8 @@ import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -23,6 +25,7 @@ import org.openhab.core.automation.handler.BaseModuleHandlerFactory; import org.openhab.core.automation.handler.ModuleHandler; import org.openhab.core.automation.handler.ModuleHandlerFactory; +import org.openhab.core.automation.module.script.ScriptDependencyTracker; import org.openhab.core.automation.module.script.ScriptEngineManager; import org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler; import org.openhab.core.automation.module.script.internal.handler.ScriptConditionHandler; @@ -39,8 +42,8 @@ * @author Kai Kreuzer - Initial contribution */ @NonNullByDefault -@Component(service = ModuleHandlerFactory.class) -public class ScriptModuleHandlerFactory extends BaseModuleHandlerFactory { +@Component(service = { ModuleHandlerFactory.class, ScriptDependencyTracker.Listener.class }) +public class ScriptModuleHandlerFactory extends BaseModuleHandlerFactory implements ScriptDependencyTracker.Listener { private final Logger logger = LoggerFactory.getLogger(ScriptModuleHandlerFactory.class); @@ -48,6 +51,8 @@ public class ScriptModuleHandlerFactory extends BaseModuleHandlerFactory { ScriptConditionHandler.TYPE_ID); private @NonNullByDefault({}) ScriptEngineManager scriptEngineManager; + private Map trackedHandlers = new ConcurrentHashMap<>(); + @Override @Deactivate protected void deactivate() { @@ -68,7 +73,9 @@ public Collection getTypes() { scriptEngineManager); return handler; } else if (ScriptActionHandler.TYPE_ID.equals(moduleTypeUID) && module instanceof Action) { - ScriptActionHandler handler = new ScriptActionHandler((Action) module, ruleUID, scriptEngineManager); + ScriptActionHandler handler = new ScriptActionHandler((Action) module, ruleUID, scriptEngineManager, + this::onHandlerRemoval); + trackedHandlers.put(handler.getEngineIdentifier(), handler); return handler; } else { logger.error("The ModuleHandler is not supported: {}", moduleTypeUID); @@ -84,4 +91,16 @@ public void setScriptEngineManager(ScriptEngineManager scriptEngineManager) { public void unsetScriptEngineManager(ScriptEngineManager scriptEngineManager) { this.scriptEngineManager = null; } + + private void onHandlerRemoval(ScriptActionHandler handler) { + trackedHandlers.values().remove(handler); + } + + @Override + public void onDependencyChange(String engineIdentifier) { + ScriptActionHandler handler = trackedHandlers.get(engineIdentifier); + if (handler != null) { + handler.resetScriptEngine(); + } + } } diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java index 41d0dd402ef..a3bda9b2cbd 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java @@ -84,6 +84,24 @@ public void dispose() { scriptEngineManager.removeEngine(engineIdentifier); } + /** + * Reset the script engine to force a script reload + * + */ + public synchronized void resetScriptEngine() { + scriptEngineManager.removeEngine(engineIdentifier); + scriptEngine = Optional.empty(); + } + + /** + * Gets the script engine identifier for this module + * + * @return the engine identifier string + */ + public String getEngineIdentifier() { + return engineIdentifier; + } + protected Optional getScriptEngine() { return scriptEngine.isPresent() ? scriptEngine : createScriptEngine(); } diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java index 0ea27b747fa..61d93125400 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java @@ -14,6 +14,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.function.Consumer; import javax.script.ScriptException; @@ -37,15 +38,27 @@ public class ScriptActionHandler extends AbstractScriptModuleHandler imp public static final String TYPE_ID = "script.ScriptAction"; private final Logger logger = LoggerFactory.getLogger(ScriptActionHandler.class); + private final Consumer onRemoval; /** * constructs a new ScriptActionHandler * * @param module the module * @param ruleUID the UID of the rule this handler is used for + * @param onRemoval called on removal of this script */ - public ScriptActionHandler(Action module, String ruleUID, ScriptEngineManager scriptEngineManager) { + public ScriptActionHandler(Action module, String ruleUID, ScriptEngineManager scriptEngineManager, + Consumer onRemoval) { super(module, ruleUID, scriptEngineManager); + + this.onRemoval = onRemoval; + } + + @Override + public void dispose() { + onRemoval.accept(this); + + super.dispose(); } @Override From 36410cd49060bc48d072f7d3272405fe31fc94db Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 20 Nov 2022 21:05:18 +0100 Subject: [PATCH 4/8] improvements Signed-off-by: Jan N. Klug --- .../internal/loader/ScriptLibraryWatcher.java | 66 ------------------- .../AbstractScriptDependencyTracker.java | 46 ++++++++++--- .../AbstractScriptDependencyTrackerTest.java | 21 +++--- 3 files changed, 47 insertions(+), 86 deletions(-) delete mode 100644 bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java deleted file mode 100644 index d47ec6e27ba..00000000000 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/loader/ScriptLibraryWatcher.java +++ /dev/null @@ -1,66 +0,0 @@ -/** - * 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.core.automation.module.script.rulesupport.internal.loader; - -import static java.nio.file.StandardWatchEventKinds.*; - -import java.io.File; -import java.nio.file.Path; -import java.nio.file.WatchEvent; -import java.util.function.Consumer; - -import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; -import org.openhab.core.service.AbstractWatchService; - -/** - * Listens for changes to script libraries - * - * @author Jonathan Gilbert - Initial contribution - */ -@NonNullByDefault -public class ScriptLibraryWatcher extends AbstractWatchService { - - // package private for testing - final Consumer listener; - - public ScriptLibraryWatcher(final String libraryPath, Consumer listener) { - super(libraryPath); - - this.listener = listener; - } - - @Override - protected boolean watchSubDirectories() { - return true; - } - - @Override - protected WatchEvent.Kind @Nullable [] getWatchEventKinds(Path path) { - return new WatchEvent.Kind[] { ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY }; - } - - @Override - protected void processWatchEvent(WatchEvent watchEvent, WatchEvent.Kind kind, Path path) { - File file = path.toFile(); - if (!file.isHidden()) { - if (kind.equals(ENTRY_DELETE)) { - listener.accept(file.getPath()); - } - - if (file.canRead() && (kind.equals(ENTRY_CREATE) || kind.equals(ENTRY_MODIFY))) { - listener.accept(file.getPath()); - } - } - } -} diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java index b799a8e23e3..9c43e227a7f 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java @@ -12,6 +12,9 @@ */ package org.openhab.core.automation.module.script.rulesupport.loader; +import java.io.File; +import java.nio.file.Path; +import java.nio.file.WatchEvent; import java.util.HashSet; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -20,17 +23,22 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.automation.module.script.ScriptDependencyTracker; -import org.openhab.core.automation.module.script.rulesupport.internal.loader.ScriptLibraryWatcher; import org.openhab.core.automation.module.script.rulesupport.internal.loader.collection.BidiSetBag; +import org.openhab.core.service.AbstractWatchService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE; +import static java.nio.file.StandardWatchEventKinds.ENTRY_DELETE; +import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY; + /** * The {@link AbstractScriptDependencyTracker} tracks dependencies between scripts and reloads dependees * It needs to be sub-classed for each {@link org.openhab.core.automation.module.script.ScriptEngineFactory} * that wants to support dependency tracking * * @author Jonathan Gilbert - Initial contribution + * @author Jan N. Klug - Refactored to OSGi service */ @NonNullByDefault public abstract class AbstractScriptDependencyTracker implements ScriptDependencyTracker { @@ -41,27 +49,45 @@ public abstract class AbstractScriptDependencyTracker implements ScriptDependenc private final Set dependencyChangeListeners = ConcurrentHashMap.newKeySet(); private final BidiSetBag scriptToLibs = new BidiSetBag<>(); - private @Nullable ScriptLibraryWatcher scriptLibraryWatcher; + private @Nullable AbstractWatchService dependencyWatchService; public AbstractScriptDependencyTracker(final String libraryPath) { this.libraryPath = libraryPath; } public void activate() { - ScriptLibraryWatcher scriptLibraryWatcher = createScriptLibraryWatcher(); - scriptLibraryWatcher.activate(); - this.scriptLibraryWatcher = scriptLibraryWatcher; + AbstractWatchService dependencyWatchService = createDependencyWatchService(); + dependencyWatchService.activate(); + this.dependencyWatchService = dependencyWatchService; } public void deactivate() { - ScriptLibraryWatcher scriptLibraryWatcher = this.scriptLibraryWatcher; - if (scriptLibraryWatcher != null) { - scriptLibraryWatcher.deactivate(); + AbstractWatchService dependencyWatchService = this.dependencyWatchService; + if (dependencyWatchService != null) { + dependencyWatchService.deactivate(); } } - protected ScriptLibraryWatcher createScriptLibraryWatcher() { - return new ScriptLibraryWatcher(libraryPath, this::dependencyChanged); + protected AbstractWatchService createDependencyWatchService() { + return new AbstractWatchService(libraryPath) { + @Override + protected boolean watchSubDirectories() { + return true; + } + + @Override + protected WatchEvent.Kind @Nullable [] getWatchEventKinds(Path path) { + return new WatchEvent.Kind[] { ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY }; + } + + @Override + protected void processWatchEvent(WatchEvent watchEvent, WatchEvent.Kind kind, Path path) { + File file = path.toFile(); + if (!file.isHidden() && (kind.equals(ENTRY_DELETE) + || (file.canRead() && (kind.equals(ENTRY_CREATE) || kind.equals(ENTRY_MODIFY))))) { + AbstractScriptDependencyTracker.this.dependencyChanged(file.getPath()); + } + }}; } protected void dependencyChanged(String dependency) { diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java index c77f1cb9e0d..30384084ea7 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTrackerTest.java @@ -30,7 +30,7 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; import org.openhab.core.automation.module.script.ScriptDependencyTracker; -import org.openhab.core.automation.module.script.rulesupport.internal.loader.ScriptLibraryWatcher; +import org.openhab.core.service.AbstractWatchService; /** * The {@link AbstractScriptDependencyTrackerTest} contains tests for the {@link AbstractScriptDependencyTracker} @@ -42,7 +42,7 @@ public class AbstractScriptDependencyTrackerTest { private static final String WATCH_DIR = "test"; - private @Nullable ScriptLibraryWatcher scriptLibraryWatcher; + private @Nullable AbstractWatchService dependencyWatchService; private @NonNullByDefault({}) AbstractScriptDependencyTracker scriptDependencyTracker; @@ -51,10 +51,10 @@ public void setup() { scriptDependencyTracker = new AbstractScriptDependencyTracker(WATCH_DIR) { @Override - protected ScriptLibraryWatcher createScriptLibraryWatcher() { - ScriptLibraryWatcher scriptLibraryWatcher = Mockito.spy(super.createScriptLibraryWatcher()); - AbstractScriptDependencyTrackerTest.this.scriptLibraryWatcher = scriptLibraryWatcher; - return scriptLibraryWatcher; + protected AbstractWatchService createDependencyWatchService() { + AbstractWatchService dependencyWatchService = Mockito.spy(super.createDependencyWatchService()); + AbstractScriptDependencyTrackerTest.this.dependencyWatchService = dependencyWatchService; + return dependencyWatchService; } @Override @@ -62,6 +62,7 @@ public void dependencyChanged(String dependency) { super.dependencyChanged(dependency); } }; + scriptDependencyTracker.activate(); } @@ -72,18 +73,18 @@ public void tearDown() { @Test public void testScriptLibraryWatcherIsCreatedAndActivated() { - assertThat(scriptLibraryWatcher, is(notNullValue())); + assertThat(dependencyWatchService, is(notNullValue())); - assertThat(scriptLibraryWatcher.getSourcePath(), is(Path.of(WATCH_DIR))); + assertThat(dependencyWatchService.getSourcePath(), is(Path.of(WATCH_DIR))); - verify(scriptLibraryWatcher).activate(); + verify(dependencyWatchService).activate(); } @Test public void testScriptLibraryWatchersIsDeactivatedOnShutdown() { scriptDependencyTracker.deactivate(); - verify(scriptLibraryWatcher).deactivate(); + verify(dependencyWatchService).deactivate(); } @Test From fe06c0fa5c9827a487e64e306e1637c417da133c Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 20 Nov 2022 21:17:20 +0100 Subject: [PATCH 5/8] improvement Signed-off-by: Jan N. Klug --- .../loader/AbstractScriptDependencyTracker.java | 11 ++++++----- .../internal/factory/ScriptModuleHandlerFactory.java | 1 + 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java index 9c43e227a7f..4cbb5404a90 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java @@ -12,6 +12,10 @@ */ package org.openhab.core.automation.module.script.rulesupport.loader; +import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE; +import static java.nio.file.StandardWatchEventKinds.ENTRY_DELETE; +import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY; + import java.io.File; import java.nio.file.Path; import java.nio.file.WatchEvent; @@ -28,10 +32,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE; -import static java.nio.file.StandardWatchEventKinds.ENTRY_DELETE; -import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY; - /** * The {@link AbstractScriptDependencyTracker} tracks dependencies between scripts and reloads dependees * It needs to be sub-classed for each {@link org.openhab.core.automation.module.script.ScriptEngineFactory} @@ -87,7 +87,8 @@ protected void processWatchEvent(WatchEvent watchEvent, WatchEvent.Kind ki || (file.canRead() && (kind.equals(ENTRY_CREATE) || kind.equals(ENTRY_MODIFY))))) { AbstractScriptDependencyTracker.this.dependencyChanged(file.getPath()); } - }}; + } + }; } protected void dependencyChanged(String dependency) { diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java index d662846b657..9433eb95c1a 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/factory/ScriptModuleHandlerFactory.java @@ -100,6 +100,7 @@ private void onHandlerRemoval(ScriptActionHandler handler) { public void onDependencyChange(String engineIdentifier) { ScriptActionHandler handler = trackedHandlers.get(engineIdentifier); if (handler != null) { + logger.debug("Resetting script engine for script {}", engineIdentifier); handler.resetScriptEngine(); } } From 40bdf7f8673a936057edafc01930266226d12579 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 20 Nov 2022 21:59:16 +0100 Subject: [PATCH 6/8] improvement Signed-off-by: Jan N. Klug --- bundles/org.openhab.core.automation.module.script/pom.xml | 6 ------ .../module/script/internal/ScriptEngineManagerImplTest.java | 3 +-- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script/pom.xml b/bundles/org.openhab.core.automation.module.script/pom.xml index ea137aa528b..bfdfea949ea 100644 --- a/bundles/org.openhab.core.automation.module.script/pom.xml +++ b/bundles/org.openhab.core.automation.module.script/pom.xml @@ -25,12 +25,6 @@ org.openhab.core.transform ${project.version} - - org.openhab.core.bundles - org.openhab.core.test - ${project.version} - test - diff --git a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java index 36852a08865..b24bf7b649b 100644 --- a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java +++ b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java @@ -40,7 +40,6 @@ import org.mockito.quality.Strictness; import org.openhab.core.automation.module.script.ScriptDependencyTracker; import org.openhab.core.automation.module.script.ScriptEngineFactory; -import org.openhab.core.test.java.JavaTest; /** * The {@link ScriptEngineManagerImplTest} is a test class for the {@link ScriptEngineManagerImpl} @@ -50,7 +49,7 @@ @NonNullByDefault @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) -public class ScriptEngineManagerImplTest extends JavaTest { +public class ScriptEngineManagerImplTest { private static final String SUPPORTED_SCRIPT_TYPE = "supported"; From 27e34630fb519776c7e3ebb2f1dd0a3f8e7f87f2 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Tue, 29 Nov 2022 20:07:09 +0100 Subject: [PATCH 7/8] address review comments Signed-off-by: Jan N. Klug --- .../loader/AbstractScriptDependencyTracker.java | 7 +++---- .../loader/AbstractScriptFileWatcherTest.java | 9 +++------ 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java index 4cbb5404a90..867926a15b6 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptDependencyTracker.java @@ -85,7 +85,7 @@ protected void processWatchEvent(WatchEvent watchEvent, WatchEvent.Kind ki File file = path.toFile(); if (!file.isHidden() && (kind.equals(ENTRY_DELETE) || (file.canRead() && (kind.equals(ENTRY_CREATE) || kind.equals(ENTRY_MODIFY))))) { - AbstractScriptDependencyTracker.this.dependencyChanged(file.getPath()); + dependencyChanged(file.getPath()); } } }; @@ -93,8 +93,7 @@ protected void processWatchEvent(WatchEvent watchEvent, WatchEvent.Kind ki protected void dependencyChanged(String dependency) { Set scripts = new HashSet<>(scriptToLibs.getKeys(dependency)); // take a copy as it will change as we - AbstractScriptDependencyTracker.this.logger.debug("Library {} changed; reimporting {} scripts...", libraryPath, - scripts.size()); + logger.debug("Library {} changed; reimporting {} scripts...", libraryPath, scripts.size()); for (String scriptUrl : scripts) { for (ScriptDependencyTracker.Listener listener : dependencyChangeListeners) { try { @@ -123,7 +122,7 @@ protected void startTracking(String scriptId, String libPath) { /** * Add a dependency change listener * - * Since this is done via service injection and OSGi annotations are not inherited it is required that sub-classes + * Since this is done via service injection and OSGi annotations are not inherited it is required that subclasses * expose this method with proper annotation * * @param listener the dependency change listener diff --git a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java index ed5e94ec4dc..07e75f4f538 100644 --- a/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java +++ b/bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java @@ -59,12 +59,9 @@ class AbstractScriptFileWatcherTest { private @NonNullByDefault({}) AbstractScriptFileWatcher scriptFileWatcher; - @Mock - private @NonNullByDefault({}) ScriptEngineManager scriptEngineManagerMock; - @Mock - private @NonNullByDefault({}) ScriptDependencyTracker scriptDependencyTrackerMock; - @Mock - private @NonNullByDefault({}) ReadyService readyService; + private @Mock @NonNullByDefault({}) ScriptEngineManager scriptEngineManagerMock; + private @Mock @NonNullByDefault({}) ScriptDependencyTracker scriptDependencyTrackerMock; + private @Mock @NonNullByDefault({}) ReadyService readyService; protected @NonNullByDefault({}) @TempDir Path tempScriptDir; From 915a1d53d9de4708daf3833ccb02c55132e6149f Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Tue, 29 Nov 2022 20:43:31 +0100 Subject: [PATCH 8/8] another one Signed-off-by: Jan N. Klug --- .../internal/ScriptEngineManagerImplTest.java | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java index b24bf7b649b..01d876d1a40 100644 --- a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java +++ b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java @@ -53,26 +53,13 @@ public class ScriptEngineManagerImplTest { private static final String SUPPORTED_SCRIPT_TYPE = "supported"; - @Mock - private @NonNullByDefault({}) ScriptExtensionManager scriptExtensionManagerMock; - - @Mock - private @NonNullByDefault({}) ScriptEngineFactory scriptEngineFactoryMock; - - @Mock - private @NonNullByDefault({}) ScriptEngine scriptEngineMock; - - @Mock - private @NonNullByDefault({}) javax.script.ScriptEngineFactory internalScriptEngineFactoryMock; - - @Mock - private @NonNullByDefault({}) ScriptDependencyTracker scriptDependencyTrackerMock; - - @Mock - private @NonNullByDefault({}) ScriptContext scriptContextMock; - - @Mock - private @NonNullByDefault({}) Consumer dependencyListenerMock; + private @Mock @NonNullByDefault({}) ScriptExtensionManager scriptExtensionManagerMock; + private @Mock @NonNullByDefault({}) ScriptEngineFactory scriptEngineFactoryMock; + private @Mock @NonNullByDefault({}) ScriptEngine scriptEngineMock; + private @Mock @NonNullByDefault({}) javax.script.ScriptEngineFactory internalScriptEngineFactoryMock; + private @Mock @NonNullByDefault({}) ScriptDependencyTracker scriptDependencyTrackerMock; + private @Mock @NonNullByDefault({}) ScriptContext scriptContextMock; + private @Mock @NonNullByDefault({}) Consumer dependencyListenerMock; private @NonNullByDefault({}) ScriptEngineManagerImpl scriptEngineManager;