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

[PnP] Update PayloadCollection to accept dictionary of values #2171

Merged
merged 2 commits into from
Sep 20, 2021
Merged

[PnP] Update PayloadCollection to accept dictionary of values #2171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d4e5b5
fix(iot-device): Update PayloadCollection to accept dictionary of values
abhipsaMisra Sep 17, 2021
7013c5a
minor
abhipsaMisra Sep 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 58 additions & 8 deletions iothub/device/src/ClientPropertyCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,16 @@ public void AddRootProperty(string propertyName, object propertyValue)
/// <param name="componentName">The component with the property to add.</param>
/// <param name="propertyName">The name of the property to add.</param>
/// <param name="propertyValue">The value of the property to add.</param>
/// <exception cref="ArgumentNullException"><paramref name="componentName"/> or <paramref name="propertyName"/> is <c>null</c>.</exception>
public void AddComponentProperty(string componentName, string propertyName, object propertyValue)
=> AddInternal(new Dictionary<string, object> { { propertyName, propertyValue } }, componentName, false);
{
if (componentName == null)
{
throw new ArgumentNullException(nameof(componentName));
}

AddInternal(new Dictionary<string, object> { { propertyName, propertyValue } }, componentName, false);
}

/// <inheritdoc path="/remarks" cref="AddRootProperty(string, object)" />
/// <inheritdoc path="/seealso" cref="AddInternal(IDictionary{string, object}, string, bool)" />
Expand All @@ -55,8 +63,16 @@ public void AddComponentProperty(string componentName, string propertyName, obje
/// <param name="componentName">The component with the properties to add.</param>
/// <param name="properties">A collection of properties to add.</param>
/// <exception cref="ArgumentException">A property name in <paramref name="properties"/> already exists in the collection.</exception>
/// <exception cref="ArgumentNullException"><paramref name="componentName"/> or a property name in <paramref name="properties"/> is <c>null</c>.</exception>
public void AddComponentProperties(string componentName, IDictionary<string, object> properties)
=> AddInternal(properties, componentName, true);
{
if (componentName == null)
{
throw new ArgumentNullException(nameof(componentName));
}

AddInternal(properties, componentName, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the final parameter here be true? If not, why is it being changed from its previous behavior here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final parameter here determines if the operation is a "force" update. This is set to false for the "Add" APIs and is set to true for the "AddOrUpdate" APIs.
This was incorrectly set to true previously.

}

/// <inheritdoc path="/seealso" cref="AddInternal(IDictionary{string, object}, string, bool)" />
/// <inheritdoc path="/exception" cref="AddRootProperty(string, object)" />
Expand Down Expand Up @@ -100,9 +116,16 @@ public void AddOrUpdateRootProperty(string propertyName, object propertyValue)
/// <param name="componentName">The component with the property to add or update.</param>
/// <param name="propertyName">The name of the property to add or update.</param>
/// <param name="propertyValue">The value of the property to add or update.</param>
/// <exception cref="ArgumentNullException"><paramref name="propertyName"/> is <c>null</c>.</exception>
/// <exception cref="ArgumentNullException"><paramref name="componentName"/> or <paramref name="propertyName"/> is <c>null</c>.</exception>
public void AddOrUpdateComponentProperty(string componentName, string propertyName, object propertyValue)
=> AddInternal(new Dictionary<string, object> { { propertyName, propertyValue } }, componentName, true);
{
if (componentName == null)
{
throw new ArgumentNullException(nameof(componentName));
}

AddInternal(new Dictionary<string, object> { { propertyName, propertyValue } }, componentName, true);
}

/// <inheritdoc path="/summary" cref="AddInternal(IDictionary{string, object}, string, bool)" />
/// <inheritdoc path="/seealso" cref="AddInternal(IDictionary{string, object}, string, bool)" />
Expand All @@ -116,8 +139,16 @@ public void AddOrUpdateComponentProperty(string componentName, string propertyNa
/// </remarks>
/// <param name="componentName">The component with the properties to add or update.</param>
/// <param name="properties">A collection of properties to add or update.</param>
/// <exception cref="ArgumentNullException"><paramref name="componentName"/> or a property name in <paramref name="properties"/> is <c>null</c>.</exception>
public void AddOrUpdateComponentProperties(string componentName, IDictionary<string, object> properties)
=> AddInternal(properties, componentName, true);
{
if (componentName == null)
{
throw new ArgumentNullException(nameof(componentName));
}

AddInternal(properties, componentName, true);
}

/// <inheritdoc path="/summary" cref="AddInternal(IDictionary{string, object}, string, bool)" />
/// <inheritdoc path="/seealso" cref="AddInternal(IDictionary{string, object}, string, bool)" />
Expand All @@ -133,9 +164,16 @@ public void AddOrUpdateComponentProperties(string componentName, IDictionary<str
/// <param name="properties">A collection of properties to add or update.</param>
/// <exception cref="ArgumentNullException"><paramref name="properties"/> is <c>null</c>.</exception>
public void AddOrUpdateRootProperties(IDictionary<string, object> properties)
=> properties
{
if (properties == null)
{
throw new ArgumentNullException(nameof(properties));
}

properties
.ToList()
.ForEach(entry => Collection[entry.Key] = entry.Value);
}

/// <summary>
/// Determines whether the specified property is present.
Expand Down Expand Up @@ -426,14 +464,14 @@ internal static ClientPropertyCollection FromClientPropertiesAsDictionary(IDicti
/// <param name="componentName">The component with the properties to add or update.</param>
/// <param name="forceUpdate">Forces the collection to use the Add or Update behavior.
/// Setting to true will simply overwrite the value. Setting to false will use <see cref="IDictionary{TKey, TValue}.Add(TKey, TValue)"/></param>
/// <exception cref="ArgumentNullException"><paramref name="properties"/> is <c>null</c> for a top-level property operation.</exception>
/// <exception cref="ArgumentNullException"><paramref name="properties"/> is <c>null</c>.</exception>
private void AddInternal(IDictionary<string, object> properties, string componentName = default, bool forceUpdate = false)
{
// If the componentName is null then simply add the key-value pair to Collection dictionary.
// This will either insert a property or overwrite it if it already exists.
if (componentName == null)
{
// If both the component name and properties collection are null then throw a ArgumentNullException.
// If both the component name and properties collection are null then throw an ArgumentNullException.
// This is not a valid use-case.
if (properties == null)
{
Expand All @@ -442,6 +480,12 @@ private void AddInternal(IDictionary<string, object> properties, string componen

foreach (KeyValuePair<string, object> entry in properties)
{
// A null property key is not allowed. Throw an ArgumentNullException.
if (entry.Key == null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, a dictionary will not allow you to have a null value as a key. Might be overkill to check for this unless we're thinking that someone will violate the IDictionary<> Add method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense; however, our public API does take in an IDictionary (and not a Dictionary), so this might be worth holding on to. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will likely save us from some headache since we don't want to be in a position where we would try to serialize a null key

{
throw new ArgumentNullException(nameof(entry.Key));
}

if (forceUpdate)
{
Collection[entry.Key] = entry.Value;
Expand Down Expand Up @@ -471,6 +515,12 @@ private void AddInternal(IDictionary<string, object> properties, string componen

foreach (KeyValuePair<string, object> entry in properties)
{
// A null property key is not allowed. Throw an ArgumentNullException.
if (entry.Key == null)
{
throw new ArgumentNullException(nameof(entry.Key));
}

if (forceUpdate)
{
componentProperties[entry.Key] = entry.Value;
Expand Down
39 changes: 39 additions & 0 deletions iothub/device/src/TelemetryCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.Azure.Devices.Client
{
Expand Down Expand Up @@ -32,5 +34,42 @@ public override void AddOrUpdate(string telemetryName, object telemetryValue)
{
base.AddOrUpdate(telemetryName, telemetryValue);
}

/// <summary>
/// Adds the telemetry values to the telemetry collection.
/// </summary>
/// <inheritdoc cref="AddOrUpdate(string, object)" path="/param['telemetryName']"/>
/// <inheritdoc cref="AddOrUpdate(string, object)" path="/param['telemetryValue']"/>
/// <exception cref="ArgumentException">An element with the same key already exists in the collection.</exception>
/// <exception cref="ArgumentNullException"><paramref name="telemetryValues"/> is <c>null</c>.</exception>
public void Add(IDictionary<string, object> telemetryValues)
{
if (telemetryValues == null)
{
throw new ArgumentNullException(nameof(telemetryValues));
}

telemetryValues
.ToList()
.ForEach(entry => base.Add(entry.Key, entry.Value));
}

/// <summary>
/// Adds or updates the telemetry values in the telemetry collection.
/// </summary>
/// <inheritdoc cref="AddOrUpdate(string, object)" path="/param['telemetryName']"/>
/// <inheritdoc cref="AddOrUpdate(string, object)" path="/param['telemetryValue']"/>
/// <exception cref="ArgumentNullException"><paramref name="telemetryValues"/> is <c>null</c>.</exception>
public void AddOrUpdate(IDictionary<string, object> telemetryValues)
{
if (telemetryValues == null)
{
throw new ArgumentNullException(nameof(telemetryValues));
}

telemetryValues
.ToList()
.ForEach(entry => base.AddOrUpdate(entry.Key, entry.Value));
}
}
}
Loading