From 960cf0c1796a793ff6a83891d95d405c67d9a1c5 Mon Sep 17 00:00:00 2001 From: J-N-K Date: Sun, 7 May 2023 20:46:42 +0200 Subject: [PATCH] Improve thing updates (#3576) * Improve thing updates Signed-off-by: Jan N. Klug --- .../OH-INF/i18n/validation.properties | 5 +- .../OH-INF/i18n/validation_cs.properties | 3 - .../OH-INF/i18n/validation_de.properties | 3 - .../OH-INF/i18n/validation_he.properties | 3 - .../OH-INF/i18n/validation_hu.properties | 3 - .../OH-INF/i18n/validation_it.properties | 3 - .../OH-INF/i18n/validation_no.properties | 3 - .../OH-INF/i18n/validation_pl.properties | 3 - .../OH-INF/i18n/validation_sl.properties | 3 - .../thing/internal/ThingFactoryHelper.java | 4 +- .../core/thing/internal/ThingManagerImpl.java | 28 ++-- .../update/ThingUpdateInstructionReader.java | 85 +----------- .../ThingUpdateInstructionReaderImpl.java | 122 ++++++++++++++++++ .../update/UpdateChannelInstructionImpl.java | 45 ++++++- .../thing/internal/ThingManagerImplTest.java | 6 +- .../internal/update/ThingUpdateOSGiTest.java | 36 +++--- 16 files changed, 207 insertions(+), 148 deletions(-) create mode 100644 bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReaderImpl.java diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation.properties index aff65cbe021..07d06aa9e2f 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation.properties @@ -13,6 +13,5 @@ options_violated=The value {0} does not match allowed parameter options. Allowed multiple_limit_violated=Only {0} elements are allowed but {1} are provided. bridge_not_configured=Configuring a bridge is mandatory. -type_description_missing=Type description for '{0}' not found also we checked the presence before. -config_description_missing=Config description for '{0}' not found also we checked the presence before. - +type_description_missing=Type description {0} for {1} not found, although we checked the presence before. +config_description_missing=Config description {0} for {1} not found, although we checked the presence before. diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_cs.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_cs.properties index 7b9fb8c12fc..7306d12294b 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_cs.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_cs.properties @@ -13,6 +13,3 @@ options_violated=Hodnota {0} neodpovídá parametru povolených možností. Povo multiple_limit_violated=Pouze {0} prvků je povoleno, ale {1} je k dispozici. bridge_not_configured=Konfigurace mostu (bridge) je povinná. -type_description_missing=Popis typu pro ''{0}'' nebyl nalezen, také jsme zkontrolovali předchozí. -config_description_missing=Popis konfigurace pro ''{0}'' nebyl nalezen, také jsme zkontrolovali předchozí. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_de.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_de.properties index 093bbb739b0..0321c872143 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_de.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_de.properties @@ -13,6 +13,3 @@ options_violated=Der Wert {0} ist nicht in den erlaubten Optionen enthalten. Erl multiple_limit_violated=Nur {0} Optionen sind zulässig, aber {1} wurden übergeben. bridge_not_configured=Es wird eine Bridge benötigt. -type_description_missing=Typbeschreibung für ''{0}'' nicht gefunden, auch das Vorhandensein wurde vorher überprüft. -config_description_missing=Konfigurationsbeschreibung für ''{0}'' nicht gefunden, auch das Vorhandensein wurde vorher überprüft. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_he.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_he.properties index 0047c2555db..41f4b6ef97d 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_he.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_he.properties @@ -13,6 +13,3 @@ options_violated=הערך {0} אינו תואם לאפשרויות הפרמטר multiple_limit_violated=רק {0} אלמנטים מותרים אך הוזנו {1}. bridge_not_configured=הגדרת מגשר היא חובה. -type_description_missing=סוג תיאור עבור ''{0}'' לא נמצא גם בדקנו את הנוכחות בעבר. -config_description_missing=תיאור התצורה של ''{0}'' לא נמצא גם בדקנו את הנוכחות בעבר. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_hu.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_hu.properties index 13308502ae6..3d2bdf1cc5c 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_hu.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_hu.properties @@ -13,6 +13,3 @@ options_violated=A(z) {0} érték nem egyezik a megengedett paraméter-beállít multiple_limit_violated=Csak {0} elem engedélyezett, de {1} szerepel. bridge_not_configured=A híd beállítása kötelező. -type_description_missing=A {0} típus leírása nem található még az előzményekben sem. -config_description_missing=A {0} beállítás leírása nem található még az előzményekben sem. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_it.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_it.properties index cbd65218bd9..c3e941a1a42 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_it.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_it.properties @@ -13,6 +13,3 @@ options_violated=Il valore {0} non è tra le opzioni consentite per il parametro multiple_limit_violated=Sono consentiti {0} elementi, ma ne sono stati forniti {1}. bridge_not_configured=Configurare un bridge è obbligatorio. -type_description_missing=Descrizione del tipo per ''{0}'' non trovato anche se abbiamo prima controllato la presenza. -config_description_missing=Descrizione della configurazione per ''{0}'' non trovato anche se abbiamo prima controllato la presenza. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_no.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_no.properties index 03cb5ef14d0..0d20d68d8b7 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_no.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_no.properties @@ -13,6 +13,3 @@ options_violated=Verdien {0} samsvarer ikke med tillatte parameteralternativer. multiple_limit_violated=Kun {0} elementer er tillatt mens {1} er angitt. bridge_not_configured=Konfigurasjon av en bro er obligatorisk. -type_description_missing=Typebeskivelsen for ''{0}'' ble ikke funnet, vi sjekket også tilstedeværelsen tidligere. -config_description_missing=Konfigurasjonsbeskrivelsen for ''{0}'' ble ikke funnet, vi sjekket også tilstedeværelsen tidligere. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_pl.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_pl.properties index 077b74545bd..5bc41d2c0de 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_pl.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_pl.properties @@ -13,6 +13,3 @@ options_violated=Wartość {0} nie pasuje do dozwolonych opcji parametrów. Dozw multiple_limit_violated=Tylko {0} elementy są dozwolone, ale {1} jest zapewniony. bridge_not_configured=Konfiguracja mostka jest obowiązkowa. -type_description_missing=Nie znaleziono opisu dla ''{0}'' również sprawdzono wcześniejsze opisy. -config_description_missing=Nie znaleziono opisu konfiguracji dla ''{0}'' również sprawdzono wcześniejsze opisy. - diff --git a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_sl.properties b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_sl.properties index 6e81dbf2794..c7d6d05b96f 100644 --- a/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_sl.properties +++ b/bundles/org.openhab.core.config.core/src/main/resources/OH-INF/i18n/validation_sl.properties @@ -13,6 +13,3 @@ options_violated=Vrednost {0} se ne ujema z dovoljenimim izborom parametrov. Dov multiple_limit_violated=Dovoljenih je le {0} elementov, podanih pa je {1}. bridge_not_configured=Konfiguriranje mostu je obvezno. -type_description_missing=Opis vrste za ''{0}'' ni bil najden. Obstoj smo preverili tudi že prej. -config_description_missing=Opis konfiguracije za ''{0}'' ni bil najden. Obstoj smo preverili tudi že prej. - diff --git a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingFactoryHelper.java b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingFactoryHelper.java index eb3194fcd55..3322f845f0b 100644 --- a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingFactoryHelper.java +++ b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingFactoryHelper.java @@ -144,7 +144,7 @@ private static Channel createChannel(ChannelDefinition channelDefinition, ThingU return channelBuilder.withProperties(channelDefinition.getProperties()).build(); } - static ChannelBuilder createChannelBuilder(ChannelUID channelUID, ChannelType channelType, + public static ChannelBuilder createChannelBuilder(ChannelUID channelUID, ChannelType channelType, ConfigDescriptionRegistry configDescriptionRegistry) { final ChannelBuilder channelBuilder = ChannelBuilder.create(channelUID, channelType.getItemType()) // .withType(channelType.getUID()) // @@ -168,7 +168,7 @@ static ChannelBuilder createChannelBuilder(ChannelUID channelUID, ChannelType ch return channelBuilder; } - static ChannelBuilder createChannelBuilder(ChannelUID channelUID, ChannelDefinition channelDefinition, + public static ChannelBuilder createChannelBuilder(ChannelUID channelUID, ChannelDefinition channelDefinition, ConfigDescriptionRegistry configDescriptionRegistry) { ChannelType channelType = withChannelTypeRegistry(channelTypeRegistry -> (channelTypeRegistry != null) ? channelTypeRegistry.getChannelType(channelDefinition.getChannelTypeUID()) diff --git a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java index 32a6b2a93b0..380e3cd998c 100644 --- a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java +++ b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java @@ -186,8 +186,10 @@ public ThingManagerImpl( // final @Reference StorageService storageService, // final @Reference ThingRegistry thingRegistry, final @Reference ThingStatusInfoI18nLocalizationService thingStatusInfoI18nLocalizationService, - final @Reference ThingTypeRegistry thingTypeRegistry, final @Reference BundleResolver bundleResolver, - final @Reference TranslationProvider translationProvider, final BundleContext bundleContext) { + final @Reference ThingTypeRegistry thingTypeRegistry, + final @Reference ThingUpdateInstructionReader thingUpdateInstructionReader, + final @Reference BundleResolver bundleResolver, final @Reference TranslationProvider translationProvider, + final BundleContext bundleContext) { this.channelGroupTypeRegistry = channelGroupTypeRegistry; this.channelTypeRegistry = channelTypeRegistry; this.communicationManager = communicationManager; @@ -198,7 +200,7 @@ public ThingManagerImpl( // this.readyService = readyService; this.safeCaller = safeCaller; this.thingRegistry = (ThingRegistryImpl) thingRegistry; - this.thingUpdateInstructionReader = new ThingUpdateInstructionReader(bundleResolver); + this.thingUpdateInstructionReader = thingUpdateInstructionReader; this.thingStatusInfoI18nLocalizationService = thingStatusInfoI18nLocalizationService; this.thingTypeRegistry = thingTypeRegistry; this.translationProvider = translationProvider; @@ -404,13 +406,13 @@ public void thingUpdated(Thing oldThing, Thing newThing, ThingTrackerEvent thing try { normalizeThingConfiguration(oldThing); } catch (ConfigValidationException e) { - logger.warn("Failed to normalize configuration for thing '{}': {}", oldThing.getUID(), + logger.debug("Failed to normalize configuration for old thing during update '{}': {}", oldThing.getUID(), e.getValidationMessages(null)); } try { normalizeThingConfiguration(newThing); } catch (ConfigValidationException e) { - logger.warn("Failed to normalize configuration´for thing '{}': {}", newThing.getUID(), + logger.warn("Failed to normalize configuration for new thing during update '{}': {}", newThing.getUID(), e.getValidationMessages(null)); } if (thingUpdatedLock.contains(thingUID)) { @@ -661,23 +663,23 @@ private void normalizeThingConfiguration(Thing thing) throws ConfigValidationExc thing.getUID()); return; } - normalizeConfiguration(thingType, thing.getUID(), thing.getConfiguration()); + normalizeConfiguration(thingType, thing.getThingTypeUID(), thing.getUID(), thing.getConfiguration()); for (Channel channel : thing.getChannels()) { ChannelTypeUID channelTypeUID = channel.getChannelTypeUID(); if (channelTypeUID != null) { ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUID); - normalizeConfiguration(channelType, channel.getUID(), channel.getConfiguration()); + normalizeConfiguration(channelType, channelTypeUID, channel.getUID(), channel.getConfiguration()); } } } - private void normalizeConfiguration(@Nullable AbstractDescriptionType prototype, UID targetUID, + private void normalizeConfiguration(@Nullable AbstractDescriptionType prototype, UID prototypeUID, UID targetUID, Configuration configuration) throws ConfigValidationException { if (prototype == null) { ConfigValidationMessage message = new ConfigValidationMessage("thing/channel", - "Type description for '{0}' not found although we checked the presence before.", - "type_description_missing", targetUID.toString()); + "Type description {0} for {1} not found, although we checked the presence before.", + "type_description_missing", prototypeUID.toString(), targetUID.toString()); throw new ConfigValidationException(bundleContext.getBundle(), translationProvider, List.of(message)); } @@ -691,8 +693,8 @@ private void normalizeConfiguration(@Nullable AbstractDescriptionType prototype, ConfigDescription configDescription = configDescriptionRegistry.getConfigDescription(configDescriptionURI); if (configDescription == null) { ConfigValidationMessage message = new ConfigValidationMessage("thing/channel", - "Config description for '{0}' not found also we checked the presence before.", - "config_description_missing", targetUID); + "Config description {0} for {1} not found, although we checked the presence before.", + "config_description_missing", configDescriptionURI.toString(), targetUID.toString()); throw new ConfigValidationException(bundleContext.getBundle(), translationProvider, List.of(message)); } @@ -1262,7 +1264,7 @@ public synchronized boolean isReady() { timesChecked++; if (timesChecked > MAX_CHECK_PREREQUISITE_TIME / CHECK_INTERVAL) { logger.warn( - "Channel types or config descriptions for thing '{}' are missing in the respective registry for more than {}s. This should be fixed in the binding.", + "Channel types or config descriptions for thing '{}' are missing in the respective registry for more than {}s. In case it does not happen immediately after an upgrade, it should be fixed in the binding.", thingUID, MAX_CHECK_PREREQUISITE_TIME); channelTypeUIDs.clear(); configDescriptionUris.clear(); diff --git a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReader.java b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReader.java index 0ebe8949e6b..a56f104399b 100644 --- a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReader.java +++ b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReader.java @@ -12,100 +12,23 @@ */ package org.openhab.core.thing.internal.update; -import java.net.URL; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.Enumeration; -import java.util.HashMap; import java.util.List; import java.util.Map; -import javax.xml.bind.JAXBContext; -import javax.xml.bind.JAXBException; -import javax.xml.bind.Unmarshaller; - import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.thing.ThingTypeUID; import org.openhab.core.thing.binding.ThingHandlerFactory; -import org.openhab.core.thing.internal.update.dto.XmlAddChannel; -import org.openhab.core.thing.internal.update.dto.XmlInstructionSet; -import org.openhab.core.thing.internal.update.dto.XmlRemoveChannel; -import org.openhab.core.thing.internal.update.dto.XmlThingType; -import org.openhab.core.thing.internal.update.dto.XmlUpdateChannel; -import org.openhab.core.thing.internal.update.dto.XmlUpdateDescriptions; -import org.openhab.core.util.BundleResolver; -import org.osgi.framework.Bundle; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * The {@link ThingUpdateInstructionReader} is used to read instructions for a given {@link ThingHandlerFactory} and - * * create a list of {@link ThingUpdateInstruction}s + * create a list of {@link ThingUpdateInstruction}s * * @author Jan N. Klug - Initial contribution */ @NonNullByDefault -public class ThingUpdateInstructionReader { - private final Logger logger = LoggerFactory.getLogger(ThingUpdateInstructionReader.class); - private final BundleResolver bundleResolver; - - public ThingUpdateInstructionReader(BundleResolver bundleResolver) { - this.bundleResolver = bundleResolver; - } - - public Map> readForFactory(ThingHandlerFactory factory) { - Bundle bundle = bundleResolver.resolveBundle(factory.getClass()); - if (bundle == null) { - logger.error( - "Could not get bundle for '{}', thing type updates will fail. If this occurs outside of tests, it is a bug.", - factory.getClass()); - return Map.of(); - } - - Map> updateInstructions = new HashMap<>(); - Enumeration entries = bundle.findEntries("OH-INF/update", "*.xml", true); - if (entries != null) { - while (entries.hasMoreElements()) { - URL url = entries.nextElement(); - try { - JAXBContext context = JAXBContext.newInstance(XmlUpdateDescriptions.class); - Unmarshaller u = context.createUnmarshaller(); - XmlUpdateDescriptions updateDescriptions = (XmlUpdateDescriptions) u.unmarshal(url); - - for (XmlThingType thingType : updateDescriptions.getThingType()) { - ThingTypeUID thingTypeUID = new ThingTypeUID(thingType.getUid()); - UpdateInstructionKey key = new UpdateInstructionKey(factory, thingTypeUID); - List instructions = new ArrayList<>(); - List instructionSets = thingType.getInstructionSet().stream() - .sorted(Comparator.comparing(XmlInstructionSet::getTargetVersion)).toList(); - for (XmlInstructionSet instructionSet : instructionSets) { - int targetVersion = instructionSet.getTargetVersion(); - for (Object instruction : instructionSet.getInstructions()) { - if (instruction instanceof XmlAddChannel addChannelType) { - instructions.add(new UpdateChannelInstructionImpl(targetVersion, addChannelType)); - } else if (instruction instanceof XmlUpdateChannel updateChannelType) { - instructions - .add(new UpdateChannelInstructionImpl(targetVersion, updateChannelType)); - } else if (instruction instanceof XmlRemoveChannel removeChannelType) { - instructions - .add(new RemoveChannelInstructionImpl(targetVersion, removeChannelType)); - } else { - logger.warn("Instruction type '{}' is unknown.", instruction.getClass()); - } - } - } - updateInstructions.put(key, instructions); - } - logger.trace("Reading update instructions from '{}'", url.getPath()); - } catch (IllegalArgumentException | JAXBException e) { - logger.warn("Failed to parse update instructions from '{}':", url, e); - } - } - } - - return updateInstructions; - } +public interface ThingUpdateInstructionReader { + Map> readForFactory(ThingHandlerFactory factory); - public record UpdateInstructionKey(ThingHandlerFactory factory, ThingTypeUID thingTypeId) { + record UpdateInstructionKey(ThingHandlerFactory factory, ThingTypeUID thingTypeId) { } } diff --git a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReaderImpl.java b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReaderImpl.java new file mode 100644 index 00000000000..8cb809bd527 --- /dev/null +++ b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReaderImpl.java @@ -0,0 +1,122 @@ +/** + * Copyright (c) 2010-2023 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.thing.internal.update; + +import java.net.URL; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import javax.xml.bind.JAXBContext; +import javax.xml.bind.JAXBException; +import javax.xml.bind.Unmarshaller; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.openhab.core.config.core.ConfigDescriptionRegistry; +import org.openhab.core.thing.ThingTypeUID; +import org.openhab.core.thing.binding.ThingHandlerFactory; +import org.openhab.core.thing.internal.update.dto.XmlAddChannel; +import org.openhab.core.thing.internal.update.dto.XmlInstructionSet; +import org.openhab.core.thing.internal.update.dto.XmlRemoveChannel; +import org.openhab.core.thing.internal.update.dto.XmlThingType; +import org.openhab.core.thing.internal.update.dto.XmlUpdateChannel; +import org.openhab.core.thing.internal.update.dto.XmlUpdateDescriptions; +import org.openhab.core.thing.type.ChannelTypeRegistry; +import org.openhab.core.util.BundleResolver; +import org.osgi.framework.Bundle; +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; + +/** + * The {@link ThingUpdateInstructionReaderImpl} is an implementation of {@link ThingUpdateInstructionReader} + * + * @author Jan N. Klug - Initial contribution + */ +@Component(service = ThingUpdateInstructionReader.class) +@NonNullByDefault +public class ThingUpdateInstructionReaderImpl implements ThingUpdateInstructionReader { + private final Logger logger = LoggerFactory.getLogger(ThingUpdateInstructionReaderImpl.class); + private final BundleResolver bundleResolver; + private final ChannelTypeRegistry channelTypeRegistry; + private final ConfigDescriptionRegistry configDescriptionRegistry; + + @Activate + public ThingUpdateInstructionReaderImpl(@Reference BundleResolver bundleResolver, + @Reference ChannelTypeRegistry channelTypeRegistry, + @Reference ConfigDescriptionRegistry configDescriptionRegistry) { + this.bundleResolver = bundleResolver; + this.channelTypeRegistry = channelTypeRegistry; + this.configDescriptionRegistry = configDescriptionRegistry; + } + + @Override + public Map> readForFactory(ThingHandlerFactory factory) { + Bundle bundle = bundleResolver.resolveBundle(factory.getClass()); + if (bundle == null) { + logger.error( + "Could not get bundle for '{}', thing type updates will fail. If this occurs outside of tests, it is a bug.", + factory.getClass()); + return Map.of(); + } + + Map> updateInstructions = new HashMap<>(); + Enumeration entries = bundle.findEntries("OH-INF/update", "*.xml", true); + if (entries != null) { + while (entries.hasMoreElements()) { + URL url = entries.nextElement(); + try { + JAXBContext context = JAXBContext.newInstance(XmlUpdateDescriptions.class); + Unmarshaller u = context.createUnmarshaller(); + XmlUpdateDescriptions updateDescriptions = (XmlUpdateDescriptions) u.unmarshal(url); + + for (XmlThingType thingType : updateDescriptions.getThingType()) { + ThingTypeUID thingTypeUID = new ThingTypeUID(thingType.getUid()); + UpdateInstructionKey key = new UpdateInstructionKey(factory, thingTypeUID); + List instructions = new ArrayList<>(); + List instructionSets = thingType.getInstructionSet().stream() + .sorted(Comparator.comparing(XmlInstructionSet::getTargetVersion)).toList(); + for (XmlInstructionSet instructionSet : instructionSets) { + int targetVersion = instructionSet.getTargetVersion(); + for (Object instruction : instructionSet.getInstructions()) { + if (instruction instanceof XmlAddChannel addChannelType) { + instructions.add(new UpdateChannelInstructionImpl(targetVersion, addChannelType, + channelTypeRegistry, configDescriptionRegistry)); + } else if (instruction instanceof XmlUpdateChannel updateChannelType) { + instructions.add(new UpdateChannelInstructionImpl(targetVersion, updateChannelType, + channelTypeRegistry, configDescriptionRegistry)); + } else if (instruction instanceof XmlRemoveChannel removeChannelType) { + instructions + .add(new RemoveChannelInstructionImpl(targetVersion, removeChannelType)); + } else { + logger.warn("Instruction type '{}' is unknown.", instruction.getClass()); + } + } + } + updateInstructions.put(key, instructions); + } + logger.trace("Reading update instructions from '{}'", url.getPath()); + } catch (IllegalArgumentException | JAXBException e) { + logger.warn("Failed to parse update instructions from '{}':", url, e); + } + } + } + + return updateInstructions; + } +} diff --git a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/UpdateChannelInstructionImpl.java b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/UpdateChannelInstructionImpl.java index 7433ad9f0f3..88c26aae26a 100644 --- a/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/UpdateChannelInstructionImpl.java +++ b/bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/UpdateChannelInstructionImpl.java @@ -19,14 +19,20 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.config.core.ConfigDescriptionRegistry; import org.openhab.core.thing.Channel; import org.openhab.core.thing.ChannelUID; import org.openhab.core.thing.Thing; import org.openhab.core.thing.binding.builder.ChannelBuilder; import org.openhab.core.thing.binding.builder.ThingBuilder; +import org.openhab.core.thing.internal.ThingFactoryHelper; import org.openhab.core.thing.internal.update.dto.XmlAddChannel; import org.openhab.core.thing.internal.update.dto.XmlUpdateChannel; +import org.openhab.core.thing.type.ChannelType; +import org.openhab.core.thing.type.ChannelTypeRegistry; import org.openhab.core.thing.type.ChannelTypeUID; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * The {@link UpdateChannelInstructionImpl} implements a {@link ThingUpdateInstruction} that updates a channel of a @@ -36,40 +42,49 @@ */ @NonNullByDefault public class UpdateChannelInstructionImpl implements ThingUpdateInstruction { + private final Logger logger = LoggerFactory.getLogger(UpdateChannelInstructionImpl.class); private final boolean removeOldChannel; private final int thingTypeVersion; private final boolean preserveConfig; private final List groupIds; private final String channelId; - private final String channelTypeUid; + private final ChannelTypeUID channelTypeUid; private final @Nullable String label; private final @Nullable String description; private final @Nullable List tags; + private final ChannelTypeRegistry channelTypeRegistry; + private final ConfigDescriptionRegistry configDescriptionRegistry; - UpdateChannelInstructionImpl(int thingTypeVersion, XmlUpdateChannel updateChannel) { + UpdateChannelInstructionImpl(int thingTypeVersion, XmlUpdateChannel updateChannel, + ChannelTypeRegistry channelTypeRegistry, ConfigDescriptionRegistry configDescriptionRegistry) { this.removeOldChannel = true; this.thingTypeVersion = thingTypeVersion; this.channelId = updateChannel.getId(); - this.channelTypeUid = updateChannel.getType(); + this.channelTypeUid = new ChannelTypeUID(updateChannel.getType()); String rawGroupIds = updateChannel.getGroupIds(); this.groupIds = rawGroupIds != null ? Arrays.asList(rawGroupIds.split(",")) : List.of(); this.label = updateChannel.getLabel(); this.description = updateChannel.getDescription(); this.tags = updateChannel.getTags(); this.preserveConfig = updateChannel.isPreserveConfiguration(); + this.channelTypeRegistry = channelTypeRegistry; + this.configDescriptionRegistry = configDescriptionRegistry; } - UpdateChannelInstructionImpl(int thingTypeVersion, XmlAddChannel addChannel) { + UpdateChannelInstructionImpl(int thingTypeVersion, XmlAddChannel addChannel, + ChannelTypeRegistry channelTypeRegistry, ConfigDescriptionRegistry configDescriptionRegistry) { this.removeOldChannel = false; this.thingTypeVersion = thingTypeVersion; this.channelId = addChannel.getId(); - this.channelTypeUid = addChannel.getType(); + this.channelTypeUid = new ChannelTypeUID(addChannel.getType()); String rawGroupIds = addChannel.getGroupIds(); this.groupIds = rawGroupIds != null ? Arrays.asList(rawGroupIds.split(",")) : List.of(); this.label = addChannel.getLabel(); this.description = addChannel.getDescription(); this.tags = addChannel.getTags(); this.preserveConfig = false; + this.channelTypeRegistry = channelTypeRegistry; + this.configDescriptionRegistry = configDescriptionRegistry; } @Override @@ -79,6 +94,7 @@ public int getThingTypeVersion() { @Override public void perform(Thing thing, ThingBuilder thingBuilder) { + logger.debug("Applying {} to thing '{}'", this, thing.getUID()); if (groupIds.isEmpty()) { doChannel(thing, thingBuilder, new ChannelUID(thing.getUID(), channelId)); } else { @@ -92,8 +108,15 @@ private void doChannel(Thing thing, ThingBuilder thingBuilder, ChannelUID affect thingBuilder.withoutChannel(affectedChannelUid); } - ChannelBuilder channelBuilder = ChannelBuilder.create(affectedChannelUid) - .withType(new ChannelTypeUID(channelTypeUid)); + ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUid); + if (channelType == null) { + logger.error("Failed to create channel '{}' because channel type '{}' could not be found.", + affectedChannelUid, channelTypeUid); + return; + } + + ChannelBuilder channelBuilder = ThingFactoryHelper.createChannelBuilder(affectedChannelUid, channelType, + configDescriptionRegistry); if (preserveConfig) { Channel oldChannel = thing.getChannel(affectedChannelUid); @@ -115,4 +138,12 @@ private void doChannel(Thing thing, ThingBuilder thingBuilder, ChannelUID affect thingBuilder.withChannel(channelBuilder.build()); } + + @Override + public String toString() { + return "UpdateChannelInstructionImpl{" + "removeOldChannel=" + removeOldChannel + ", thingTypeVersion=" + + thingTypeVersion + ", preserveConfig=" + preserveConfig + ", groupIds=" + groupIds + ", channelId='" + + channelId + "'" + ", channelTypeUid=" + channelTypeUid + ", label='" + label + "'" + ", description='" + + description + "'" + ", tags=" + tags + "}"; + } } diff --git a/bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/internal/ThingManagerImplTest.java b/bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/internal/ThingManagerImplTest.java index 861cd7728d1..bf635ce2c29 100644 --- a/bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/internal/ThingManagerImplTest.java +++ b/bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/internal/ThingManagerImplTest.java @@ -41,6 +41,7 @@ import org.openhab.core.thing.binding.ThingHandlerFactory; import org.openhab.core.thing.i18n.ThingStatusInfoI18nLocalizationService; import org.openhab.core.thing.internal.ThingTracker.ThingTrackerEvent; +import org.openhab.core.thing.internal.update.ThingUpdateInstructionReader; import org.openhab.core.thing.link.ItemChannelLinkRegistry; import org.openhab.core.thing.type.ChannelGroupTypeRegistry; import org.openhab.core.thing.type.ChannelTypeRegistry; @@ -72,6 +73,7 @@ public class ThingManagerImplTest extends JavaTest { private @Mock @NonNullByDefault({}) Thing thingMock; private @Mock @NonNullByDefault({}) ThingRegistryImpl thingRegistryMock; private @Mock @NonNullByDefault({}) BundleResolver bundleResolverMock; + private @Mock @NonNullByDefault({}) ThingUpdateInstructionReader thingUpdateInstructionReaderMock; private @Mock @NonNullByDefault({}) TranslationProvider translationProviderMock; private @Mock @NonNullByDefault({}) BundleContext bundleContextMock; private @Mock @NonNullByDefault({}) ThingType thingTypeMock; @@ -92,8 +94,8 @@ private ThingManagerImpl createThingManager() { return new ThingManagerImpl(channelGroupTypeRegistryMock, channelTypeRegistryMock, communicationManagerMock, configDescriptionRegistryMock, configDescriptionValidatorMock, eventPublisherMock, itemChannelLinkRegistryMock, readyServiceMock, safeCallerMock, storageServiceMock, thingRegistryMock, - thingStatusInfoI18nLocalizationService, thingTypeRegistryMock, bundleResolverMock, - translationProviderMock, bundleContextMock); + thingStatusInfoI18nLocalizationService, thingTypeRegistryMock, thingUpdateInstructionReaderMock, + bundleResolverMock, translationProviderMock, bundleContextMock); } @Test diff --git a/itests/org.openhab.core.thing.tests/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateOSGiTest.java b/itests/org.openhab.core.thing.tests/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateOSGiTest.java index 18a3f616d4b..b0c698c7812 100644 --- a/itests/org.openhab.core.thing.tests/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateOSGiTest.java +++ b/itests/org.openhab.core.thing.tests/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateOSGiTest.java @@ -69,7 +69,7 @@ import org.slf4j.LoggerFactory; /** - * Tests for {@link ThingUpdateInstructionReader} and {@link ThingUpdateInstruction} implementations. + * Tests for {@link ThingUpdateInstructionReaderImpl} and {@link ThingUpdateInstruction} implementations. * * @author Jan N. Klug - Initial contribution */ @@ -149,14 +149,14 @@ public void testSingleChannelAddition() { assertThat(updatedThing.getChannels(), hasSize(3)); Channel channel1 = updatedThing.getChannel("testChannel1"); - assertChannel(channel1, channelTypeUID, null, null); + assertChannel(channel1, channelTypeUID, "Switch", "typeLabel", null); Channel channel2 = updatedThing.getChannel("testChannel2"); - assertChannel(channel2, channelTypeUID, "Test Label", null); + assertChannel(channel2, channelTypeUID, "Switch", "Test Label", null); assertThat(channel2.getDefaultTags(), containsInAnyOrder(ADDED_TAGS)); Channel channel3 = updatedThing.getChannel("testChannel3"); - assertChannel(channel3, channelTypeUID, "Test Label", "Test Description"); + assertChannel(channel3, channelTypeUID, "Switch", "Test Label", "Test Description"); } @Test @@ -186,11 +186,11 @@ public void testSingleChannelUpdate() { assertThat(updatedThing.getChannels(), hasSize(2)); Channel channel1 = updatedThing.getChannel(channelUID1); - assertChannel(channel1, channelTypeNewUID, "New Test Label", null); + assertChannel(channel1, channelTypeNewUID, "Number", "New Test Label", null); assertThat(channel1.getConfiguration(), is(channelConfig)); Channel channel2 = updatedThing.getChannel(channelUID2); - assertChannel(channel2, channelTypeNewUID, null, null); + assertChannel(channel2, channelTypeNewUID, "Number", "typeLabel", null); assertThat(channel2.getConfiguration().getProperties(), is(anEmptyMap())); } @@ -235,10 +235,10 @@ public void testMultipleChannelUpdates() { assertThat(updatedThing.getChannels(), hasSize(2)); Channel channel1 = updatedThing.getChannel("testChannel1"); - assertChannel(channel1, channelTypeNewUID, "Test Label", null); + assertChannel(channel1, channelTypeNewUID, "Number", "Test Label", null); Channel channel2 = updatedThing.getChannel("testChannel2"); - assertChannel(channel2, channelTypeOldUID, "TestLabel", null); + assertChannel(channel2, channelTypeOldUID, "Switch", "TestLabel", null); } @Test @@ -253,8 +253,10 @@ public void testOnlyMatchingInstructionsUpdate() { ChannelUID channelUID1 = new ChannelUID(thingUID, "testChannel1"); Thing thing = ThingBuilder.create(MULTIPLE_CHANNEL_THING_TYPE_UID, thingUID) - .withChannel(ChannelBuilder.create(channelUID0).withType(channelTypeOldUID).build()) - .withChannel(ChannelBuilder.create(channelUID1).withType(channelTypeOldUID).build()) + .withChannel(ChannelBuilder.create(channelUID0).withType(channelTypeOldUID) + .withAcceptedItemType("Switch").build()) + .withChannel(ChannelBuilder.create(channelUID1).withType(channelTypeOldUID) + .withAcceptedItemType("Switch").build()) .withProperty(PROPERTY_THING_TYPE_VERSION, "2").build(); managedThingProvider.add(thing); @@ -263,7 +265,7 @@ public void testOnlyMatchingInstructionsUpdate() { assertThat(updatedThing.getChannels(), hasSize(1)); Channel channel1 = updatedThing.getChannel("testChannel1"); - assertChannel(channel1, channelTypeOldUID, null, null); + assertChannel(channel1, channelTypeOldUID, "Switch", null, null); } @Test @@ -283,11 +285,11 @@ public void testSingleChannelAdditionGroup() { List channels1 = updatedThing.getChannelsOfGroup("group1"); assertThat(channels1, hasSize(1)); - assertChannel(channels1.get(0), channelTypeUID, null, null); + assertChannel(channels1.get(0), channelTypeUID, "Switch", "typeLabel", null); List channels2 = updatedThing.getChannelsOfGroup("group2"); assertThat(channels2, hasSize(1)); - assertChannel(channels2.get(0), channelTypeUID, null, null); + assertChannel(channels2.get(0), channelTypeUID, "Switch", "typeLabel", null); } @Test @@ -328,10 +330,11 @@ private Thing assertThing(Thing oldThing, int expectedNewThingTypeVersion) { return updatedThing; } - private void assertChannel(@Nullable Channel channel, ChannelTypeUID channelTypeUID, @Nullable String label, - @Nullable String description) { + private void assertChannel(@Nullable Channel channel, ChannelTypeUID channelTypeUID, String expectedItemType, + @Nullable String label, @Nullable String description) { assertThat(channel, is(notNullValue())); assertThat(channel.getChannelTypeUID(), is(channelTypeUID)); + assertThat(channel.getAcceptedItemType(), is(expectedItemType)); if (label != null) { assertThat(channel.getLabel(), is(label)); } else { @@ -361,7 +364,8 @@ private void registerChannelTypes(ChannelTypeUID... channelTypeUIDs) { ChannelTypeRegistry channelTypeRegistry = mock(ChannelTypeRegistry.class); for (ChannelTypeUID channelTypeUID : channelTypeUIDs) { - ChannelType channelType = ChannelTypeBuilder.state(channelTypeUID, "label", "Number").build(); + String itemType = channelTypeUID.getId().contains("New") ? "Number" : "Switch"; + ChannelType channelType = ChannelTypeBuilder.state(channelTypeUID, "typeLabel", itemType).build(); when(channelTypeProvider.getChannelType(eq(channelTypeUID), nullable(Locale.class))) .thenReturn(channelType); when(channelTypeRegistry.getChannelType(eq(channelTypeUID))).thenReturn(channelType);