Code review changes

* Fixed @nullable annotations
* Fixed too many logger messages
* Rewritten configuration to use configuration class

Signed-off-by: Jan Vybíral <jan.vybiral1@gmail.com>

bundles/org.openhab.binding.nuki/README.md

@@ -24,7 +24,7 @@ The Sheet [NukiBridgeAPI](https://docs.google.com/spreadsheets/d/1SGKWhqwqRyOGbv
 
 ## Supported Bridges
 
-This binding supports just one bridge type: The Nuki Bridge. Create one `bridge` per Nuki Bridge available in your home automation environment.  
+This binding supports just one bridge type: The Nuki Bridge (`nuki:bridge`). Create one `bridge` per Nuki Bridge available in your home automation environment.  
 
 The following configuration options are available:  
 
@@ -51,7 +51,7 @@ be created but token must be set manually for binding to work.
 
 ## Supported Things
 
-This binding supports 2 things - Nuki Smart Lock and Nuki Opener. Both devices can be added using discovery after bridge they are 
+This binding supports 2 things - Nuki Smart Lock (`nuki:smartlock`) and Nuki Opener (`nuki:opener`). Both devices can be added using discovery after bridge they are 
 connected to is configured and online.
 
 ### Nuki Smart Lock
@@ -80,10 +80,10 @@ The following configuration options are available:
   | 5      | Lock 'n' Go with Unlatch |
 
   Supported lock states are:
-  
+
   | State  | Name                     |
   |--------|--------------------------|
-  | 0      | Uncalibared              |
+  | 0      | Uncalibrated             |
   | 1      | Locked                   |
   | 2      | Unlocking                |
   | 3      | Unlocked                 |
@@ -110,7 +110,8 @@ The following configuration options are available:
 
 - **doorsensorState** (Number)
     Use this channel if you want to display the current door state provided by the door sensor.
-
+
+
   | Action | Name                     |
   |--------|--------------------------|
   | 0      | Unavailable              |

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/NukiHandlerFactory.java

@@ -42,6 +42,7 @@
  * handlers.
  *
  * @author Markus Katter - Initial contribution
+ * @contributer Jan Vybíral - Improved thing id generation
  */
 @Component(service = ThingHandlerFactory.class, configurationPid = "binding.nuki")
 @NonNullByDefault
@@ -68,43 +69,35 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
 
     @Override
     protected @Nullable ThingHandler createHandler(Thing thing) {
-        logger.debug("NukiHandlerFactory:createHandler({})", thing);
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (NukiBindingConstants.THING_TYPE_BRIDGE_UIDS.contains(thingTypeUID)) {
             String callbackUrl = createCallbackUrl(thing.getUID().getId());
             NukiBridgeHandler nukiBridgeHandler = new NukiBridgeHandler((Bridge) thing, httpClient, callbackUrl);
-            if (!nukiBridgeHandler.isInitializable()) {
-                return null;
-            }
             nukiApiServlet.add(nukiBridgeHandler);
             return nukiBridgeHandler;
         } else if (NukiBindingConstants.THING_TYPE_SMARTLOCK_UIDS.contains(thingTypeUID)) {
             return new NukiSmartLockHandler(thing);
         } else if (NukiBindingConstants.THING_TYPE_OPENER_UIDS.contains(thingTypeUID)) {
             return new NukiOpenerHandler(thing);
         }
-        logger.trace("No valid Handler found for Thing[{}]!", thingTypeUID);
+        logger.warn("No valid Handler found for Thing[{}]!", thingTypeUID);
         return null;
     }
 
     @Override
     public void unregisterHandler(Thing thing) {
         super.unregisterHandler(thing);
-        logger.trace("NukiHandlerFactory:unregisterHandler({})", thing);
-        @Nullable
         ThingHandler handler = thing.getHandler();
         if (handler instanceof NukiBridgeHandler) {
             nukiApiServlet.remove((NukiBridgeHandler) handler);
         }
     }
 
     private @Nullable String createCallbackUrl(String bridgeId) {
-        logger.trace("createCallbackUrl()");
-        @Nullable
         final String ipAddress = networkAddressService.getPrimaryIpv4HostAddress();
         if (ipAddress == null) {
-            logger.warn("No network interface could be found.");
+            logger.warn("No network interface could be found to get callback address");
             return null;
         }
         // we do not use SSL as it can cause certificate validation issues.

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/configuration/NukiBridgeConfiguration.java

@@ -0,0 +1,26 @@
+/**
+ * Copyright (c) 2010-2021 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.nuki.internal.configuration;
+
+/**
+ * Configuration for Nuki Bridge
+ *
+ * @author Jan Vybíral - Initial contribution
+ */
+public class NukiBridgeConfiguration {
+    public String ip;
+    public Integer port;
+    public String apiToken;
+    public boolean manageCallbacks = true;
+    public boolean secureToken = true;
+}

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/constants/NukiBindingConstants.java

@@ -59,9 +59,6 @@ public class NukiBindingConstants {
     public static final String PROPERTY_NAME = "name";
     public static final String PROPERTY_NUKI_ID = "nukiId";
     public static final String PROPERTY_BRIDGE_ID = "bridgeId";
-    public static final String PROPERTY_BRIDGE_TOKEN = "bridgeToken";
-    public static final String PROPERTY_BRIDGE_IP = "bridgeIp";
-    public static final String PROPERTY_BRIDGE_PORT = "bridgePort";
 
     // List of all Smart Lock Channel ids
     public static final String CHANNEL_SMARTLOCK_LOCK = "lock";

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/constants/NukiLinkBuilder.java

@@ -70,7 +70,6 @@ public NukiLinkBuilder(String host, int port, String token, boolean secureToken)
         this.port = port;
         this.token = token;
         this.secureToken = secureToken;
-        logger.debug("Instantiating NukiLinkBuilder({}:{}, secured: {})", host, port, secureToken);
     }
 
     public static URI getAuthUri(String host, int port) {

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/constants/OpenerAction.java

@@ -34,8 +34,7 @@ public enum OpenerAction {
         this.action = action;
     }
 
-    @Nullable
-    public static OpenerAction fromAction(int action) {
+    public static @Nullable OpenerAction fromAction(int action) {
         for (OpenerAction value : values()) {
             if (value.action == action) {
                 return value;

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/constants/SmartLockAction.java

@@ -34,8 +34,7 @@ public enum SmartLockAction {
         this.action = action;
     }
 
-    @Nullable
-    public static SmartLockAction fromAction(int action) {
+    public static @Nullable SmartLockAction fromAction(int action) {
         for (SmartLockAction value : values()) {
             if (value.action == action) {
                 return value;

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/BridgeCallbackListResponse.java

@@ -16,7 +16,6 @@
 import java.util.Collections;
 import java.util.List;
 
-import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.nuki.internal.dto.BridgeApiCallbackListCallbackDto;
@@ -47,7 +46,6 @@ public BridgeCallbackListResponse(NukiBaseResponse nukiBaseResponse) {
         super(nukiBaseResponse.getStatus(), nukiBaseResponse.getMessage());
     }
 
-    @NonNull
     public List<BridgeApiCallbackListCallbackDto> getCallbacks() {
         return callbacks;
     }

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiApiServlet.java

@@ -25,7 +25,6 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
-import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.jetty.http.HttpStatus;
@@ -62,7 +61,6 @@ public class NukiApiServlet extends HttpServlet {
     private final Gson gson;
 
     public NukiApiServlet(HttpService httpService) {
-        logger.debug("Instantiating NukiApiServlet({})", httpService);
         this.httpService = httpService;
         gson = new Gson();
     }
@@ -88,32 +86,30 @@ public void remove(NukiBridgeHandler nukiBridgeHandler) {
     }
 
     private void activate() {
-        logger.debug("Activating NukiApiServlet.");
         Dictionary<String, String> servletParams = new Hashtable<>();
         try {
             httpService.registerServlet(NukiLinkBuilder.CALLBACK_ENDPOINT, this, servletParams,
                     httpService.createDefaultHttpContext());
             logger.debug("Started NukiApiServlet at path[{}]", NukiLinkBuilder.CALLBACK_ENDPOINT);
         } catch (ServletException | NamespaceException e) {
-            logger.error("ERROR: {}", e.getMessage(), e);
+            logger.error("Error activating NukiApiServlet: {}", e.getMessage(), e);
         }
     }
 
     private void deactivate() {
-        logger.trace("deactivate()");
         httpService.unregister(NukiLinkBuilder.CALLBACK_ENDPOINT);
     }
 
     @Override
-    protected void service(HttpServletRequest request, HttpServletResponse response)
-            throws ServletException, IOException {
+    protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException {
         logger.debug("Servlet Request at URI[{}] request[{}]", request.getRequestURI(), request);
         BridgeApiLockStateRequestDto bridgeApiLockStateRequestDto = getBridgeApiLockStateRequestDto(request);
 
         ResponseEntity responseEntity;
         if (bridgeApiLockStateRequestDto == null) {
-            logger.error("Could not handle Bridge CallBack Request - Discarding!");
-            logger.error("Please report a bug, if this request was done by the Nuki Bridge!");
+            logger.warn(
+                    "Could not handle Bridge CallBack Request, Please report a bug, if this request was done by the Nuki Bridge! - {}",
+                    request);
 
             responseEntity = new ResponseEntity(HttpStatus.BAD_REQUEST_400,
                     new NukiHttpServerStatusResponseDto("Invalid BCB-Request!"));
@@ -133,7 +129,7 @@ private ResponseEntity doHandle(BridgeApiLockStateRequestDto request, @Nullable
             logger.trace("Searching Bridge[{}] with NukiBridgeHandler[{}] for nukiId[{}].",
                     nukiBridgeHandler.getThing().getConfiguration().get(NukiBindingConstants.CONFIG_IP),
                     nukiBridgeHandler.getThing().getUID(), nukiId);
-            List<@NonNull Thing> allSmartLocks = nukiBridgeHandler.getThing().getThings();
+            List<Thing> allSmartLocks = nukiBridgeHandler.getThing().getThings();
             for (Thing thing : allSmartLocks) {
                 nukiIdThing = thing.getProperties().get(NukiBindingConstants.PROPERTY_NUKI_ID);
                 if (nukiIdThing != null && nukiIdThing.equals(nukiId)) {
@@ -154,23 +150,19 @@ private ResponseEntity doHandle(BridgeApiLockStateRequestDto request, @Nullable
 
     @Nullable
     private BridgeApiLockStateRequestDto getBridgeApiLockStateRequestDto(HttpServletRequest request) {
-        logger.trace("getBridgeApiLockStateRequestDto(...)");
-        @Nullable
         String requestContent = null;
         try (BufferedReader reader = request.getReader()) {
             requestContent = readAll(reader);
-            @Nullable
             BridgeApiLockStateRequestDto bridgeApiLockStateRequestDto = gson.fromJson(requestContent,
                     BridgeApiLockStateRequestDto.class);
             if (bridgeApiLockStateRequestDto != null && bridgeApiLockStateRequestDto.getNukiId() != null) {
                 logger.trace("requestContent[{}]", requestContent);
                 return bridgeApiLockStateRequestDto;
             } else {
-                logger.error("Invalid BCB-Request payload data!");
-                logger.error("requestContent[{}]", requestContent);
+                logger.debug("Invalid BCB-Request payload data! {}", requestContent);
             }
         } catch (IOException e) {
-            logger.error("Could not read payload from BCB-Request! Message[{}]", e.getMessage());
+            logger.debug("Could not read payload from BCB-Request! Message[{}]", e.getMessage());
         } catch (Exception e) {
             logger.error("Could not create BridgeApiLockStateRequestDto from BCB-Request! Message[{}]", e.getMessage());
             logger.error("requestContent[{}]", requestContent);
@@ -188,10 +180,7 @@ private String readAll(BufferedReader reader) throws IOException {
         return sb.toString();
     }
 
-    @Nullable
-    private AbstractNukiDeviceHandler getDeviceHandler(Thing thing) {
-        logger.trace("getDeviceHandler(...) from thing[{}]", thing.getUID());
-        @Nullable
+    private @Nullable AbstractNukiDeviceHandler getDeviceHandler(Thing thing) {
         AbstractNukiDeviceHandler nsh = (AbstractNukiDeviceHandler) thing.getHandler();
         if (nsh == null) {
             logger.debug("Could not get AbstractNukiDeviceHandler for ThingUID[{}]!", thing.getUID());

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiBaseResponse.java

@@ -14,19 +14,24 @@
 
 import java.time.Instant;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+
 /**
  * The {@link NukiBaseResponse} class is the base class for API Responses.
  *
  * @author Markus Katter - Initial contribution
  */
+@NonNullByDefault
 public class NukiBaseResponse {
 
     private int status;
+    @Nullable
     private String message;
     private boolean success;
     private Instant created;
 
-    public NukiBaseResponse(int status, String message) {
+    public NukiBaseResponse(int status, @Nullable String message) {
         this.status = status;
         this.message = message;
         this.created = Instant.now();
@@ -40,7 +45,7 @@ public void setStatus(int status) {
         this.status = status;
     }
 
-    public String getMessage() {
+    public @Nullable String getMessage() {
         return message;
     }
 

bundles/org.openhab.binding.nuki/src/main/java/org/openhab/binding/nuki/internal/dataexchange/NukiHttpClient.java

@@ -157,7 +157,7 @@ public BridgeLockStateResponse getBridgeLockState(String nukiId, int deviceType)
                 return new BridgeLockStateResponse(status, contentResponse.getReason(), null);
             }
         } catch (Exception e) {
-            logger.debug("Could not get Bridge Lock State! Exception[{}]", e.getMessage());
+            logger.debug("Could not get Bridge Lock State!", e);
             return new BridgeLockStateResponse(handleException(e));
         }
     }