Skip to content

Commit

Permalink
Avoid NPE during Thing update (#758)
Browse files Browse the repository at this point in the history
Compare equality using Objects.equals(Object, Object) which is null-safe.

Possible fix for #755.

Also mentioned here: https://community.openhab.org/t/item-update-results-in-exception-occurred-while-calling-thing-updated-at-thinghandler/34526/2

It’s very hard to add a unit test because the updateNodeProperties() method is private and 144 lines long. Would patches to split this function out be acceptable?

An alternative fix would be to simply do:

```java
update = config.equals(originalConfig);
```

because `org.eclipse.smarthome.config.core.Configuration` implements the `equals` method, but then we wouldn’t be able to have the debug messages showing what changed. Having said that, you’ll only see the first changed key logged anyway because of the `break` statement.

Signed-off-by: Jon Evans <[email protected]> (github: evansj)
  • Loading branch information
evansj authored and cdjackson committed Dec 3, 2017
1 parent 40c5103 commit 76b568f
Showing 1 changed file with 2 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.TimeZone;
import java.util.concurrent.ScheduledFuture;
Expand Down Expand Up @@ -1396,7 +1397,7 @@ private void updateNodeProperties() {
for (String property : config.getProperties().keySet()) {
logger.debug("NODE {}: Property to update: {}, {}, {}", nodeId, property, config.get(property),
originalConfig.get(property));
if (config.get(property).equals(originalConfig.get(property)) == false) {
if (!Objects.equals(config.get(property), originalConfig.get(property))) {
update = true;
break;
}
Expand Down

0 comments on commit 76b568f

Please sign in to comment.