[avmfritz] Avoid a hidden NPE when getIdentifier() of an uninitialize…

…d ThingHandler is called (openhab#9040) * Avoid a hidden NPE when getIdentifier() of an uninitialized ThingHandler is called * Fixed Powerline546EHandler Signed-off-by: Christoph Weitkamp <[email protected]>
bosch-io · Nov 16, 2020 · aafd3be · aafd3be
1 parent 1a113f6
commit aafd3be
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 48 deletions.
diff --git a/...rc/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java b/...rc/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java
@@ -20,7 +20,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -126,7 +125,7 @@ public void initialize() {
         AVMFritzBoxConfiguration config = getConfigAs(AVMFritzBoxConfiguration.class);
 
         String localIpAddress = config.ipAddress;
-        if (localIpAddress == null || localIpAddress.trim().isEmpty()) {
+        if (localIpAddress == null || localIpAddress.isBlank()) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
                     "The 'ipAddress' parameter must be configured.");
             configValid = false;
@@ -271,10 +270,8 @@ public void onDeviceListAdded(List<AVMFritzBaseModel> deviceList) {
         getThing().getThings().forEach(childThing -> {
             final AVMFritzBaseThingHandler childHandler = (AVMFritzBaseThingHandler) childThing.getHandler();
             if (childHandler != null) {
-                final Optional<AVMFritzBaseModel> optionalDevice = Optional
-                        .ofNullable(deviceIdentifierMap.get(childHandler.getIdentifier()));
-                if (optionalDevice.isPresent()) {
-                    final AVMFritzBaseModel device = optionalDevice.get();
+                final AVMFritzBaseModel device = deviceIdentifierMap.get(childHandler.getIdentifier());
+                if (device != null) {
                     deviceList.remove(device);
                     listeners.forEach(listener -> listener.onDeviceUpdated(childThing.getUID(), device));
                 } else {

diff --git a/...src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseThingHandler.java b/...src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseThingHandler.java
@@ -82,7 +82,7 @@ public abstract class AVMFritzBaseThingHandler extends BaseThingHandler implemen
      * keeps track of the current state for handling of increase/decrease
      */
     private @Nullable AVMFritzBaseModel state;
-    private @NonNullByDefault({}) AVMFritzDeviceConfiguration config;
+    private @Nullable String identifier;
 
     /**
      * Constructor
@@ -95,13 +95,13 @@ public AVMFritzBaseThingHandler(Thing thing) {
 
     @Override
     public void initialize() {
-        config = getConfigAs(AVMFritzDeviceConfiguration.class);
-
-        String newIdentifier = config.ain;
-        if (newIdentifier == null || newIdentifier.trim().isEmpty()) {
+        final AVMFritzDeviceConfiguration config = getConfigAs(AVMFritzDeviceConfiguration.class);
+        final String newIdentifier = config.ain;
+        if (newIdentifier == null || newIdentifier.isBlank()) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
                     "The 'ain' parameter must be configured.");
         } else {
+            this.identifier = newIdentifier;
             updateStatus(ThingStatus.UNKNOWN);
         }
     }
@@ -472,6 +472,6 @@ private void handleRefreshCommand() {
      * @return the AIN
      */
     public @Nullable String getIdentifier() {
-        return config.ain;
+        return identifier;
     }
 }
diff --git a/...itz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java b/...itz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java
@@ -17,7 +17,6 @@
 import java.math.BigDecimal;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.function.Predicate;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -32,7 +31,6 @@
 import org.openhab.binding.avmfritz.internal.dto.SwitchModel;
 import org.openhab.binding.avmfritz.internal.hardware.FritzAhaStatusListener;
 import org.openhab.binding.avmfritz.internal.hardware.FritzAhaWebInterface;
-import org.openhab.core.config.core.Configuration;
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.OpenClosedType;
 import org.openhab.core.library.types.QuantityType;
@@ -72,7 +70,7 @@ public class Powerline546EHandler extends AVMFritzBaseBridgeHandler implements F
      * keeps track of the current state for handling of increase/decrease
      */
     private @Nullable AVMFritzBaseModel state;
-    private @Nullable AVMFritzDeviceConfiguration config;
+    private @Nullable String identifier;
 
     /**
      * Constructor
@@ -86,32 +84,27 @@ public Powerline546EHandler(Bridge bridge, HttpClient httpClient,
 
     @Override
     public void initialize() {
-        config = getConfigAs(AVMFritzDeviceConfiguration.class);
-
-        registerStatusListener(this);
+        final AVMFritzDeviceConfiguration config = getConfigAs(AVMFritzDeviceConfiguration.class);
+        final String newIdentifier = config.ain;
+        if (newIdentifier != null && !newIdentifier.isBlank()) {
+            this.identifier = newIdentifier;
+        }
 
         super.initialize();
     }
 
-    @Override
-    public void dispose() {
-        unregisterStatusListener(this);
-
-        super.dispose();
-    }
-
+    @SuppressWarnings({ "null", "unused" })
     @Override
     public void onDeviceListAdded(List<AVMFritzBaseModel> devicelist) {
-        final String identifier = getIdentifier();
-        final Predicate<AVMFritzBaseModel> predicate = identifier == null ? it -> thing.getUID().equals(getThingUID(it))
-                : it -> identifier.equals(it.getIdentifier());
-        final Optional<AVMFritzBaseModel> optionalDevice = devicelist.stream().filter(predicate).findFirst();
-        if (optionalDevice.isPresent()) {
-            final AVMFritzBaseModel device = optionalDevice.get();
+        final String ain = getIdentifier();
+        final Predicate<AVMFritzBaseModel> predicate = ain == null ? it -> thing.getUID().equals(getThingUID(it))
+                : it -> ain.equals(it.getIdentifier());
+        final AVMFritzBaseModel device = devicelist.stream().filter(predicate).findFirst().orElse(null);
+        if (device != null) {
             devicelist.remove(device);
-            listeners.stream().forEach(listener -> listener.onDeviceUpdated(thing.getUID(), device));
+            onDeviceUpdated(thing.getUID(), device);
         } else {
-            listeners.stream().forEach(listener -> listener.onDeviceGone(thing.getUID()));
+            onDeviceGone(thing.getUID());
         }
         super.onDeviceListAdded(devicelist);
     }
@@ -125,8 +118,8 @@ public void onDeviceAdded(AVMFritzBaseModel device) {
     public void onDeviceUpdated(ThingUID thingUID, AVMFritzBaseModel device) {
         if (thing.getUID().equals(thingUID)) {
             // save AIN to config for FRITZ!Powerline 546E stand-alone
-            if (config == null) {
-                updateConfiguration(device);
+            if (this.identifier == null) {
+                this.identifier = device.getIdentifier();
             }
 
             logger.debug("Update self '{}' with device model: {}", thingUID, device);
@@ -185,17 +178,6 @@ private void updateProperties(AVMFritzBaseModel device) {
         updateProperties(editProperties);
     }
 
-    /**
-     * Updates thing configuration.
-     *
-     * @param device the {@link AVMFritzBaseModel}
-     */
-    private void updateConfiguration(AVMFritzBaseModel device) {
-        Configuration editConfig = editConfiguration();
-        editConfig.put(CONFIG_AIN, device.getIdentifier());
-        updateConfiguration(editConfig);
-    }
-
     /**
      * Updates thing channels and creates dynamic channels if missing.
      *
@@ -311,7 +293,6 @@ public void handleCommand(ChannelUID channelUID, Command command) {
      * @return the AIN
      */
     public @Nullable String getIdentifier() {
-        AVMFritzDeviceConfiguration localConfig = config;
-        return localConfig != null ? localConfig.ain : null;
+        return identifier;
     }
 }