Adding parameter check for track 2 Azure.ResourceManager.core (#19162)

* initial checkin * adding test cases
Azure · Mar 3, 2021 · b066d3f · b066d3f
1 parent dafcd2b
commit b066d3f
Show file tree

Hide file tree

Showing 12 changed files with 137 additions and 38 deletions.
diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmResponse.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmResponse.cs
@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
+
 namespace Azure.ResourceManager.Core
 {
     /// <summary>
@@ -14,8 +16,12 @@ public sealed class ArmResponse : ArmResponse<Response>
         /// Initializes a new instance of the <see cref="ArmResponse"/> class.
         /// </summary>
         /// <param name="response"> The azure response object to wrap. </param>
+        /// <exception cref="ArgumentNullException"> If <see cref="Response"/> is null. </exception>
         public ArmResponse(Response response)
         {
+            if (response is null)
+                throw new ArgumentNullException(nameof(response));
+
             _response = response;
         }
 

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmVoidOperation.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmVoidOperation.cs
@@ -21,6 +21,9 @@ public sealed class ArmVoidOperation : ArmOperation<Response>
         public ArmVoidOperation(Operation<Response> other)
             : base(null)
         {
+            if (other is null)
+                throw new ArgumentNullException(nameof(other));
+
             _wrapped = other;
         }
 
@@ -31,6 +34,8 @@ public ArmVoidOperation(Operation<Response> other)
         public ArmVoidOperation(Response other)
             : base(other)
         {
+            if (other is null)
+                throw new ArgumentNullException(nameof(other));
         }
 
         /// <inheritdoc/>

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClient.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClient.cs
@@ -28,12 +28,13 @@ public class AzureResourceManagerClient
         /// Initializes a new instance of the <see cref="AzureResourceManagerClient"/> class for mocking.
         /// </summary>
         protected AzureResourceManagerClient()
-            : this(null, null, new DefaultAzureCredential(), new AzureResourceManagerClientOptions())
+            : this(null, null, new DefaultAzureCredential(), null)
         {
         }
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="AzureResourceManagerClient"/> class.
+        /// Initializes a new instance of the <see cref="AzureResourceManagerClient"/> class
+        /// with <see cref="DefaultAzureCredential"/> as credential.
         /// </summary>
         /// <param name="options"> The client parameters to use in these operations. </param>
         public AzureResourceManagerClient(AzureResourceManagerClientOptions options = default)
@@ -46,6 +47,7 @@ public AzureResourceManagerClient(AzureResourceManagerClientOptions options = de
         /// </summary>
         /// <param name="credential"> A credential used to authenticate to an Azure Service. </param>
         /// <param name="options"> The client parameters to use in these operations. </param>
+        /// <exception cref="ArgumentNullException"> If <see cref="TokenCredential"/> is null. </exception>
         public AzureResourceManagerClient(TokenCredential credential, AzureResourceManagerClientOptions options = default)
             : this(null, null, credential, options)
         {
@@ -57,18 +59,26 @@ public AzureResourceManagerClient(TokenCredential credential, AzureResourceManag
         /// <param name="defaultSubscriptionId"> The id of the default Azure subscription. </param>
         /// <param name="credential"> A credential used to authenticate to an Azure Service. </param>
         /// <param name="options"> The client parameters to use in these operations. </param>
-        public AzureResourceManagerClient(string defaultSubscriptionId, TokenCredential credential, AzureResourceManagerClientOptions options = default)
+        /// <exception cref="ArgumentNullException"> If <see cref="TokenCredential"/> is null. </exception> 
+        public AzureResourceManagerClient(
+            string defaultSubscriptionId,
+            TokenCredential credential,
+            AzureResourceManagerClientOptions options = default)
             : this(defaultSubscriptionId, null, credential, options)
         {
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="AzureResourceManagerClient"/> class.
         /// </summary>
-        /// <param name="baseUri"> The base URI of the service. </param>
+        /// <param name="baseUri"> The base URI of the Azure management endpoint. </param>
         /// <param name="credential"> A credential used to authenticate to an Azure Service. </param>
         /// <param name="options"> The client parameters to use in these operations. </param>
-        public AzureResourceManagerClient(Uri baseUri, TokenCredential credential, AzureResourceManagerClientOptions options = default)
+        /// <exception cref="ArgumentNullException"> If <see cref="TokenCredential"/> is null. </exception>
+        public AzureResourceManagerClient(
+            Uri baseUri,
+            TokenCredential credential,
+            AzureResourceManagerClientOptions options = default)
             : this(null, baseUri, credential, options)
         {
         }
@@ -80,20 +90,22 @@ public AzureResourceManagerClient(Uri baseUri, TokenCredential credential, Azure
         /// <param name="baseUri"> The base URI of the service. </param>
         /// <param name="credential"> A credential used to authenticate to an Azure Service. </param>
         /// <param name="options"> The client parameters to use in these operations. </param>
-        private AzureResourceManagerClient(string defaultSubscriptionId, Uri baseUri, TokenCredential credential, AzureResourceManagerClientOptions options = default)
+        private AzureResourceManagerClient(
+            string defaultSubscriptionId,
+            Uri baseUri,
+            TokenCredential credential,
+            AzureResourceManagerClientOptions options)
         {
+            if (credential is null)
+                throw new ArgumentNullException(nameof(credential));
+
             _credentials = credential;
             _baseUri = baseUri;
             ClientOptions = options ?? new AzureResourceManagerClientOptions();
 
-            if (string.IsNullOrWhiteSpace(defaultSubscriptionId))
-            {
-                DefaultSubscription = GetDefaultSubscription();
-            }
-            else
-            {
-                DefaultSubscription = GetSubscriptionOperations(defaultSubscriptionId).Get().Value;
-            }
+            DefaultSubscription = string.IsNullOrWhiteSpace(defaultSubscriptionId) 
+                ? GetDefaultSubscription()
+                : GetSubscriptionOperations(defaultSubscriptionId).Get().Value;
 
             ApiVersionOverrides = new Dictionary<string, string>();
         }

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClientOptions.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClientOptions.cs
@@ -4,6 +4,7 @@
 using Azure.Core;
 using Azure.Core.Pipeline;
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.ComponentModel;
 
@@ -14,9 +15,7 @@ namespace Azure.ResourceManager.Core
     /// </summary>
     public sealed class AzureResourceManagerClientOptions : ClientOptions
     {
-        private static readonly object _overridesLock = new object();
-
-        private Dictionary<Type, object> _overrides = new Dictionary<Type, object>();
+        private readonly ConcurrentDictionary<Type, object> _overrides = new ConcurrentDictionary<Type, object>();
 
         /// <summary>
         /// Initializes a new instance of the <see cref="AzureResourceManagerClientOptions"/> class.
@@ -40,8 +39,12 @@ public AzureResourceManagerClientOptions(LocationData defaultLocation)
         /// </summary>
         /// <param name="defaultLocation"> The default location to use if can't be inherited from parent. </param>
         /// <param name="other"> The client parameters to use in these operations. </param>
+        /// <exception cref="ArgumentNullException"> If <see cref="LocationData"/> is null. </exception>
         internal AzureResourceManagerClientOptions(LocationData defaultLocation, AzureResourceManagerClientOptions other = null)
         {
+            if (defaultLocation is null)
+                throw new ArgumentNullException(nameof(defaultLocation));
+
             // Will go away when moved into core since we will have directy acces the policies and transport, so just need to set those
             if (!ReferenceEquals(other, null))
                 Copy(other);
@@ -93,9 +96,13 @@ public T Convert<T>()
         /// </summary>
         /// <param name="policy"> The http call policy in the pipeline. </param>
         /// <param name="position"> The position of the http call policy in the pipeline. </param>
+        /// <exception cref="ArgumentNullException"> If <see cref="HttpPipelinePolicy"/> is null. </exception>
         public new void AddPolicy(HttpPipelinePolicy policy, HttpPipelinePosition position)
         {
-            // TODO policy lists are internal hence we don't have acces to them by inheriting ClientOptions in this Asembly, this is a wrapper for now to convert to the concrete
+            if (policy is null)
+                throw new ArgumentNullException(nameof(policy));
+
+            // TODO policy lists are internal hence we don't have access to them by inheriting ClientOptions in this Assembly, this is a wrapper for now to convert to the concrete
             // policy options.
             switch (position)
             {
@@ -116,26 +123,15 @@ public T Convert<T>()
         /// Gets override object.
         /// </summary>
         /// <typeparam name="T"> The type of the underlying model this class wraps. </typeparam>
-        /// <param name="ctor"> A function which returns an object. </param>
+        /// <param name="objectConstructor"> A function used to construct a new object if none was found. </param>
         /// <returns> The override object. </returns>
         [EditorBrowsable(EditorBrowsableState.Never)]
-        public object GetOverrideObject<T>(Func<object> ctor)
+        public object GetOverrideObject<T>(Func<object> objectConstructor)
         {
-            object overrideObject;
-            Type type = typeof(T);
-            if (!_overrides.TryGetValue(type, out overrideObject))
-            {
-                lock (_overridesLock)
-                {
-                    if (!_overrides.TryGetValue(type, out overrideObject))
-                    {
-                        overrideObject = ctor();
-                        _overrides[type] = overrideObject;
-                    }
-                }
-            }
+            if (objectConstructor is null)
+                throw new ArgumentNullException(nameof(objectConstructor));
 
-            return overrideObject;
+            return _overrides.GetOrAdd(typeof(T), objectConstructor());
         }
 
         // Will be removed like AddPolicy when we move to azure core

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/ResourceType.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/ResourceType.cs
@@ -8,7 +8,7 @@
 namespace Azure.ResourceManager.Core
 {
     /// <summary>
-    /// Structure respresenting a resource type
+    /// Structure representing a resource type
     /// </summary>
     public sealed class ResourceType : IEquatable<ResourceType>, IEquatable<string>, IComparable<ResourceType>,
         IComparable<string>

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/ResourceTypeFilter.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/ResourceTypeFilter.cs
@@ -14,8 +14,12 @@ public class ResourceTypeFilter : GenericResourceFilter, IEquatable<ResourceType
         /// Initializes a new instance of the <see cref="ResourceTypeFilter"/> class.
         /// </summary>
         /// <param name="resourceType"> The resource type to filter by. </param>
+        /// <exception cref="ArgumentNullException"> If <see cref="ResourceType"/> is null. </exception>
         public ResourceTypeFilter(ResourceType resourceType)
         {
+            if (resourceType is null)
+                throw new ArgumentNullException(nameof(resourceType));
+
             ResourceType = resourceType;
         }
 

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/Sku.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/Sku.cs
@@ -7,7 +7,7 @@
 namespace Azure.ResourceManager.Core
 {
     /// <summary>
-    /// Representaion of ARM SKU
+    /// A class representing SKU for resource
     /// </summary>
     public sealed class Sku : IEquatable<Sku>, IComparable<Sku>
     {

diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmClientOptionsTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmClientOptionsTests.cs
@@ -1,4 +1,6 @@
-using NUnit.Framework;
+using System;
+using Azure.Core;
+using NUnit.Framework;
 
 namespace Azure.ResourceManager.Core.Tests
 {
@@ -21,5 +23,15 @@ public void MultiClientSeparateVersions()
             Assert.AreEqual(FakeResourceApiVersions.Default, options1.FakeRpApiVersions().FakeResourceVersion);
             Assert.AreEqual(FakeResourceApiVersions.V2019_12_01, options2.FakeRpApiVersions().FakeResourceVersion);
         }
+
+        [TestCase]
+        public void TestClientOptionsParamCheck()
+        {
+            Assert.Throws<ArgumentNullException>(() => { new AzureResourceManagerClientOptions(null); });
+            Assert.Throws<ArgumentNullException>(() => { new AzureResourceManagerClientOptions(null, null); });
+
+            var options = new AzureResourceManagerClientOptions();
+            Assert.Throws<ArgumentNullException>(() => { options.AddPolicy(null, HttpPipelinePosition.PerCall); });
+        }
     }
 }
diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmClientTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmClientTests.cs
@@ -1,4 +1,5 @@
-using NUnit.Framework;
+using System;
+using NUnit.Framework;
 
 namespace Azure.ResourceManager.Core.Tests
 {
@@ -11,5 +12,13 @@ public void CreateResourceFromId()
             //public ArmResponse<TOperations> CreateResource<TContainer, TOperations, TResource>(string subscription, string resourceGroup, string name, TResource model, azure_proto_core.Location location = default)
             Assert.Ignore();
         }
+
+        [TestCase]
+        public void TestArmClientParamCheck()
+        {
+            Assert.Throws<ArgumentNullException>(() => { new AzureResourceManagerClient(null, null); });
+            Assert.Throws<ArgumentNullException>(() => { new AzureResourceManagerClient(baseUri:null, null, null); });
+            Assert.Throws<ArgumentNullException>(() => { new AzureResourceManagerClient(defaultSubscriptionId: null, null, null); });
+        }
     }
 }
diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmResponseTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmResponseTests.cs
@@ -0,0 +1,18 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using NUnit.Framework;
+
+namespace Azure.ResourceManager.Core.Tests
+{
+    public class ArmResponseTests
+    {
+        [TestCase]
+        public void TestArmResponseParamCheck()
+        {
+            Assert.Throws<ArgumentNullException>(() => { new ArmResponse(null); });
+
+        }
+    }
+}
diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmVoidOperationTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmVoidOperationTests.cs
@@ -0,0 +1,18 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using Azure.ResourceManager.Core.Resources;
+using NUnit.Framework;
+using System;
+
+namespace Azure.ResourceManager.Core.Tests
+{
+    public class ArmVoidOperationTests
+    {
+        [TestCase]
+        public void TestArmVoidOperationParamCheck()
+        {
+            Assert.Throws<ArgumentNullException>(() => { new ResourceTypeFilter(null); });
+        }
+    }
+}
diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ResourceTypeFilterTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ResourceTypeFilterTests.cs
@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using Azure.ResourceManager.Core.Resources;
+using NUnit.Framework;
+using System;
+
+namespace Azure.ResourceManager.Core.Tests
+{
+    public class ResourceTypeFilterTests
+    {
+        [TestCase]
+        public void TestResourceTypeFilterParamCheck()
+        {
+            Assert.Throws<ArgumentNullException>(() => { new ArmVoidOperation((Operation<Response>)null); });
+            Assert.Throws<ArgumentNullException>(() => { new ArmVoidOperation((Response)null); });
+        }
+    }
+}