Improve thing updates (#3576)

* Improve thing updates

Signed-off-by: Jan N. Klug <github@klug.nrw>

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.
+type_description_missing=Type description {0} for {1} not found, although we checked the presence before.
-config_description_missing=Config description for '{0}' not found also we checked the presence before.
+config_description_missing=Config description {0} for {1} not found, although we checked the presence before.
-

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í.
-

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.
-

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}'' לא נמצא גם בדקנו את הנוכחות בעבר.
-

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.
-

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.
-

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.
-

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.
-

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.
-

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingFactoryHelper.java

@@ -144,7 +144,7 @@ public class ThingFactoryHelper {
         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 @@ public class ThingFactoryHelper {
         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())

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/ThingManagerImpl.java

@@ -186,8 +186,10 @@ public class ThingManagerImpl implements ReadyTracker, ThingManager, ThingTracke
             final @Reference StorageService storageService, //
             final @Reference ThingRegistry thingRegistry,
             final @Reference ThingStatusInfoI18nLocalizationService thingStatusInfoI18nLocalizationService,
-            final @Reference ThingTypeRegistry thingTypeRegistry, final @Reference BundleResolver bundleResolver,
+            final @Reference ThingTypeRegistry thingTypeRegistry,
-            final @Reference TranslationProvider translationProvider, final BundleContext bundleContext) {
+            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 class ThingManagerImpl implements ReadyTracker, ThingManager, ThingTracke
         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 class ThingManagerImpl implements ReadyTracker, ThingManager, ThingTracke
         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 @@ public class ThingManagerImpl implements ReadyTracker, ThingManager, ThingTracke
                     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 {0} for {1} not found, although we checked the presence before.",
-                    "type_description_missing", targetUID.toString());
+                    "type_description_missing", prototypeUID.toString(), targetUID.toString());
             throw new ConfigValidationException(bundleContext.getBundle(), translationProvider, List.of(message));
         }
 
@@ -691,8 +693,8 @@ public class ThingManagerImpl implements ReadyTracker, ThingManager, ThingTracke
         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 {0} for {1} not found, although we checked the presence before.",
-                    "config_description_missing", targetUID);
+                    "config_description_missing", configDescriptionURI.toString(), targetUID.toString());
             throw new ConfigValidationException(bundleContext.getBundle(), translationProvider, List.of(message));
         }
 
@@ -1262,7 +1264,7 @@ public class ThingManagerImpl implements ReadyTracker, ThingManager, ThingTracke
                 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();

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 {
+public interface ThingUpdateInstructionReader {
-    private final Logger logger = LoggerFactory.getLogger(ThingUpdateInstructionReader.class);
+    Map<UpdateInstructionKey, List<ThingUpdateInstruction>> readForFactory(ThingHandlerFactory factory);
-    private final BundleResolver bundleResolver;
 
-    public ThingUpdateInstructionReader(BundleResolver bundleResolver) {
+    record UpdateInstructionKey(ThingHandlerFactory factory, ThingTypeUID thingTypeId) {
-        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 record UpdateInstructionKey(ThingHandlerFactory factory, ThingTypeUID thingTypeId) {
     }
 }

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<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,
+                                            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;
+    }
+}

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/internal/update/UpdateChannelInstructionImpl.java

@@ -19,14 +19,20 @@ import java.util.Set;
 
 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 @@ import org.openhab.core.thing.type.ChannelTypeUID;
  */
 @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<String> 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<String> 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 class UpdateChannelInstructionImpl implements ThingUpdateInstruction {
 
     @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 @@ public class UpdateChannelInstructionImpl implements ThingUpdateInstruction {
             thingBuilder.withoutChannel(affectedChannelUid);
         }
 
-        ChannelBuilder channelBuilder = ChannelBuilder.create(affectedChannelUid)
+        ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUid);
-                .withType(new ChannelTypeUID(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 @@ public class UpdateChannelInstructionImpl implements ThingUpdateInstruction {
 
         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 + "}";
+    }
 }

bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/internal/ThingManagerImplTest.java

@@ -41,6 +41,7 @@ import org.openhab.core.thing.ThingUID;
 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 @@ public class ThingManagerImplTest extends JavaTest {
         return new ThingManagerImpl(channelGroupTypeRegistryMock, channelTypeRegistryMock, communicationManagerMock,
                 configDescriptionRegistryMock, configDescriptionValidatorMock, eventPublisherMock,
                 itemChannelLinkRegistryMock, readyServiceMock, safeCallerMock, storageServiceMock, thingRegistryMock,
-                thingStatusInfoI18nLocalizationService, thingTypeRegistryMock, bundleResolverMock,
+                thingStatusInfoI18nLocalizationService, thingTypeRegistryMock, thingUpdateInstructionReaderMock,
-                translationProviderMock, bundleContextMock);
+                bundleResolverMock, translationProviderMock, bundleContextMock);
     }
 
     @Test

itests/org.openhab.core.thing.tests/src/main/java/org/openhab/core/thing/internal/update/ThingUpdateOSGiTest.java

@@ -69,7 +69,7 @@ import org.slf4j.Logger;
 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 class ThingUpdateOSGiTest extends JavaOSGiTest {
         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 class ThingUpdateOSGiTest extends JavaOSGiTest {
         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 class ThingUpdateOSGiTest extends JavaOSGiTest {
         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 class ThingUpdateOSGiTest extends JavaOSGiTest {
         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(channelUID0).withType(channelTypeOldUID)
-                .withChannel(ChannelBuilder.create(channelUID1).withType(channelTypeOldUID).build())
+                        .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 class ThingUpdateOSGiTest extends JavaOSGiTest {
         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 class ThingUpdateOSGiTest extends JavaOSGiTest {
 
         List<Channel> channels1 = updatedThing.getChannelsOfGroup("group1");
         assertThat(channels1, hasSize(1));
-        assertChannel(channels1.get(0), channelTypeUID, null, null);
+        assertChannel(channels1.get(0), channelTypeUID, "Switch", "typeLabel", null);
 
         List<Channel> 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 @@ public class ThingUpdateOSGiTest extends JavaOSGiTest {
         return updatedThing;
     }
 
-    private void assertChannel(@Nullable Channel channel, ChannelTypeUID channelTypeUID, @Nullable String label,
+    private void assertChannel(@Nullable Channel channel, ChannelTypeUID channelTypeUID, String expectedItemType,
-            @Nullable String description) {
+            @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 @@ public class ThingUpdateOSGiTest extends JavaOSGiTest {
         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);