diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPolling.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPolling.java index 0cc9a13f24c0d..5c307002ee40e 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPolling.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPolling.java @@ -211,7 +211,7 @@ private void onLongPollComplete(BoschHttpClient httpClient, String subscriptionI * @param subscriptionId Id of subscription the response is for * @param content Content of the response */ - private void handleLongPollResponse(BoschHttpClient httpClient, String subscriptionId, @Nullable String content) { + void handleLongPollResponse(BoschHttpClient httpClient, String subscriptionId, @Nullable String content) { logger.debug("Long poll response: {}", content); try { @@ -239,6 +239,10 @@ private void handleLongPollResponse(BoschHttpClient httpClient, String subscript this.handleFailure.accept( new LongPollingFailedException("Could not deserialize long poll response: '" + content + "'", e)); return; + } catch (Exception e) { + this.handleFailure.accept( + new LongPollingFailedException("Error while handling long poll response: '" + content + "'", e)); + return; } // Execute next run diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedState.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedState.java index ff1615b4801a9..6a0464645fbf9 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedState.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedState.java @@ -37,7 +37,7 @@ public class UserDefinedState extends BoschSHCServiceState { private boolean state; public UserDefinedState() { - super("UserDefinedState"); + super("userDefinedState"); } public String getId() { diff --git a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializer.java b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializer.java index 41b02cfbfb8d2..ffaa617c82036 100644 --- a/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializer.java +++ b/bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializer.java @@ -32,6 +32,7 @@ * Utility class for JSON deserialization of device data and triggered scenarios using Google Gson. * * @author Patrick Gell - Initial contribution + * @author David Pace - Fixed NPEs and simplified code, added sanity check * */ @NonNullByDefault @@ -42,36 +43,18 @@ public class BoschServiceDataDeserializer implements JsonDeserializer { - var deviceServiceData = new DeviceServiceData(); - deviceServiceData.deviceId = jsonObject.get("deviceId").getAsString(); - deviceServiceData.state = jsonObject.get("state"); - deviceServiceData.id = jsonObject.get("id").getAsString(); - deviceServiceData.path = jsonObject.get("path").getAsString(); - return deviceServiceData; - } - case "scenarioTriggered" -> { - var scenario = new Scenario(); - scenario.id = jsonObject.get("id").getAsString(); - scenario.name = jsonObject.get("name").getAsString(); - scenario.lastTimeTriggered = jsonObject.get("lastTimeTriggered").getAsString(); - return scenario; - } - case "userDefinedState" -> { - var state = new UserDefinedState(); - state.setId(jsonObject.get("id").getAsString()); - state.setName(jsonObject.get("name").getAsString()); - state.setState(jsonObject.get("state").getAsBoolean()); - return state; - } - case "message" -> { - return GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(jsonElement, Message.class); - } - default -> { - return new BoschSHCServiceState(dataType.getAsString()); - } + JsonElement dataTypeElement = jsonObject.get("@type"); + if (dataTypeElement == null) { + throw new IllegalArgumentException("Received a service state without a @type property: " + jsonObject); } + + String dataType = dataTypeElement.getAsString(); + return switch (dataType) { + case "DeviceServiceData" -> GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(jsonObject, DeviceServiceData.class); + case "scenarioTriggered" -> GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(jsonObject, Scenario.class); + case "userDefinedState" -> GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(jsonObject, UserDefinedState.class); + case "message" -> GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(jsonElement, Message.class); + default -> new BoschSHCServiceState(dataType); + }; } } diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPollingTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPollingTest.java index 6ae484982520b..4fd17ada2089a 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPollingTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPollingTest.java @@ -14,6 +14,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -24,6 +25,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -461,10 +463,35 @@ void startLongPollingInvalidLongPollResponse() ArgumentCaptor throwableCaptor = ArgumentCaptor.forClass(Throwable.class); verify(failureHandler).accept(throwableCaptor.capture()); Throwable t = throwableCaptor.getValue(); - assertEquals( - "Could not deserialize long poll response: '400

400 Unsupported HTTP Protocol Version: /remote/json-rpcHTTP/1.1

'", - t.getMessage()); - assertTrue(t.getCause() instanceof JsonSyntaxException); + assertThat(t.getMessage(), is( + "Could not deserialize long poll response: '400

400 Unsupported HTTP Protocol Version: /remote/json-rpcHTTP/1.1

'")); + assertThat(t.getCause(), instanceOf(JsonSyntaxException.class)); + } + + @Test + void testHandleLongPollResponseNPE() { + doThrow(NullPointerException.class).when(longPollHandler).accept(any()); + + var resultJson = """ + { + "result": [ + { + "@type": "DeviceServiceData", + "deleted": true, + "id": "CommunicationQuality", + "deviceId": "hdm:ZigBee:30fb10fffe46d732" + } + ], + "jsonrpc":"2.0" + } + """; + fixture.handleLongPollResponse(httpClient, "subscriptionId", resultJson); + + ArgumentCaptor throwableCaptor = ArgumentCaptor.forClass(Throwable.class); + verify(failureHandler).accept(throwableCaptor.capture()); + Throwable t = throwableCaptor.getValue(); + assertThat(t.getMessage(), is("Error while handling long poll response: '" + resultJson + "'")); + assertThat(t.getCause(), instanceOf(NullPointerException.class)); } @AfterEach diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedStateTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedStateTest.java index dbff9b7b0da83..b3b62cef5f0e8 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedStateTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedStateTest.java @@ -53,7 +53,7 @@ void testIsValid() { @Test void testToString() { assertEquals( - String.format("UserDefinedState{id='%s', name='test user state', state=true, type='UserDefinedState'}", + String.format("UserDefinedState{id='%s', name='test user state', state=true, type='userDefinedState'}", testId), fixture.toString()); } diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializerTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializerTest.java index 252983adbe543..e77860dd0f499 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializerTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializerTest.java @@ -12,22 +12,29 @@ */ package org.openhab.binding.boschshc.internal.serialization; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.hasSize; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.HashSet; +import java.util.List; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; import org.openhab.binding.boschshc.internal.devices.bridge.dto.DeviceServiceData; import org.openhab.binding.boschshc.internal.devices.bridge.dto.LongPollResult; import org.openhab.binding.boschshc.internal.devices.bridge.dto.Scenario; +import org.openhab.binding.boschshc.internal.devices.bridge.dto.UserDefinedState; +import org.openhab.binding.boschshc.internal.services.dto.BoschSHCServiceState; /** * Unit tests for {@link BoschServiceDataDeserializer}. * * @author Patrick Gell - Initial contribution + * @author David Pace - Added tests for all supported service data classes * */ @NonNullByDefault @@ -60,12 +67,104 @@ void deserializationOfLongPollingResult() { """; var longPollResult = GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(resultJson, LongPollResult.class); + // note: when using assertThat() to check that the value is non-null, we get compiler warnings. assertNotNull(longPollResult); - assertEquals(2, longPollResult.result.size()); + List results = longPollResult.result; + assertThat(results, is(notNullValue())); + assertThat(results, hasSize(2)); - var resultClasses = new HashSet<>(longPollResult.result.stream().map(e -> e.getClass().getName()).toList()); - assertEquals(2, resultClasses.size()); - assertTrue(resultClasses.contains(DeviceServiceData.class.getName())); - assertTrue(resultClasses.contains(Scenario.class.getName())); + var resultClasses = new HashSet<>(results.stream().map(e -> e.getClass().getName()).toList()); + assertThat(resultClasses, hasSize(2)); + assertThat(resultClasses, containsInAnyOrder(DeviceServiceData.class.getName(), Scenario.class.getName())); + } + + @Test + void testDeserializeDeletedDeviceServiceData() { + var resultJson = """ + { + "result": [ + { + "@type": "DeviceServiceData", + "deleted": true, + "id": "CommunicationQuality", + "deviceId": "hdm:ZigBee:30fb10fffe46d732" + } + ], + "jsonrpc":"2.0" + } + """; + + var longPollResult = GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(resultJson, LongPollResult.class); + // note: when using assertThat() to check that the value is non-null, we get compiler warnings. + assertNotNull(longPollResult); + List results = longPollResult.result; + assertThat(results, is(notNullValue())); + assertThat(results, hasSize(1)); + + DeviceServiceData deviceServiceData = (DeviceServiceData) longPollResult.result.get(0); + assertThat(deviceServiceData.type, is("DeviceServiceData")); + assertThat(deviceServiceData.id, is("CommunicationQuality")); + assertThat(deviceServiceData.deviceId, is("hdm:ZigBee:30fb10fffe46d732")); + } + + @Test + void testDeserializeScenarioTriggered() { + String resultJson = """ + { + "result": [ + { + "@type": "scenarioTriggered", + "name": "My Scenario", + "id": "509bd737-eed0-40b7-8caa-e8686a714399", + "lastTimeTriggered": "1693758693032" + } + ], + "jsonrpc": "2.0" + } + """; + + var longPollResult = GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(resultJson, LongPollResult.class); + // note: when using assertThat() to check that the value is non-null, we get compiler warnings. + assertNotNull(longPollResult); + List results = longPollResult.result; + assertThat(results, is(notNullValue())); + assertThat(results, hasSize(1)); + + Scenario scenario = (Scenario) longPollResult.result.get(0); + assertThat(scenario.type, is("scenarioTriggered")); + assertThat(scenario.name, is("My Scenario")); + assertThat(scenario.id, is("509bd737-eed0-40b7-8caa-e8686a714399")); + assertThat(scenario.lastTimeTriggered, is("1693758693032")); + } + + @Test + void testDeserializeUserDefinedState() { + String resultJson = """ + { + "result": [ + { + "@type": "userDefinedState", + "deleted": false, + "name": "Test State", + "id": "3d8023d6-69ca-4e79-89dd-7090295cefbf", + "state": true + } + ], + "jsonrpc": "2.0" + } + """; + + var longPollResult = GsonUtils.DEFAULT_GSON_INSTANCE.fromJson(resultJson, LongPollResult.class); + // note: when using assertThat() to check that the value is non-null, we get compiler warnings. + assertNotNull(longPollResult); + List results = longPollResult.result; + assertThat(results, is(notNullValue())); + assertThat(results, hasSize(1)); + + UserDefinedState userDefinedState = (UserDefinedState) longPollResult.result.get(0); + assertThat(userDefinedState.type, is("userDefinedState")); + assertThat(userDefinedState.getName(), is("Test State")); + assertThat(userDefinedState.getId(), is("3d8023d6-69ca-4e79-89dd-7090295cefbf")); + assertThat(userDefinedState.isState(), is(true)); } } diff --git a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/services/dto/BoschSHCServiceStateTest.java b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/services/dto/BoschSHCServiceStateTest.java index 9a127e9fed1a3..a1e8295892ddd 100644 --- a/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/services/dto/BoschSHCServiceStateTest.java +++ b/bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/services/dto/BoschSHCServiceStateTest.java @@ -12,7 +12,12 @@ */ package org.openhab.binding.boschshc.internal.services.dto; -import static org.junit.jupiter.api.Assertions.*; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import org.eclipse.jdt.annotation.NonNullByDefault; import org.junit.jupiter.api.Test; @@ -85,8 +90,9 @@ void fromJsonStateObjectForValidJsonAfterOtherState() { @Test void fromJsonReturnsUserStateServiceStateForValidJson() { var state = BoschSHCServiceState.fromJson(new JsonPrimitive("false"), UserStateServiceState.class); - assertNotEquals(null, state); - assertTrue(state.getClass().isAssignableFrom(UserStateServiceState.class)); - assertFalse(state.isState()); + // note: when using assertThat() to check that the value is non-null, we get compiler warnings. + assertNotNull(state); + assertThat(state, instanceOf(UserStateServiceState.class)); + assertThat(state.isState(), is(false)); } }