Improve thing updates (openhab#3576)

* Improve thing updates Signed-off-by: Jan N. Klug <[email protected]>
ConnectorIO · May 7, 2023 · 960cf0c · 960cf0c
1 parent 2e00efc
commit 960cf0c
Show file tree

Hide file tree

Showing 16 changed files with 207 additions and 148 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/....openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingFactoryHelper.java b/....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/...rg.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java b/...rg.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/...ng/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateInstructionReader.java b/...ng/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<UpdateInstructionKey, List<ThingUpdateInstruction>> 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<UpdateInstructionKey, List<ThingUpdateInstruction>> updateInstructions = new HashMap<>();
-        Enumeration<URL> 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<ThingUpdateInstruction> instructions = new ArrayList<>();
-                        List<XmlInstructionSet> 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<UpdateInstructionKey, List<ThingUpdateInstruction>> readForFactory(ThingHandlerFactory factory);
 
-    public record UpdateInstructionKey(ThingHandlerFactory factory, ThingTypeUID thingTypeId) {
+    record UpdateInstructionKey(ThingHandlerFactory factory, ThingTypeUID thingTypeId) {
     }
 }