From 923f720a5c256613fbefb4d4bae7634732a14d87 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Jan=20Vyb=C3=ADral?= <jan.vybiral1@gmail.com>
Date: Sun, 27 Feb 2022 19:46:32 +0100
Subject: [PATCH] [nuki] Fixed configuration reload on initialization (#12276)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

* Fixed bug where thing configuration was not reloaded after reinitialization
* Added better logging when processing bridge callback

Signed-off-by: Jan Vybíral <jan.vybiral1@gmail.com>
---
 .../nuki/internal/NukiHandlerFactory.java     |  6 ----
 .../internal/dataexchange/NukiApiServlet.java |  9 ++++--
 .../handler/AbstractNukiDeviceHandler.java    | 31 ++++++++++++++-----
 .../internal/handler/NukiOpenerHandler.java   |  4 ++-
 .../handler/NukiSmartLockHandler.java         |  5 ---
 5 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/NukiHandlerFactory.java b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/NukiHandlerFactory.java
index ab4cd67514..8e92155b16 100644
--- a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/NukiHandlerFactory.java
+++ b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/NukiHandlerFactory.java
@@ -21,7 +21,6 @@ import org.openhab.binding.nuki.internal.dataexchange.NukiApiServlet;
 import org.openhab.binding.nuki.internal.handler.NukiBridgeHandler;
 import org.openhab.binding.nuki.internal.handler.NukiOpenerHandler;
 import org.openhab.binding.nuki.internal.handler.NukiSmartLockHandler;
-import org.openhab.core.config.core.Configuration;
 import org.openhab.core.id.InstanceUUID;
 import org.openhab.core.io.net.http.HttpClientFactory;
 import org.openhab.core.net.HttpServiceUtil;
@@ -88,11 +87,6 @@ public class NukiHandlerFactory extends BaseThingHandlerFactory {
         return null;
     }
 
-    @Override
-    protected @Nullable Thing createThing(ThingTypeUID thingTypeUID, Configuration configuration, ThingUID thingUID) {
-        return super.createThing(thingTypeUID, configuration, thingUID);
-    }
-
     @Override
     public void removeThing(ThingUID thingUID) {
         super.removeThing(thingUID);
diff --git a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiApiServlet.java b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiApiServlet.java
index 597d508533..675efd4d50 100644
--- a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiApiServlet.java
+++ b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiApiServlet.java
@@ -114,7 +114,12 @@ public class NukiApiServlet extends HttpServlet {
             responseEntity = new ResponseEntity(HttpStatus.BAD_REQUEST_400,
                     new NukiHttpServerStatusResponseDto("Invalid BCB-Request!"));
         } else {
-            responseEntity = doHandle(bridgeApiLockStateRequestDto, request.getParameter("bridgeId"));
+            try {
+                responseEntity = doHandle(bridgeApiLockStateRequestDto);
+            } catch (Exception e) {
+                logger.warn("Error processing request '{}'", gson.toJson(bridgeApiLockStateRequestDto), e);
+                throw e;
+            }
         }
 
         setHeaders(response);
@@ -122,7 +127,7 @@ public class NukiApiServlet extends HttpServlet {
         response.getWriter().write(gson.toJson(responseEntity.getData()));
     }
 
-    private ResponseEntity doHandle(BridgeApiLockStateRequestDto request, @Nullable String bridgeId) {
+    private ResponseEntity doHandle(BridgeApiLockStateRequestDto request) {
         String nukiId = request.getNukiId().toString();
         for (NukiBridgeHandler nukiBridgeHandler : nukiBridgeHandlers) {
             logger.trace("Searching Bridge[{}] with NukiBridgeHandler[{}] for nukiId[{}].",
diff --git a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/AbstractNukiDeviceHandler.java b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/AbstractNukiDeviceHandler.java
index 7e572721da..23387dcd76 100644
--- a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/AbstractNukiDeviceHandler.java
+++ b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/AbstractNukiDeviceHandler.java
@@ -32,6 +32,7 @@ import org.openhab.binding.nuki.internal.dataexchange.NukiBaseResponse;
 import org.openhab.binding.nuki.internal.dataexchange.NukiHttpClient;
 import org.openhab.binding.nuki.internal.dto.BridgeApiDeviceStateDto;
 import org.openhab.binding.nuki.internal.dto.BridgeApiListDeviceDto;
+import org.openhab.core.config.core.Configuration;
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.OnOffType;
@@ -70,6 +71,11 @@ public abstract class AbstractNukiDeviceHandler<T extends NukiDeviceConfiguratio
     @Nullable
     private NukiHttpClient nukiHttpClient;
 
+    public AbstractNukiDeviceHandler(Thing thing) {
+        super(thing);
+        this.configuration = getConfigAs(getConfigurationClass());
+    }
+
     private static String hexToDecimal(String hexString) {
         return String.valueOf(Integer.parseInt(hexString, 16));
     }
@@ -92,23 +98,34 @@ public abstract class AbstractNukiDeviceHandler<T extends NukiDeviceConfiguratio
         }
     }
 
-    public AbstractNukiDeviceHandler(Thing thing) {
-        super(thing);
-        this.configuration = getConfigAs(getConfigurationClass());
+    /**
+     * Performs migration of old device configuration
+     * 
+     * @return true if configuration was change and reload is needed
+     */
+    protected boolean migrateConfiguration() {
+        String nukiId = getConfig().get(NukiBindingConstants.PROPERTY_NUKI_ID).toString();
         // legacy support - check if nukiId is hexadecimal (which might have been set by previous binding version)
         // and convert it to decimal
-        if (NUKI_ID_HEX_PATTERN.matcher(this.configuration.nukiId).matches()) {
+        if (NUKI_ID_HEX_PATTERN.matcher(nukiId).matches()) {
             logger.warn(
                     "SmartLock '{}' was created by old version of binding. It is recommended to delete it and discover again",
                     thing.getUID());
-            this.thing.getConfiguration().put(NukiBindingConstants.PROPERTY_NUKI_ID,
-                    hexToDecimal(configuration.nukiId));
-            this.configuration = getConfigAs(getConfigurationClass());
+            Configuration newConfig = editConfiguration();
+            newConfig.put(NukiBindingConstants.PROPERTY_NUKI_ID, hexToDecimal(nukiId));
+            updateConfiguration(newConfig);
+            return true;
+        } else {
+            return false;
         }
     }
 
     @Override
     public void initialize() {
+        this.configuration = getConfigAs(getConfigurationClass());
+        if (migrateConfiguration()) {
+            this.configuration = getConfigAs(getConfigurationClass());
+        }
         scheduler.execute(this::initializeHandler);
     }
 
diff --git a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiOpenerHandler.java b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiOpenerHandler.java
index 196f8882e3..662c580614 100644
--- a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiOpenerHandler.java
+++ b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiOpenerHandler.java
@@ -14,6 +14,7 @@ package org.openhab.binding.nuki.internal.handler;
 
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Objects;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.binding.nuki.internal.configuration.NukiDeviceConfiguration;
@@ -49,7 +50,8 @@ public class NukiOpenerHandler extends AbstractNukiDeviceHandler<NukiDeviceConfi
         updateState(NukiBindingConstants.CHANNEL_OPENER_RING_ACTION_TIMESTAMP, state.getRingactionTimestamp(),
                 this::toDateTime);
 
-        if (state.getRingactionState() && Duration.between(lastRingAction, Instant.now()).getSeconds() > 30) {
+        if (Objects.equals(state.getRingactionState(), true)
+                && Duration.between(lastRingAction, Instant.now()).getSeconds() > 30) {
             triggerChannel(NukiBindingConstants.CHANNEL_OPENER_RING_ACTION_STATE, NukiBindingConstants.EVENT_RINGING);
             lastRingAction = Instant.now();
         }
diff --git a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiSmartLockHandler.java b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiSmartLockHandler.java
index 50caba669c..39d2db1fe2 100644
--- a/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiSmartLockHandler.java
+++ b/bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/handler/NukiSmartLockHandler.java
@@ -39,11 +39,6 @@ public class NukiSmartLockHandler extends AbstractNukiDeviceHandler<NukiSmartLoc
         super(thing);
     }
 
-    @Override
-    public void initialize() {
-        super.initialize();
-    }
-
     @Override
     public void refreshState(BridgeApiDeviceStateDto state) {
         updateState(NukiBindingConstants.CHANNEL_SMARTLOCK_LOCK,
-- 
2.47.3