From c7a2026346bdb526b7e4370393493d599875a3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20K=C3=BCper?= Date: Wed, 28 Aug 2024 15:54:57 +0200 Subject: [PATCH] [homematic] Fixed #16940: Spaces in URLs are not allowed (#17206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * #16940: Added check in handler for invalid configuration values. Signed-off-by: Sönke Küper --- .../org.openhab.binding.homematic/README.md | 2 +- .../handler/HomematicBridgeHandler.java | 23 +++++-- .../handler/HomematicBridgeHandlerMock.java | 45 +++++++++++++ .../handler/HomematicBridgeHandlerTest.java | 65 +++++++++++++++++++ 4 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerMock.java create mode 100644 bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerTest.java diff --git a/bundles/org.openhab.binding.homematic/README.md b/bundles/org.openhab.binding.homematic/README.md index 0dd85b61c77..2b13cfb6208 100644 --- a/bundles/org.openhab.binding.homematic/README.md +++ b/bundles/org.openhab.binding.homematic/README.md @@ -141,7 +141,7 @@ Network address of the Homematic gateway Hint for the binding to identify the gateway type (auto|ccu|noccu) (default = "auto"). - **callbackHost** -Callback network address of the system runtime, default is auto-discovery +Callback network address of the system runtime, default is auto-discovery. This value must not contain any white spaces. - **xmlCallbackPort** Callback port of the binding's XML-RPC server, default is 9125 and counts up for each additional bridge diff --git a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandler.java b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandler.java index 5e0e358d1d4..2d4bb424006 100644 --- a/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandler.java +++ b/bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandler.java @@ -20,6 +20,7 @@ import java.util.Collection; import java.util.Map; import java.util.Set; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import org.eclipse.jdt.annotation.NonNull; @@ -58,11 +59,14 @@ import org.slf4j.LoggerFactory; * @author Gerhard Riegler - Initial contribution */ public class HomematicBridgeHandler extends BaseBridgeHandler implements HomematicGatewayAdapter { + + protected ScheduledExecutorService executorService = scheduler; + private final Logger logger = LoggerFactory.getLogger(HomematicBridgeHandler.class); private static final long REINITIALIZE_DELAY_SECONDS = 10; private static final int DUTY_CYCLE_RATIO_LIMIT = 99; private static final int DUTY_CYCLE_DISCONNECTED = -1; - private static SimplePortPool portPool = new SimplePortPool(); + private static final SimplePortPool portPool = new SimplePortPool(); private final Object dutyCycleRatioUpdateLock = new Object(); private final Object initDisposeLock = new Object(); @@ -93,7 +97,7 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat public void initialize() { synchronized (initDisposeLock) { isDisposed = false; - initializeFuture = scheduler.submit(this::initializeInternal); + initializeFuture = executorService.submit(this::initializeInternal); } } @@ -106,6 +110,8 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat config = createHomematicConfig(); try { + this.checkForConfigurationErrors(); + String id = getThing().getUID().getId(); gateway = HomematicGatewayFactory.createGateway(id, config, this, httpClient); configureThingProperties(); @@ -140,6 +146,15 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat } } + /** + * Validates, if the configuration contains errors. + */ + private void checkForConfigurationErrors() { + if (this.config.getCallbackHost().contains(" ")) { + throw new ConfigurationException("The callback host mut not contain white spaces."); + } + } + private void configureThingProperties() { final HmGatewayInfo info = config.getGatewayInfo(); final Map properties = getThing().getProperties(); @@ -160,7 +175,7 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat */ private void scheduleReinitialize() { if (!isDisposed) { - initializeFuture = scheduler.schedule(this::initializeInternal, REINITIALIZE_DELAY_SECONDS, + initializeFuture = executorService.schedule(this::initializeInternal, REINITIALIZE_DELAY_SECONDS, TimeUnit.SECONDS); } } @@ -434,7 +449,7 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat * @param defer true will delete the device once it becomes available. */ public void deleteFromGateway(String address, boolean reset, boolean force, boolean defer) { - scheduler.submit(() -> { + executorService.submit(() -> { logger.debug("Deleting the device '{}' from gateway '{}'", address, getBridge()); getGateway().deleteDevice(address, reset, force, defer); }); diff --git a/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerMock.java b/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerMock.java new file mode 100644 index 00000000000..56a3e085062 --- /dev/null +++ b/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerMock.java @@ -0,0 +1,45 @@ +/** + * Copyright (c) 2010-2024 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.binding.homematic.internal.handler; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; + +import java.util.concurrent.ScheduledExecutorService; + +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jetty.client.HttpClient; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.openhab.binding.homematic.internal.type.HomematicTypeGenerator; +import org.openhab.core.thing.Bridge; + +/** + * The {@link HomematicBridgeHandlerMock} is responsible for mocking {@link HomematicBridgeHandler} + * + * @author Sönke Küper - Initial contribution + */ +@NonNullByDefault +public class HomematicBridgeHandlerMock extends HomematicBridgeHandler { + + public HomematicBridgeHandlerMock(@NonNull Bridge bridge, HomematicTypeGenerator typeGenerator, String ipv4Address, + HttpClient httpClient) { + super(bridge, typeGenerator, ipv4Address, httpClient); + executorService = Mockito.mock(ScheduledExecutorService.class); + doAnswer((InvocationOnMock invocation) -> { + ((Runnable) invocation.getArguments()[0]).run(); + return null; + }).when(executorService).submit(any(Runnable.class)); + } +} diff --git a/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerTest.java b/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerTest.java new file mode 100644 index 00000000000..6c60e907000 --- /dev/null +++ b/bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerTest.java @@ -0,0 +1,65 @@ +/** + * Copyright (c) 2010-2024 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.binding.homematic.internal.handler; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import java.util.Objects; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jetty.client.HttpClient; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.openhab.binding.homematic.internal.HomematicBindingConstants; +import org.openhab.binding.homematic.internal.type.HomematicTypeGenerator; +import org.openhab.core.thing.Bridge; +import org.openhab.core.thing.ThingStatus; +import org.openhab.core.thing.ThingStatusDetail; +import org.openhab.core.thing.binding.ThingHandlerCallback; +import org.openhab.core.thing.internal.BridgeImpl; + +/** + * @author Sönke Küper - Initial contribution + */ +@NonNullByDefault +public class HomematicBridgeHandlerTest { + + @Test + public void testGetRpcCallbackUrlDoesNotContainsSpaces() { + HttpClient httpClient = Mockito.mock(HttpClient.class); + + Bridge bridge = new BridgeImpl(HomematicBindingConstants.THING_TYPE_BRIDGE, "1234"); + bridge.getConfiguration().put("callbackHost", " 192. 168.1.1"); + assertThat(bridge.getStatus(), is(ThingStatus.UNINITIALIZED)); + HomematicTypeGenerator typeGenerator = mock(HomematicTypeGenerator.class); + + HomematicBridgeHandlerMock handler = new HomematicBridgeHandlerMock(bridge, typeGenerator, "1.2.3.4", + httpClient); + ThingHandlerCallback callback = mock(ThingHandlerCallback.class); + handler.setCallback(callback); + handler.initialize(); + + try { + verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.OFFLINE) + && arg.getStatusDetail().equals(ThingStatusDetail.CONFIGURATION_ERROR) + && Objects.equals(arg.getDescription(), "The callback host mut not contain white spaces."))); + } finally { + handler.dispose(); + } + } +}