From 51ff29116ffd45c5b35993ae287c8ea834bf489f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Lange?= Date: Sun, 14 Aug 2022 21:31:31 +0200 Subject: [PATCH] [mielecloud] Adapt to inbox api changes (#13185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * No unneccessary calls to thingRegistry.get * Handle Inbox.add now returning CompletableFuture Signed-off-by: Björn Lange --- .../BridgeCreationFailedException.java | 8 +++ .../config/servlet/CreateBridgeServlet.java | 57 +++++++++------ .../servlet/CreateBridgeServletTest.java | 72 ++++++++++++++++--- 3 files changed, 105 insertions(+), 32 deletions(-) diff --git a/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/exception/BridgeCreationFailedException.java b/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/exception/BridgeCreationFailedException.java index f148b06a1..e293fddfa 100644 --- a/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/exception/BridgeCreationFailedException.java +++ b/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/exception/BridgeCreationFailedException.java @@ -22,4 +22,12 @@ import org.eclipse.jdt.annotation.NonNullByDefault; @NonNullByDefault public final class BridgeCreationFailedException extends RuntimeException { private static final long serialVersionUID = -6150154333256723312L; + + public BridgeCreationFailedException(String message) { + super(message); + } + + public BridgeCreationFailedException(String message, Throwable cause) { + super(message, cause); + } } diff --git a/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServlet.java b/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServlet.java index 764c016e2..91100dddc 100644 --- a/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServlet.java +++ b/bundles/org.openhab.binding.mielecloud/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServlet.java @@ -12,7 +12,9 @@ */ package org.openhab.binding.mielecloud.internal.config.servlet; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.function.BooleanSupplier; import javax.servlet.http.HttpServletRequest; @@ -56,6 +58,9 @@ public final class CreateBridgeServlet extends AbstractRedirectionServlet { private static final long DISCOVERY_COMPLETION_TIMEOUT_IN_MILLISECONDS = 5000; private static final long CHECK_INTERVAL_IN_MILLISECONDS = 100; + private static final long INBOX_ENTRY_CREATION_TIMEOUT = 15; + private static final TimeUnit INBOX_ENTRY_CREATION_TIMEOUT_UNIT = TimeUnit.SECONDS; + private final Logger logger = LoggerFactory.getLogger(CreateBridgeServlet.class); private final Inbox inbox; @@ -109,12 +114,12 @@ public final class CreateBridgeServlet extends AbstractRedirectionServlet { waitForBridgeToComeOnline(bridge); return "/mielecloud"; } catch (BridgeReconfigurationFailedException e) { - logger.warn("{}", e.getMessage()); + logger.warn("Thing reconfiguration failed: {}", e.getMessage(), e); return "/mielecloud/success?" + SuccessServlet.BRIDGE_RECONFIGURATION_FAILED_PARAMETER_NAME + "=true&" + SuccessServlet.BRIDGE_UID_PARAMETER_NAME + "=" + bridgeUidString + "&" + SuccessServlet.EMAIL_PARAMETER_NAME + "=" + email; } catch (BridgeCreationFailedException e) { - logger.warn("Thing creation failed because there was no binding available that supports the thing."); + logger.warn("Thing creation failed: {}", e.getMessage(), e); return "/mielecloud/success?" + SuccessServlet.BRIDGE_CREATION_FAILED_PARAMETER_NAME + "=true&" + SuccessServlet.BRIDGE_UID_PARAMETER_NAME + "=" + bridgeUidString + "&" + SuccessServlet.EMAIL_PARAMETER_NAME + "=" + email; @@ -122,37 +127,47 @@ public final class CreateBridgeServlet extends AbstractRedirectionServlet { } private Thing pairOrReconfigureBridge(String locale, ThingUID bridgeUid, String email) { + var thing = thingRegistry.get(bridgeUid); + if (thing != null) { + reconfigureBridge(thing); + return thing; + } else { + return pairBridge(bridgeUid, locale, email); + } + } + + private Thing pairBridge(ThingUID bridgeUid, String locale, String email) { DiscoveryResult result = DiscoveryResultBuilder.create(bridgeUid) .withRepresentationProperty(Thing.PROPERTY_MODEL_ID).withLabel(MIELE_CLOUD_BRIDGE_LABEL) .withProperty(Thing.PROPERTY_MODEL_ID, MIELE_CLOUD_BRIDGE_NAME) .withProperty(MieleCloudBindingConstants.CONFIG_PARAM_LOCALE, locale) .withProperty(MieleCloudBindingConstants.CONFIG_PARAM_EMAIL, email).build(); - if (thingRegistry.get(bridgeUid) != null) { - return reconfigureBridge(bridgeUid); - } else { - inbox.add(result); - return pairBridge(bridgeUid); - } - } - private Thing pairBridge(ThingUID thingUid) { - Thing thing = inbox.approve(thingUid, MIELE_CLOUD_BRIDGE_LABEL, null); + try { + var success = inbox.add(result).get(INBOX_ENTRY_CREATION_TIMEOUT, INBOX_ENTRY_CREATION_TIMEOUT_UNIT); + if (!Boolean.TRUE.equals(success)) { + throw new BridgeCreationFailedException("Adding bridge to inbox failed"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new BridgeCreationFailedException("Interrupted while adding bridge to inbox", e); + } catch (ExecutionException e) { + throw new BridgeCreationFailedException("Adding bridge to inbox failed", e); + } catch (TimeoutException e) { + throw new BridgeCreationFailedException("Adding bridge to inbox failed: Timeout", e); + } + + Thing thing = inbox.approve(bridgeUid, MIELE_CLOUD_BRIDGE_LABEL, null); if (thing == null) { - throw new BridgeCreationFailedException(); + throw new BridgeCreationFailedException("Approving thing from inbox failed"); } - logger.debug("Successfully created bridge {}", thingUid); + logger.debug("Successfully created bridge {}", bridgeUid); return thing; } - private Thing reconfigureBridge(ThingUID thingUid) { + private void reconfigureBridge(Thing thing) { logger.debug("Thing already exists. Modifying configuration."); - Thing thing = thingRegistry.get(thingUid); - if (thing == null) { - throw new BridgeReconfigurationFailedException( - "Cannot modify non existing bridge: Could neither add bridge via inbox nor find existing bridge."); - } - ThingHandler handler = thing.getHandler(); if (handler == null) { throw new BridgeReconfigurationFailedException("Bridge exists but has no handler."); @@ -165,8 +180,6 @@ public final class CreateBridgeServlet extends AbstractRedirectionServlet { MieleBridgeHandler bridgeHandler = (MieleBridgeHandler) handler; bridgeHandler.disposeWebservice(); bridgeHandler.initializeWebservice(); - - return thing; } private String getValidLocale(@Nullable String localeParameterValue) { diff --git a/itests/org.openhab.binding.mielecloud.tests/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServletTest.java b/itests/org.openhab.binding.mielecloud.tests/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServletTest.java index d86b7224c..52bcbe386 100644 --- a/itests/org.openhab.binding.mielecloud.tests/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServletTest.java +++ b/itests/org.openhab.binding.mielecloud.tests/src/main/java/org/openhab/binding/mielecloud/internal/config/servlet/CreateBridgeServletTest.java @@ -19,13 +19,15 @@ import static org.openhab.binding.mielecloud.internal.util.ReflectionUtil.setPri import java.util.Objects; import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; import org.openhab.binding.mielecloud.internal.MieleCloudBindingConstants; import org.openhab.binding.mielecloud.internal.auth.OAuthTokenRefresher; import org.openhab.binding.mielecloud.internal.config.MieleCloudConfigService; -import org.openhab.binding.mielecloud.internal.config.exception.BridgeReconfigurationFailedException; import org.openhab.binding.mielecloud.internal.util.AbstractConfigFlowTest; import org.openhab.binding.mielecloud.internal.util.MieleCloudBindingIntegrationTestConstants; import org.openhab.binding.mielecloud.internal.util.Website; @@ -39,8 +41,8 @@ import org.openhab.core.thing.binding.ThingHandler; */ @NonNullByDefault public class CreateBridgeServletTest extends AbstractConfigFlowTest { - @Test - public void whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage() throws Exception { + private void whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage( + CompletableFuture addInboxEntryResult) throws Exception { // given: MieleCloudConfigService configService = getService(MieleCloudConfigService.class); assertNotNull(configService); @@ -48,8 +50,12 @@ public class CreateBridgeServletTest extends AbstractConfigFlowTest { CreateBridgeServlet createBridgeServlet = configService.getCreateBridgeServlet(); assertNotNull(createBridgeServlet); + ThingRegistry thingRegistry = mock(ThingRegistry.class); + when(thingRegistry.get(MieleCloudBindingIntegrationTestConstants.BRIDGE_THING_UID)).thenReturn(null); + setPrivate(Objects.requireNonNull(createBridgeServlet), "thingRegistry", thingRegistry); + Inbox inbox = mock(Inbox.class); - when(inbox.approve(any(), anyString(), anyString())).thenReturn(null); + when(inbox.add(any())).thenReturn(addInboxEntryResult); setPrivate(Objects.requireNonNull(createBridgeServlet), "inbox", inbox); // when: @@ -65,7 +71,51 @@ public class CreateBridgeServletTest extends AbstractConfigFlowTest { } @Test - public void whenBridgeReconfigurationFailsDueToMissingBridgeThenAWarningIsShownOnTheSuccessPage() throws Exception { + public void whenBridgeCreationFailsBecauseInboxEntryCannotBeAddedThenAWarningIsShownOnTheSuccessPage() + throws Exception { + whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(CompletableFuture.completedFuture(false)); + } + + @Test + public void whenBridgeCreationFailsBecauseInboxEntryAddResultIsNullThenAWarningIsShownOnTheSuccessPage() + throws Exception { + whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(CompletableFuture.completedFuture(null)); + } + + @SuppressWarnings("unchecked") + private CompletableFuture mockBooleanResultCompletableFuture() { + return mock(CompletableFuture.class); + } + + @Test + public void whenBridgeCreationFailBecauseInboxEntryCreationIsInterruptedThenAWarningIsShownOnTheSuccessPage() + throws Exception { + CompletableFuture future = mockBooleanResultCompletableFuture(); + when(future.get(anyLong(), any())).thenThrow(new InterruptedException()); + + whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(future); + } + + @Test + public void whenBridgeCreationFailBecauseInboxEntryCreationFailsThenAWarningIsShownOnTheSuccessPage() + throws Exception { + CompletableFuture future = mockBooleanResultCompletableFuture(); + when(future.get(anyLong(), any())).thenThrow(new ExecutionException(new NullPointerException())); + + whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(future); + } + + @Test + public void whenBridgeCreationFailBecauseInboxEntryCreationTimesOutThenAWarningIsShownOnTheSuccessPage() + throws Exception { + CompletableFuture future = mockBooleanResultCompletableFuture(); + when(future.get(anyLong(), any())).thenThrow(new TimeoutException()); + + whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(future); + } + + @Test + public void whenBridgeCreationFailBecauseInboxApprovalFailsThenAWarningIsShownOnTheSuccessPage() throws Exception { // given: MieleCloudConfigService configService = getService(MieleCloudConfigService.class); assertNotNull(configService); @@ -73,13 +123,15 @@ public class CreateBridgeServletTest extends AbstractConfigFlowTest { CreateBridgeServlet createBridgeServlet = configService.getCreateBridgeServlet(); assertNotNull(createBridgeServlet); - Inbox inbox = mock(Inbox.class); - setPrivate(Objects.requireNonNull(createBridgeServlet), "inbox", inbox); - ThingRegistry thingRegistry = mock(ThingRegistry.class); - when(thingRegistry.get(any())).thenThrow(new BridgeReconfigurationFailedException("")); + when(thingRegistry.get(MieleCloudBindingIntegrationTestConstants.BRIDGE_THING_UID)).thenReturn(null); setPrivate(Objects.requireNonNull(createBridgeServlet), "thingRegistry", thingRegistry); + Inbox inbox = mock(Inbox.class); + when(inbox.add(any())).thenReturn(CompletableFuture.completedFuture(true)); + when(inbox.approve(any(), anyString(), anyString())).thenReturn(null); + setPrivate(Objects.requireNonNull(createBridgeServlet), "inbox", inbox); + // when: Website website = getCrawler().doGetRelative("/mielecloud/createBridgeThing?" + CreateBridgeServlet.BRIDGE_UID_PARAMETER_NAME + "=" @@ -89,7 +141,7 @@ public class CreateBridgeServletTest extends AbstractConfigFlowTest { // then: assertTrue(website.contains("Pairing successful!")); assertTrue(website.contains( - "Could not auto reconfigure the bridge. Bridge thing or thing handler is not available. Please try the configuration flow again.")); + "Could not auto configure the bridge. Failed to approve the bridge from the inbox. Please try the configuration flow again.")); } @Test