From 4d30ca892a66efcef680e89c6d33cb95929604f6 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Sun, 15 Nov 2020 12:14:31 +0100 Subject: [PATCH 1/3] Avoid a hidden NPE when getIdentifier() of an uninitialized ThingHandler is colled Signed-off-by: Christoph Weitkamp --- .../handler/AVMFritzBaseBridgeHandler.java | 8 +++---- .../handler/AVMFritzBaseThingHandler.java | 12 +++++----- .../handler/Powerline546EHandler.java | 22 +++++++++---------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java index 9a4ac1d734274..4cc0907187ae2 100644 --- a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java +++ b/bundles/org.openhab.binding.avmfritz/src/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; @@ -270,11 +269,10 @@ public void onDeviceListAdded(List deviceList) { .collect(Collectors.toMap(it -> it.getIdentifier(), Function.identity())); getThing().getThings().forEach(childThing -> { final AVMFritzBaseThingHandler childHandler = (AVMFritzBaseThingHandler) childThing.getHandler(); + // if (childHandler != null && ThingHandlerHelper.isHandlerInitialized(childHandler)) { if (childHandler != null) { - final Optional 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/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseThingHandler.java b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseThingHandler.java index e8c6bcdf24e4a..74731db006590 100644 --- a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseThingHandler.java +++ b/bundles/org.openhab.binding.avmfritz/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/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java index 6d4a305e89585..5dc675eaa75db 100644 --- a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java +++ b/bundles/org.openhab.binding.avmfritz/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; @@ -72,7 +71,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,7 +85,11 @@ public Powerline546EHandler(Bridge bridge, HttpClient httpClient, @Override public void initialize() { - config = getConfigAs(AVMFritzDeviceConfiguration.class); + final AVMFritzDeviceConfiguration config = getConfigAs(AVMFritzDeviceConfiguration.class); + final String newIdentifier = config.ain; + if (newIdentifier != null && !newIdentifier.isBlank()) { + this.identifier = newIdentifier; + } registerStatusListener(this); @@ -100,14 +103,14 @@ public void dispose() { super.dispose(); } + @SuppressWarnings({ "null", "unused" }) @Override public void onDeviceListAdded(List devicelist) { final String identifier = getIdentifier(); final Predicate predicate = identifier == null ? it -> thing.getUID().equals(getThingUID(it)) : it -> identifier.equals(it.getIdentifier()); - final Optional optionalDevice = devicelist.stream().filter(predicate).findFirst(); - if (optionalDevice.isPresent()) { - final AVMFritzBaseModel device = optionalDevice.get(); + 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)); } else { @@ -125,9 +128,7 @@ 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); - } + updateConfiguration(device); logger.debug("Update self '{}' with device model: {}", thingUID, device); if (device.getPresent() == 1) { @@ -311,7 +312,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; } } From e3115704f0818d9fb804c4893fb43ebd883cda42 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Sun, 15 Nov 2020 12:49:03 +0100 Subject: [PATCH 2/3] Fixed Powerline546EHandler Signed-off-by: Christoph Weitkamp --- .../handler/AVMFritzBaseBridgeHandler.java | 2 +- .../handler/Powerline546EHandler.java | 35 +++++-------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java index 4cc0907187ae2..0b1f53f6b67f2 100644 --- a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java +++ b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java @@ -125,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; diff --git a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java index 5dc675eaa75db..089ee9b8123fa 100644 --- a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java +++ b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/Powerline546EHandler.java @@ -31,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; @@ -91,30 +90,21 @@ public void initialize() { this.identifier = newIdentifier; } - registerStatusListener(this); - super.initialize(); } - @Override - public void dispose() { - unregisterStatusListener(this); - - super.dispose(); - } - @SuppressWarnings({ "null", "unused" }) @Override public void onDeviceListAdded(List devicelist) { - final String identifier = getIdentifier(); - final Predicate predicate = identifier == null ? it -> thing.getUID().equals(getThingUID(it)) - : it -> identifier.equals(it.getIdentifier()); + final String ain = getIdentifier(); + final Predicate 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); } @@ -128,7 +118,9 @@ 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 - updateConfiguration(device); + if (this.identifier == null) { + this.identifier = device.getIdentifier(); + } logger.debug("Update self '{}' with device model: {}", thingUID, device); if (device.getPresent() == 1) { @@ -186,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. * From d6bdcfd0d46c4e340f6807624d9aad9a0421a258 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Mon, 16 Nov 2020 07:43:23 +0100 Subject: [PATCH 3/3] Removed comment Signed-off-by: Christoph Weitkamp --- .../avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java index 0b1f53f6b67f2..1639a24d54f83 100644 --- a/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java +++ b/bundles/org.openhab.binding.avmfritz/src/main/java/org/openhab/binding/avmfritz/internal/handler/AVMFritzBaseBridgeHandler.java @@ -269,7 +269,6 @@ public void onDeviceListAdded(List deviceList) { .collect(Collectors.toMap(it -> it.getIdentifier(), Function.identity())); getThing().getThings().forEach(childThing -> { final AVMFritzBaseThingHandler childHandler = (AVMFritzBaseThingHandler) childThing.getHandler(); - // if (childHandler != null && ThingHandlerHelper.isHandlerInitialized(childHandler)) { if (childHandler != null) { final AVMFritzBaseModel device = deviceIdentifierMap.get(childHandler.getIdentifier()); if (device != null) {