Avoid NPE during Thing update

[evansj]

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:

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)

[cdjackson]

Thanks. Looks good - if you wanted to add test you can use reflection to change visibility in the test.

Yes, if you want to split out the methods, that's also fine. But note that this will be deleted, so it's probably not worth spending too much time on it here - better to spend time on any such refactoring on a PR into the development branch.

Thanks
Chris

[evansj]

Thanks Chris,

Please can you explain "this will be deleted"?

I will do some refactoring against the development branch instead, and create some more PRs.

Cheers
Jon

[cdjackson]

Sorry - deleted might be the wrong word, but this branch will ‘cease’ and the development branch will become master - it won’t be rebased as they are too different, so changes in master will ultimately be lost (not deleted - wrong word). Thanks.

…

On 29 Nov 2017, at 17:15, Jon Evans ***@***.***> wrote: Thanks Chris, Please can you explain "this will be deleted"? I will do some refactoring against the development branch instead, and create some more PRs. Cheers Jon — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#758 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_kQ0veT03FvLHvRY9cFZuOEoMGNoPMks5s7ZE4gaJpZM4Qu6yo>.

[evansj]

Makes sense! Thanks.

[cdjackson]

Just reopening to kick off Jenkins again to see if this failure is permanent (shouldn't be I think).

[5iver]

Will this be merged into the development branch?

[cdjackson]

I looked at this earlier and don’t think it’s an issue in the development branch as there is a null check.

…

On 9 Dec 2017, at 18:10, Scott Rushworth ***@***.***> wrote: Will this be merged into the development branch? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#758 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA_kQ7DiLpi0zTmcvLceUgtt7OQbZC1Tks5s-sz5gaJpZM4Qu6yo>.