Add unit test coverage to v2 service client (#3137)

* Improve unit test coverages for Devices V2 (#3046) * Add test cases * Add more tests * Add more unit tests * Address comments * Address comments * Address comments * Add unit tests for ConfigurationsClient * remove unused import statements * Add more test cases and address comments * Add a simple message unit test * Npatilsen/improve unit test coverage (#3047) * connection string tests -- connection string to 90+, connection string parser to 85+, sasbuilder to 100 * added clientTwinProperties test * TwinsClient tests up to 92% and 76% * dmcr tests * dcmr part 2 * direct method service request coverage * direct method service request coverage p2 * exponentialretrypolicy * address comments * Improve code coverage DevicesClient V2 (#3073) * Add more unit tests * Address comments * Create new tests * Work in progress * Work in progress * Work in progress * Add more unit tests * Make methods virtual and write more unit test cases * Add more test * Add comment * Rename test case * remove extra line * remove extra line * Address comments * Add comments * Npatilsen improve code coverage 2 (#3063) * iothubserviceclient tests started * iothubservice client part 2 * fixeddelayretrypolicy * extra fixedretry and incrementalretry policy tests * connection properties and sas properties tests * addressed comments * reformatted logic for connection properties hostname * more comments * Fix error (#3078) * added serialized client twins to http response (#3079) * Remove invalid test * Add new unit tests in V2 Devices to improve coverage (#3089) * Add tests for IotHubTokenCredentialProperties and deserialization * Rename tests * work in progress * Work in progress * Work in progress * Add more tests * Added more tests * Fix broken test * Make changes * Make changes * Address comments 1 * Address comments * Address comments * Address comments * Update test method * minor change * add a test * address comments * address comments * Add additional assertion for validation * address comment * fix typo * address comments * remove extra assertions * address comments * Improve code coverage for AMQP Devices V2 (#3099) * Add tests for IotHubTokenCredentialProperties and deserialization * Work in progress * Work in progress * Add more tests * Added more tests * Fix broken test * Make changes * Address comments 1 * Address comments * Address comments * minor change * address comments * address comments * Add additional assertion for validation * remove extra assertions * work in progress * test * work in progress * work in progress * work in progress * Work in progress * Work in progress * Fix * Fix * Fix * Add more tests * Small updates * remove duplicate test file * minor fixes * minor change * Remove unnecessary test case * Address comments * address comments * address pr comments * added subclient test coverage (serviceClient) (#3088) * rebase and digitalTwinsClient tests * digitalTwinsClient bugs fixed * query client tests outlines * query test attempts * scheduled jobs client and retry policy base fix * debugged query client tests * addressed comments p1 * addressed comments p2 * fixes part 2 * fixed retry policy base * fixes p3 * additional comments * made location more realistic * better test names * final comments addressed * Npatilsen/misc files missing tests (#3118) * rebase and digitalTwinsClient tests * digitalTwinsClient bugs fixed * query client tests outlines * query test attempts * scheduled jobs client and retry policy base fix * debugged query client tests * addressed comments p1 * addressed comments p2 * fixes part 2 * fixed retry policy base * new classes p1 * new classes p1 * merge conflict fixes * merge conflict fixes p2 * retry policy base revert * serialize jobrequests * scheduledjobsoptions serialization * queryresponse * addressed comments * addressed comments * nit comments * rebase onto previews/v2 * Delete haproxy.pem * unsealed amqp classes to make mockable * feedback and error fixes * address error message * compiler warning fix * compiler warning fix p2 * fix failing tests --------- Co-authored-by: Sophia Ji Who Choi <[email protected]> Co-authored-by: Sophia Choi <[email protected]>
Azure · Mar 1, 2023 · 4a77f6d · 4a77f6d
1 parent 72366f2
commit 4a77f6d
Show file tree

Hide file tree

Showing 62 changed files with 5,826 additions and 186 deletions.
diff --git a/iothub/service/src/Amqp/AmqpCbsSessionHandler.cs b/iothub/service/src/Amqp/AmqpCbsSessionHandler.cs
@@ -5,15 +5,14 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Azure.Amqp;
-using Microsoft.Azure.Devices.Common;
 
 namespace Microsoft.Azure.Devices.Amqp
 {
     /// <summary>
     /// Handles a single CBS session (for SAS token renewal) including the inital authentication and scheduling all subsequent
     /// authentication attempts.
     /// </summary>
-    internal sealed class AmqpCbsSessionHandler : IDisposable
+    internal class AmqpCbsSessionHandler : IDisposable
     {
         // There is no AmqpSession object to track here because it is encapsulated by the AmqpCbsLink class.
         private readonly IotHubConnectionProperties _credential;
@@ -26,6 +25,8 @@ internal sealed class AmqpCbsSessionHandler : IDisposable
         private static readonly TimeSpan s_defaultOperationTimeout = TimeSpan.FromMinutes(1);
         private readonly IOThreadTimerSlim _refreshTokenTimer;
 
+        protected AmqpCbsSessionHandler() { }
+
         public AmqpCbsSessionHandler(IotHubConnectionProperties credential, EventHandler connectionLossHandler)
         {
             _credential = credential;
@@ -35,10 +36,11 @@ public AmqpCbsSessionHandler(IotHubConnectionProperties credential, EventHandler
 
         /// <summary>
         /// Opens the session, then opens the CBS links, then sends the initial authentication message.
+        /// Marked virtual for unit testing purposes only.
         /// </summary>
         /// <param name="connection">The connection to attach this session to.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
-        public async Task OpenAsync(AmqpConnection connection, CancellationToken cancellationToken)
+        public virtual async Task OpenAsync(AmqpConnection connection, CancellationToken cancellationToken)
         {
             if (Logging.IsEnabled)
                 Logging.Enter(this, $"Opening CBS session.");
@@ -82,7 +84,6 @@ public void Close()
         /// Returns true if this session and its CBS link are open. Returns false otherwise.
         /// </summary>
         /// <returns>True if this session and its CBS link are open. False otherwise.</returns>
-
         public bool IsOpen()
         {
             return _cbsLink != null;

diff --git a/iothub/service/src/Amqp/AmqpConnectionHandler.cs b/iothub/service/src/Amqp/AmqpConnectionHandler.cs
@@ -21,7 +21,7 @@ namespace Microsoft.Azure.Devices.Amqp
     /// <remarks>
     /// This class intentionally abstracts away details about sessions and links for simplicity at the service client level.
     /// </remarks>
-    internal sealed class AmqpConnectionHandler : IDisposable
+    internal class AmqpConnectionHandler : IDisposable
     {
         private static readonly AmqpVersion s_amqpVersion_1_0_0 = new(1, 0, 0);
 
@@ -41,6 +41,35 @@ internal sealed class AmqpConnectionHandler : IDisposable
 
         // The current delivery tag. Increments after each send operation to give a unique value.
         private int _sendingDeliveryTag;
+
+        /// <summary>
+        /// Creates an instance of this class. Provided for unit testing purposes only.
+        /// </summary>
+        protected internal AmqpConnectionHandler()
+        { }
+
+        /// <summary>
+        /// Creates an instance of this class. Provided for unit testing purposes only.
+        /// </summary>
+        internal AmqpConnectionHandler(
+            IotHubConnectionProperties credential,
+            IotHubTransportProtocol protocol,
+            string linkAddress,
+            IotHubServiceClientOptions options,
+            EventHandler connectionLossHandler,
+            AmqpCbsSessionHandler cbsSession,
+            AmqpSessionHandler workerSession)
+        {
+            _credential = credential;
+            _useWebSocketOnly = protocol == IotHubTransportProtocol.WebSocket;
+            _linkAddress = linkAddress;
+            _options = options;
+            _connectionLossHandler = connectionLossHandler;
+            _cbsSession = cbsSession;
+            _workerSession = workerSession;
+
+            _sendingDeliveryTag = 0;
+        }
 
         internal AmqpConnectionHandler(
             IotHubConnectionProperties credential,
@@ -64,9 +93,10 @@ internal AmqpConnectionHandler(
         /// <summary>
         /// Returns true if this connection, its sessions and its sessions' links are all open.
         /// Returns false otherwise.
+        /// Marked virtual for unit testing purposes only.
         /// </summary>
         /// <returns>True if this connection, its sessions and its sessions' links are all open. False otherwise.</returns>
-        internal bool IsOpen => _connection != null
+        internal virtual bool IsOpen => _connection != null
             && _connection.State == AmqpObjectState.Opened
             && _cbsSession != null
             && _cbsSession.IsOpen()
@@ -76,9 +106,10 @@ internal AmqpConnectionHandler(
         /// <summary>
         /// Opens the AMQP connection. This involves creating the needed TCP or Websocket transport and
         /// then opening all the required sessions and links.
+        /// Marked virtual for unit testing purposes only.
         /// </summary>
         /// <param name="cancellationToken">The cancellation token.</param>
-        internal async Task OpenAsync(CancellationToken cancellationToken)
+        internal virtual async Task OpenAsync(CancellationToken cancellationToken)
         {
             if (Logging.IsEnabled)
                 Logging.Enter(this, "Opening amqp connection.", nameof(OpenAsync));
@@ -185,9 +216,10 @@ internal async Task OpenAsync(CancellationToken cancellationToken)
 
         /// <summary>
         /// Closes the AMQP connection. This closes all the open links and sessions prior to closing the connection.
+        /// Marked virtual for unit testing purposes only.
         /// </summary>
         /// <param name="cancellationToken">The cancellation token.</param>
-        internal async Task CloseAsync(CancellationToken cancellationToken)
+        internal virtual async Task CloseAsync(CancellationToken cancellationToken)
         {
             if (Logging.IsEnabled)
                 Logging.Enter(this, "Closing amqp connection.", nameof(CloseAsync));
@@ -233,7 +265,7 @@ internal async Task CloseAsync(CancellationToken cancellationToken)
         /// </summary>
         /// <param name="message">The message to send.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
-        internal async Task<Outcome> SendAsync(AmqpMessage message, CancellationToken cancellationToken)
+        internal virtual async Task<Outcome> SendAsync(AmqpMessage message, CancellationToken cancellationToken)
         {
             ArraySegment<byte> deliveryTag = GetNextDeliveryTag();
             return await _workerSession.SendAsync(message, deliveryTag, cancellationToken).ConfigureAwait(false);

diff --git a/iothub/service/src/Amqp/AmqpSessionHandler.cs b/iothub/service/src/Amqp/AmqpSessionHandler.cs
@@ -13,7 +13,7 @@ namespace Microsoft.Azure.Devices.Amqp
     /// Handles a single AMQP session that holds the sender or receiver link that does the "work"
     /// for the AMQP connection (receiving file upload notification, sending cloud to device messages, etc).
     /// </summary>
-    internal sealed class AmqpSessionHandler
+    internal class AmqpSessionHandler
     {
         private readonly AmqpSendingLinkHandler _sendingLinkHandler;
         private readonly AmqpReceivingLinkHandler _receivingLinkHandler;
@@ -22,6 +22,12 @@ internal sealed class AmqpSessionHandler
 
         private AmqpSession _session;
 
+        /// <summary>
+        /// Creates an instance of this class. Provided for unit testing purposes only.
+        /// </summary>
+        protected internal AmqpSessionHandler()
+        { }
+
         /// <summary>
         /// Construct an AMQP session for handling sending cloud to device messaging, receiving file
         /// upload notifications or receiving feedback messages.
@@ -74,10 +80,11 @@ internal bool IsOpen
 
         /// <summary>
         /// Opens the session and then opens the worker link.
+        /// Marked virtual for unit testing purposes only.
         /// </summary>
         /// <param name="connection">The connection to open this session on.</param>
         /// <param name="cancellationToken">The timeout for the open operation.</param>
-        internal async Task OpenAsync(AmqpConnection connection, CancellationToken cancellationToken)
+        internal virtual async Task OpenAsync(AmqpConnection connection, CancellationToken cancellationToken)
         {
             if (Logging.IsEnabled)
                 Logging.Enter(this, "Opening worker session.", nameof(OpenAsync));
@@ -146,11 +153,12 @@ internal async Task CloseAsync(CancellationToken cancellationToken)
 
         /// <summary>
         /// Sends the cloud to device message via the worker link.
+        /// Marked virtual for unit testing purposes only.
         /// </summary>
         /// <param name="message">The message to send.</param>
         /// <param name="deliveryTag">The message delivery tag. Used for correlating messages and acknowledgements.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
-        internal async Task<Outcome> SendAsync(AmqpMessage message, ArraySegment<byte> deliveryTag, CancellationToken cancellationToken)
+        internal virtual async Task<Outcome> SendAsync(AmqpMessage message, ArraySegment<byte> deliveryTag, CancellationToken cancellationToken)
         {
             if (_sendingLinkHandler == null)
             {

diff --git a/iothub/service/src/Authentication/IotHubConnectionProperties.cs b/iothub/service/src/Authentication/IotHubConnectionProperties.cs
@@ -49,9 +49,14 @@ internal static string GetIotHubName(string hostName)
             }
 
             int index = hostName.IndexOf(IotHubConnectionStringConstants.HostNameSeparator, StringComparison.OrdinalIgnoreCase);
-            string iotHubName = index >= 0
-                ? hostName.Substring(0, index)
-                : hostName;
+
+            // throw if hostname is invalid format
+            if (index < 0)
+            {
+                throw new ArgumentException("Invalid host name format. Host names should be delimited by periods. E.g, \"IOTHUB_NAME.azure-devices.net\" for public endpoints.");
+            }
+
+            string iotHubName = hostName.Substring(0, index);
             return iotHubName;
         }
     }

diff --git a/iothub/service/src/Authentication/IotHubTokenCredentialProperties.cs b/iothub/service/src/Authentication/IotHubTokenCredentialProperties.cs
@@ -12,7 +12,7 @@ namespace Microsoft.Azure.Devices
     /// <summary>
     /// The properties required for authentication to IoT hub using a token credential.
     /// </summary>
-    internal sealed class IotHubTokenCredentialProperties : IotHubConnectionProperties
+    internal class IotHubTokenCredentialProperties : IotHubConnectionProperties
     {
         private const string TokenType = "Bearer";
         private static readonly string[] s_iotHubAadTokenScopes = new string[] { "https://iothubs.azure.net/.default" };
@@ -21,6 +21,13 @@ internal sealed class IotHubTokenCredentialProperties : IotHubConnectionProperti
         private readonly object _tokenLock = new();
         private AccessToken? _cachedAccessToken;
 
+        // Creates an instance of this class. Provided for unit testing purposes only.
+        protected internal IotHubTokenCredentialProperties(string hostName, TokenCredential credential, AccessToken? accessToken) : base(hostName)
+        {
+            _credential = credential;
+            _cachedAccessToken = accessToken;
+        }
+
         public IotHubTokenCredentialProperties(string hostName, TokenCredential credential) : base(hostName)
         {
             _credential = credential;

diff --git a/iothub/service/src/DigitalTwin/DigitalTwinsClient.cs b/iothub/service/src/DigitalTwin/DigitalTwinsClient.cs
@@ -106,10 +106,9 @@ await _internalRetryHandler
                 }
                 throw new IotHubServiceException(ex.Message, HttpStatusCode.RequestTimeout, IotHubServiceErrorCode.Unknown, null, ex);
             }
-            catch (Exception ex)
+            catch (Exception ex) when (Logging.IsEnabled)
             {
-                if (Logging.IsEnabled)
-                    Logging.Error(this, $"Getting digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(GetAsync));
+                Logging.Error(this, $"Getting digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(GetAsync));
                 throw;
             }
             finally
@@ -206,10 +205,9 @@ await _internalRetryHandler
                 }
                 throw new IotHubServiceException(ex.Message, HttpStatusCode.RequestTimeout, IotHubServiceErrorCode.Unknown, null, ex);
             }
-            catch (Exception ex)
+            catch (Exception ex) when (Logging.IsEnabled)
             {
-                if (Logging.IsEnabled)
-                    Logging.Error(this, $"Updating digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(UpdateAsync));
+                Logging.Error(this, $"Updating digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(UpdateAsync));
                 throw;
             }
             finally
@@ -292,10 +290,9 @@ await _internalRetryHandler
                 }
                 throw new IotHubServiceException(ex.Message, HttpStatusCode.RequestTimeout, IotHubServiceErrorCode.Unknown, null, ex);
             }
-            catch (Exception ex)
+            catch (Exception ex) when (Logging.IsEnabled)
             {
-                if (Logging.IsEnabled)
-                    Logging.Error(this, $"Invoking command on digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(InvokeCommandAsync));
+                Logging.Error(this, $"Invoking command on digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(InvokeCommandAsync));
                 throw;
             }
             finally
@@ -381,10 +378,9 @@ await _internalRetryHandler
                 }
                 throw new IotHubServiceException(ex.Message, HttpStatusCode.RequestTimeout, IotHubServiceErrorCode.Unknown, null, ex);
             }
-            catch (Exception ex)
+            catch (Exception ex) when (Logging.IsEnabled)
             {
-                if (Logging.IsEnabled)
-                    Logging.Error(this, $"Invoking component command on digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(InvokeComponentCommandAsync));
+                Logging.Error(this, $"Invoking component command on digital twin with Id {digitalTwinId} threw an exception: {ex}", nameof(InvokeComponentCommandAsync));
                 throw;
             }
             finally

diff --git a/iothub/service/src/DigitalTwin/Models/InvokeDigitalTwinCommandResponse.cs b/iothub/service/src/DigitalTwin/Models/InvokeDigitalTwinCommandResponse.cs
@@ -9,7 +9,7 @@ namespace Microsoft.Azure.Devices
     public class InvokeDigitalTwinCommandResponse
     {
         /// <summary>
-        /// This constructor is for deserialization and unit test mocking purposes.
+        /// This constructor is for unit test mocking purposes.
         /// </summary>
         /// <remarks>
         /// To unit test methods that use this type as a response, inherit from this class and give it a constructor

diff --git a/iothub/service/src/DigitalTwin/Models/UpdateDigitalTwinOptions.cs b/iothub/service/src/DigitalTwin/Models/UpdateDigitalTwinOptions.cs
@@ -11,7 +11,7 @@ namespace Microsoft.Azure.Devices
     public class UpdateDigitalTwinOptions
     {
         /// <summary>
-        /// A string representing a weak ETag for the entity that this request performs an operation against, as per RFC7232.
+        /// A weak ETag for the entity that this request performs an operation against, as per RFC7232.
         /// </summary>
         /// <remarks>
         /// The request's operation is performed only if this ETag matches the value maintained by the server,

diff --git a/iothub/service/src/Exceptions/IotHubServiceException.cs b/iothub/service/src/Exceptions/IotHubServiceException.cs
@@ -58,6 +58,7 @@ public IotHubServiceException(
         /// </summary>
         /// <param name="info">The <see cref="SerializationInfo"/> that holds the serialized object data about the exception being thrown.</param>
         /// <param name="context">The <see cref="StreamingContext"/> that contains contextual information about the source or destination.</param>
+        /// <exception cref="ArgumentNullException">When the provided <paramref name="info"/> is null.</exception>
         protected IotHubServiceException(SerializationInfo info, StreamingContext context)
             : base(info, context)
         {
@@ -95,6 +96,7 @@ protected IotHubServiceException(SerializationInfo info, StreamingContext contex
         /// </summary>
         /// <param name="info">The <see cref="SerializationInfo"/> that holds the serialized object data about the exception being thrown.</param>
         /// <param name="context">The <see cref="StreamingContext"/> that contains contextual information about the source or destination.</param>
+        /// <exception cref="ArgumentNullException">When the provided <paramref name="info"/> is null.</exception>
         public override void GetObjectData(SerializationInfo info, StreamingContext context)
         {
             base.GetObjectData(info, context);

diff --git a/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs b/iothub/service/src/Feedback/MessageFeedbackProcessorClient.cs
@@ -29,6 +29,22 @@ public class MessageFeedbackProcessorClient : IDisposable
         protected MessageFeedbackProcessorClient()
         { }
 
+        /// <summary>
+        /// Creates an instance of this class. Provided for unit testing purposes only.
+        /// </summary>
+        internal MessageFeedbackProcessorClient(
+            string hostName,
+            IotHubConnectionProperties credentialProvider,
+            IotHubServiceClientOptions options,
+            RetryHandler retryHandler,
+            AmqpConnectionHandler amqpConnection)
+        {
+            _hostName = hostName;
+            _credentialProvider = credentialProvider;
+            _internalRetryHandler = retryHandler;
+            _amqpConnection = amqpConnection;
+        }
+
         internal MessageFeedbackProcessorClient(
             string hostName,
             IotHubConnectionProperties credentialProvider,

diff --git a/iothub/service/src/Feedback/Models/FeedbackBatch.cs b/iothub/service/src/Feedback/Models/FeedbackBatch.cs
@@ -15,7 +15,7 @@ namespace Microsoft.Azure.Devices
     public class FeedbackBatch
     {
         /// <summary>
-        /// This constructor is for deserialization and unit test mocking purposes.
+        /// This constructor is for unit test mocking purposes.
         /// </summary>
         /// <remarks>
         /// To unit test methods that use this type as a response, inherit from this class and give it a constructor

diff --git a/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs b/iothub/service/src/FileUpload/FileUploadNotificationProcessorClient.cs
@@ -27,6 +27,21 @@ public class FileUploadNotificationProcessorClient : IDisposable
         protected FileUploadNotificationProcessorClient()
         { }
 
+        /// <summary>
+        /// Creates an instance of this class. Provided for unit testing purposes only.
+        /// </summary>
+        internal FileUploadNotificationProcessorClient(
+            string hostName,
+            IotHubConnectionProperties credentialProvider,
+            RetryHandler retryHandler,
+            AmqpConnectionHandler amqpConnection)
+        {
+            _hostName = hostName;
+            _credentialProvider = credentialProvider;
+            _internalRetryHandler = retryHandler;
+            _amqpConnection = amqpConnection;
+        }
+
         internal FileUploadNotificationProcessorClient(
             string hostName,
             IotHubConnectionProperties credentialProvider,

diff --git a/iothub/service/src/FileUpload/Models/FileUploadNotification.cs b/iothub/service/src/FileUpload/Models/FileUploadNotification.cs
@@ -30,9 +30,8 @@ protected internal FileUploadNotification()
         /// <summary>
         /// URI path of the uploaded file.
         /// </summary>
-        // TODO: consider changing this to System.Uri before GA
         [JsonProperty("blobUri")]
-        public string BlobUriPath { get; protected internal set; }
+        public Uri BlobUriPath { get; protected internal set; }
 
         /// <summary>
         /// Name of the uploaded file.