From fe9af132aa1953d68a22257b002267c460f3d01c Mon Sep 17 00:00:00 2001 From: Arne Seime Date: Sat, 15 Feb 2025 17:28:46 +0100 Subject: [PATCH] Rule Template installation fixes (#4591) * Fix file based rule templates * Add YAML Template parser * Refactor marketplace rule template parsing * Prevent file system access for WatchService DELETE events Trying to check if deleted files are hidden, are readable or are directories will result in IOExceptions on many file systems, so that no action will be taken for deletions. Signed-off-by: Arne Seime --- .../MarketplaceRuleTemplateProvider.java | 147 ++++++++++++------ .../CommunityRuleTemplateAddonHandler.java | 6 +- .../AbstractScriptDependencyTracker.java | 2 +- .../jackson/AbstractJacksonYAMLParser.java | 48 ++++++ .../parser/jackson/TemplateYAMLParser.java | 69 ++++++++ .../provider/file/AbstractFileProvider.java | 27 +++- .../provider/file/AutomationWatchService.java | 21 ++- .../file/ModuleTypeFileProviderWatcher.java | 15 ++ .../file/TemplateFileProviderWatcher.java | 19 +++ .../core/automation/parser/Parser.java | 7 +- .../parser/ParsingNestedException.java | 4 +- .../parser/ValidationException.java | 124 +++++++++++++++ .../internal/ConfigDispatcherFileWatcher.java | 2 +- .../internal/YamlModelRepositoryImpl.java | 53 +++---- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- .../itest.bndrun | 4 +- 23 files changed, 484 insertions(+), 96 deletions(-) create mode 100644 bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/AbstractJacksonYAMLParser.java create mode 100644 bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/TemplateYAMLParser.java create mode 100644 bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/parser/ValidationException.java diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/automation/MarketplaceRuleTemplateProvider.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/automation/MarketplaceRuleTemplateProvider.java index 7ab911d4ff..d0f0b5d63a 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/automation/MarketplaceRuleTemplateProvider.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/automation/MarketplaceRuleTemplateProvider.java @@ -19,14 +19,20 @@ import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.HashSet; import java.util.Locale; +import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.automation.Module; import org.openhab.core.automation.dto.RuleTemplateDTO; import org.openhab.core.automation.dto.RuleTemplateDTOMapper; import org.openhab.core.automation.parser.Parser; import org.openhab.core.automation.parser.ParsingException; +import org.openhab.core.automation.parser.ParsingNestedException; +import org.openhab.core.automation.parser.ValidationException; +import org.openhab.core.automation.parser.ValidationException.ObjectType; import org.openhab.core.automation.template.RuleTemplate; import org.openhab.core.automation.template.RuleTemplateProvider; import org.openhab.core.common.registry.AbstractManagedProvider; @@ -34,8 +40,8 @@ import org.openhab.core.storage.StorageService; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.osgi.service.component.annotations.ReferencePolicy; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; @@ -46,27 +52,48 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; * * @author Kai Kreuzer - Initial contribution and API * @author Yannick Schaus - refactoring - * + * @author Arne Seime - refactored rule template parsing */ @NonNullByDefault @Component(service = { MarketplaceRuleTemplateProvider.class, RuleTemplateProvider.class }) public class MarketplaceRuleTemplateProvider extends AbstractManagedProvider implements RuleTemplateProvider { - private final Logger logger = LoggerFactory.getLogger(MarketplaceRuleTemplateProvider.class); - - private final Parser parser; + private final Map> parsers = new ConcurrentHashMap<>(); ObjectMapper yamlMapper; @Activate - public MarketplaceRuleTemplateProvider(final @Reference StorageService storageService, - final @Reference(target = "(&(format=json)(parser.type=parser.template))") Parser parser) { + public MarketplaceRuleTemplateProvider(final @Reference StorageService storageService) { super(storageService); - this.parser = parser; this.yamlMapper = new ObjectMapper(new YAMLFactory()); yamlMapper.findAndRegisterModules(); } + /** + * Registers a {@link Parser}. + * + * @param parser the {@link Parser} service to register. + * @param properties the properties. + */ + @Reference(cardinality = ReferenceCardinality.AT_LEAST_ONE, policy = ReferencePolicy.DYNAMIC, target = "(parser.type=parser.template)") + public void addParser(Parser parser, Map properties) { + String parserType = properties.get(Parser.FORMAT); + parserType = parserType == null ? Parser.FORMAT_JSON : parserType; + parsers.put(parserType, parser); + } + + /** + * Unregisters a {@link Parser}. + * + * @param parser the {@link Parser} service to unregister. + * @param properties the properties. + */ + public void removeParser(Parser parser, Map properties) { + String parserType = properties.get(Parser.FORMAT); + parserType = parserType == null ? Parser.FORMAT_JSON : parserType; + parsers.remove(parserType); + } + @Override public @Nullable RuleTemplate getTemplate(String uid, @Nullable Locale locale) { return get(uid); @@ -98,55 +125,83 @@ public class MarketplaceRuleTemplateProvider extends AbstractManagedProvider parser = parsers.get(format); + + // The parser might not have been registered yet + if (parser == null) { + throw new ParsingException(new ParsingNestedException(ParsingNestedException.TEMPLATE, + "No " + format.toUpperCase(Locale.ROOT) + " parser available", null)); + } + try (InputStreamReader isr = new InputStreamReader( - new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8)))) { + new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8)))) { Set templates = parser.parse(isr); - if (templates.size() != 1) { - throw new IllegalArgumentException("JSON must contain exactly one template!"); - } else { - RuleTemplate entry = templates.iterator().next(); - // add a tag with the add-on ID to be able to identify the widget in the registry - entry.getTags().add(uid); - RuleTemplate template = new RuleTemplate(entry.getUID(), entry.getLabel(), entry.getDescription(), - entry.getTags(), entry.getTriggers(), entry.getConditions(), entry.getActions(), - entry.getConfigurationDescriptions(), entry.getVisibility()); - add(template); + + // Add a tag with the marketplace add-on ID to be able to identify the template in the registry + Set tags; + for (RuleTemplate template : templates) { + validateTemplate(template); + tags = new HashSet(template.getTags()); + tags.add(uid); + add(new RuleTemplate(template.getUID(), template.getLabel(), template.getDescription(), tags, + template.getTriggers(), template.getConditions(), template.getActions(), + template.getConfigurationDescriptions(), template.getVisibility())); } } catch (IOException e) { - logger.error("Cannot close input stream.", e); + // Impossible for ByteArrayInputStream } } /** - * This adds a new rule template to the persistent storage from its YAML representation. + * Validates that the parsed template is valid. * - * @param uid the UID to be used for the template - * @param yaml the template content as a YAML string - * - * @throws ParsingException if the content cannot be parsed correctly + * @param template the {@link RuleTemplate} to validate. + * @throws ValidationException If the validation failed. */ - public void addTemplateAsYAML(String uid, String yaml) throws ParsingException { - try { - RuleTemplateDTO dto = yamlMapper.readValue(yaml, RuleTemplateDTO.class); - // add a tag with the add-on ID to be able to identify the widget in the registry - dto.tags = new HashSet<@Nullable String>((dto.tags != null) ? dto.tags : new HashSet<>()); - dto.tags.add(uid); - RuleTemplate entry = RuleTemplateDTOMapper.map(dto); - RuleTemplate template = new RuleTemplate(entry.getUID(), entry.getLabel(), entry.getDescription(), - entry.getTags(), entry.getTriggers(), entry.getConditions(), entry.getActions(), - entry.getConfigurationDescriptions(), entry.getVisibility()); - add(template); - } catch (IOException e) { - logger.error("Unable to parse YAML: {}", e.getMessage()); - throw new IllegalArgumentException("Unable to parse YAML"); + @SuppressWarnings("null") + protected void validateTemplate(RuleTemplate template) throws ValidationException { + String s; + if ((s = template.getUID()) == null || s.isBlank()) { + throw new ValidationException(ObjectType.TEMPLATE, null, "UID cannot be blank"); + } + if ((s = template.getLabel()) == null || s.isBlank()) { + throw new ValidationException(ObjectType.TEMPLATE, template.getUID(), "Label cannot be blank"); + } + if (template.getModules(Module.class).isEmpty()) { + throw new ValidationException(ObjectType.TEMPLATE, template.getUID(), "There must be at least one module"); } } } diff --git a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java index 82e4e844f2..4608882ab9 100644 --- a/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java +++ b/bundles/org.openhab.core.addon.marketplace/src/main/java/org/openhab/core/addon/marketplace/internal/community/CommunityRuleTemplateAddonHandler.java @@ -85,10 +85,10 @@ public class CommunityRuleTemplateAddonHandler implements MarketplaceAddonHandle } } catch (IOException e) { logger.error("Rule template from marketplace cannot be downloaded: {}", e.getMessage()); - throw new MarketplaceHandlerException("Template cannot be downloaded.", e); + throw new MarketplaceHandlerException("Rule template cannot be downloaded", e); } catch (Exception e) { - logger.error("Rule template from marketplace is invalid: {}", e.getMessage()); - throw new MarketplaceHandlerException("Template is not valid.", e); + logger.error("Failed to add rule template from the marketplace: {}", e.getMessage()); + throw new MarketplaceHandlerException("Rule template is invalid", e); } } 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 93920dc373..ccd5af86b8 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 @@ -80,7 +80,7 @@ public abstract class AbstractScriptDependencyTracker @Override public void processWatchEvent(WatchService.Kind kind, Path path) { File file = libraryPath.resolve(path).toFile(); - if (!file.isHidden() && (kind == DELETE || (file.canRead() && (kind == CREATE || kind == MODIFY)))) { + if (kind == DELETE || (!file.isHidden() && file.canRead() && (kind == CREATE || kind == MODIFY))) { dependencyChanged(file.toString()); } } diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/AbstractJacksonYAMLParser.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/AbstractJacksonYAMLParser.java new file mode 100644 index 0000000000..47c0c1a266 --- /dev/null +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/AbstractJacksonYAMLParser.java @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2010-2025 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.internal.parser.jackson; + +import java.io.OutputStreamWriter; +import java.util.Set; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.openhab.core.automation.parser.Parser; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; + +/** + * Abstract class that can be used by YAML parsers for the different entity types. + * + * @param the type of the entities to parse + * + * @author Arne Seime - Initial contribution + */ +@NonNullByDefault +public abstract class AbstractJacksonYAMLParser implements Parser { + + /** The YAML object mapper instance */ + protected static final ObjectMapper yamlMapper; + + static { + yamlMapper = new ObjectMapper(new YAMLFactory()); + yamlMapper.findAndRegisterModules(); + } + + @Override + public void serialize(Set dataObjects, OutputStreamWriter writer) throws Exception { + for (T dataObject : dataObjects) { + yamlMapper.writeValue(writer, dataObject); + } + } +} diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/TemplateYAMLParser.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/TemplateYAMLParser.java new file mode 100644 index 0000000000..cc8359f387 --- /dev/null +++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/parser/jackson/TemplateYAMLParser.java @@ -0,0 +1,69 @@ +/* + * Copyright (c) 2010-2025 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.internal.parser.jackson; + +import java.io.IOException; +import java.io.InputStreamReader; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.openhab.core.automation.dto.RuleTemplateDTO; +import org.openhab.core.automation.dto.RuleTemplateDTOMapper; +import org.openhab.core.automation.parser.Parser; +import org.openhab.core.automation.parser.ParsingException; +import org.openhab.core.automation.parser.ParsingNestedException; +import org.openhab.core.automation.template.Template; +import org.osgi.service.component.annotations.Component; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; + +/** + * This class can parse and serialize sets of {@link Template}s. + * + * @author Arne Seime - Initial contribution + */ +@NonNullByDefault +@Component(immediate = true, service = Parser.class, property = { "parser.type=parser.template", "format=yaml" }) +public class TemplateYAMLParser extends AbstractJacksonYAMLParser