Fix cofniguration handling

Signed-off-by: Leo Siepel <leosiepel@gmail.com>

Author: lsiepel

bundles/org.openhab.binding.mqtt/src/main/java/org/openhab/binding/mqtt/generic/internal/handler/GenericMQTTThingHandler.java

@@ -151,6 +151,15 @@ public void initialize() {
             }
             final ChannelConfig channelConfig = channel.getConfiguration().as(ChannelConfig.class);
 
+            boolean triggerChannel = channelConfig.trigger
+                    || MqttBindingConstants.TRIGGER.equals(channelTypeUID.getId());
+            if (channelConfig.stateTopic.isBlank() && (triggerChannel || channelConfig.commandTopic.isBlank())) {
+                String requiredTopics = triggerChannel ? "stateTopic" : "stateTopic or commandTopic";
+                logger.warn("Channel '{}' must define {}", channel.getUID(), requiredTopics);
+                configErrors.add(channel.getUID());
+                continue;
+            }
+
             if (channelTypeUID
                     .equals(new ChannelTypeUID(MqttBindingConstants.BINDING_ID, MqttBindingConstants.NUMBER))) {
                 Unit<?> unit = UnitUtils.parseUnit(channelConfig.unit);
@@ -197,7 +206,7 @@ public void initialize() {
         // If some channels could not start up, put the entire thing offline and display the channels
         // in question to the user.
         if (!configErrors.isEmpty()) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Remove and recreate: "
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Invalid channel configuration: "
                     + configErrors.stream().map(ChannelUID::getAsString).collect(Collectors.joining(",")));
             return;
         }

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/generic/internal/handler/GenericThingHandlerTests.java

@@ -19,6 +19,8 @@
 import static org.mockito.Mockito.*;
 import static org.openhab.binding.mqtt.generic.internal.handler.ThingChannelConstants.*;
 
+import java.util.List;
+import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -42,12 +44,15 @@
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
 import org.openhab.core.library.types.StringType;
+import org.openhab.core.thing.Channel;
 import org.openhab.core.thing.ChannelUID;
 import org.openhab.core.thing.Thing;
 import org.openhab.core.thing.ThingStatus;
 import org.openhab.core.thing.ThingStatusDetail;
 import org.openhab.core.thing.ThingStatusInfo;
 import org.openhab.core.thing.binding.ThingHandlerCallback;
+import org.openhab.core.thing.binding.builder.ChannelBuilder;
+import org.openhab.core.thing.type.ChannelKind;
 import org.openhab.core.types.RefreshType;
 import org.openhab.core.types.UnDefType;
 
@@ -125,6 +130,55 @@ public void initialize() {
                 && ThingStatusDetail.NONE.equals(arg.getStatusDetail())));
     }
 
+    @Test
+    public void initializeWithStateOnlyChannel() {
+        Channel channel = cb("stateOnly", "String", new Configuration(Map.of("stateTopic", "test/state")),
+                TEXT_CHANNEL);
+        when(thingMock.getChannels()).thenReturn(List.of(channel));
+
+        thingHandler.initialize();
+
+        assertThat(thingHandler.channelStateByChannelUID.containsKey(channel.getUID()), is(true));
+        verify(connectionMock).subscribe(eq("test/state"), any());
+    }
+
+    @Test
+    public void initializeWithCommandOnlyChannel() {
+        Channel channel = cb("commandOnly", "String", new Configuration(Map.of("commandTopic", "test/command")),
+                TEXT_CHANNEL);
+        when(thingMock.getChannels()).thenReturn(List.of(channel));
+
+        thingHandler.initialize();
+
+        assertThat(thingHandler.channelStateByChannelUID.containsKey(channel.getUID()), is(true));
+        verify(connectionMock, never()).subscribe(any(), any());
+    }
+
+    @Test
+    public void initializeRejectsChannelWithUnknownPropertiesInsteadOfTopics() {
+        Channel channel = cb("invalid", "String",
+                new Configuration(Map.of("status", "test/state", "trans", "JSONPATH:$.value")), TEXT_CHANNEL);
+
+        assertConfigurationError(channel);
+    }
+
+    @Test
+    public void initializeRejectsTypedTriggerWithoutStateTopic() {
+        Channel channel = cb("invalidTypedTrigger", "String",
+                new Configuration(Map.of("commandTopic", "test/command", "trigger", true)), TEXT_CHANNEL);
+
+        assertConfigurationError(channel);
+    }
+
+    @Test
+    public void initializeRejectsTriggerWithoutStateTopic() {
+        ChannelUID channelUID = new ChannelUID(TEST_GENERIC_THING, "invalidTrigger");
+        Channel channel = ChannelBuilder.create(channelUID).withType(TRIGGER_CHANNEL).withKind(ChannelKind.TRIGGER)
+                .withConfiguration(new Configuration(Map.of("commandTopic", "test/command"))).build();
+
+        assertConfigurationError(channel);
+    }
+
     @Test
     public void handleCommandRefresh() {
         TextValue value = spy(new TextValue());
@@ -206,4 +260,17 @@ public void handleBridgeStatusChange() {
         thingHandler.bridgeStatusChanged(new ThingStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, null));
         verify(connectionMock, times(2)).subscribe(eq("test/LWT"), any());
     }
+
+    private void assertConfigurationError(Channel channel) {
+        when(thingMock.getChannels()).thenReturn(List.of(channel));
+
+        thingHandler.initialize();
+
+        assertThat(thingHandler.channelStateByChannelUID.containsKey(channel.getUID()), is(false));
+        verify(thingHandler, never()).start(any());
+        verify(callbackMock).statusUpdated(eq(thingMock),
+                argThat(arg -> ThingStatus.OFFLINE.equals(arg.getStatus())
+                        && ThingStatusDetail.CONFIGURATION_ERROR.equals(arg.getStatusDetail())
+                        && ("Invalid channel configuration: " + channel.getUID()).equals(arg.getDescription())));
+    }
 }

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/generic/internal/handler/ThingChannelConstants.java

@@ -45,6 +45,7 @@ public class ThingChannelConstants {
     public static final ChannelTypeUID ON_OFF_CHANNEL = new ChannelTypeUID(BINDING_ID, MqttBindingConstants.SWITCH);
     public static final ChannelTypeUID NUMBER_CHANNEL = new ChannelTypeUID(BINDING_ID, MqttBindingConstants.NUMBER);
     public static final ChannelTypeUID PERCENTAGE_CHANNEL = new ChannelTypeUID(BINDING_ID, MqttBindingConstants.DIMMER);
+    public static final ChannelTypeUID TRIGGER_CHANNEL = new ChannelTypeUID(BINDING_ID, MqttBindingConstants.TRIGGER);
     public static final ChannelTypeUID UNKNOWN_CHANNEL = new ChannelTypeUID(BINDING_ID, "unknown");
 
     public static final ChannelUID TEXT_CHANNEL_UID = new ChannelUID(TEST_GENERIC_THING, "mytext");