Skip to content

Commit

Permalink
Allow device/module client to send reported properties without specif…
Browse files Browse the repository at this point in the history 
…ying 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 <jakubowski.bartosz@gmail.com>
  • Loading branch information
@timtay-microsoft @dzakub
timtay-microsoft and dzakub authored Mar 11, 2024
1 parent 59b4e9f commit be93242
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 50 deletions.
4 changes: 2 additions & 2 deletions .../src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/setup/TwinCommon.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
21 changes: 21 additions & 0 deletions ...on/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/twin/TwinTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down
10 changes: 5 additions & 5 deletions ...nt/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
29 changes: 7 additions & 22 deletions ...vice-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
{
Expand Down Expand Up @@ -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)
Expand All @@ -195,6 +179,7 @@ public void send(final IotHubTransportMessage message) throws TransportException
}
else
{
String publishTopic = buildTopic(message);
this.publish(publishTopic, message);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/com/microsoft/azure/sdk/iot/device/twin/ReportedPropertiesUpdateResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ public class ReportedPropertiesUpdateResponse
* does not apply this reported properties update immediately.
*/
@Getter
private final int version;
private final Integer version;
}
4 changes: 2 additions & 2 deletions ...rc/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportMessageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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);
Expand Down
54 changes: 46 additions & 8 deletions ...-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/mqtt/MqttTwinTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.]
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
22 changes: 12 additions & 10 deletions ...e-client/src/test/java/com/microsoft/azure/sdk/iot/device/twin/DeviceTwinMessageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}

/*
Expand All @@ -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());
}

/*
Expand All @@ -108,7 +110,7 @@ public void getVersionRetrievesVersion()
int version = msg.getVersion();

//assert
assertEquals(version, 12);
assertEquals(12, version);

}

Expand Down

0 comments on commit be93242

Please sign in to comment.