[mielecloud] Adapt to inbox api changes (#13185)

* No unneccessary calls to thingRegistry.get
* Handle Inbox.add now returning CompletableFuture

Signed-off-by: Björn Lange <bjoern.lange@itemis.de>

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);
+    }
 }

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) {
+        try {
-        Thing thing = inbox.approve(thingUid, MIELE_CLOUD_BRIDGE_LABEL, null);
+            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) {

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
+    private void whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(
-    public void whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage() throws Exception {
+            CompletableFuture<Boolean> 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<Boolean> mockBooleanResultCompletableFuture() {
+        return mock(CompletableFuture.class);
+    }
+
+    @Test
+    public void whenBridgeCreationFailBecauseInboxEntryCreationIsInterruptedThenAWarningIsShownOnTheSuccessPage()
+            throws Exception {
+        CompletableFuture<Boolean> future = mockBooleanResultCompletableFuture();
+        when(future.get(anyLong(), any())).thenThrow(new InterruptedException());
+
+        whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(future);
+    }
+
+    @Test
+    public void whenBridgeCreationFailBecauseInboxEntryCreationFailsThenAWarningIsShownOnTheSuccessPage()
+            throws Exception {
+        CompletableFuture<Boolean> future = mockBooleanResultCompletableFuture();
+        when(future.get(anyLong(), any())).thenThrow(new ExecutionException(new NullPointerException()));
+
+        whenBridgeCreationFailsThenAWarningIsShownOnTheSuccessPage(future);
+    }
+
+    @Test
+    public void whenBridgeCreationFailBecauseInboxEntryCreationTimesOutThenAWarningIsShownOnTheSuccessPage()
+            throws Exception {
+        CompletableFuture<Boolean> 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