demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2026-07-29 12:34:22 +02:00
Code Issues 1 Packages Projects Releases Wiki Activity

Improve AbstractScriptFileWatcher initialization order and file handling (#3451)

Browse Source 
* Remove initialImport from the constructor of AbstractScriptFileWatcher
- Calling initialImport inside the constructor may cause an NPE in child class.
* Refactor processWatchEvent in AbstractScriptFileWatcher
- Remove directory deletion handling and adapt test to check
for file removals instead.
- Include hidden files in deletions in case they were previously
loaded while not hidden, set to hidden, and then deleted.

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
This commit is contained in:
jimtng
2023-03-18 09:15:20 +01:00
committed by GitHub
parent 79c7ec8379
commit 1e55914e55
2 changed files with 43 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcher.java
+35 -54
View File
@@ -104,16 +104,9 @@ public abstract class AbstractScriptFileWatcher implements WatchService.WatchEve
this.readyService = readyService;
this.watchSubDirectories = watchSubDirectories;
this.watchPath = watchService.getWatchPath().resolve(fileDirectory);
manager.addFactoryChangeListener(this);
readyService.registerTracker(this, new ReadyMarkerFilter().withType(StartLevelService.STARTLEVEL_MARKER_TYPE));
this.scheduler = getScheduler();
currentStartLevel = startLevelService.getStartLevel();
if (currentStartLevel > StartLevelService.STARTLEVEL_MODEL) {
initialImport();
}
// Start with the lowest level to ensure the code in onReadyMarkerAdded runs
this.currentStartLevel = StartLevelService.STARTLEVEL_OSGI;
}
/**
@@ -144,6 +137,9 @@ public abstract class AbstractScriptFileWatcher implements WatchService.WatchEve
} else if (!Files.isDirectory(watchPath)) {
logger.warn("Trying to watch directory {}, however it is a file", watchPath);
}
manager.addFactoryChangeListener(this);
readyService.registerTracker(this, new ReadyMarkerFilter().withType(StartLevelService.STARTLEVEL_MARKER_TYPE));
watchService.registerListener(this, watchPath, watchSubDirectories);
}
@@ -223,27 +219,22 @@ public abstract class AbstractScriptFileWatcher implements WatchService.WatchEve
@Override
public void processWatchEvent(WatchService.Kind kind, Path path) {
if (!initialized.isDone()) {
// discard events if the initial import has not finished
return;
}
Path fullPath = watchPath.resolve(path);
File file = fullPath.toFile();
if (!file.isHidden()) {
if (kind == DELETE) {
if (file.isDirectory()) {
if (watchSubDirectories) {
synchronized (this) {
String prefix = fullPath.getParent().toString();
Set<String> toRemove = scriptMap.keySet().stream().filter(ref -> ref.startsWith(prefix))
.collect(Collectors.toSet());
toRemove.forEach(this::removeFile);
}
}
} else {
removeFile(ScriptFileReference.getScriptIdentifier(file.toPath()));
}
}
if (file.canRead() && (kind == CREATE || kind == MODIFY)) {
addFiles(listFiles(file.toPath(), watchSubDirectories));
// Subdirectory events are filtered out by WatchService, so we only need to deal with files
if (kind == DELETE) {
String scriptIdentifier = ScriptFileReference.getScriptIdentifier(fullPath);
if (scriptMap.containsKey(scriptIdentifier)) {
removeFile(scriptIdentifier);
}
} else if (!file.isHidden() && file.canRead() && (kind == CREATE || kind == MODIFY)) {
addFiles(listFiles(fullPath, watchSubDirectories));
}
}
@@ -269,20 +260,21 @@ public abstract class AbstractScriptFileWatcher implements WatchService.WatchEve
return CompletableFuture.runAsync(() -> {
Lock lock = getLockForScript(scriptIdentifier);
try {
ScriptFileReference ref = scriptMap.remove(scriptIdentifier);
scriptMap.computeIfPresent(scriptIdentifier, (id, ref) -> {
if (ref.getLoadedStatus().get()) {
manager.removeEngine(scriptIdentifier);
logger.debug("Unloaded script '{}'", scriptIdentifier);
} else {
logger.debug("Dequeued script '{}'", scriptIdentifier);
}
if (ref == null) {
logger.warn("Failed to unload script '{}': script reference not found.", scriptIdentifier);
return;
}
if (ref.getLoadedStatus().get()) {
manager.removeEngine(scriptIdentifier);
logger.debug("Unloaded script '{}'", scriptIdentifier);
} else {
logger.debug("Dequeued script '{}'", scriptIdentifier);
}
return null;
});
} finally {
if (scriptMap.containsKey(scriptIdentifier)) {
logger.warn("Failed to unload script '{}'", scriptIdentifier);
}
scriptLockMap.remove(scriptIdentifier);
lock.unlock();
}
@@ -353,20 +345,6 @@ public abstract class AbstractScriptFileWatcher implements WatchService.WatchEve
return false;
}
private void initialImport() {
File directory = watchPath.toFile();
if (!directory.exists()) {
if (!directory.mkdirs()) {
logger.warn("Failed to create watched directory: {}", watchPath);
}
} else if (directory.isFile()) {
logger.warn("Trying to watch directory {}, however it is a file", watchPath);
}
addFiles(listFiles(directory.toPath(), watchSubDirectories)).thenRun(() -> initialized.complete(null));
}
@Override
public void onDependencyChange(String scriptIdentifier) {
logger.debug("Reimporting {}...", scriptIdentifier);
@@ -381,13 +359,16 @@ public abstract class AbstractScriptFileWatcher implements WatchService.WatchEve
int previousLevel = currentStartLevel;
currentStartLevel = Integer.parseInt(readyMarker.getIdentifier());
logger.trace("Added ready marker {}: start level changed from {} to {}. watchPath: {}", readyMarker,
previousLevel, currentStartLevel, watchPath);
if (currentStartLevel < StartLevelService.STARTLEVEL_STATES) {
// ignore start level less than 30
return;
}
if (currentStartLevel == StartLevelService.STARTLEVEL_STATES) {
initialImport();
if (!initialized.isDone()) {
addFiles(listFiles(watchPath, watchSubDirectories)).thenRun(() -> initialized.complete(null));
} else {
scriptMap.values().stream().sorted()
.filter(ref -> needsStartLevelProcessing(ref, previousLevel, currentStartLevel))
bundles/org.openhab.core.automation.module.script.rulesupport/src/test/java/org/openhab/core/automation/module/script/rulesupport/loader/AbstractScriptFileWatcherTest.java
+8 -3
View File
@@ -554,7 +554,7 @@ class AbstractScriptFileWatcherTest extends JavaTest {
}
@Test
public void testDirectoryRemoved() {
public void testFileRemoved() {
scriptFileWatcher = createScriptFileWatcher(true);
when(scriptEngineManagerMock.isSupported("js")).thenReturn(true);
ScriptEngineContainer scriptEngineContainer = mock(ScriptEngineContainer.class);
@@ -565,16 +565,18 @@ class AbstractScriptFileWatcherTest extends JavaTest {
Path p1 = getFile("dir/script.js");
Path p2 = getFile("dir/script2.js");
Path d = p1.getParent();
scriptFileWatcher.processWatchEvent(CREATE, p1);
scriptFileWatcher.processWatchEvent(CREATE, p2);
scriptFileWatcher.processWatchEvent(DELETE, d);
awaitEmptyQueue();
verify(scriptEngineManagerMock, timeout(DEFAULT_TEST_TIMEOUT_MS)).createScriptEngine("js", p1.toString());
verify(scriptEngineManagerMock, timeout(DEFAULT_TEST_TIMEOUT_MS)).createScriptEngine("js", p2.toString());
scriptFileWatcher.processWatchEvent(DELETE, p1);
scriptFileWatcher.processWatchEvent(DELETE, p2);
verify(scriptEngineManagerMock, timeout(DEFAULT_TEST_TIMEOUT_MS)).removeEngine(p1.toString());
verify(scriptEngineManagerMock, timeout(DEFAULT_TEST_TIMEOUT_MS)).removeEngine(p2.toString());
}
@@ -622,6 +624,9 @@ class AbstractScriptFileWatcherTest extends JavaTest {
scriptFileWatcher = createScriptFileWatcher(true);
when(startLevelServiceMock.getStartLevel()).thenReturn(StartLevelService.STARTLEVEL_RULEENGINE);
AbstractScriptFileWatcher watcher = createScriptFileWatcher();
watcher.activate();
watcher.onReadyMarkerAdded(new ReadyMarker(StartLevelService.STARTLEVEL_MARKER_TYPE,
Integer.toString(StartLevelService.STARTLEVEL_STATES)));
waitForAssert(() -> assertThat(watcher.ifInitialized().isDone(), is(true)));