[innogysmarthome] NullPointerException fixed which could occur when no device state is available (#9660) (#9677)

* [innogysmarthome] Bug-fix - NPE solved which could occur when devices without a corresponding device state were returned by the API. That happened with the virtual device "NotificationSender" which is unimportant for the binding. Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] JavaDoc fixed Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimizations (warnings fixed) Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code refactoring (InnogyClient class split to make it smaller) Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization ; Potential bug with the reachable attribute solved (it was set multiple times within a loop) Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Tests added Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Tests added Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code changed to not overwrite the reachable state provided by the API (when no corresponding message is available) Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Copyright notice updated Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> * [innogysmarthome] Code and performance optimization Signed-off-by: Sven Strohschein <sven.strohschein@gmail.com> Co-authored-by: Sven Strohschein <sven.strohschein@gmail.com>
2021-01-08 23:07:38 +01:00
parent fcb774ca35
commit 3557094c13
8 changed files with 496 additions and 368 deletions
--- a/bundles/org.openhab.binding.innogysmarthome/src/test/java/org/openhab/binding/innogysmarthome/internal/client/entity/device/DeviceTest.java
+++ b/bundles/org.openhab.binding.innogysmarthome/src/test/java/org/openhab/binding/innogysmarthome/internal/client/entity/device/DeviceTest.java
@@ -0,0 +1,194 @@
+/**
+ * Copyright (c) 2010-2021 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.innogysmarthome.internal.client.entity.device;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.openhab.binding.innogysmarthome.internal.client.entity.message.Message;
+import org.openhab.binding.innogysmarthome.internal.client.entity.state.BooleanState;
+
+/**
+ * @author Sven Strohschein - Initial contribution
+ */
+public class DeviceTest {
+
+    @Test
+    public void testSetMessageListLowBatteryMessage() {
+        Device device = createDevice();
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        device.setMessageList(Collections.singletonList(createMessage(Message.TYPE_DEVICE_LOW_BATTERY)));
+
+        assertTrue(device.isReachable());
+        assertTrue(device.hasLowBattery());
+    }
+
+    @Test
+    public void testSetMessageListUnreachableMessage() {
+        Device device = createDevice();
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        device.setMessageList(Collections.singletonList(createMessage(Message.TYPE_DEVICE_UNREACHABLE)));
+
+        assertFalse(device.isReachable());
+        assertFalse(device.hasLowBattery());
+    }
+
+    @Test
+    public void testSetMessageListResetByEmpty() {
+        Device device = createDevice();
+
+        assertNull(device.getMessageList());
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        List<Message> messages = Arrays.asList(createMessage(Message.TYPE_DEVICE_LOW_BATTERY),
+                createMessage(Message.TYPE_DEVICE_UNREACHABLE));
+        device.setMessageList(messages);
+
+        assertEquals(messages, device.getMessageList());
+        assertFalse(device.isReachable());
+        assertTrue(device.hasLowBattery());
+
+        device.setMessageList(Collections.emptyList());
+
+        // Nothing should get changed.
+        // New messages are only set in real-life when the device is refreshed with new data of the API.
+        // Therefore the data of the API should be kept / not overwritten when no corresponding messages are available.
+        assertEquals(Collections.emptyList(), device.getMessageList());
+        assertFalse(device.isReachable());
+        assertTrue(device.hasLowBattery());
+    }
+
+    @Test
+    public void testSetMessageListResetByNULL() {
+        Device device = createDevice();
+
+        assertNull(device.getMessageList());
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        List<Message> messages = Arrays.asList(createMessage(Message.TYPE_DEVICE_LOW_BATTERY),
+                createMessage(Message.TYPE_DEVICE_UNREACHABLE));
+        device.setMessageList(messages);
+
+        assertEquals(messages, device.getMessageList());
+        assertFalse(device.isReachable());
+        assertTrue(device.hasLowBattery());
+
+        device.setMessageList(null);
+
+        // Nothing should get changed.
+        // New messages are only set in real-life when the device is refreshed with new data of the API.
+        // Therefore the data of the API should be kept / not overwritten when no corresponding messages are available.
+        assertNull(device.getMessageList());
+        assertFalse(device.isReachable());
+        assertTrue(device.hasLowBattery());
+    }
+
+    @Test
+    public void testSetMessageListResetByUnimportantMessage() {
+        Device device = createDevice();
+
+        assertNull(device.getMessageList());
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        List<Message> messages = Arrays.asList(createMessage(Message.TYPE_DEVICE_LOW_BATTERY),
+                createMessage(Message.TYPE_DEVICE_UNREACHABLE));
+        device.setMessageList(messages);
+
+        assertEquals(messages, device.getMessageList());
+        assertFalse(device.isReachable());
+        assertTrue(device.hasLowBattery());
+
+        messages = Collections.singletonList(createMessage("UNKNOWN"));
+        device.setMessageList(messages);
+
+        // Nothing should get changed.
+        // New messages are only set in real-life when the device is refreshed with new data of the API.
+        // Therefore the data of the API should be kept / not overwritten when no corresponding messages are available.
+        assertEquals(messages, device.getMessageList());
+        assertFalse(device.isReachable());
+        assertTrue(device.hasLowBattery());
+    }
+
+    @Test
+    public void testSetMessageListUnimportantMessage() {
+        Device device = createDevice();
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        device.setMessageList(Collections.singletonList(createMessage("UNKNOWN")));
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+    }
+
+    private Message createMessage(String messageType) {
+        Message message = new Message();
+        message.setType(messageType);
+        return message;
+    }
+
+    @Test
+    public void testSetMessageListNULL() {
+        Device device = createDevice();
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        device.setMessageList(null);
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+    }
+
+    @Test
+    public void testSetMessageListEmpty() {
+        Device device = createDevice();
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+
+        device.setMessageList(Collections.emptyList());
+
+        assertTrue(device.isReachable());
+        assertFalse(device.hasLowBattery());
+    }
+
+    private static Device createDevice() {
+        BooleanState isReachableState = new BooleanState();
+        isReachableState.setValue(true);
+
+        State state = new State();
+        state.setIsReachable(isReachableState);
+
+        DeviceState deviceState = new DeviceState();
+        deviceState.setState(state);
+
+        Device device = new Device();
+        device.setDeviceState(deviceState);
+        return device;
+    }
+}
--- a/bundles/org.openhab.binding.innogysmarthome/src/test/java/org/openhab/binding/innogysmarthome/internal/handler/InnogyBridgeHandlerTest.java
+++ b/bundles/org.openhab.binding.innogysmarthome/src/test/java/org/openhab/binding/innogysmarthome/internal/handler/InnogyBridgeHandlerTest.java
@@ -30,6 +30,7 @@ import org.openhab.binding.innogysmarthome.internal.InnogyWebSocket;
 import org.openhab.binding.innogysmarthome.internal.client.InnogyClient;
 import org.openhab.binding.innogysmarthome.internal.client.entity.device.Device;
 import org.openhab.binding.innogysmarthome.internal.client.entity.device.DeviceConfig;
+import org.openhab.binding.innogysmarthome.internal.manager.FullDeviceManager;
 import org.openhab.core.auth.client.oauth2.OAuthClientService;
 import org.openhab.core.auth.client.oauth2.OAuthFactory;
 import org.openhab.core.config.core.Configuration;
@@ -174,6 +175,7 @@ public class InnogyBridgeHandlerTest {
    private class InnogyBridgeHandlerAccessible extends InnogyBridgeHandler {

        private final InnogyClient innogyClientMock;
+        private final FullDeviceManager fullDeviceManagerMock;
        private final ScheduledExecutorService schedulerMock;
        private int executionCount;
        private int directExecutionCount;
@@ -188,7 +190,8 @@ public class InnogyBridgeHandlerTest {
            bridgeDevice.setConfig(new DeviceConfig());

            innogyClientMock = mock(InnogyClient.class);
-            when(innogyClientMock.getFullDevices()).thenReturn(Collections.singletonList(bridgeDevice));
+            fullDeviceManagerMock = mock(FullDeviceManager.class);
+            when(fullDeviceManagerMock.getFullDevices()).thenReturn(Collections.singletonList(bridgeDevice));

            schedulerMock = mock(ScheduledExecutorService.class);

@@ -218,6 +221,12 @@ public class InnogyBridgeHandlerTest {
            return directExecutionCount;
        }

+        @Override
+        @NonNull
+        FullDeviceManager createFullDeviceManager(@NonNull InnogyClient client) {
+            return fullDeviceManagerMock;
+        }
+
        @Override
        @NonNull
        InnogyClient createInnogyClient(@NonNull OAuthClientService oAuthService, @NonNull HttpClient httpClient) {