Fix rules stay uninitialized when using Java 17 (#2787)

On Java 17 there is no Nashorn scripting engine so it takes a bit longer before ScriptEngines are available. Rules would stay uninitialized forever because the ScriptModuleTypeProvider did not notify its listeners whenever script.ScriptAction, script.ScriptCondition became available. Signed-off-by: Wouter Born <[email protected]>
openhab · Feb 20, 2022 · 2e0b242 · 2e0b242
1 parent 5e33cfc
commit 2e0b242
Showing 1 changed file with 42 additions and 34 deletions.
diff --git a/...org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java b/...org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java
@@ -17,10 +17,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Optional;
 import java.util.TreeMap;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import javax.script.ScriptEngine;
 
@@ -36,12 +33,13 @@
 import org.openhab.core.automation.type.ModuleType;
 import org.openhab.core.automation.type.ModuleTypeProvider;
 import org.openhab.core.automation.type.Output;
-import org.openhab.core.common.registry.ProviderChangeListener;
+import org.openhab.core.common.registry.AbstractProvider;
 import org.openhab.core.config.core.ConfigDescriptionParameter;
 import org.openhab.core.config.core.ConfigDescriptionParameter.Type;
 import org.openhab.core.config.core.ConfigDescriptionParameterBuilder;
 import org.openhab.core.config.core.ParameterOption;
 import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.osgi.service.component.annotations.ReferencePolicy;
@@ -56,15 +54,23 @@
  */
 @NonNullByDefault
 @Component
-public class ScriptModuleTypeProvider implements ModuleTypeProvider {
+public class ScriptModuleTypeProvider extends AbstractProvider<ModuleType> implements ModuleTypeProvider {
 
     private final Logger logger = LoggerFactory.getLogger(ScriptModuleTypeProvider.class);
     private final Map<String, String> parameterOptions = new TreeMap<>();
 
+    @Deactivate
+    public void deactivate() {
+        listeners.clear();
+        parameterOptions.clear();
+    }
+
     @SuppressWarnings("unchecked")
     @Override
     public @Nullable ModuleType getModuleType(String UID, @Nullable Locale locale) {
-        if (ScriptActionHandler.TYPE_ID.equals(UID)) {
+        if (parameterOptions.isEmpty()) {
+            return null;
+        } else if (ScriptActionHandler.TYPE_ID.equals(UID)) {
             return getScriptActionType(locale);
         } else if (ScriptConditionHandler.TYPE_ID.equals(UID)) {
             return getScriptConditionType(locale);
@@ -73,26 +79,22 @@ public class ScriptModuleTypeProvider implements ModuleTypeProvider {
         }
     }
 
-    private @Nullable ModuleType getScriptActionType(@Nullable Locale locale) {
-        if (parameterOptions.isEmpty()) {
-            return null;
-        } else {
-            List<Output> outputs = new ArrayList<>();
-            Output result = new Output("result", "java.lang.Object", "result", "the script result", null, null, null);
-            outputs.add(result);
-            return new ActionType(ScriptActionHandler.TYPE_ID, getConfigDescriptions(locale), "execute a given script",
-                    "Allows the execution of a user-defined script.", null, Visibility.VISIBLE, null, outputs);
-        }
+    private ModuleType getScriptActionType(@Nullable Locale locale) {
+        List<Output> outputs = new ArrayList<>();
+        Output result = new Output("result", "java.lang.Object", "result", "the script result", null, null, null);
+        outputs.add(result);
+        return new ActionType(ScriptActionHandler.TYPE_ID, getConfigDescriptions(locale), "execute a given script",
+                "Allows the execution of a user-defined script.", null, Visibility.VISIBLE, null, outputs);
     }
 
-    private @Nullable ModuleType getScriptConditionType(@Nullable Locale locale) {
-        if (parameterOptions.isEmpty()) {
-            return null;
-        } else {
-            return new ConditionType(ScriptConditionHandler.TYPE_ID, getConfigDescriptions(locale),
-                    "a given script evaluates to true", "Allows the definition of a condition through a script.", null,
-                    Visibility.VISIBLE, null);
-        }
+    private ModuleType getScriptConditionType(@Nullable Locale locale) {
+        return new ConditionType(ScriptConditionHandler.TYPE_ID, getConfigDescriptions(locale),
+                "a given script evaluates to true", "Allows the definition of a condition through a script.", null,
+                Visibility.VISIBLE, null);
+    }
+
+    private List<ModuleType> getModuleTypesUnconditionally(@Nullable Locale locale) {
+        return List.of(getScriptActionType(locale), getScriptConditionType(locale));
     }
 
     /**
@@ -118,25 +120,24 @@ private List<ConfigDescriptionParameter> getConfigDescriptions(@Nullable Locale
 
     @Override
     public Collection<ModuleType> getModuleTypes(@Nullable Locale locale) {
-        return Stream
-                .of(Optional.ofNullable(getScriptActionType(locale)),
-                        Optional.ofNullable(getScriptConditionType(locale)))
-                .filter(Optional::isPresent).map(Optional::get).collect(Collectors.toUnmodifiableList());
+        return parameterOptions.isEmpty() ? List.of() : getModuleTypesUnconditionally(locale);
     }
 
     @Override
     public Collection<ModuleType> getAll() {
         return getModuleTypes(null);
     }
 
-    @Override
-    public void addProviderChangeListener(ProviderChangeListener<ModuleType> listener) {
-        // does nothing because this provider does not change
+    private void notifyModuleTypesAdded() {
+        for (ModuleType moduleType : getModuleTypesUnconditionally(null)) {
+            notifyListenersAboutAddedElement(moduleType);
+        }
     }
 
-    @Override
-    public void removeProviderChangeListener(ProviderChangeListener<ModuleType> listener) {
-        // does nothing because this provider does not change
+    private void notifyModuleTypesRemoved() {
+        for (ModuleType moduleType : getModuleTypesUnconditionally(null)) {
+            notifyListenersAboutRemovedElement(moduleType);
+        }
     }
 
     /**
@@ -149,8 +150,12 @@ public void setScriptEngineFactory(ScriptEngineFactory engineFactory) {
         if (!scriptTypes.isEmpty()) {
             ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0));
             if (scriptEngine != null) {
+                boolean notifyListeners = parameterOptions.isEmpty();
                 parameterOptions.put(getPreferredMimeType(engineFactory), getLanguageName(scriptEngine.getFactory()));
                 logger.trace("ParameterOptions: {}", parameterOptions);
+                if (notifyListeners) {
+                    notifyModuleTypesAdded();
+                }
             } else {
                 logger.trace("setScriptEngineFactory: engine was null");
             }
@@ -166,6 +171,9 @@ public void unsetScriptEngineFactory(ScriptEngineFactory engineFactory) {
             if (scriptEngine != null) {
                 parameterOptions.remove(getPreferredMimeType(engineFactory));
                 logger.trace("ParameterOptions: {}", parameterOptions);
+                if (parameterOptions.isEmpty()) {
+                    notifyModuleTypesRemoved();
+                }
             } else {
                 logger.trace("unsetScriptEngineFactory: engine was null");
             }