Check valid properties for channel (#5416)

Null key or value are rejected and an IllegalArgumentException is thrown.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

bundles/org.openhab.core.thing/src/main/java/org/openhab/core/thing/binding/builder/ChannelBuilder.java

@@ -15,6 +15,7 @@ package org.openhab.core.thing.binding.builder;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -26,6 +27,8 @@ import org.openhab.core.thing.type.AutoUpdatePolicy;
 import org.openhab.core.thing.type.ChannelKind;
 import org.openhab.core.thing.type.ChannelType;
 import org.openhab.core.thing.type.ChannelTypeUID;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * {@link ChannelBuilder} is responsible for creating {@link Channel}s.
@@ -47,6 +50,8 @@ public class ChannelBuilder {
         }
     }
 
+    private final Logger logger = LoggerFactory.getLogger(ChannelBuilder.class);
+
     private final ChannelUID channelUID;
     private @Nullable String acceptedItemType;
     private ChannelKind kind;
@@ -137,6 +142,16 @@ public class ChannelBuilder {
      * @return channel builder
      */
     public ChannelBuilder withProperties(Map<String, String> properties) {
+        String propertiesWithNullKeyOrValue = properties.entrySet().stream()
+                .filter(e -> e.getKey() == null || e.getValue() == null).map(e -> String.valueOf(e.getKey())).sorted()
+                .collect(Collectors.joining(", "));
+        if (!propertiesWithNullKeyOrValue.isEmpty()) {
+            logger.error(
+                    "Unexpected properties ({}) with null key or value for channel {}; probably a bug in the related binding!",
+                    propertiesWithNullKeyOrValue, channelUID);
+            throw new IllegalArgumentException("Unexpected properties (%s) with null key or value for channel %s"
+                    .formatted(propertiesWithNullKeyOrValue, channelUID.getAsString()));
+        }
         this.properties = properties;
         return this;
     }

bundles/org.openhab.core.thing/src/test/java/org/openhab/core/thing/binding/builder/ChannelBuilderTest.java

@@ -15,11 +15,14 @@ package org.openhab.core.thing.binding.builder;
 import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.collection.IsCollectionWithSize.hasSize;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.openhab.core.thing.DefaultSystemChannelTypeProvider.SYSTEM_OUTDOOR_TEMPERATURE;
 
+import java.util.HashMap;
 import java.util.Map;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.openhab.core.library.CoreItemFactory;
@@ -41,6 +44,7 @@ public class ChannelBuilderTest {
 
     private static final String KEY1 = "key1";
     private static final String KEY2 = "key2";
+    private static final String KEY3 = "key3";
     private static final String VALUE1 = "value1";
     private static final String VALUE2 = "value2";
 
@@ -101,4 +105,25 @@ public class ChannelBuilderTest {
         assertThat(otherChannel.getAcceptedItemType(), is(not(channel.getAcceptedItemType())));
         assertThat(otherChannel.getProperties().size(), is(not(channel.getProperties().size())));
     }
+
+    @Test
+    public void testChannelBuilderWithInvalidProperties() {
+        Map<@Nullable String, @Nullable String> map = new HashMap<>();
+        map.put(KEY1, null);
+        map.put(KEY2, VALUE2);
+        map.put(KEY3, null);
+        map.put(null, VALUE1);
+
+        IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> {
+            builder.withProperties(toNonNullStringMap(map));
+        });
+        assertEquals(
+                "Unexpected properties (key1, key3, null) with null key or value for channel bindingId:thingTypeId:thingId:temperature",
+                exception.getMessage());
+    }
+
+    @SuppressWarnings("unchecked") // Map may contain null keys or values; this is intentional for this test
+    private static Map<String, String> toNonNullStringMap(Map<@Nullable String, @Nullable String> source) {
+        return (Map<String, String>) (Map<?, ?>) source;
+    }
 }