Skip to content

Commit

Permalink
Docs and tests for disposed module client operations (#3216)
Browse files Browse the repository at this point in the history
  • Loading branch information
David R. Williamson authored Mar 31, 2023
1 parent 2a70b99 commit f57411a
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 75 deletions.
1 change: 0 additions & 1 deletion iothub/device/src/ClientSettings/ITransportSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Net;
using Microsoft.Azure.Devices.Shared;

namespace Microsoft.Azure.Devices.Client
{
Expand Down
42 changes: 19 additions & 23 deletions iothub/device/src/DeviceClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private DeviceClient(InternalClient internalClient)
/// <param name="hostname">The fully-qualified DNS host name of IoT hub</param>
/// <param name="authenticationMethod">The authentication method that is used</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient Create(string hostname, IAuthenticationMethod authenticationMethod, ClientOptions options = default)
{
return Create(() => ClientFactory.Create(hostname, authenticationMethod, options));
Expand All @@ -62,7 +62,7 @@ public static DeviceClient Create(string hostname, IAuthenticationMethod authent
/// <param name="gatewayHostname">The fully-qualified DNS host name of Gateway</param>
/// <param name="authenticationMethod">The authentication method that is used</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient Create(
string hostname,
string gatewayHostname,
Expand All @@ -79,7 +79,7 @@ public static DeviceClient Create(
/// <param name="authenticationMethod">The authentication method that is used</param>
/// <param name="transportType">The transportType used (HTTP1, AMQP or MQTT), <see cref="TransportType"/></param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient Create(
string hostname,
IAuthenticationMethod authenticationMethod,
Expand All @@ -97,7 +97,7 @@ public static DeviceClient Create(
/// <param name="authenticationMethod">The authentication method that is used</param>
/// <param name="transportType">The transportType used (Http1, AMQP or MQTT), <see cref="TransportType"/></param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient Create(
string hostname,
string gatewayHostname,
Expand All @@ -115,7 +115,7 @@ public static DeviceClient Create(
/// <param name="authenticationMethod">The authentication method that is used</param>
/// <param name="transportSettings">Prioritized list of transportTypes and their settings</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient Create(string hostname, IAuthenticationMethod authenticationMethod,
ITransportSettings[] transportSettings, ClientOptions options = default)
{
Expand All @@ -130,7 +130,7 @@ public static DeviceClient Create(string hostname, IAuthenticationMethod authent
/// <param name="authenticationMethod">The authentication method that is used</param>
/// <param name="transportSettings">Prioritized list of transportTypes and their settings</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient Create(string hostname, string gatewayHostname, IAuthenticationMethod authenticationMethod,
ITransportSettings[] transportSettings, ClientOptions options = default)
{
Expand All @@ -142,7 +142,7 @@ public static DeviceClient Create(string hostname, string gatewayHostname, IAuth
/// </summary>
/// <param name="connectionString">Connection string for the IoT hub (including DeviceId)</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient CreateFromConnectionString(string connectionString, ClientOptions options = default)
{
return Create(() => ClientFactory.CreateFromConnectionString(connectionString, options));
Expand All @@ -154,7 +154,7 @@ public static DeviceClient CreateFromConnectionString(string connectionString, C
/// <param name="connectionString">IoT hub-Scope Connection string for the IoT hub (without DeviceId)</param>
/// <param name="deviceId">Id of the Device</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient CreateFromConnectionString(string connectionString, string deviceId, ClientOptions options = default)
{
return Create(() => ClientFactory.CreateFromConnectionString(connectionString, deviceId, options));
Expand All @@ -166,7 +166,7 @@ public static DeviceClient CreateFromConnectionString(string connectionString, s
/// <param name="connectionString">Connection string for the IoT hub (including DeviceId)</param>
/// <param name="transportType">Specifies whether Http1, AMQP or MQTT transport is used, <see cref="TransportType"/></param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient CreateFromConnectionString(
string connectionString,
TransportType transportType,
Expand All @@ -182,7 +182,7 @@ public static DeviceClient CreateFromConnectionString(
/// <param name="deviceId">Id of the device</param>
/// <param name="transportType">The transportType used (Http1, AMQP or MQTT), <see cref="TransportType"/></param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient CreateFromConnectionString(
string connectionString,
string deviceId,
Expand All @@ -198,7 +198,7 @@ public static DeviceClient CreateFromConnectionString(
/// <param name="connectionString">Connection string for the IoT hub (with DeviceId)</param>
/// <param name="transportSettings">Prioritized list of transports and their settings</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient CreateFromConnectionString(
string connectionString,
ITransportSettings[] transportSettings,
Expand All @@ -214,7 +214,7 @@ public static DeviceClient CreateFromConnectionString(
/// <param name="deviceId">Id of the device</param>
/// <param name="transportSettings">Prioritized list of transportTypes and their settings</param>
/// <param name="options">The options that allow configuration of the device client instance during initialization.</param>
/// <returns>A disposable DeviceClient instance</returns>
/// <returns>A disposable DeviceClient instance.</returns>
public static DeviceClient CreateFromConnectionString(
string connectionString,
string deviceId,
Expand Down Expand Up @@ -283,9 +283,7 @@ public RetryPolicyType RetryPolicy
/// The change will take effect after any in-progress operations.
/// </summary>
/// <param name="retryPolicy">The retry policy. The default is
/// <c>new ExponentialBackoff(int.MaxValue, TimeSpan.FromMilliseconds(100), TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(100));</c></param>
// Codes_SRS_DEVICECLIENT_28_001: [This property shall be defaulted to the exponential retry strategy with back-off
// parameters for calculating delay in between retries.]
/// <code>new ExponentialBackoff(int.MaxValue, TimeSpan.FromMilliseconds(100), TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(100));</code></param>
public void SetRetryPolicy(IRetryPolicy retryPolicy)
{
InternalClient.SetRetryPolicy(retryPolicy);
Expand Down Expand Up @@ -735,12 +733,10 @@ public void SetMethodHandler(string methodName, MethodCallback methodHandler, ob
/// Sets a new delegate for the connection status changed callback.
/// </summary>
/// <remarks>
/// If a delegate is already associated,
/// it will be replaced with the new delegate. Note that this callback will never be called if the client is configured to use
/// HTTP, as that protocol is stateless.
/// If a delegate is already associated, it will be replaced with the new delegate. Note that this callback will never be called
/// if the client is configured to use HTTP, as that protocol is stateless.
/// </remarks>
/// <param name="statusChangesHandler">The name of the method to associate with the delegate.</param>
/// <exception cref="ObjectDisposedException">When the client has been disposed.</exception>
public void SetConnectionStatusChangesHandler(ConnectionStatusChangesHandler statusChangesHandler) =>
InternalClient.SetConnectionStatusChangesHandler(statusChangesHandler);

Expand All @@ -766,11 +762,11 @@ public void Dispose()
/// Includes a call to <see cref="CloseAsync()"/>.
/// </remarks>
/// <example>
/// <c>
/// <code language="csharp">
/// await using var client = DeviceClient.CreateFromConnectionString(...);
/// </c>
/// </code>
/// or
/// <c>
/// <code language="csharp">
/// var client = DeviceClient.CreateFromConnectionString(...);
/// try
/// {
Expand All @@ -780,7 +776,7 @@ public void Dispose()
/// {
/// await client.DisposeAsync();
/// }
/// </c>
/// </code>
/// </example>
[SuppressMessage("Usage", "CA1816:Dispose methods should call SuppressFinalize", Justification = "SuppressFinalize is called by Dispose(), which this method calls.")]
public async ValueTask DisposeAsync()
Expand Down
11 changes: 5 additions & 6 deletions iothub/device/src/InternalClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ internal class InternalClient : IDisposable
private readonly HttpTransportHandler _fileUploadHttpTransportHandler;
private readonly ITransportSettings[] _transportSettings;
private readonly ClientOptions _clientOptions;
private volatile bool _isDisposed;

// Stores message input names supported by the client module and their associated delegate.
private volatile Dictionary<string, Tuple<MessageHandler, object>> _receiveEventEndpoints;
Expand Down Expand Up @@ -200,6 +199,7 @@ public InternalClient(
if (Logging.IsEnabled)
Logging.Exit(this, transportSettings, pipelineBuilder, nameof(InternalClient) + "_ctor");
}
internal bool IsDisposed { get; private set; }

/// <summary>
/// Diagnostic sampling percentage value, [0-100];
Expand Down Expand Up @@ -1120,7 +1120,6 @@ public async Task<Twin> GetTwinAsync()
/// <returns>The device twin object for the current device</returns>
public Task<Twin> GetTwinAsync(CancellationToken cancellationToken)
{
// `GetTwinAsync` shall call `SendTwinGetAsync` on the transport to get the twin state.
try
{
return InnerHandler.SendTwinGetAsync(cancellationToken);
Expand Down Expand Up @@ -1379,7 +1378,7 @@ internal Task<FileUploadSasUriResponse> GetFileUploadSasUriAsync(
FileUploadSasUriRequest request,
CancellationToken cancellationToken = default)
{
if (_isDisposed)
if (IsDisposed)
{
throw new ObjectDisposedException("IoT client", DefaultDelegatingHandler.ClientDisposedMessage);
}
Expand All @@ -1391,7 +1390,7 @@ internal Task CompleteFileUploadAsync(
FileUploadCompletionNotification notification,
CancellationToken cancellationToken = default)
{
if (_isDisposed)
if (IsDisposed)
{
throw new ObjectDisposedException("IoT client", DefaultDelegatingHandler.ClientDisposedMessage);
}
Expand Down Expand Up @@ -1429,7 +1428,7 @@ public Task UploadToBlobAsync(string blobName, Stream source, CancellationToken

try
{
if (_isDisposed)
if (IsDisposed)
{
throw new ObjectDisposedException("IoT client", DefaultDelegatingHandler.ClientDisposedMessage);
}
Expand Down Expand Up @@ -1846,7 +1845,7 @@ internal void ValidateModuleTransportHandler(string apiName)

public void Dispose()
{
_isDisposed = true;
IsDisposed = true;
InnerHandler?.Dispose();
_methodsSemaphore?.Dispose();
_moduleReceiveMessageSemaphore?.Dispose();
Expand Down
Loading

0 comments on commit f57411a

Please sign in to comment.