Skip to content

Commit

Permalink
[homematic] Fixed openhab#16940: Spaces in URLs are not allowed (open…
Browse files Browse the repository at this point in the history
…hab#17206)

* openhab#16940: Added check in handler for invalid configuration values.

Signed-off-by: Sönke Küper <[email protected]>
  • Loading branch information
soenkekueper authored Aug 28, 2024
1 parent 198b9b1 commit c7a2026
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 5 deletions.
2 changes: 1 addition & 1 deletion bundles/org.openhab.binding.homematic/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
Expand Down Expand Up @@ -58,11 +59,14 @@
* @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();
Expand Down Expand Up @@ -93,7 +97,7 @@ public HomematicBridgeHandler(@NonNull Bridge bridge, HomematicTypeGenerator typ
public void initialize() {
synchronized (initDisposeLock) {
isDisposed = false;
initializeFuture = scheduler.submit(this::initializeInternal);
initializeFuture = executorService.submit(this::initializeInternal);
}
}

Expand All @@ -106,6 +110,8 @@ private void initializeInternal() {
config = createHomematicConfig();

try {
this.checkForConfigurationErrors();

String id = getThing().getUID().getId();
gateway = HomematicGatewayFactory.createGateway(id, config, this, httpClient);
configureThingProperties();
Expand Down Expand Up @@ -140,6 +146,15 @@ private void initializeInternal() {
}
}

/**
* 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<String, String> properties = getThing().getProperties();
Expand All @@ -160,7 +175,7 @@ private void configureThingProperties() {
*/
private void scheduleReinitialize() {
if (!isDisposed) {
initializeFuture = scheduler.schedule(this::initializeInternal, REINITIALIZE_DELAY_SECONDS,
initializeFuture = executorService.schedule(this::initializeInternal, REINITIALIZE_DELAY_SECONDS,
TimeUnit.SECONDS);
}
}
Expand Down Expand Up @@ -434,7 +449,7 @@ public void updateDatapoint(HmDatapoint dp) throws IOException {
* @param defer <i>true</i> 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);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
}

0 comments on commit c7a2026

Please sign in to comment.