PR review: fix npe potential NPP and use record instead of class for DTO

Signed-off-by: Jordan Martin <jo69270@gmail.com>

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

@@ -1,6 +1,6 @@
 # The Keys Binding
 
-This is the binding for [TheKeys Smartlock](https://www.the-keys.eu/fr/produits/8-serrure-connectee.html).
+This is the binding for [TheKeys Smartlock](https://www.the-keys.eu/en/produits/8-serrure-connectee.html).
 This binding allows you to integrate, view, control and configure TheKeys Gateway and TheKeys Smartlock.
 
 ![The Keys smartlock](doc/thekeys-smartlock.png)
@@ -79,7 +79,7 @@ A manual setup through files could look like this:
 
 ```
 Bridge thekeys:gateway:tk-gateway [ host="192.168.1.50", code="secretcode", refreshInterval=5, apiTimeout=30 ] {
- Thing smartlock tk-smartlock [ lockId=1234 ]
+ Thing smartlock 1234 [ lockId=1234 ]
 }
 ```
 
@@ -90,12 +90,12 @@ Bridge thekeys:gateway:tk-gateway [ host="192.168.1.50", code="secretcode", refr
 Group Smartlock "Smartlock" ["Equipment"]
 
 // Points
-String Smartlock_Lock_status "Lock status" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:status" } 
-Number Smartlock_Niveau_Batterie "Battery level" <Battery> (Smartlock) ["Measurement", "Energy"] { channel="thekeys:smartlock:tk-gateway:1234:batteryLevel" } 
-Switch Smartlock_Batterie_Faible "Low battery" <LowBattery> (Smartlock) ["Energy", "LowBattery"] { channel="thekeys:smartlock:tk-gateway:1234:lowBattery" } 
-Number Smartlock_Bluetooth_rssi  "Bluetooth rssi" <QualityOfService> (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:rssi" } 
-Number Smartlock_Smartlock_position "Smartlock position" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:position" } 
-Switch Smartlock_Synchronization_in_progress "Synchronization in progress" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:syncInProgress" } 
-DateTime Smartlock_Last_sync "Last sync" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:lastSync" } 
-Switch Smartlock_Lock "Lock" <Lock> (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:lock" } 
+String Smartlock_Lock_Status  "Lock Status" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:status" } 
+Number Smartlock_Battery_Level  "Battery Level" <Battery> (Smartlock) ["Measurement", "Energy"] { channel="thekeys:smartlock:tk-gateway:1234:batteryLevel" } 
+Switch Smartlock_Low_Battery  "Low Battery" <LowBattery> (Smartlock) ["Energy", "LowBattery"] { channel="thekeys:smartlock:tk-gateway:1234:lowBattery" } 
+Number:Power Smartlock_Bluetooth_RSSI  "Bluetooth RSSI" <QualityOfService> (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:rssi" } 
+Number Smartlock_Smartlock_Position  "Smartlock Position" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:position" } 
+Switch  Smartlock_Synchronization_In_Progress "Synchronization In Progress" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:syncInProgress" } 
+DateTime Smartlock_Last_Sync  "Last Sync" (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:lastSync" } 
+Switch  Smartlock_Lock "Lock" <Lock> (Smartlock) ["Point"] { channel="thekeys:smartlock:tk-gateway:1234:lock" }  
 ```

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/GatewayService.java

@@ -43,11 +43,8 @@
  */
 @NonNullByDefault
 public class GatewayService {
-
- private final Logger logger = LoggerFactory.getLogger(GatewayService.class);
-
  private static final String API_SCHEME = "http://";
-
+ private final Logger logger = LoggerFactory.getLogger(GatewayService.class);
  private final TheKeysGatewayConfiguration configuration;
  private final TheKeysHttpClient httpClient;
 
@@ -142,9 +139,10 @@ private <T> T get(String path, Class<T> responseType, int remainingRetryCount) t
  } catch (Exception e) {
  Throwable rootCause = ExceptionUtils.getRootCause(e);
  // Retry the same request because the gateway failed sometimes to handle the request
- if (remainingRetryCount > 0
- && (rootCause instanceof ConnectException || rootCause instanceof EOFException)) {
- logger.debug("Request failed. {}, retrying {} more times", rootCause.getClass(), remainingRetryCount);
+ if ((remainingRetryCount > 0
+ && (rootCause instanceof ConnectException || rootCause instanceof EOFException))) {
+ String rootCauseString = rootCause == null ? "" : String.valueOf(rootCause.getClass());
+ logger.debug("Request failed. {}, retrying {} more times", rootCauseString, remainingRetryCount);
  return get(path, responseType, --remainingRetryCount);
  }
  throw e;
@@ -191,7 +189,7 @@ private String createRequestBody(int lockId) {
  */
  String computeRequestBodyWithHash(String timestamp, int lockId) {
  String hash = hmacSha256(timestamp, configuration.code);
- return String.format("identifier=%s&ts=%s&hash=%s", lockId, timestamp, hash);
+ return "identifier=%s&ts=%s&hash=%s".formatted(lockId, timestamp, hash);
  }
 
  /**

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/TheKeysError.java

@@ -12,6 +12,8 @@
  */
 package org.openhab.binding.thekeys.internal.api;
 
+import java.io.Serial;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 
@@ -23,6 +25,9 @@
 @NonNullByDefault
 public class TheKeysError extends RuntimeException {
 
+ @Serial
+ private static final long serialVersionUID = 1L;
+
  public TheKeysError(String message) {
  super(message);
  }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/TheKeysHttpClient.java

@@ -15,6 +15,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.core.io.net.http.HttpUtil;
 
@@ -36,14 +37,14 @@ public TheKeysHttpClient() {
  this.gson = new GsonBuilder().setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES).create();
  }
 
- public <T> T get(String url, int timeoutMs, Class<T> responseType) throws IOException {
+ public <T> T get(String url, int timeoutMs, Class<@NonNull T> responseType) throws IOException {
  String json = HttpUtil.executeUrl("GET", url, timeoutMs);
- return gson.fromJson(json, responseType);
+ return (T) gson.fromJson(json, responseType);
  }
 
  public <T> T post(String url, String body, int timeout, Class<T> responseType) throws IOException {
  ByteArrayInputStream bodyInputStream = new ByteArrayInputStream(body.getBytes());
  String json = HttpUtil.executeUrl("POST", url, bodyInputStream, "application/x-www-form-urlencoded", timeout);
- return gson.fromJson(json, responseType);
+ return (T) gson.fromJson(json, responseType);
  }
 }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/model/GatewayInfosDTO.java

@@ -17,24 +17,5 @@
  * 
  * @author Jordan Martin - Initial contribution
  */
-public class GatewayInfosDTO {
-
- private int version;
- private String currentStatus;
-
- public int getVersion() {
- return version;
- }
-
- public void setVersion(int version) {
- this.version = version;
- }
-
- public String getCurrentStatus() {
- return currentStatus;
- }
-
- public void setCurrentStatus(String currentStatus) {
- this.currentStatus = currentStatus;
- }
+public record GatewayInfosDTO(int version, String currentStatus) {
 }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/model/LockerDTO.java

@@ -17,41 +17,5 @@
  *
  * @author Jordan Martin - Initial contribution
  */
-public class LockerDTO {
- private int identifier;
- private int rssi;
- private int battery;
- private int lastLog;
-
- public int getIdentifier() {
- return identifier;
- }
-
- public void setIdentifier(int identifier) {
- this.identifier = identifier;
- }
-
- public int getRssi() {
- return rssi;
- }
-
- public void setRssi(int rssi) {
- this.rssi = rssi;
- }
-
- public int getBattery() {
- return battery;
- }
-
- public void setBattery(int battery) {
- this.battery = battery;
- }
-
- public int getLastLog() {
- return lastLog;
- }
-
- public void setLastLog(int lastLog) {
- this.lastLog = lastLog;
- }
+public record LockerDTO(int identifier, int rssi, int battery, int lastLog) {
 }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/model/OpenCloseDTO.java

@@ -17,33 +17,5 @@
  *
  * @author Jordan Martin - Initial contribution
  */
-public class OpenCloseDTO {
-
- private String status;
- private String code;
- private String cause;
-
- public String getStatus() {
- return status;
- }
-
- public void setStatus(String status) {
- this.status = status;
- }
-
- public String getCode() {
- return code;
- }
-
- public void setCode(String code) {
- this.code = code;
- }
-
- public String getCause() {
- return cause;
- }
-
- public void setCause(String cause) {
- this.cause = cause;
- }
+public record OpenCloseDTO(String status, String code, String cause) {
 }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/api/model/SynchronizeDTO.java

@@ -17,33 +17,5 @@
  *
  * @author Jordan Martin - Initial contribution
  */
-public class SynchronizeDTO {
-
- private String status;
- private int code;
- private String message;
-
- public String getStatus() {
- return status;
- }
-
- public void setStatus(String status) {
- this.status = status;
- }
-
- public int getCode() {
- return code;
- }
-
- public void setCode(int code) {
- this.code = code;
- }
-
- public String getMessage() {
- return message;
- }
-
- public void setMessage(String message) {
- this.message = message;
- }
+public record SynchronizeDTO(String status, int code, String message) {
 }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/gateway/TheKeysGatewayHandler.java

@@ -14,10 +14,7 @@
 
 import static org.openhab.core.thing.Thing.PROPERTY_FIRMWARE_VERSION;
 
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
@@ -82,8 +79,8 @@ public void initialize() {
  */
  private void pollGateway() {
  try {
- GatewayInfosDTO gwInfos = api.getGatewayInfos();
- updateProperty(PROPERTY_FIRMWARE_VERSION, String.valueOf(gwInfos.getVersion()));
+ GatewayInfosDTO gwInfos = Objects.requireNonNull(api).getGatewayInfos();
+ updateProperty(PROPERTY_FIRMWARE_VERSION, String.valueOf(gwInfos.version()));
 
  // Here the api answer without errors => back online
  if (getThing().getStatus() != ThingStatus.ONLINE) {
@@ -99,8 +96,8 @@ private void pollGateway() {
  List<TheKeysSmartlockHandler> locksThing = getThing().getThings().stream().map(Thing::getHandler)
  .map(TheKeysSmartlockHandler.class::cast).toList();
 
- Map<Integer, LockerDTO> locksResponse = api.getLocks().stream()
- .collect(Collectors.toMap(LockerDTO::getIdentifier, Function.identity()));
+ Map<Integer, LockerDTO> locksResponse = Objects.requireNonNull(api).getLocks().stream()
+ .collect(Collectors.toMap(LockerDTO::identifier, Function.identity()));
 
  // Update each handler with corresponding data
  for (TheKeysSmartlockHandler lockHandler : locksThing) {
@@ -130,9 +127,10 @@ private boolean noLockThingIsEnabled() {
 
  @Override
  public void dispose() {
+ ScheduledFuture<?> gwPollingJob = this.gwPollingJob;
  if (gwPollingJob != null && !gwPollingJob.isCancelled()) {
  gwPollingJob.cancel(true);
- gwPollingJob = null;
+ this.gwPollingJob = null;
  }
  super.dispose();
  }

bundles/org.openhab.binding.thekeys/src/main/java/org/openhab/binding/thekeys/internal/smartlock/TheKeysDiscoveryService.java

@@ -15,11 +15,12 @@
 import static org.openhab.binding.thekeys.internal.TheKeysBindingConstants.CONF_SMARTLOCK_LOCKID;
 import static org.openhab.binding.thekeys.internal.TheKeysBindingConstants.THING_TYPE_SMARTLOCK;
 
+import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Stream;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.binding.thekeys.internal.api.GatewayService;
 import org.openhab.binding.thekeys.internal.api.model.LockerDTO;
 import org.openhab.binding.thekeys.internal.gateway.TheKeysGatewayHandler;
 import org.openhab.core.config.discovery.AbstractDiscoveryService;
@@ -50,13 +51,8 @@ public TheKeysDiscoveryService() {
 
  @Override
  protected void startScan() {
- if (gatewayHandler == null) {
- return;
- }
-
  try {
- Stream<LockerDTO> locks = gatewayHandler.getApi().getLocks().stream();
- locks.map(this::createDiscoveryResult).forEach(this::thingDiscovered);
+ getGatewayService().getLocks().stream().map(this::createDiscoveryResult).forEach(this::thingDiscovered);
  } catch (Exception e) {
  logger.warn("Cannot start TheKeys discovery : {}", e.getMessage(), e);
  }
@@ -69,14 +65,13 @@ protected void startScan() {
  * @return The discoveryResult associated with the gateway
  */
  private DiscoveryResult createDiscoveryResult(LockerDTO lock) {
- if (gatewayHandler == null) {
- throw new IllegalStateException();
- }
+ TheKeysGatewayHandler gatewayHandler = Objects.requireNonNull(this.gatewayHandler);
  ThingUID gatewayThingUID = gatewayHandler.getThing().getUID();
  String label = gatewayHandler.getTranslationProvider().getText("discovery.thekeys.smartlock.label");
+
  return DiscoveryResultBuilder.create(createThingUID(lock)).withBridge(gatewayThingUID)
- .withLabel(label + " " + lock.getIdentifier()).withRepresentationProperty(CONF_SMARTLOCK_LOCKID)
- .withProperty(CONF_SMARTLOCK_LOCKID, lock.getIdentifier()).build();
+ .withLabel(label + " " + lock.identifier()).withRepresentationProperty(CONF_SMARTLOCK_LOCKID)
+ .withProperty(CONF_SMARTLOCK_LOCKID, lock.identifier()).build();
  }
 
  /**
@@ -86,11 +81,9 @@ private DiscoveryResult createDiscoveryResult(LockerDTO lock) {
  * @return The thingUID
  */
  private ThingUID createThingUID(LockerDTO lock) {
- if (gatewayHandler == null) {
- throw new IllegalStateException();
- }
+ TheKeysGatewayHandler gatewayHandler = Objects.requireNonNull(this.gatewayHandler);
  ThingUID gatewayThingUID = gatewayHandler.getThing().getUID();
- String lockId = String.valueOf(lock.getIdentifier());
+ String lockId = String.valueOf(lock.identifier());
  return new ThingUID(THING_TYPE_SMARTLOCK, gatewayThingUID, lockId);
  }
 
@@ -107,12 +100,17 @@ protected synchronized void stopScan() {
 
  @Override
  public void setThingHandler(@Nullable ThingHandler handler) {
- if (handler instanceof TheKeysGatewayHandler) {
- gatewayHandler = (TheKeysGatewayHandler) handler;
+ if (handler instanceof TheKeysGatewayHandler theKeysGatewayHandler) {
+ gatewayHandler = theKeysGatewayHandler;
  }
  }
 
  @Override
  public void deactivate() {
+ super.deactivate();
+ }
+
+ private GatewayService getGatewayService() {
+ return Objects.requireNonNull(Objects.requireNonNull(this.gatewayHandler).getApi());
  }
 }