demus/openhab-addons
1
0
Fork 0
mirror of https://github.com/openhab/openhab-addons.git synced 2025-01-10 15:11:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

[boschshc] Fix NPE during deserialization, make long polling more robust (#17190)

Browse Source 
* Fix NPE while deserializing service data JSON objects
* Catch exceptions while handling long poll results
* Correct @type name for user-defined states
* Add unit tests and enhance existing unit tests

Signed-off-by: David Pace <dev@davidpace.de>
This commit is contained in:
David Pace 2024-08-13 20:20:51 +02:00 committed by GitHub
parent d6cac81e4a
commit c2613c7759
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 167 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPolling.java
View File

@ -211,7 +211,7 @@ public class LongPolling {
* @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 @@ public class LongPolling {
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

2
bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedState.java
View File

@ -37,7 +37,7 @@ public class UserDefinedState extends BoschSHCServiceState {
private boolean state;
public UserDefinedState() {
super("UserDefinedState");
super("userDefinedState");
}
public String getId() {

43
bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializer.java
View File

@ -32,6 +32,7 @@ import com.google.gson.JsonParseException;
* 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<BoschSHCSe
public BoschSHCServiceState deserialize(JsonElement jsonElement, Type type,
JsonDeserializationContext jsonDeserializationContext) throws JsonParseException {
JsonObject jsonObject = jsonElement.getAsJsonObject();
JsonElement dataType = jsonObject.get("@type");
switch (dataType.getAsString()) {
case "DeviceServiceData" -> {
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);
};
}
}

35
bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPollingTest.java
View File

@ -14,6 +14,7 @@ package org.openhab.binding.boschshc.internal.devices.bridge;
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.any;
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 @@ class LongPollingTest {
ArgumentCaptor<Throwable> throwableCaptor = ArgumentCaptor.forClass(Throwable.class);
verify(failureHandler).accept(throwableCaptor.capture());
Throwable t = throwableCaptor.getValue();
assertEquals(
"Could not deserialize long poll response: '<HTML><HEAD><TITLE>400</TITLE></HEAD><BODY><H1>400 Unsupported HTTP Protocol Version: /remote/json-rpcHTTP/1.1</H1></BODY></HTML>'",
t.getMessage());
assertTrue(t.getCause() instanceof JsonSyntaxException);
assertThat(t.getMessage(), is(
"Could not deserialize long poll response: '<HTML><HEAD><TITLE>400</TITLE></HEAD><BODY><H1>400 Unsupported HTTP Protocol Version: /remote/json-rpcHTTP/1.1</H1></BODY></HTML>'"));
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<Throwable> 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

2
bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/dto/UserDefinedStateTest.java
View File

@ -53,7 +53,7 @@ public class UserDefinedStateTest {
@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());
}

113
bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/serialization/BoschServiceDataDeserializerTest.java
View File

@ -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 @@ class BoschServiceDataDeserializerTest {
""";
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<BoschSHCServiceState> 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<BoschSHCServiceState> 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<BoschSHCServiceState> 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<BoschSHCServiceState> 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));
}
}

14
bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/services/dto/BoschSHCServiceStateTest.java
View File

@ -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 @@ class BoschSHCServiceStateTest {
@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));
}
}