[avmfritz] Avoid a hidden NPE when getIdentifier() of an uninitialized ThingHandler is called (#9040)

* Avoid a hidden NPE when getIdentifier() of an uninitialized ThingHandler is called
* Fixed Powerline546EHandler

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
This commit is contained in:
Christoph Weitkamp 2020-11-16 18:09:30 +01:00 committed by GitHub
parent 57177ccdff
commit 896640db3f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 48 deletions

View File

@ -20,7 +20,6 @@ import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
@ -126,7 +125,7 @@ public abstract class AVMFritzBaseBridgeHandler extends BaseBridgeHandler {
AVMFritzBoxConfiguration config = getConfigAs(AVMFritzBoxConfiguration.class); AVMFritzBoxConfiguration config = getConfigAs(AVMFritzBoxConfiguration.class);
String localIpAddress = config.ipAddress; String localIpAddress = config.ipAddress;
if (localIpAddress == null || localIpAddress.trim().isEmpty()) { if (localIpAddress == null || localIpAddress.isBlank()) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
"The 'ipAddress' parameter must be configured."); "The 'ipAddress' parameter must be configured.");
configValid = false; configValid = false;
@ -271,10 +270,8 @@ public abstract class AVMFritzBaseBridgeHandler extends BaseBridgeHandler {
getThing().getThings().forEach(childThing -> { getThing().getThings().forEach(childThing -> {
final AVMFritzBaseThingHandler childHandler = (AVMFritzBaseThingHandler) childThing.getHandler(); final AVMFritzBaseThingHandler childHandler = (AVMFritzBaseThingHandler) childThing.getHandler();
if (childHandler != null) { if (childHandler != null) {
final Optional<AVMFritzBaseModel> optionalDevice = Optional final AVMFritzBaseModel device = deviceIdentifierMap.get(childHandler.getIdentifier());
.ofNullable(deviceIdentifierMap.get(childHandler.getIdentifier())); if (device != null) {
if (optionalDevice.isPresent()) {
final AVMFritzBaseModel device = optionalDevice.get();
deviceList.remove(device); deviceList.remove(device);
listeners.forEach(listener -> listener.onDeviceUpdated(childThing.getUID(), device)); listeners.forEach(listener -> listener.onDeviceUpdated(childThing.getUID(), device));
} else { } else {

View File

@ -82,7 +82,7 @@ public abstract class AVMFritzBaseThingHandler extends BaseThingHandler implemen
* keeps track of the current state for handling of increase/decrease * keeps track of the current state for handling of increase/decrease
*/ */
private @Nullable AVMFritzBaseModel state; private @Nullable AVMFritzBaseModel state;
private @NonNullByDefault({}) AVMFritzDeviceConfiguration config; private @Nullable String identifier;
/** /**
* Constructor * Constructor
@ -95,13 +95,13 @@ public abstract class AVMFritzBaseThingHandler extends BaseThingHandler implemen
@Override @Override
public void initialize() { public void initialize() {
config = getConfigAs(AVMFritzDeviceConfiguration.class); final AVMFritzDeviceConfiguration config = getConfigAs(AVMFritzDeviceConfiguration.class);
final String newIdentifier = config.ain;
String newIdentifier = config.ain; if (newIdentifier == null || newIdentifier.isBlank()) {
if (newIdentifier == null || newIdentifier.trim().isEmpty()) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
"The 'ain' parameter must be configured."); "The 'ain' parameter must be configured.");
} else { } else {
this.identifier = newIdentifier;
updateStatus(ThingStatus.UNKNOWN); updateStatus(ThingStatus.UNKNOWN);
} }
} }
@ -472,6 +472,6 @@ public abstract class AVMFritzBaseThingHandler extends BaseThingHandler implemen
* @return the AIN * @return the AIN
*/ */
public @Nullable String getIdentifier() { public @Nullable String getIdentifier() {
return config.ain; return identifier;
} }
} }

View File

@ -17,7 +17,6 @@ import static org.openhab.binding.avmfritz.internal.AVMFritzBindingConstants.*;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.function.Predicate; import java.util.function.Predicate;
import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.NonNullByDefault;
@ -32,7 +31,6 @@ import org.openhab.binding.avmfritz.internal.dto.PowerMeterModel;
import org.openhab.binding.avmfritz.internal.dto.SwitchModel; import org.openhab.binding.avmfritz.internal.dto.SwitchModel;
import org.openhab.binding.avmfritz.internal.hardware.FritzAhaStatusListener; import org.openhab.binding.avmfritz.internal.hardware.FritzAhaStatusListener;
import org.openhab.binding.avmfritz.internal.hardware.FritzAhaWebInterface; 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.OnOffType;
import org.openhab.core.library.types.OpenClosedType; import org.openhab.core.library.types.OpenClosedType;
import org.openhab.core.library.types.QuantityType; 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 * keeps track of the current state for handling of increase/decrease
*/ */
private @Nullable AVMFritzBaseModel state; private @Nullable AVMFritzBaseModel state;
private @Nullable AVMFritzDeviceConfiguration config; private @Nullable String identifier;
/** /**
* Constructor * Constructor
@ -86,32 +84,27 @@ public class Powerline546EHandler extends AVMFritzBaseBridgeHandler implements F
@Override @Override
public void initialize() { public void initialize() {
config = getConfigAs(AVMFritzDeviceConfiguration.class); final AVMFritzDeviceConfiguration config = getConfigAs(AVMFritzDeviceConfiguration.class);
final String newIdentifier = config.ain;
registerStatusListener(this); if (newIdentifier != null && !newIdentifier.isBlank()) {
this.identifier = newIdentifier;
}
super.initialize(); super.initialize();
} }
@Override @SuppressWarnings({ "null", "unused" })
public void dispose() {
unregisterStatusListener(this);
super.dispose();
}
@Override @Override
public void onDeviceListAdded(List<AVMFritzBaseModel> devicelist) { public void onDeviceListAdded(List<AVMFritzBaseModel> devicelist) {
final String identifier = getIdentifier(); final String ain = getIdentifier();
final Predicate<AVMFritzBaseModel> predicate = identifier == null ? it -> thing.getUID().equals(getThingUID(it)) final Predicate<AVMFritzBaseModel> predicate = ain == null ? it -> thing.getUID().equals(getThingUID(it))
: it -> identifier.equals(it.getIdentifier()); : it -> ain.equals(it.getIdentifier());
final Optional<AVMFritzBaseModel> optionalDevice = devicelist.stream().filter(predicate).findFirst(); final AVMFritzBaseModel device = devicelist.stream().filter(predicate).findFirst().orElse(null);
if (optionalDevice.isPresent()) { if (device != null) {
final AVMFritzBaseModel device = optionalDevice.get();
devicelist.remove(device); devicelist.remove(device);
listeners.stream().forEach(listener -> listener.onDeviceUpdated(thing.getUID(), device)); onDeviceUpdated(thing.getUID(), device);
} else { } else {
listeners.stream().forEach(listener -> listener.onDeviceGone(thing.getUID())); onDeviceGone(thing.getUID());
} }
super.onDeviceListAdded(devicelist); super.onDeviceListAdded(devicelist);
} }
@ -125,8 +118,8 @@ public class Powerline546EHandler extends AVMFritzBaseBridgeHandler implements F
public void onDeviceUpdated(ThingUID thingUID, AVMFritzBaseModel device) { public void onDeviceUpdated(ThingUID thingUID, AVMFritzBaseModel device) {
if (thing.getUID().equals(thingUID)) { if (thing.getUID().equals(thingUID)) {
// save AIN to config for FRITZ!Powerline 546E stand-alone // save AIN to config for FRITZ!Powerline 546E stand-alone
if (config == null) { if (this.identifier == null) {
updateConfiguration(device); this.identifier = device.getIdentifier();
} }
logger.debug("Update self '{}' with device model: {}", thingUID, device); logger.debug("Update self '{}' with device model: {}", thingUID, device);
@ -185,17 +178,6 @@ public class Powerline546EHandler extends AVMFritzBaseBridgeHandler implements F
updateProperties(editProperties); 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. * Updates thing channels and creates dynamic channels if missing.
* *
@ -311,7 +293,6 @@ public class Powerline546EHandler extends AVMFritzBaseBridgeHandler implements F
* @return the AIN * @return the AIN
*/ */
public @Nullable String getIdentifier() { public @Nullable String getIdentifier() {
AVMFritzDeviceConfiguration localConfig = config; return identifier;
return localConfig != null ? localConfig.ain : null;
} }
} }