From cd61160d85a86c8a3613863316bd8ae64c89b08c Mon Sep 17 00:00:00 2001 From: Reza Jooyandeh Date: Fri, 5 Feb 2021 10:58:50 -0800 Subject: [PATCH] [Communication] - Ensure RestClients are created based on ClientOptions.ApiVersion (#18464) --- .../src/CommunicationIdentityClient.cs | 85 +++++++++--------- ...mmunication.PhoneNumbers.netstandard2.0.cs | 3 +- .../src/PhoneNumberAdministrationClient.cs | 84 ++++++++++-------- .../Azure.Communication.Sms/src/SmsClient.cs | 86 +++++++++---------- 4 files changed, 133 insertions(+), 125 deletions(-) diff --git a/sdk/communication/Azure.Communication.Identity/src/CommunicationIdentityClient.cs b/sdk/communication/Azure.Communication.Identity/src/CommunicationIdentityClient.cs index e3d1667ac106b..4a1976638997e 100644 --- a/sdk/communication/Azure.Communication.Identity/src/CommunicationIdentityClient.cs +++ b/sdk/communication/Azure.Communication.Identity/src/CommunicationIdentityClient.cs @@ -20,23 +20,14 @@ public class CommunicationIdentityClient private readonly ClientDiagnostics _clientDiagnostics; internal CommunicationIdentityRestClient RestClient { get; } - /// Initializes a new instance of . - /// The URI of the Azure Communication Services resource. - /// The used to authenticate requests. - /// Client option exposing , , , etc. - public CommunicationIdentityClient(Uri endpoint, AzureKeyCredential keyCredential, CommunicationIdentityClientOptions options = default) - : this( - AssertNotNull(endpoint, nameof(endpoint)), - options ?? new CommunicationIdentityClientOptions(), - AssertNotNull(keyCredential, nameof(keyCredential))) - { } + #region public constructors - all argument need null check /// Initializes a new instance of . /// Connection string acquired from the Azure Communication Services resource. public CommunicationIdentityClient(string connectionString) : this( - new CommunicationIdentityClientOptions(), - ConnectionString.Parse(AssertNotNull(connectionString, nameof(connectionString)))) + ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString))), + new CommunicationIdentityClientOptions()) { } /// Initializes a new instance of . @@ -44,8 +35,19 @@ public CommunicationIdentityClient(string connectionString) /// Client option exposing , , , etc. public CommunicationIdentityClient(string connectionString, CommunicationIdentityClientOptions options) : this( - options ?? new CommunicationIdentityClientOptions(), - ConnectionString.Parse(AssertNotNull(connectionString, nameof(connectionString)))) + ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString))), + options ?? new CommunicationIdentityClientOptions()) + { } + + /// Initializes a new instance of . + /// The URI of the Azure Communication Services resource. + /// The used to authenticate requests. + /// Client option exposing , , , etc. + public CommunicationIdentityClient(Uri endpoint, AzureKeyCredential keyCredential, CommunicationIdentityClientOptions options = default) + : this( + AssertNotNull(endpoint, nameof(endpoint)).AbsoluteUri, + AssertNotNull(keyCredential, nameof(keyCredential)), + options ?? new CommunicationIdentityClientOptions()) { } /// Initializes a new instance of . @@ -54,43 +56,40 @@ public CommunicationIdentityClient(string connectionString, CommunicationIdentit /// Client option exposing , , , etc. public CommunicationIdentityClient(Uri endpoint, TokenCredential tokenCredential, CommunicationIdentityClientOptions options = default) : this( - AssertNotNull(endpoint, nameof(endpoint)), - options ?? new CommunicationIdentityClientOptions(), - AssertNotNull(tokenCredential, nameof(tokenCredential))) + AssertNotNull(endpoint, nameof(endpoint)).AbsoluteUri, + AssertNotNull(tokenCredential, nameof(tokenCredential)), + options ?? new CommunicationIdentityClientOptions()) { } - private CommunicationIdentityClient(CommunicationIdentityClientOptions options, ConnectionString connectionString) - { - _clientDiagnostics = new ClientDiagnostics(options); - RestClient = new CommunicationIdentityRestClient( - _clientDiagnostics, - options.BuildHttpPipeline(connectionString), - connectionString.GetRequired("endpoint")); - } + #endregion - private CommunicationIdentityClient(Uri endpoint, CommunicationIdentityClientOptions options, AzureKeyCredential credential) - { - _clientDiagnostics = new ClientDiagnostics(options); - RestClient = new CommunicationIdentityRestClient( - _clientDiagnostics, - options.BuildHttpPipeline(credential), - endpoint.AbsoluteUri); - } + #region private constructors + + private CommunicationIdentityClient(ConnectionString connectionString, CommunicationIdentityClientOptions options) + : this(connectionString.GetRequired("endpoint"), options.BuildHttpPipeline(connectionString), options) + { } + + private CommunicationIdentityClient(string endpoint, AzureKeyCredential keyCredential, CommunicationIdentityClientOptions options) + : this(endpoint, options.BuildHttpPipeline(keyCredential), options) + { } + + private CommunicationIdentityClient(string endpoint, TokenCredential tokenCredential, CommunicationIdentityClientOptions options) + : this(endpoint, options.BuildHttpPipeline(tokenCredential), options) + { } - private CommunicationIdentityClient(Uri endpoint, CommunicationIdentityClientOptions options, TokenCredential tokenCredential) + private CommunicationIdentityClient(string endpoint, HttpPipeline httpPipeline, CommunicationIdentityClientOptions options) { _clientDiagnostics = new ClientDiagnostics(options); - RestClient = new CommunicationIdentityRestClient( - _clientDiagnostics, - options.BuildHttpPipeline(tokenCredential), - endpoint.AbsoluteUri); + RestClient = new CommunicationIdentityRestClient(_clientDiagnostics, httpPipeline, endpoint, options.ApiVersion); } + #endregion + /// Initializes a new instance of for mocking. protected CommunicationIdentityClient() { - _clientDiagnostics = null!; - RestClient = null!; + _clientDiagnostics = null; + RestClient = null; } /// Creates a new . @@ -296,5 +295,11 @@ private static T AssertNotNull(T argument, string argumentName) Argument.AssertNotNull(argument, argumentName); return argument; } + + private static string AssertNotNullOrEmpty(string argument, string argumentName) + { + Argument.AssertNotNullOrEmpty(argument, argumentName); + return argument; + } } } diff --git a/sdk/communication/Azure.Communication.PhoneNumbers/api/Azure.Communication.PhoneNumbers.netstandard2.0.cs b/sdk/communication/Azure.Communication.PhoneNumbers/api/Azure.Communication.PhoneNumbers.netstandard2.0.cs index c77d51511b9a7..4341f376067b4 100644 --- a/sdk/communication/Azure.Communication.PhoneNumbers/api/Azure.Communication.PhoneNumbers.netstandard2.0.cs +++ b/sdk/communication/Azure.Communication.PhoneNumbers/api/Azure.Communication.PhoneNumbers.netstandard2.0.cs @@ -3,7 +3,8 @@ namespace Azure.Communication.PhoneNumbers public partial class PhoneNumberAdministrationClient { protected PhoneNumberAdministrationClient() { } - public PhoneNumberAdministrationClient(string connectionString, Azure.Communication.PhoneNumbers.PhoneNumberAdministrationClientOptions? options = null) { } + public PhoneNumberAdministrationClient(string connectionString) { } + public PhoneNumberAdministrationClient(string connectionString, Azure.Communication.PhoneNumbers.PhoneNumberAdministrationClientOptions options) { } public PhoneNumberAdministrationClient(System.Uri endpoint, Azure.AzureKeyCredential keyCredential, Azure.Communication.PhoneNumbers.PhoneNumberAdministrationClientOptions? options = null) { } public PhoneNumberAdministrationClient(System.Uri endpoint, Azure.Core.TokenCredential tokenCredential, Azure.Communication.PhoneNumbers.PhoneNumberAdministrationClientOptions? options = null) { } public virtual Azure.Response CancelReservation(string reservationId, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } diff --git a/sdk/communication/Azure.Communication.PhoneNumbers/src/PhoneNumberAdministrationClient.cs b/sdk/communication/Azure.Communication.PhoneNumbers/src/PhoneNumberAdministrationClient.cs index f3580359f9256..91418196bf5d3 100644 --- a/sdk/communication/Azure.Communication.PhoneNumbers/src/PhoneNumberAdministrationClient.cs +++ b/sdk/communication/Azure.Communication.PhoneNumbers/src/PhoneNumberAdministrationClient.cs @@ -21,24 +21,35 @@ public class PhoneNumberAdministrationClient internal ClientDiagnostics ClientDiagnostics { get; private set; } internal PhoneNumberAdministrationRestClient RestClient { get; } - /// Initializes a new instance of . - /// The URI of the Azure Communication Services resource. - /// The used to authenticate requests. - /// Client option exposing , , , etc. - public PhoneNumberAdministrationClient(Uri endpoint, AzureKeyCredential keyCredential, PhoneNumberAdministrationClientOptions? options = default) + #region public constructors - all arguments need null check + + /// + /// Initializes a phone number administration client with an Azure resource connection string and client options. + /// + public PhoneNumberAdministrationClient(string connectionString) : this( - AssertNotNull(endpoint, nameof(endpoint)), - options ?? new PhoneNumberAdministrationClientOptions(), - AssertNotNull(keyCredential, nameof(keyCredential))) + ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString))), + new PhoneNumberAdministrationClientOptions()) { } /// /// Initializes a phone number administration client with an Azure resource connection string and client options. /// - public PhoneNumberAdministrationClient(string connectionString, PhoneNumberAdministrationClientOptions? options = default) + public PhoneNumberAdministrationClient(string connectionString, PhoneNumberAdministrationClientOptions options) + : this( + ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString))), + options ?? new PhoneNumberAdministrationClientOptions()) + { } + + /// Initializes a new instance of . + /// The URI of the Azure Communication Services resource. + /// The used to authenticate requests. + /// Client option exposing , , , etc. + public PhoneNumberAdministrationClient(Uri endpoint, AzureKeyCredential keyCredential, PhoneNumberAdministrationClientOptions? options = default) : this( - options ?? new PhoneNumberAdministrationClientOptions(), - ConnectionString.Parse(AssertNotNull(connectionString, nameof(connectionString)))) + AssertNotNull(endpoint, nameof(endpoint)).AbsoluteUri, + AssertNotNull(keyCredential, nameof(keyCredential)), + options ?? new PhoneNumberAdministrationClientOptions()) { } /// @@ -49,42 +60,35 @@ public PhoneNumberAdministrationClient(string connectionString, PhoneNumberAdmin /// public PhoneNumberAdministrationClient(Uri endpoint, TokenCredential tokenCredential, PhoneNumberAdministrationClientOptions? options = default) : this( - endpoint, - options ?? new PhoneNumberAdministrationClientOptions(), - tokenCredential) + AssertNotNull(endpoint, nameof(endpoint)).AbsoluteUri, + AssertNotNull(tokenCredential, nameof(tokenCredential)), + options ?? new PhoneNumberAdministrationClientOptions()) { } - internal PhoneNumberAdministrationClient(PhoneNumberAdministrationClientOptions options, ConnectionString connectionString) - : this(new ClientDiagnostics(options), options.BuildHttpPipeline(connectionString), connectionString.GetRequired("endpoint")) + #endregion + + #region private constructors + + private PhoneNumberAdministrationClient(ConnectionString connectionString, PhoneNumberAdministrationClientOptions options) + : this(connectionString.GetRequired("endpoint"), options.BuildHttpPipeline(connectionString), options) { } - internal PhoneNumberAdministrationClient(ClientDiagnostics clientDiagnostics, HttpPipeline pipeline, string endpointUrl) - { - RestClient = new PhoneNumberAdministrationRestClient(clientDiagnostics, pipeline, endpointUrl); - ClientDiagnostics = clientDiagnostics; - } + private PhoneNumberAdministrationClient(string endpoint, AzureKeyCredential keyCredential, PhoneNumberAdministrationClientOptions options) + : this(endpoint, options.BuildHttpPipeline(keyCredential), options) + { } - internal PhoneNumberAdministrationClient(Uri endpoint, PhoneNumberAdministrationClientOptions options, AzureKeyCredential credential) - { - ClientDiagnostics = new ClientDiagnostics(options); - RestClient = new PhoneNumberAdministrationRestClient( - ClientDiagnostics, - options.BuildHttpPipeline(credential), - endpoint.AbsoluteUri); - } + private PhoneNumberAdministrationClient(string endpoint, TokenCredential tokenCredential, PhoneNumberAdministrationClientOptions options) + : this(endpoint, options.BuildHttpPipeline(tokenCredential), options) + { } - internal PhoneNumberAdministrationClient(Uri endpoint, PhoneNumberAdministrationClientOptions options, TokenCredential tokenCredential) + private PhoneNumberAdministrationClient(string endpoint, HttpPipeline httpPipeline, PhoneNumberAdministrationClientOptions options) { - Argument.AssertNotNull(endpoint, nameof(endpoint)); - Argument.AssertNotNull(tokenCredential, nameof(tokenCredential)); - ClientDiagnostics = new ClientDiagnostics(options); - RestClient = new PhoneNumberAdministrationRestClient( - ClientDiagnostics, - options.BuildHttpPipeline(tokenCredential), - endpoint.AbsoluteUri); + RestClient = new PhoneNumberAdministrationRestClient(ClientDiagnostics, httpPipeline, endpoint, options.ApiVersion); } + #endregion + /// Initializes a new instance of for mocking. protected PhoneNumberAdministrationClient() { @@ -989,5 +993,11 @@ private static T AssertNotNull(T argument, string argumentName) Argument.AssertNotNull(argument, argumentName); return argument; } + + private static string AssertNotNullOrEmpty(string argument, string argumentName) + { + Argument.AssertNotNullOrEmpty(argument, argumentName); + return argument; + } } } diff --git a/sdk/communication/Azure.Communication.Sms/src/SmsClient.cs b/sdk/communication/Azure.Communication.Sms/src/SmsClient.cs index 27d29ada32374..2af4d33951269 100644 --- a/sdk/communication/Azure.Communication.Sms/src/SmsClient.cs +++ b/sdk/communication/Azure.Communication.Sms/src/SmsClient.cs @@ -20,23 +20,14 @@ public class SmsClient private readonly ClientDiagnostics _clientDiagnostics; internal SmsRestClient RestClient { get; } - /// Initializes a new instance of . - /// The URI of the Azure Communication Services resource. - /// The used to authenticate requests. - /// Client option exposing , , , etc. - public SmsClient(Uri endpoint, AzureKeyCredential keyCredential, SmsClientOptions options = default) - : this( - AssertNotNull(endpoint, nameof(endpoint)), - options ?? new SmsClientOptions(), - AssertNotNull(keyCredential, nameof(keyCredential))) - { } + #region public constructors - all arguments need null check /// Initializes a new instance of . /// Connection string acquired from the Azure Communication Services resource. public SmsClient(string connectionString) : this( - new SmsClientOptions(), - ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString)))) + ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString))), + new SmsClientOptions()) { } /// Initializes a new instance of . @@ -44,8 +35,19 @@ public SmsClient(string connectionString) /// Client option exposing , , , etc. public SmsClient(string connectionString, SmsClientOptions options) : this( - options ?? new SmsClientOptions(), - ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString)))) + ConnectionString.Parse(AssertNotNullOrEmpty(connectionString, nameof(connectionString))), + options ?? new SmsClientOptions()) + { } + + /// Initializes a new instance of . + /// The URI of the Azure Communication Services resource. + /// The used to authenticate requests. + /// Client option exposing , , , etc. + public SmsClient(Uri endpoint, AzureKeyCredential keyCredential, SmsClientOptions options = default) + : this( + AssertNotNull(endpoint, nameof(endpoint)).AbsoluteUri, + AssertNotNull(keyCredential, nameof(keyCredential)), + options ?? new SmsClientOptions()) { } /// Initializes a new instance of . @@ -54,50 +56,40 @@ public SmsClient(string connectionString, SmsClientOptions options) /// Client option exposing , , , etc. public SmsClient(Uri endpoint, TokenCredential tokenCredential, SmsClientOptions options = default) : this( - endpoint, - options ?? new SmsClientOptions(), - tokenCredential) + AssertNotNull(endpoint, nameof(endpoint)).AbsoluteUri, + AssertNotNull(tokenCredential, nameof(tokenCredential)), + options ?? new SmsClientOptions()) { } - /// Initializes a new instance of for mocking. - protected SmsClient() - { - _clientDiagnostics = null!; - RestClient = null!; - } + #endregion - private SmsClient(ClientDiagnostics clientDiagnostics, HttpPipeline pipeline, string endpointUrl) - { - RestClient = new SmsRestClient(clientDiagnostics, pipeline, endpointUrl); - _clientDiagnostics = clientDiagnostics; - } + #region private constructors - private SmsClient(SmsClientOptions options, ConnectionString connectionString) - : this( - clientDiagnostics: new ClientDiagnostics(options), - pipeline: options.BuildHttpPipeline(connectionString), - endpointUrl: connectionString.GetRequired("endpoint")) + private SmsClient(ConnectionString connectionString, SmsClientOptions options) + : this(connectionString.GetRequired("endpoint"), options.BuildHttpPipeline(connectionString), options) { } - private SmsClient(Uri endpoint, SmsClientOptions options, TokenCredential tokenCredential) - { - Argument.AssertNotNull(endpoint, nameof(endpoint)); - Argument.AssertNotNull(tokenCredential, nameof(tokenCredential)); + private SmsClient(string endpoint, TokenCredential tokenCredential, SmsClientOptions options) + : this(endpoint, options.BuildHttpPipeline(tokenCredential), options) + { } + private SmsClient(string endpoint, AzureKeyCredential keyCredential, SmsClientOptions options) + : this(endpoint, options.BuildHttpPipeline(keyCredential), options) + { } + + private SmsClient(string endpoint, HttpPipeline httpPipeline, SmsClientOptions options) + { _clientDiagnostics = new ClientDiagnostics(options); - RestClient = new SmsRestClient( - _clientDiagnostics, - options.BuildHttpPipeline(tokenCredential), - endpoint.AbsoluteUri); + RestClient = new SmsRestClient(_clientDiagnostics, httpPipeline, endpoint, options.ApiVersion); } - private SmsClient(Uri endpoint, SmsClientOptions options, AzureKeyCredential credential) + #endregion + + /// Initializes a new instance of for mocking. + protected SmsClient() { - _clientDiagnostics = new ClientDiagnostics(options); - RestClient = new SmsRestClient( - _clientDiagnostics, - options.BuildHttpPipeline(credential), - endpoint.AbsoluteUri); + _clientDiagnostics = null; + RestClient = null; } ///