From 6273de309e7375acab5acbe99080f883ff757d35 Mon Sep 17 00:00:00 2001 From: Allen Zhang Date: Wed, 3 Mar 2021 00:00:23 -0800 Subject: [PATCH 1/2] initial checkin --- .../src/ArmResponse.cs | 6 +++ .../src/ArmVoidOperation.cs | 3 ++ .../src/AzureResourceManagerClient.cs | 40 ++++++++++++------- .../src/AzureResourceManagerClientOptions.cs | 36 ++++++++--------- .../src/Resources/ResourceType.cs | 2 +- .../src/Resources/ResourceTypeFilter.cs | 4 ++ .../src/Resources/Sku.cs | 2 +- 7 files changed, 57 insertions(+), 36 deletions(-) diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmResponse.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmResponse.cs index d130fc818631..c9f4e005822d 100644 --- 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 { /// @@ -14,8 +16,12 @@ public sealed class ArmResponse : ArmResponse /// Initializes a new instance of the class. /// /// The azure response object to wrap. + /// If is null. 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 index bbab26ea2456..68e3bca5771c 100644 --- 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 public ArmVoidOperation(Operation other) : base(null) { + if (other is null) + throw new ArgumentNullException(nameof(other)); + _wrapped = other; } diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClient.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClient.cs index e5db0c8bee2e..2772642ffc63 100644 --- 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 class for mocking. /// protected AzureResourceManagerClient() - : this(null, null, new DefaultAzureCredential(), new AzureResourceManagerClientOptions()) + : this(null, null, new DefaultAzureCredential(), null) { } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class + /// with as credential. /// /// The client parameters to use in these operations. public AzureResourceManagerClient(AzureResourceManagerClientOptions options = default) @@ -46,6 +47,7 @@ public AzureResourceManagerClient(AzureResourceManagerClientOptions options = de /// /// A credential used to authenticate to an Azure Service. /// The client parameters to use in these operations. + /// If is null. public AzureResourceManagerClient(TokenCredential credential, AzureResourceManagerClientOptions options = default) : this(null, null, credential, options) { @@ -57,7 +59,11 @@ public AzureResourceManagerClient(TokenCredential credential, AzureResourceManag /// The id of the default Azure subscription. /// A credential used to authenticate to an Azure Service. /// The client parameters to use in these operations. - public AzureResourceManagerClient(string defaultSubscriptionId, TokenCredential credential, AzureResourceManagerClientOptions options = default) + /// If is null. + public AzureResourceManagerClient( + string defaultSubscriptionId, + TokenCredential credential, + AzureResourceManagerClientOptions options = default) : this(defaultSubscriptionId, null, credential, options) { } @@ -65,10 +71,14 @@ public AzureResourceManagerClient(string defaultSubscriptionId, TokenCredential /// /// Initializes a new instance of the class. /// - /// The base URI of the service. + /// The base URI of the Azure management endpoint. /// A credential used to authenticate to an Azure Service. /// The client parameters to use in these operations. - public AzureResourceManagerClient(Uri baseUri, TokenCredential credential, AzureResourceManagerClientOptions options = default) + /// If is null. + 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 /// The base URI of the service. /// A credential used to authenticate to an Azure Service. /// The client parameters to use in these operations. - 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(); } diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClientOptions.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/AzureResourceManagerClientOptions.cs index 2b6e0a8a2753..ba82c50013e5 100644 --- 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 /// public sealed class AzureResourceManagerClientOptions : ClientOptions { - private static readonly object _overridesLock = new object(); - - private Dictionary _overrides = new Dictionary(); + private readonly ConcurrentDictionary _overrides = new ConcurrentDictionary(); /// /// Initializes a new instance of the class. @@ -40,8 +39,12 @@ public AzureResourceManagerClientOptions(LocationData defaultLocation) /// /// The default location to use if can't be inherited from parent. /// The client parameters to use in these operations. + /// If is null. 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() /// /// The http call policy in the pipeline. /// The position of the http call policy in the pipeline. + /// If is null. 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() /// Gets override object. /// /// The type of the underlying model this class wraps. - /// A function which returns an object. + /// A function used to construct a new object if none was found. /// The override object. [EditorBrowsable(EditorBrowsableState.Never)] - public object GetOverrideObject(Func ctor) + public object GetOverrideObject(Func 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 index b4dc44088dfe..b5fad5e15de4 100644 --- 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 { /// - /// Structure respresenting a resource type + /// Structure representing a resource type /// public sealed class ResourceType : IEquatable, IEquatable, IComparable, IComparable diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/ResourceTypeFilter.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/Resources/ResourceTypeFilter.cs index 099584c5d399..33e43b366342 100644 --- 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 class. /// /// The resource type to filter by. + /// If is null. 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 index 25008bfb6f51..79da9befc1c5 100644 --- 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 { /// - /// Representaion of ARM SKU + /// A class representing SKU for resource /// public sealed class Sku : IEquatable, IComparable { From fcf9b35541a46747a07570ed1a1800ffa0780054 Mon Sep 17 00:00:00 2001 From: Allen Zhang Date: Wed, 3 Mar 2021 10:16:26 -0800 Subject: [PATCH 2/2] adding test cases --- .../src/ArmVoidOperation.cs | 2 ++ .../tests/ArmClientOptionsTests.cs | 14 +++++++++++++- .../tests/ArmClientTests.cs | 11 ++++++++++- .../tests/ArmResponseTests.cs | 18 ++++++++++++++++++ .../tests/ArmVoidOperationTests.cs | 18 ++++++++++++++++++ .../tests/ResourceTypeFilterTests.cs | 19 +++++++++++++++++++ 6 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmResponseTests.cs create mode 100644 sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmVoidOperationTests.cs create mode 100644 sdk/resourcemanager/Azure.ResourceManager.Core/tests/ResourceTypeFilterTests.cs diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmVoidOperation.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmVoidOperation.cs index 68e3bca5771c..f0d24e3e714e 100644 --- a/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmVoidOperation.cs +++ b/sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmVoidOperation.cs @@ -34,6 +34,8 @@ public ArmVoidOperation(Operation other) public ArmVoidOperation(Response other) : base(other) { + if (other is null) + throw new ArgumentNullException(nameof(other)); } /// diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmClientOptionsTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmClientOptionsTests.cs index 7ce6a32f5a40..dc8230e914f4 100644 --- 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(() => { new AzureResourceManagerClientOptions(null); }); + Assert.Throws(() => { new AzureResourceManagerClientOptions(null, null); }); + + var options = new AzureResourceManagerClientOptions(); + Assert.Throws(() => { 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 index 04b52e63ba37..12b3f925e6b4 100644 --- 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 CreateResource(string subscription, string resourceGroup, string name, TResource model, azure_proto_core.Location location = default) Assert.Ignore(); } + + [TestCase] + public void TestArmClientParamCheck() + { + Assert.Throws(() => { new AzureResourceManagerClient(null, null); }); + Assert.Throws(() => { new AzureResourceManagerClient(baseUri:null, null, null); }); + Assert.Throws(() => { 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 new file mode 100644 index 000000000000..cc1f44728f9d --- /dev/null +++ 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(() => { new ArmResponse(null); }); + + } + } +} diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmVoidOperationTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ArmVoidOperationTests.cs new file mode 100644 index 000000000000..e73427f50b57 --- /dev/null +++ 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(() => { new ResourceTypeFilter(null); }); + } + } +} diff --git a/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ResourceTypeFilterTests.cs b/sdk/resourcemanager/Azure.ResourceManager.Core/tests/ResourceTypeFilterTests.cs new file mode 100644 index 000000000000..9b2fc4af4be4 --- /dev/null +++ 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(() => { new ArmVoidOperation((Operation)null); }); + Assert.Throws(() => { new ArmVoidOperation((Response)null); }); + } + } +}