Fix thing reloading from things file (#3526)

It was found that things from textual configuration are not properly updated if changes are only made in configuration or label of a channel. The reason is that for equality only uid and accepted item-type where checked.

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

bundles/org.openhab.core.model.thing/src/org/openhab/core/model/thing/internal/GenericThingProvider.xtend

@@ -602,19 +602,20 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
     }
 
     def private createThingsFromModelForThingHandlerFactory(String modelName, ThingHandlerFactory factory) {
-        if (!loadedXmlThingTypes.contains(factory.bundleName)) {
+        if (!loadedXmlThingTypes.contains(factory.bundleName) || modelRepository == null) {
             return
         }
         val oldThings = thingsMap.get(modelName).clone
         val newThings = newArrayList()
-        if (modelRepository !== null) {
+
-            val model = modelRepository.getModel(modelName) as ThingModel
+        val model = modelRepository.getModel(modelName) as ThingModel
-            if (model !== null) {
+        if (model !== null) {
-                flattenModelThings(model.things).forEach [
+            flattenModelThings(model.things).forEach [
-                    createThing(newThings, factory)
+                createThing(newThings, factory)
-                ]
+            ]
-            }
         }
+        thingsMap.put(modelName, newThings)
+
         newThings.forEach [ newThing |
             val oldThing = oldThings.findFirst[it.UID == newThing.UID]
             if (oldThing !== null) {
@@ -624,7 +625,6 @@ class GenericThingProvider extends AbstractProviderLazyNullness<Thing> implement
                 }
             } else {
                 logger.debug("Adding thing '{}' from model '{}'.", newThing.UID, modelName);
-                thingsMap.get(modelName).add(newThing)
                 newThing.notifyListenersAboutAddedElement
             }
         ]

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/Channel.java

@@ -14,6 +14,7 @@ package org.openhab.core.thing;
 
 import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -178,4 +179,27 @@ public class Channel {
     public @Nullable AutoUpdatePolicy getAutoUpdatePolicy() {
         return autoUpdatePolicy;
     }
+
+    @Override
+    public boolean equals(@Nullable Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        Channel channel = (Channel) o;
+        return Objects.equals(acceptedItemType, channel.acceptedItemType) && kind == channel.kind
+                && Objects.equals(uid, channel.uid) && Objects.equals(channelTypeUID, channel.channelTypeUID)
+                && Objects.equals(label, channel.label) && Objects.equals(description, channel.description)
+                && Objects.equals(configuration, channel.configuration)
+                && Objects.equals(properties, channel.properties) && Objects.equals(defaultTags, channel.defaultTags)
+                && autoUpdatePolicy == channel.autoUpdatePolicy;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(acceptedItemType, kind, uid, channelTypeUID, label, description, configuration, properties,
+                defaultTags, autoUpdatePolicy);
+    }
 }

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/util/ThingHelper.java

@@ -12,10 +12,8 @@
  */
 package org.openhab.core.thing.util;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -79,24 +77,10 @@ public class ThingHelper {
             return false;
         }
         // channels
-        List<Channel> channelsOfA = a.getChannels();
+        Set<Channel> channelsOfA = new HashSet<>(a.getChannels());
-        List<Channel> channelsOfB = b.getChannels();
+        Set<Channel> channelsOfB = new HashSet<>(b.getChannels());
-        if (channelsOfA.size() != channelsOfB.size()) {
-            return false;
-        }
-        if (!toString(channelsOfA).equals(toString(channelsOfB))) {
-            return false;
-        }
-        return true;
-    }
 
-    private static String toString(List<Channel> channels) {
+        return channelsOfA.equals(channelsOfB);
-        List<String> strings = new ArrayList<>(channels.size());
-        for (Channel channel : channels) {
-            strings.add(channel.getUID().toString() + '#' + channel.getAcceptedItemType() + '#' + channel.getKind());
-        }
-        Collections.sort(strings);
-        return String.join(",", strings);
     }
 
     public static void addChannelsToThing(Thing thing, Collection<Channel> channels) {

bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/util/ThingHelperTest.java

@@ -16,6 +16,7 @@ import static java.util.stream.Collectors.toList;
 import static org.junit.jupiter.api.Assertions.*;
 
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -142,4 +143,21 @@ public class ThingHelperTest {
                                 ChannelBuilder.create(new ChannelUID(thingUID, "channel3"), "").build())
                                 .collect(toList())));
     }
+
+    @Test
+    public void asserThatChannelsWithDifferentConfigurationAreDetectedAsDifferent() {
+        Thing thingA = ThingBuilder.create(THING_TYPE_UID, THING_UID)
+                .withChannels(ChannelBuilder.create(new ChannelUID("binding:type:thingId:channel1"), "itemType")
+                        .withConfiguration(new Configuration(Map.of("key", "v1"))).build())
+                .withConfiguration(new Configuration()).build();
+
+        assertTrue(ThingHelper.equals(thingA, thingA));
+
+        Thing thingB = ThingBuilder.create(THING_TYPE_UID, THING_UID)
+                .withChannels(ChannelBuilder.create(new ChannelUID("binding:type:thingId:channel1"), "itemType")
+                        .withConfiguration(new Configuration(Map.of("key", "v2"))).build())
+                .withConfiguration(new Configuration()).build();
+
+        assertFalse(ThingHelper.equals(thingA, thingB));
+    }
 }