Skip to content

Commit

Permalink
Fixed IndexOutOfBoundsException in ScriptModuleTypeProvider (#1730)
Browse files Browse the repository at this point in the history 
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
  • Loading branch information
@cweitkamp
cweitkamp committed Oct 19, 2020
1 parent b2c045d commit 7d70a97
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 42 deletions.
35 changes: 17 additions & 18 deletions ...main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@
public class ScriptEngineManagerImpl implements ScriptEngineManager {

private final Logger logger = LoggerFactory.getLogger(ScriptEngineManagerImpl.class);
private final Map<String, ScriptEngineContainer> loadedScriptEngineInstances = new HashMap<>();
private final Map<String, ScriptEngineFactory> customSupport = new HashMap<>();
private final Map<String, ScriptEngineFactory> genericSupport = new HashMap<>();
private final Map<String, @Nullable ScriptEngineContainer> loadedScriptEngineInstances = new HashMap<>();
private final Map<String, @Nullable ScriptEngineFactory> customSupport = new HashMap<>();
private final Map<String, @Nullable ScriptEngineFactory> genericSupport = new HashMap<>();
private final ScriptExtensionManager scriptExtensionManager;

@Activate
Expand All @@ -57,7 +57,6 @@ public ScriptEngineManagerImpl(final @Reference ScriptExtensionManager scriptExt
@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
public void addScriptEngineFactory(ScriptEngineFactory engineFactory) {
List<String> scriptTypes = engineFactory.getScriptTypes();

logger.trace("{}.getScriptTypes(): {}", engineFactory.getClass().getSimpleName(), scriptTypes);
for (String scriptType : scriptTypes) {
if (isCustomFactory(engineFactory)) {
Expand All @@ -66,26 +65,27 @@ public void addScriptEngineFactory(ScriptEngineFactory engineFactory) {
this.genericSupport.put(scriptType, engineFactory);
}
}
if (!engineFactory.getScriptTypes().isEmpty()) {
ScriptEngine scriptEngine = engineFactory.createScriptEngine(engineFactory.getScriptTypes().get(0));
if (scriptEngine != null) {
javax.script.ScriptEngineFactory factory = scriptEngine.getFactory();
logger.debug(
"Initialized a {} ScriptEngineFactory for {} ({}): supports {} ({}) with file extensions {}, names {}, and mimetypes {}",
(isCustomFactory(engineFactory)) ? "custom" : "generic", factory.getEngineName(),
factory.getEngineVersion(), factory.getLanguageName(), factory.getLanguageVersion(),
factory.getExtensions(), factory.getNames(), factory.getMimeTypes());
if (logger.isDebugEnabled()) {
if (!scriptTypes.isEmpty()) {
ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0));
if (scriptEngine != null) {
javax.script.ScriptEngineFactory factory = scriptEngine.getFactory();
logger.debug(
"Initialized a {} ScriptEngineFactory for {} ({}): supports {} ({}) with file extensions {}, names {}, and mimetypes {}",
(isCustomFactory(engineFactory)) ? "custom" : "generic", factory.getEngineName(),
factory.getEngineVersion(), factory.getLanguageName(), factory.getLanguageVersion(),
factory.getExtensions(), factory.getNames(), factory.getMimeTypes());
} else {
logger.trace("addScriptEngineFactory: engine was null");
}
} else {
logger.trace("addScriptEngineFactory: engine was null");
logger.trace("addScriptEngineFactory: scriptTypes was empty");
}
} else {
logger.trace("addScriptEngineFactory: scriptTypes was empty");
}
}

public void removeScriptEngineFactory(ScriptEngineFactory engineFactory) {
List<String> scriptTypes = engineFactory.getScriptTypes();

logger.trace("{}.getScriptTypes(): {}", engineFactory.getClass().getSimpleName(), scriptTypes);
for (String scriptType : scriptTypes) {
if (isCustomFactory(engineFactory)) {
Expand Down Expand Up @@ -150,7 +150,6 @@ public void loadScript(String engineIdentifier, InputStreamReader scriptData) {
ScriptEngine engine = container.getScriptEngine();
try {
engine.eval(scriptData);

if (engine instanceof Invocable) {
Invocable inv = (Invocable) engine;
try {
Expand Down
42 changes: 24 additions & 18 deletions ...org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import java.util.Locale;
import java.util.Map;
import java.util.TreeMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.script.ScriptEngine;

Expand Down Expand Up @@ -77,7 +75,6 @@ public class ScriptModuleTypeProvider implements ModuleTypeProvider {
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",
Expand Down Expand Up @@ -113,13 +110,12 @@ private List<ConfigDescriptionParameter> getConfigDescriptions(@Nullable Locale
final ConfigDescriptionParameter script = ConfigDescriptionParameterBuilder.create("script", Type.TEXT)
.withRequired(true).withReadOnly(false).withMultiple(false).withLabel("Script").withContext("script")
.withDescription("the script to execute").build();
return Stream.of(scriptType, script).collect(Collectors.toList());
return List.of(scriptType, script);
}

@Override
public Collection<ModuleType> getModuleTypes(@Nullable Locale locale) {
return (Collection<ModuleType>) Stream.of(getScriptActionType(locale), getScriptConditionType(locale))
.collect(Collectors.toList());
return (Collection<ModuleType>) List.of(getScriptActionType(locale), getScriptConditionType(locale));
}

@Override
Expand All @@ -143,24 +139,34 @@ public void removeProviderChangeListener(ProviderChangeListener<ModuleType> list
*/
@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
public void setScriptEngineFactory(ScriptEngineFactory engineFactory) {
ScriptEngine scriptEngine = engineFactory.createScriptEngine(engineFactory.getScriptTypes().get(0));
if (scriptEngine != null) {
javax.script.ScriptEngineFactory factory = scriptEngine.getFactory();
parameterOptions.put(getPreferredMimeType(factory), getLanguageName(factory));
logger.trace("ParameterOptions: {}", parameterOptions);
List<String> scriptTypes = engineFactory.getScriptTypes();
if (!scriptTypes.isEmpty()) {
ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0));
if (scriptEngine != null) {
javax.script.ScriptEngineFactory factory = scriptEngine.getFactory();
parameterOptions.put(getPreferredMimeType(factory), getLanguageName(factory));
logger.trace("ParameterOptions: {}", parameterOptions);
} else {
logger.trace("setScriptEngineFactory: engine was null");
}
} else {
logger.trace("setScriptEngineFactory: engine was null");
logger.trace("addScriptEngineFactory: scriptTypes was empty");
}
}

public void unsetScriptEngineFactory(ScriptEngineFactory engineFactory) {
ScriptEngine scriptEngine = engineFactory.createScriptEngine(engineFactory.getScriptTypes().get(0));
if (scriptEngine != null) {
javax.script.ScriptEngineFactory factory = scriptEngine.getFactory();
parameterOptions.remove(getPreferredMimeType(factory));
logger.trace("ParameterOptions: {}", parameterOptions);
List<String> scriptTypes = engineFactory.getScriptTypes();
if (!scriptTypes.isEmpty()) {
ScriptEngine scriptEngine = engineFactory.createScriptEngine(scriptTypes.get(0));
if (scriptEngine != null) {
javax.script.ScriptEngineFactory factory = scriptEngine.getFactory();
parameterOptions.remove(getPreferredMimeType(factory));
logger.trace("ParameterOptions: {}", parameterOptions);
} else {
logger.trace("unsetScriptEngineFactory: engine was null");
}
} else {
logger.trace("unsetScriptEngineFactory: engine was null");
logger.trace("unsetScriptEngineFactory: scriptTypes was empty");
}
}

Expand Down
8 changes: 5 additions & 3 deletions ...rg.openhab.core.automation/src/main/java/org/openhab/core/automation/type/ActionType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.List;
import java.util.Set;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.automation.Action;
import org.openhab.core.automation.Module;
Expand All @@ -32,6 +33,7 @@
* @author Ana Dimova - Initial contribution
* @author Vasil Ilchev - Initial contribution
*/
@NonNullByDefault
public class ActionType extends ModuleType {

/**
Expand All @@ -57,7 +59,7 @@ public class ActionType extends ModuleType {
* {@link Action} instances.
*/
public ActionType(@Nullable String UID, @Nullable List<ConfigDescriptionParameter> configDescriptions,
List<Input> inputs) {
@Nullable List<Input> inputs) {
this(UID, configDescriptions, inputs, null);
}

Expand Down Expand Up @@ -101,8 +103,8 @@ public ActionType(@Nullable String UID, @Nullable List<ConfigDescriptionParamete
@Nullable String label, @Nullable String description, @Nullable Set<String> tags,
@Nullable Visibility visibility, @Nullable List<Input> inputs, @Nullable List<Output> outputs) {
super(UID, configDescriptions, label, description, tags, visibility);
this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : Collections.emptyList();
this.outputs = outputs != null ? Collections.unmodifiableList(outputs) : Collections.emptyList();
this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : List.of();
this.outputs = outputs != null ? Collections.unmodifiableList(outputs) : List.of();
}

/**
Expand Down
7 changes: 4 additions & 3 deletions ...openhab.core.automation/src/main/java/org/openhab/core/automation/type/ConditionType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.List;
import java.util.Set;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.automation.Condition;
import org.openhab.core.automation.Visibility;
Expand All @@ -31,6 +32,7 @@
* @author Ana Dimova - Initial contribution
* @author Vasil Ilchev - Initial contribution
*/
@NonNullByDefault
public class ConditionType extends ModuleType {

private final List<Input> inputs;
Expand All @@ -47,8 +49,7 @@ public class ConditionType extends ModuleType {
*/
public ConditionType(@Nullable String UID, @Nullable List<ConfigDescriptionParameter> configDescriptions,
@Nullable List<Input> inputs) {
super(UID, configDescriptions);
this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : Collections.emptyList();
this(UID, configDescriptions, null, null, null, null, inputs);
}

/**
Expand All @@ -72,7 +73,7 @@ public ConditionType(@Nullable String UID, @Nullable List<ConfigDescriptionParam
@Nullable String label, @Nullable String description, @Nullable Set<String> tags,
@Nullable Visibility visibility, @Nullable List<Input> inputs) {
super(UID, configDescriptions, label, description, tags, visibility);
this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : Collections.emptyList();
this.inputs = inputs != null ? Collections.unmodifiableList(inputs) : List.of();
}

/**
Expand Down

0 comments on commit 7d70a97

Please sign in to comment.