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 1bec9d49f..3fd277d31 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 @@ -24,9 +24,11 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.nio.file.WatchEvent; import java.nio.file.WatchEvent.Kind; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -138,41 +140,52 @@ public abstract class AbstractScriptFileWatcher extends AbstractWatchService imp if (rootDirectory.exists()) { File[] files = rootDirectory.listFiles(); if (files != null) { + Collection resources = new TreeSet<>(); for (File f : files) { if (!f.isHidden()) { - importResources(f); + resources.addAll(collectResources(f)); } } + resources.forEach(this::importFileWhenReady); } } } /** - * Imports resources from the specified file or directory. + * Collects all resources from the specified file or directory, + * possibly including subdirectories. + * + * The results will be sorted. * * @param file the file or directory to import resources from */ - private void importResources(File file) { - if (file.exists()) { - File[] files = file.listFiles(); - if (files != null) { - if (watchSubDirectories()) { + private Collection collectResources(File file) { + Collection resources = new TreeSet<>(); + if (!file.exists()) { + return resources; + } + + if (file.isDirectory()) { + if (watchSubDirectories()) { + File[] files = file.listFiles(); + if (files != null) { for (File f : files) { if (!f.isHidden()) { - importResources(f); + resources.addAll(collectResources(f)); } } } - } else { - try { - URL url = file.toURI().toURL(); - importFileWhenReady(new ScriptFileReference(url)); - } catch (MalformedURLException e) { - // can't happen for the 'file' protocol handler with a correctly formatted URI - logger.debug("Can't create a URL", e); - } + } + } else { + try { + URL url = file.toURI().toURL(); + resources.add(new ScriptFileReference(url)); + } catch (MalformedURLException e) { + // can't happen for the 'file' protocol handler with a correctly formatted URI + logger.debug("Can't create a URL", e); } } + return resources; } @Override @@ -190,13 +203,24 @@ public abstract class AbstractScriptFileWatcher extends AbstractWatchService imp File file = path.toFile(); if (!file.isHidden()) { try { - URL fileUrl = file.toURI().toURL(); if (ENTRY_DELETE.equals(kind)) { - removeFile(new ScriptFileReference(fileUrl)); + if (file.isDirectory()) { + if (watchSubDirectories()) { + synchronized (this) { + String prefix = file.toString() + File.separator; + var toRemove = loaded.stream() + .filter(f -> f.getScriptFileURL().getFile().startsWith(prefix)) + .collect(Collectors.toList()); + toRemove.forEach(this::removeFile); + } + } + } else { + removeFile(new ScriptFileReference(file.toURI().toURL())); + } } if (file.canRead() && (ENTRY_CREATE.equals(kind) || ENTRY_MODIFY.equals(kind))) { - importFileWhenReady(new ScriptFileReference(fileUrl)); + collectResources(file).forEach(this::importFileWhenReady); } } catch (MalformedURLException e) { logger.error("malformed", e); @@ -289,9 +313,7 @@ public abstract class AbstractScriptFileWatcher extends AbstractWatchService imp pending.removeAll(newlySupported); } - for (ScriptFileReference ref : newlySupported) { - importFileWhenReady(ref); - } + newlySupported.forEach(this::importFileWhenReady); } private synchronized void onStartLevelChanged(int newLevel) { 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 07e75f4f5..b9535f235 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 @@ -363,4 +363,69 @@ class AbstractScriptFileWatcherTest { verify(scriptEngineManagerMock, timeout(10000).times(2)).createScriptEngine("js", p.toFile().toURI().toString()); } + + @Test + public void testDirectoryAdded() { + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); + ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); + when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + + updateStartLevel(100); + + Path p1 = getFile("dir/script.js"); + Path p2 = getFile("dir/script2.js"); + Path d = p1.getParent(); + + scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, d); + + verify(scriptEngineManagerMock, timeout(10000)).createScriptEngine("js", p1.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000)).createScriptEngine("js", p2.toFile().toURI().toString()); + } + + @Test + public void testSortsAllFilesInNewDirectory() { + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); + ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); + when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + + updateStartLevel(100); + + Path p20 = getFile("dir/script.sl20.js"); + Path p10 = getFile("dir/script2.sl10.js"); + Path d = p10.getParent(); + + scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, d); + + InOrder inOrder = inOrder(scriptEngineManagerMock); + + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p10.toFile().toURI().toString()); + inOrder.verify(scriptEngineManagerMock, timeout(10000).times(1)).createScriptEngine("js", + p20.toFile().toURI().toString()); + } + + @Test + public void testDirectoryRemoved() { + when(scriptEngineManagerMock.isSupported("js")).thenReturn(true); + ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class); + when(scriptEngineContainer.getScriptEngine()).thenReturn(mock(ScriptEngine.class)); + when(scriptEngineManagerMock.createScriptEngine(anyString(), anyString())).thenReturn(scriptEngineContainer); + + updateStartLevel(100); + + Path p1 = getFile("dir/script.js"); + Path p2 = getFile("dir/script2.js"); + Path d = p1.getParent(); + + scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p1); + scriptFileWatcher.processWatchEvent(null, ENTRY_CREATE, p2); + scriptFileWatcher.processWatchEvent(null, ENTRY_DELETE, d); + + verify(scriptEngineManagerMock, timeout(10000)).createScriptEngine("js", p1.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000)).createScriptEngine("js", p2.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000)).removeEngine(p1.toFile().toURI().toString()); + verify(scriptEngineManagerMock, timeout(10000)).removeEngine(p2.toFile().toURI().toString()); + } }