From be932425f21126ab534551d3d0339e7d366d2e7a Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Mon, 11 Mar 2024 11:41:39 -0700 Subject: [PATCH] Allow device/module client to send reported properties without specifying the version (#1779) * Fixing issue: Version 2.x of iot-device-client, requires version of reported properties to be sent, while 1.x not require that #1776 * Add integration test for updating reported properties without version Remove some duplicated logic around building a topic string for subscribing to desired properties from MQTT layer --------- Co-authored-by: dzakub --- .../sdk/iot/iothub/setup/TwinCommon.java | 4 +- .../azure/sdk/iot/iothub/twin/TwinTests.java | 21 ++++++++ .../transport/IotHubTransportMessage.java | 10 ++-- .../iot/device/transport/mqtt/MqttTwin.java | 29 +++------- .../ReportedPropertiesUpdateResponse.java | 2 +- .../transport/IotHubTransportMessageTest.java | 4 +- .../device/transport/mqtt/MqttTwinTest.java | 54 ++++++++++++++++--- .../device/twin/DeviceTwinMessageTest.java | 22 ++++---- 8 files changed, 96 insertions(+), 50 deletions(-) diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/setup/TwinCommon.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/setup/TwinCommon.java index 9dfb9451da..ea4cf4823d 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/setup/TwinCommon.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/setup/TwinCommon.java @@ -112,12 +112,12 @@ public void setup() throws IOException, GeneralSecurityException, IotHubExceptio { if (clientType == ClientType.DEVICE_CLIENT) { - this.testIdentity = Tools.getTestDevice(iotHubConnectionString, protocol, authenticationType, false); + this.testIdentity = Tools.getTestDevice(iotHubConnectionString, protocol, authenticationType, true); this.serviceTwin = new Twin(testIdentity.getDeviceId()); } else { - this.testIdentity = Tools.getTestModule(iotHubConnectionString, protocol, authenticationType, false); + this.testIdentity = Tools.getTestModule(iotHubConnectionString, protocol, authenticationType, true); this.serviceTwin = new Twin(testIdentity.getDeviceId(), ((TestModuleIdentity) testIdentity).getModuleId()); } diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/twin/TwinTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/twin/TwinTests.java index 08b64f2396..78c4052d26 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/twin/TwinTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/twin/TwinTests.java @@ -10,6 +10,7 @@ import com.microsoft.azure.sdk.iot.device.twin.*; import com.microsoft.azure.sdk.iot.service.auth.AuthenticationType; import com.microsoft.azure.sdk.iot.service.exceptions.IotHubException; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,6 +56,26 @@ public void testBasicTwinFlow() throws InterruptedException, IOException, IotHub super.testBasicTwinFlow(true); } + @Test + public void sendReportedPropertiesWithoutVersion() throws InterruptedException, IOException, IotHubException, TimeoutException, IotHubClientException + { + testInstance.testIdentity.getClient().open(true); + testInstance.testIdentity.getClient().subscribeToDesiredProperties( + (twin, context) -> + { + // do nothing + }, + null); + Twin twin = testInstance.testIdentity.getClient().getTwin(); + twin.getReportedProperties().put("someKey", "someValue"); + twin.getReportedProperties().setVersion(null); + testInstance.testIdentity.getClient().updateReportedProperties(twin.getReportedProperties()); + + Twin twinAfterReportedPropertiesUpdate = testInstance.testIdentity.getClient().getTwin(); + Assert.assertEquals(twin.getReportedProperties(), twinAfterReportedPropertiesUpdate.getReportedProperties()); + Assert.assertEquals(2, (int) twinAfterReportedPropertiesUpdate.getReportedProperties().getVersion()); + } + @Test public void receiveMultipleDesiredPropertiesAtOnce() throws IOException, InterruptedException, IotHubException, TimeoutException, IotHubClientException { diff --git a/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessage.java b/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessage.java index c1acecf804..4249c33ec0 100644 --- a/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessage.java +++ b/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessage.java @@ -25,7 +25,7 @@ public class IotHubTransportMessage extends Message private String uriPath; private String methodName; - private int version; + private Integer version; private String requestId; private String status; private DeviceOperations operationType; @@ -46,7 +46,7 @@ public IotHubTransportMessage(byte[] data, MessageType messageType) super(data); super.setMessageType(messageType); this.methodName = null; - this.version = 0; + this.version = null; this.requestId = null; this.status = null; this.operationType = DeviceOperations.DEVICE_OPERATION_UNKNOWN; @@ -61,7 +61,7 @@ public IotHubTransportMessage(String body) super(body); super.setMessageType(MessageType.UNKNOWN); this.methodName = null; - this.version = 0; + this.version = null; this.requestId = null; this.status = null; this.operationType = DeviceOperations.DEVICE_OPERATION_UNKNOWN; @@ -104,7 +104,7 @@ public void setMessageCallbackContext(Object messageCallbackContext) * Setter for the message version * @param version The String containing the version. */ - public void setVersion(int version) + public void setVersion(Integer version) { this.version = version; } @@ -113,7 +113,7 @@ public void setVersion(int version) * Getter for the message version * @return the String containing the version. */ - public int getVersion() + public Integer getVersion() { return this.version; } diff --git a/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwin.java b/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwin.java index ebfec4f69f..00d2701842 100644 --- a/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwin.java +++ b/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwin.java @@ -122,28 +122,13 @@ private String buildTopic(final IotHubTransportMessage message) throw new IllegalArgumentException("Request Id is Mandatory"); } - int version = message.getVersion(); - topic.append(AND); - topic.append(VERSION); - topic.append(version); - break; - } - case DEVICE_OPERATION_TWIN_SUBSCRIBE_DESIRED_PROPERTIES_REQUEST: - { - // Building $iothub/twin/PATCH/properties/desired/?$version={new version} - topic.append(PATCH); - topic.append(BACKSLASH); - topic.append(PROPERTIES); - topic.append(BACKSLASH); - topic.append(DESIRED); - - int version = message.getVersion(); - topic.append(BACKSLASH); - topic.append(QUESTION); - topic.append(VERSION); - topic.append(version); + Integer version = message.getVersion(); + if (version != null) { + topic.append(AND); + topic.append(VERSION); + topic.append(version); + } break; - } default: { @@ -177,7 +162,6 @@ public void send(final IotHubTransportMessage message) throws TransportException return; } - String publishTopic = buildTopic(message); requestMap.put(message.getRequestId(), message.getDeviceOperationType()); if (message.getDeviceOperationType() == DeviceOperations.DEVICE_OPERATION_TWIN_SUBSCRIBE_DESIRED_PROPERTIES_REQUEST) @@ -195,6 +179,7 @@ public void send(final IotHubTransportMessage message) throws TransportException } else { + String publishTopic = buildTopic(message); this.publish(publishTopic, message); } } diff --git a/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/twin/ReportedPropertiesUpdateResponse.java b/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/twin/ReportedPropertiesUpdateResponse.java index c5d830a011..d7832d7f88 100644 --- a/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/twin/ReportedPropertiesUpdateResponse.java +++ b/iothub/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/twin/ReportedPropertiesUpdateResponse.java @@ -15,5 +15,5 @@ public class ReportedPropertiesUpdateResponse * does not apply this reported properties update immediately. */ @Getter - private final int version; + private final Integer version; } diff --git a/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessageTest.java b/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessageTest.java index 09260f0dac..055eed0ee0 100644 --- a/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessageTest.java +++ b/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessageTest.java @@ -62,7 +62,7 @@ public void constructorWithByteArraySetsMessageType() // assert assertEquals(messageType, iotHubTransportMessage.getMessageType()); - assertEquals(0, iotHubTransportMessage.getVersion()); + assertNull(iotHubTransportMessage.getVersion()); assertNull(iotHubTransportMessage.getRequestId()); assertNull(iotHubTransportMessage.getStatus()); assertEquals(DEVICE_OPERATION_UNKNOWN, iotHubTransportMessage.getDeviceOperationType()); @@ -89,7 +89,7 @@ public void constructorSuccess() public void setVersionSetsTheVersion() { // arrange - int version = 1234; + Integer version = 1234; byte[] data = new byte[1]; MessageType messageType = MessageType.DEVICE_TWIN; IotHubTransportMessage iotHubTransportMessage = new IotHubTransportMessage(data, messageType); diff --git a/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwinTest.java b/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwinTest.java index 214a8559d0..325456ec7a 100644 --- a/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwinTest.java +++ b/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwinTest.java @@ -31,7 +31,7 @@ public class MqttTwinTest { final String resTopic = "$iothub/twin/res/#"; - final int mockVersion = 5; + final Integer mockVersion = 5; final String mockReqId = String.valueOf(100); @Mocked @@ -227,6 +227,44 @@ public void sendPublishesMessageForUpdateReportedPropertiesOnCorrectTopic(@Mocke } }; } + @Test + public void sendPublishesMessageForUpdateReportedPropertiesOnCorrectTopicWithoutVersion(@Mocked final Mqtt mockMqtt, @Mocked final IotHubTransportMessage mockMessage) throws TransportException + { + //arrange + final byte[] actualPayload = {0x61, 0x62, 0x63}; + final String expectedTopic = "$iothub/twin/PATCH/properties/reported/?$rid="+ mockReqId; + MqttTwin testTwin = new MqttTwin("", mockedConnectOptions, new HashMap<>(), new ConcurrentLinkedQueue<>()); + testTwin.start(); + new NonStrictExpectations() + { + { + mockMessage.getBytes(); + result = actualPayload; + mockMessage.getMessageType(); + result = MessageType.DEVICE_TWIN; + mockMessage.getDeviceOperationType(); + result = DEVICE_OPERATION_TWIN_UPDATE_REPORTED_PROPERTIES_REQUEST; + mockMessage.getRequestId(); + result = mockReqId; + mockMessage.getVersion(); + result = null; + } + }; + + //act + testTwin.send(mockMessage); + + //assert + new Verifications() + { + { + mockMessage.getBytes(); + times = 1; + Deencapsulation.invoke(mockMqtt, "publish", expectedTopic, mockMessage); + times = 1; + } + }; + } /* **Tests_SRS_MQTTDEVICETWIN_25_027: [send method shall throw an IllegalArgumentException if message contains a null or empty request id if the operation is of type DEVICE_OPERATION_TWIN_UPDATE_REPORTED_PROPERTIES_REQUEST.] */ @@ -462,7 +500,7 @@ public void receiveParsesResponseTopicForGetTwinSucceeds() throws TransportExcep assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_TWIN_GET_RESPONSE); assertEquals(receivedMessage.getRequestId(), mockReqId); assertEquals("200", receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); } } @Test @@ -624,7 +662,7 @@ public void receiveSetsReqIdOnResTopic() throws TransportException assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_TWIN_GET_RESPONSE); assertEquals(receivedMessage.getRequestId(), mockReqId); assertEquals("200", receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); } } @@ -658,7 +696,7 @@ public void receiveDoesNotSetReqIdOnResTopicIfNotFound() throws TransportExcepti assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_UNKNOWN); assertNull(receivedMessage.getRequestId()); assertEquals("200", receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); } } @@ -731,7 +769,7 @@ public void receiveDoesNotSetVersionOnResTopicIfNotFound() throws TransportExcep assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_TWIN_GET_RESPONSE); assertEquals(receivedMessage.getRequestId(), mockReqId); assertEquals("201", receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); } } @@ -768,7 +806,7 @@ public void receiveSetsDataForGetTwinResp() throws TransportException assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_TWIN_GET_RESPONSE); assertEquals(receivedMessage.getRequestId(), mockReqId); assertEquals("200", receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); byte[] receivedMessageBytes = receivedMessage.getBytes(); assertEquals(receivedMessageBytes.length, actualPayload.length); @@ -854,7 +892,7 @@ public void receiveSetsDataForDesiredPropNotifResp() throws TransportException assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_TWIN_SUBSCRIBE_DESIRED_PROPERTIES_RESPONSE); assertNull(receivedMessage.getRequestId()); assertNull(receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); byte[] receivedMessageBytes = receivedMessage.getBytes(); assertEquals(receivedMessageBytes.length, actualPayload.length); @@ -893,7 +931,7 @@ public void receiveDoesNotSetVersionForDesiredPropNotifRespIfNotFound() throws T assertSame(receivedMessage.getDeviceOperationType(), DEVICE_OPERATION_TWIN_SUBSCRIBE_DESIRED_PROPERTIES_RESPONSE); assertNull(receivedMessage.getRequestId()); assertNull(receivedMessage.getStatus()); - assertEquals(0, receivedMessage.getVersion()); + assertNull(receivedMessage.getVersion()); byte[] receivedMessageBytes = receivedMessage.getBytes(); assertEquals(receivedMessageBytes.length, actualPayload.length); diff --git a/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/twin/DeviceTwinMessageTest.java b/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/twin/DeviceTwinMessageTest.java index 1f5068d454..2b45f0095f 100644 --- a/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/twin/DeviceTwinMessageTest.java +++ b/iothub/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/twin/DeviceTwinMessageTest.java @@ -32,11 +32,12 @@ public void constructorSetsRequiredPrivateMembers() IotHubTransportMessage msg = new IotHubTransportMessage(actualData, MessageType.DEVICE_TWIN); //assert - int actualVersion = Deencapsulation.getField(msg, "version"); + Integer actualVersion = Deencapsulation.getField(msg, "version"); String actualRequestId = Deencapsulation.getField(msg, "requestId"); String actualStatus = Deencapsulation.getField(msg, "status"); DeviceOperations operationType = Deencapsulation.getField(msg, "operationType"); + assertNull(actualVersion); assertNull(actualRequestId); assertNull(actualStatus); assertEquals(operationType, DeviceOperations.DEVICE_OPERATION_UNKNOWN); @@ -60,20 +61,21 @@ public void gettersGetDefaultsIfNotSet() IotHubTransportMessage msg = new IotHubTransportMessage(actualData, MessageType.DEVICE_TWIN); //act - int testVersion = msg.getVersion(); + Integer testVersion = msg.getVersion(); String testRequestId = msg.getRequestId(); String testStatus = msg.getStatus(); DeviceOperations testOpType = msg.getDeviceOperationType(); //assert - int actualVersion = Deencapsulation.getField(msg, "version"); + Integer actualVersion = Deencapsulation.getField(msg, "version"); String actualRequestId = Deencapsulation.getField(msg, "requestId"); String actualStatus = Deencapsulation.getField(msg, "status"); - DeviceOperations operationType = Deencapsulation.getField(msg, "operationType"); + DeviceOperations actualOperationType = Deencapsulation.getField(msg, "operationType"); - assertEquals(actualRequestId, testRequestId); - assertEquals(actualStatus, testStatus); - assertEquals(actualVersion, testVersion); + assertEquals(testRequestId, actualRequestId); + assertEquals(testStatus, actualStatus); + assertEquals(testVersion, actualVersion); + assertEquals(testOpType, actualOperationType); } /* @@ -87,10 +89,10 @@ public void setVersionSetsCorrectVersion() IotHubTransportMessage msg = new IotHubTransportMessage(actualData, MessageType.DEVICE_TWIN); //act - msg.setVersion(12); + msg.setVersion(12); //assert - assertEquals(msg.getVersion(), 12); + assertEquals(12, msg.getVersion().intValue()); } /* @@ -108,7 +110,7 @@ public void getVersionRetrievesVersion() int version = msg.getVersion(); //assert - assertEquals(version, 12); + assertEquals(12, version); }