Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TryGetValue methods should check for the component identifier when applicable and should not throw any exceptions #2111

Merged
merged 8 commits into from
Jul 20, 2021
Merged

TryGetValue methods should check for the component identifier when applicable and should not throw any exceptions #2111

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9dd3e2f
fix(iot-device): TryGetValue methods should not throw any exceptions
abhipsaMisra Jul 16, 2021
ce85a42
comments1
abhipsaMisra Jul 16, 2021
166ceb7
comments2
abhipsaMisra Jul 16, 2021
9c96941
add check for component identifier
abhipsaMisra Jul 19, 2021
daa98d0
rename
abhipsaMisra Jul 19, 2021
9d321d8
arrange, act, assert
abhipsaMisra Jul 19, 2021
fc661c1
fix
abhipsaMisra Jul 19, 2021
293bc1e
one more successful/unsuccessful reference
abhipsaMisra Jul 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 56 additions & 18 deletions iothub/device/src/ClientPropertyCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,9 @@ public bool Contains(string componentName, string propertyName)
/// <typeparam name="T">The type to cast the object to.</typeparam>
/// <param name="componentName">The component which holds the required property.</param>
/// <param name="propertyName">The property to get.</param>
/// <param name="propertyValue">The value of the component-level property.</param>
/// <returns>true if the property collection contains a component level property with the specified key; otherwise, false.</returns>
/// <param name="propertyValue">When this method returns successfully, this contains the value of the component-level property.
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
/// When this method returns unsuccessfully, this contains the default value of the type <c>T</c> passed in.</param>
/// <returns>True if a component-level property of type <c>T</c> with the specified key was found; otherwise, it returns false.</returns>
public virtual bool TryGetValue<T>(string componentName, string propertyName, out T propertyValue)
{
if (Logging.IsEnabled && Convention == null)
Expand All @@ -180,35 +181,72 @@ public virtual bool TryGetValue<T>(string componentName, string propertyName, ou
$"TryGetValue will attempt to get the property value but may not behave as expected.", nameof(TryGetValue));
}

// If either the component name or the property name is null, empty or whitespace,
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
// then return unsuccessfully (false) with the default value of the type <T> passed in.
if (string.IsNullOrWhiteSpace(componentName) || string.IsNullOrWhiteSpace(propertyName))
{
propertyValue = default;
return false;
}

if (Contains(componentName, propertyName))
{
object componentProperties = Collection[componentName];

// If the ClientPropertyCollection was constructed by the user application (eg. for updating the client properties)
// then the componentProperties are retrieved as a dictionary.
// The required property value can be fetched from the dictionary directly.
if (componentProperties is IDictionary<string, object> nestedDictionary)
{
if (nestedDictionary.TryGetValue(propertyName, out object dictionaryElement))
// First verify that the retrieved dictionary contains the component identifier { "__t": "c" }.
// If not, then the retrieved nested dictionary is actually a root-level property of type map.
if (nestedDictionary.TryGetValue(ConventionBasedConstants.ComponentIdentifierKey, out object componentIdentifierValue)
&& componentIdentifierValue.Equals(ConventionBasedConstants.ComponentIdentifierValue))
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
{
// If the value is null, go ahead and return it.
if (dictionaryElement == null)
{
propertyValue = default;
return true;
}

// If the object is of type T or can be cast to type T, go ahead and return it.
if (dictionaryElement is T valueRef
|| NumericHelpers.TryCastNumericTo(dictionaryElement, out valueRef))
if (nestedDictionary.TryGetValue(propertyName, out object dictionaryElement))
{
propertyValue = valueRef;
return true;
// If the value associated with the key is null, then return successfully (true) with the default value of the type <T> passed in.
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
if (dictionaryElement == null)
{
propertyValue = default;
return true;
}

// If the object is of type T or can be cast to type T, go ahead and return it.
if (dictionaryElement is T valueRef
|| NumericHelpers.TryCastNumericTo(dictionaryElement, out valueRef))
{
propertyValue = valueRef;
return true;
}
}
}
}
else
{
// If it's not, we need to try to convert it using the serializer.
Convention.PayloadSerializer.TryGetNestedObjectValue<T>(componentProperties, propertyName, out propertyValue);
return true;
// If the ClientPropertyCollection was constructed by the SDK (eg. when retrieving the client properties)
// then the componentProperties are retrieved as the json object that is defined in the PayloadConvention.
// The required property value then needs to be deserialized accordingly.
try
{
// First verify that the retrieved dictionary contains the component identifier { "__t": "c" }.
// If not, then the retrieved nested dictionary is actually a root-level property of type map.
if (Convention
.PayloadSerializer
.TryGetNestedObjectValue(componentProperties, ConventionBasedConstants.ComponentIdentifierKey, out string componentIdentifierValue)
&& componentIdentifierValue.Equals(ConventionBasedConstants.ComponentIdentifierValue))
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
{
// Since the value cannot be cast to <T> directly, we need to try to convert it using the serializer.
// If it can be successfully converted, go ahead and return it.
Convention.PayloadSerializer.TryGetNestedObjectValue<T>(componentProperties, propertyName, out propertyValue);
return true;
}
}
catch
{
// In case the value cannot be converted using the serializer,
// then return unsuccessfully (false) with the default value of the type <T> passed in.
}
}
}

Expand Down
32 changes: 23 additions & 9 deletions iothub/device/src/PayloadCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,11 @@ public bool Contains(string key)
/// <summary>
/// Gets the value of the object from the collection.
/// </summary>
/// <remarks>
/// This class is used for both sending and receiving properties for the device.
/// </remarks>
/// <typeparam name="T">The type to cast the object to.</typeparam>
/// <param name="key">The key of the property to get.</param>
/// <param name="value">The value of the object from the collection.</param>
/// <returns>True if the collection contains an element with the specified key; otherwise, it returns false.</returns>
/// <param name="value">When this method returns successfully, this contains the value of the object from the collection.
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
/// When this method returns unsuccessfully, this contains the default value of the type <c>T</c> passed in.</param>
/// <returns>True if a value of type <c>T</c> with the specified key was found; otherwise, it returns false.</returns>
public bool TryGetValue<T>(string key, out T value)
{
if (Logging.IsEnabled && Convention == null)
Expand All @@ -122,9 +120,16 @@ public bool TryGetValue<T>(string key, out T value)
$"TryGetValue will attempt to get the property value but may not behave as expected.", nameof(TryGetValue));
}

// If the key is null, empty or whitespace, then return unsuccessfully (false) with the default value of the type <T> passed in.
if (string.IsNullOrWhiteSpace(key))
{
value = default;
return false;
}

if (Collection.ContainsKey(key))
{
// If the value is null, go ahead and return it.
// If the value associated with the key is null, then return successfully (true) with the default value of the type <T> passed in.
if (Collection[key] == null)
{
value = default;
Expand All @@ -139,9 +144,18 @@ public bool TryGetValue<T>(string key, out T value)
return true;
}

// If it's not, we need to try to convert it using the serializer.
value = Convention.PayloadSerializer.ConvertFromObject<T>(Collection[key]);
return true;
try
{
// If the value cannot be cast to <T> directly, we need to try to convert it using the serializer.
// If it can be successfully converted, go ahead and return it.
value = Convention.PayloadSerializer.ConvertFromObject<T>(Collection[key]);
return true;
}
catch
{
// In case the value cannot be converted using the serializer,
// then return unsuccessfully (false) with the default value of the type <T> passed in.
}
}

value = default;
Expand Down
83 changes: 70 additions & 13 deletions iothub/device/tests/ClientPropertyCollectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,24 +160,46 @@ public void ClientPropertyCollection_CanAddMultiplePropertyAndGetBackWithoutDevi
outIntValue.Should().Be(IntPropertyValue);
}

[TestMethod]
public void ClientPropertyCollection_TryGetValueShouldReturnFalseIfValueNotFound()
{
var clientProperties = new ClientPropertyCollection();
abhipsaMisra marked this conversation as resolved.
Show resolved Hide resolved
clientProperties.AddRootProperty(StringPropertyName, StringPropertyValue);

bool isValueRetrieved = clientProperties.TryGetValue(IntPropertyName, out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollection_TryGetValueShouldReturnFalseIfValueCouldNotBeDeserialized()
{
var clientProperties = new ClientPropertyCollection();
clientProperties.AddRootProperty(StringPropertyName, StringPropertyValue);

bool isValueRetrieved = clientProperties.TryGetValue(StringPropertyName, out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollection_CanAddSimpleObjectWithComponentAndGetBackWithoutDeviceClient()
{
var clientProperties = new ClientPropertyCollection
var componentLevelProperties = new Dictionary<string, object>
{
{ ComponentName, new Dictionary<string, object> {
{ StringPropertyName, StringPropertyValue },
{ BoolPropertyName, BoolPropertyValue },
{ DoublePropertyName, DoublePropertyValue },
{ FloatPropertyName, FloatPropertyValue },
{ IntPropertyName, IntPropertyValue },
{ ShortPropertyName, ShortPropertyValue },
{ ObjectPropertyName, s_objectPropertyValue },
{ ArrayPropertyName, s_arrayPropertyValue },
{ MapPropertyName, s_mapPropertyValue },
{ DateTimePropertyName, s_dateTimePropertyValue } }
}
{ StringPropertyName, StringPropertyValue },
{ BoolPropertyName, BoolPropertyValue },
{ DoublePropertyName, DoublePropertyValue },
{ FloatPropertyName, FloatPropertyValue },
{ IntPropertyName, IntPropertyValue },
{ ShortPropertyName, ShortPropertyValue },
{ ObjectPropertyName, s_objectPropertyValue },
{ ArrayPropertyName, s_arrayPropertyValue },
{ MapPropertyName, s_mapPropertyValue },
{ DateTimePropertyName, s_dateTimePropertyValue }
};
var clientProperties = new ClientPropertyCollection();
clientProperties.AddComponentProperties(ComponentName, componentLevelProperties);

clientProperties.TryGetValue(ComponentName, StringPropertyName, out string stringOutValue);
stringOutValue.Should().Be(StringPropertyValue);
Expand Down Expand Up @@ -308,6 +330,41 @@ public void ClientPropertyCollection_AddingComponentAddsComponentIdentifier()
outValue.Should().Be(StringPropertyValue);
componentOut.Should().Be(ConventionBasedConstants.ComponentIdentifierValue);
}

[TestMethod]
public void ClientPropertyCollection_TryGetValueWithComponentShouldReturnFalseIfValueNotFound()
{
var clientProperties = new ClientPropertyCollection();
clientProperties.AddComponentProperty(ComponentName, StringPropertyName, StringPropertyValue);

bool isValueRetrieved = clientProperties.TryGetValue(ComponentName, IntPropertyName, out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollection_TryGetValueWithComponentShouldReturnFalseIfValueCouldNotBeDeserialized()
{
var clientProperties = new ClientPropertyCollection();
clientProperties.AddComponentProperty(ComponentName, StringPropertyName, StringPropertyValue);

bool isValueRetrieved = clientProperties.TryGetValue(ComponentName, StringPropertyName, out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollection_TryGetValueWithComponentShouldReturnFalseIfNotAComponent()
{
var clientProperties = new ClientPropertyCollection();
clientProperties.AddRootProperty(MapPropertyName, s_mapPropertyValue);

string incorrectlyMappedComponentName = MapPropertyName;
string incorrectlyMappedComponentPropertyName = "key1";
bool isValueRetrieved = clientProperties.TryGetValue(incorrectlyMappedComponentName, incorrectlyMappedComponentPropertyName, out object propertyValue);
isValueRetrieved.Should().BeFalse();
propertyValue.Should().Be(default);
}
}

internal class CustomClientProperty
Expand Down
52 changes: 52 additions & 0 deletions iothub/device/tests/ClientPropertyCollectionTestsNewtonsoft.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,58 @@ public void ClientPropertyCollectionNewtonsoft_CanGetComponentIdentifier()
outValue.Should().Be(StringPropertyValue);
componentOut.Should().Be(ConventionBasedConstants.ComponentIdentifierValue);
}

[TestMethod]
public void ClientPropertyCollectionNewtonSoft_TryGetValueShouldReturnFalseIfValueNotFound()
{
var clientProperties = ClientPropertyCollection.FromTwinCollection(collectionToRoundTrip, DefaultPayloadConvention.Instance);

bool isValueRetrieved = clientProperties.TryGetValue("thisPropertyDoesNotExist", out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollectionNewtonSoft_TryGetValueWithComponentShouldReturnFalseIfValueNotFound()
{
var clientProperties = ClientPropertyCollection.FromTwinCollection(collectionWithComponentToRoundTrip, DefaultPayloadConvention.Instance);

bool isValueRetrieved = clientProperties.TryGetValue(ComponentName, "thisPropertyDoesNotExist", out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollectionNewtonSoft_TryGetValueShouldReturnFalseIfValueCouldNotBeDeserialized()
{
var clientProperties = ClientPropertyCollection.FromTwinCollection(collectionToRoundTrip, DefaultPayloadConvention.Instance);

bool isValueRetrieved = clientProperties.TryGetValue(StringPropertyName, out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollectionNewtonSoft_TryGetValueWithComponentShouldReturnFalseIfValueCouldNotBeDeserialized()
{
var clientProperties = ClientPropertyCollection.FromTwinCollection(collectionWithComponentToRoundTrip, DefaultPayloadConvention.Instance);

bool isValueRetrieved = clientProperties.TryGetValue(ComponentName, StringPropertyName, out int outIntValue);
isValueRetrieved.Should().BeFalse();
outIntValue.Should().Be(default);
}

[TestMethod]
public void ClientPropertyCollectionNewtonSoft_TryGetValueWithComponentShouldReturnFalseIfNotAComponent()
{
var clientProperties = ClientPropertyCollection.FromTwinCollection(collectionToRoundTrip, DefaultPayloadConvention.Instance);

string incorrectlyMappedComponentName = MapPropertyName;
string incorrectlyMappedComponentPropertyName = "key1";
bool isValueRetrieved = clientProperties.TryGetValue(incorrectlyMappedComponentName, incorrectlyMappedComponentPropertyName, out object propertyValue);
isValueRetrieved.Should().BeFalse();
propertyValue.Should().Be(default);
}
}

internal class RootLevelProperties
Expand Down