* Remove the ability to set http client handler on OAuth2Client

lukebakken · lukebakken · commit d6975b6b417e · 2024-08-05T14:25:31.000-07:00
* Only warn when a connection can't be deleted
* Make `RefreshToken` a nullable string
diff --git a/projects/RabbitMQ.Client.OAuth2/OAuth2Client.cs b/projects/RabbitMQ.Client.OAuth2/OAuth2Client.cs
@@ -47,7 +47,6 @@ public class OAuth2ClientBuilder
         private readonly Uri _tokenEndpoint;
         private string? _scope;
         private IDictionary<string, string>? _additionalRequestParameters;
-        private HttpClientHandler? _httpClientHandler;
 
         public OAuth2ClientBuilder(string clientId, string clientSecret, Uri tokenEndpoint)
         {
@@ -62,12 +61,6 @@ public OAuth2ClientBuilder SetScope(string scope)
             return this;
         }
 
-        public OAuth2ClientBuilder SetHttpClientHandler(HttpClientHandler handler)
-        {
-            _httpClientHandler = handler ?? throw new ArgumentNullException(nameof(handler));
-            return this;
-        }
-
         public OAuth2ClientBuilder AddRequestParameter(string param, string paramValue)
         {
             if (param == null)
@@ -92,7 +85,7 @@ public OAuth2ClientBuilder AddRequestParameter(string param, string paramValue)
         public IOAuth2Client Build()
         {
             return new OAuth2Client(_clientId, _clientSecret, _tokenEndpoint,
-                _scope, _additionalRequestParameters, _httpClientHandler);
+                _scope, _additionalRequestParameters);
         }
     }
 
@@ -123,17 +116,15 @@ internal class OAuth2Client : IOAuth2Client, IDisposable
 
         public OAuth2Client(string clientId, string clientSecret, Uri tokenEndpoint,
             string? scope,
-            IDictionary<string, string>? additionalRequestParameters,
-            HttpClientHandler? httpClientHandler)
+            IDictionary<string, string>? additionalRequestParameters)
         {
             _clientId = clientId;
             _clientSecret = clientSecret;
             _scope = scope;
             _additionalRequestParameters = additionalRequestParameters == null ? EMPTY : additionalRequestParameters;
             _tokenEndpoint = tokenEndpoint;
 
-            _httpClient = httpClientHandler == null ? new HttpClient() :
-                new HttpClient(httpClientHandler);
+            _httpClient = new HttpClient();
             _httpClient.DefaultRequestHeaders.Accept.Clear();
             _httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
         }
@@ -219,7 +210,7 @@ private Dictionary<string, string> BuildRequestParameters()
 
         private Dictionary<string, string> BuildRefreshParameters(IToken token)
         {
-            var dict = BuildRequestParameters();
+            Dictionary<string, string> dict = BuildRequestParameters();
             dict.Remove(GRANT_TYPE);
             dict.Add(GRANT_TYPE, REFRESH_TOKEN);
 
@@ -228,7 +219,10 @@ private Dictionary<string, string> BuildRefreshParameters(IToken token)
                 dict.Add(SCOPE, _scope);
             }
 
-            dict.Add(REFRESH_TOKEN, token.RefreshToken);
+            if (token.RefreshToken != null)
+            {
+                dict.Add(REFRESH_TOKEN, token.RefreshToken);
+            }
 
             return dict;
         }
@@ -263,7 +257,7 @@ public string AccessToken
         }
 
         [JsonPropertyName("refresh_token")]
-        public string RefreshToken
+        public string? RefreshToken
         {
             get; set;
         }
diff --git a/projects/RabbitMQ.Client.OAuth2/PublicAPI.Shipped.txt b/projects/RabbitMQ.Client.OAuth2/PublicAPI.Shipped.txt
@@ -9,7 +9,6 @@ RabbitMQ.Client.OAuth2.OAuth2ClientBuilder
 RabbitMQ.Client.OAuth2.OAuth2ClientBuilder.AddRequestParameter(string param, string paramValue) -> RabbitMQ.Client.OAuth2.OAuth2ClientBuilder
 RabbitMQ.Client.OAuth2.OAuth2ClientBuilder.Build() -> RabbitMQ.Client.OAuth2.IOAuth2Client
 RabbitMQ.Client.OAuth2.OAuth2ClientBuilder.OAuth2ClientBuilder(string clientId, string clientSecret, System.Uri tokenEndpoint) -> void
-RabbitMQ.Client.OAuth2.OAuth2ClientBuilder.SetHttpClientHandler(System.Net.Http.HttpClientHandler handler) -> RabbitMQ.Client.OAuth2.OAuth2ClientBuilder
 RabbitMQ.Client.OAuth2.OAuth2ClientBuilder.SetScope(string scope) -> RabbitMQ.Client.OAuth2.OAuth2ClientBuilder
 RabbitMQ.Client.OAuth2.OAuth2ClientCredentialsProvider
 RabbitMQ.Client.OAuth2.OAuth2ClientCredentialsProvider.Name.get -> string
diff --git a/projects/RabbitMQ.Client.OAuth2/Token.cs b/projects/RabbitMQ.Client.OAuth2/Token.cs
@@ -36,7 +36,7 @@ namespace RabbitMQ.Client.OAuth2
     public interface IToken
     {
         string AccessToken { get; }
-        string RefreshToken { get; }
+        string? RefreshToken { get; }
         TimeSpan ExpiresIn { get; }
         bool HasExpired { get; }
     }
@@ -60,7 +60,7 @@ public string AccessToken
             }
         }
 
-        public string RefreshToken
+        public string? RefreshToken
         {
             get
             {
diff --git a/projects/Test/Common/TestConnectionRecoveryBase.cs b/projects/Test/Common/TestConnectionRecoveryBase.cs
@@ -42,14 +42,15 @@ namespace Test
 {
     public class TestConnectionRecoveryBase : IntegrationFixture, IDisposable
     {
-        private readonly Util _util = new Util();
+        private readonly Util _util;
         protected readonly byte[] _messageBody;
         protected const ushort TotalMessageCount = 16384;
         protected const ushort CloseAtCount = 16;
 
         public TestConnectionRecoveryBase(ITestOutputHelper output)
             : base(output)
         {
+            _util = new Util(output);
             _messageBody = GetRandomBody(4096);
         }
 
diff --git a/projects/Test/Common/Util.cs b/projects/Test/Common/Util.cs
@@ -4,11 +4,13 @@
 using System.Linq;
 using System.Threading.Tasks;
 using EasyNetQ.Management.Client;
+using Xunit.Abstractions;
 
 namespace Test
 {
     public class Util : IDisposable
     {
+        private readonly ITestOutputHelper _output;
         private readonly ManagementClient _managementClient;
         private static readonly bool s_isWindows = false;
 
@@ -17,12 +19,14 @@ static Util()
             s_isWindows = InitIsWindows();
         }
 
-        public Util() : this("guest", "guest")
+        public Util(ITestOutputHelper output) : this(output, "guest", "guest")
         {
         }
 
-        public Util(string managementUsername, string managementPassword)
+        public Util(ITestOutputHelper output, string managementUsername, string managementPassword)
         {
+            _output = output;
+
             if (string.IsNullOrEmpty(managementUsername))
             {
                 managementUsername = "guest";
@@ -37,7 +41,7 @@ public Util(string managementUsername, string managementPassword)
             _managementClient = new ManagementClient(managementUri, managementUsername, managementPassword);
         }
 
-        public static string Now => DateTime.UtcNow.ToString("o", CultureInfo.InvariantCulture);
+        public static string Now => DateTime.UtcNow.ToString("s", CultureInfo.InvariantCulture);
 
         public static bool IsWindows => s_isWindows;
 
@@ -90,7 +94,8 @@ public async Task CloseConnectionAsync(string connectionClientProvidedName)
 
             if (connectionToClose == null)
             {
-                throw new InvalidOperationException($"Could not delete connection: '{connectionClientProvidedName}'");
+                _output.WriteLine("{0} [WARNING] could not find/delete connection: '{1}'",
+                    Now, connectionClientProvidedName);
             }
         }
 
diff --git a/projects/Test/OAuth2/TestOAuth2.cs b/projects/Test/OAuth2/TestOAuth2.cs
@@ -179,7 +179,7 @@ async Task CloseConnection()
                                     // TODO rabbitmq-dotnet-client-1639
                                     // Credentials httpApiCredentials = await _httpApiCredentialsProvider.GetCredentialsAsync();
                                     // closeConnectionUtil = new Util("mgt_api_client", httpApiCredentials.Password);
-                                    closeConnectionUtil = new Util();
+                                    closeConnectionUtil = new Util(_testOutputHelper);
                                     await closeConnectionUtil.CloseConnectionAsync(_connection.ClientProvidedName);
                                 }
 

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,6 @@ public class OAuth2ClientBuilder`
`47`	`47`	`private readonly Uri _tokenEndpoint;`
`48`	`48`	`private string? _scope;`
`49`	`49`	`private IDictionary<string, string>? _additionalRequestParameters;`
`50`		`- private HttpClientHandler? _httpClientHandler;`
`51`	`50`
`52`	`51`	`public OAuth2ClientBuilder(string clientId, string clientSecret, Uri tokenEndpoint)`
`53`	`52`	`{`
`@@ -62,12 +61,6 @@ public OAuth2ClientBuilder SetScope(string scope)`
`62`	`61`	`return this;`
`63`	`62`	`}`
`64`	`63`
`65`		`- public OAuth2ClientBuilder SetHttpClientHandler(HttpClientHandler handler)`
`66`		`- {`
`67`		`- _httpClientHandler = handler ?? throw new ArgumentNullException(nameof(handler));`
`68`		`- return this;`
`69`		`- }`
`70`		`-`
`71`	`64`	`public OAuth2ClientBuilder AddRequestParameter(string param, string paramValue)`
`72`	`65`	`{`
`73`	`66`	`if (param == null)`
`@@ -92,7 +85,7 @@ public OAuth2ClientBuilder AddRequestParameter(string param, string paramValue)`
`92`	`85`	`public IOAuth2Client Build()`
`93`	`86`	`{`
`94`	`87`	`return new OAuth2Client(_clientId, _clientSecret, _tokenEndpoint,`
`95`		`- _scope, _additionalRequestParameters, _httpClientHandler);`
	`88`	`+ _scope, _additionalRequestParameters);`
`96`	`89`	`}`
`97`	`90`	`}`
`98`	`91`
`@@ -123,17 +116,15 @@ internal class OAuth2Client : IOAuth2Client, IDisposable`
`123`	`116`
`124`	`117`	`public OAuth2Client(string clientId, string clientSecret, Uri tokenEndpoint,`
`125`	`118`	`string? scope,`
`126`		`- IDictionary<string, string>? additionalRequestParameters,`
`127`		`- HttpClientHandler? httpClientHandler)`
	`119`	`+ IDictionary<string, string>? additionalRequestParameters)`
`128`	`120`	`{`
`129`	`121`	`_clientId = clientId;`
`130`	`122`	`_clientSecret = clientSecret;`
`131`	`123`	`_scope = scope;`
`132`	`124`	`_additionalRequestParameters = additionalRequestParameters == null ? EMPTY : additionalRequestParameters;`
`133`	`125`	`_tokenEndpoint = tokenEndpoint;`
`134`	`126`
`135`		`- _httpClient = httpClientHandler == null ? new HttpClient() :`
`136`		`- new HttpClient(httpClientHandler);`
	`127`	`+ _httpClient = new HttpClient();`
`137`	`128`	`_httpClient.DefaultRequestHeaders.Accept.Clear();`
`138`	`129`	`_httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));`
`139`	`130`	`}`
`@@ -219,7 +210,7 @@ private Dictionary<string, string> BuildRequestParameters()`
`219`	`210`
`220`	`211`	`private Dictionary<string, string> BuildRefreshParameters(IToken token)`
`221`	`212`	`{`
`222`		`- var dict = BuildRequestParameters();`
	`213`	`+ Dictionary<string, string> dict = BuildRequestParameters();`
`223`	`214`	`dict.Remove(GRANT_TYPE);`
`224`	`215`	`dict.Add(GRANT_TYPE, REFRESH_TOKEN);`
`225`	`216`
`@@ -228,7 +219,10 @@ private Dictionary<string, string> BuildRefreshParameters(IToken token)`
`228`	`219`	`dict.Add(SCOPE, _scope);`
`229`	`220`	`}`
`230`	`221`
`231`		`- dict.Add(REFRESH_TOKEN, token.RefreshToken);`
	`222`	`+ if (token.RefreshToken != null)`
	`223`	`+ {`
	`224`	`+ dict.Add(REFRESH_TOKEN, token.RefreshToken);`
	`225`	`+ }`
`232`	`226`
`233`	`227`	`return dict;`
`234`	`228`	`}`
`@@ -263,7 +257,7 @@ public string AccessToken`
`263`	`257`	`}`
`264`	`258`
`265`	`259`	`[JsonPropertyName("refresh_token")]`
`266`		`- public string RefreshToken`
	`260`	`+ public string? RefreshToken`
`267`	`261`	`{`
`268`	`262`	`get; set;`
`269`	`263`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ namespace RabbitMQ.Client.OAuth2`
`36`	`36`	`public interface IToken`
`37`	`37`	`{`
`38`	`38`	`string AccessToken { get; }`
`39`		`- string RefreshToken { get; }`
	`39`	`+ string? RefreshToken { get; }`
`40`	`40`	`TimeSpan ExpiresIn { get; }`
`41`	`41`	`bool HasExpired { get; }`
`42`	`42`	`}`
`@@ -60,7 +60,7 @@ public string AccessToken`
`60`	`60`	`}`
`61`	`61`	`}`
`62`	`62`
`63`		`- public string RefreshToken`
	`63`	`+ public string? RefreshToken`
`64`	`64`	`{`
`65`	`65`	`get`
`66`	`66`	`{`
Original file line number	Diff line number	Diff line change
`@@ -42,14 +42,15 @@ namespace Test`
`42`	`42`	`{`
`43`	`43`	`public class TestConnectionRecoveryBase : IntegrationFixture, IDisposable`
`44`	`44`	`{`
`45`		`- private readonly Util _util = new Util();`
	`45`	`+ private readonly Util _util;`
`46`	`46`	`protected readonly byte[] _messageBody;`
`47`	`47`	`protected const ushort TotalMessageCount = 16384;`
`48`	`48`	`protected const ushort CloseAtCount = 16;`
`49`	`49`
`50`	`50`	`public TestConnectionRecoveryBase(ITestOutputHelper output)`
`51`	`51`	`: base(output)`
`52`	`52`	`{`
	`53`	`+ _util = new Util(output);`
`53`	`54`	`_messageBody = GetRandomBody(4096);`
`54`	`55`	`}`
`55`	`56`
Original file line number	Diff line number	Diff line change
`@@ -4,11 +4,13 @@`
`4`	`4`	`using System.Linq;`
`5`	`5`	`using System.Threading.Tasks;`
`6`	`6`	`using EasyNetQ.Management.Client;`
	`7`	`+using Xunit.Abstractions;`
`7`	`8`
`8`	`9`	`namespace Test`
`9`	`10`	`{`
`10`	`11`	`public class Util : IDisposable`
`11`	`12`	`{`
	`13`	`+ private readonly ITestOutputHelper _output;`
`12`	`14`	`private readonly ManagementClient _managementClient;`
`13`	`15`	`private static readonly bool s_isWindows = false;`
`14`	`16`
`@@ -17,12 +19,14 @@ static Util()`
`17`	`19`	`s_isWindows = InitIsWindows();`
`18`	`20`	`}`
`19`	`21`
`20`		`- public Util() : this("guest", "guest")`
	`22`	`+ public Util(ITestOutputHelper output) : this(output, "guest", "guest")`
`21`	`23`	`{`
`22`	`24`	`}`
`23`	`25`
`24`		`- public Util(string managementUsername, string managementPassword)`
	`26`	`+ public Util(ITestOutputHelper output, string managementUsername, string managementPassword)`
`25`	`27`	`{`
	`28`	`+ _output = output;`
	`29`	`+`
`26`	`30`	`if (string.IsNullOrEmpty(managementUsername))`
`27`	`31`	`{`
`28`	`32`	`managementUsername = "guest";`
`@@ -37,7 +41,7 @@ public Util(string managementUsername, string managementPassword)`
`37`	`41`	`_managementClient = new ManagementClient(managementUri, managementUsername, managementPassword);`
`38`	`42`	`}`
`39`	`43`
`40`		`- public static string Now => DateTime.UtcNow.ToString("o", CultureInfo.InvariantCulture);`
	`44`	`+ public static string Now => DateTime.UtcNow.ToString("s", CultureInfo.InvariantCulture);`
`41`	`45`
`42`	`46`	`public static bool IsWindows => s_isWindows;`
`43`	`47`
`@@ -90,7 +94,8 @@ public async Task CloseConnectionAsync(string connectionClientProvidedName)`
`90`	`94`
`91`	`95`	`if (connectionToClose == null)`
`92`	`96`	`{`
`93`		`- throw new InvalidOperationException($"Could not delete connection: '{connectionClientProvidedName}'");`
	`97`	`+ _output.WriteLine("{0} [WARNING] could not find/delete connection: '{1}'",`
	`98`	`+ Now, connectionClientProvidedName);`
`94`	`99`	`}`
`95`	`100`	`}`
`96`	`101`
Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@ async Task CloseConnection()`
`179`	`179`	`// TODO rabbitmq-dotnet-client-1639`
`180`	`180`	`// Credentials httpApiCredentials = await _httpApiCredentialsProvider.GetCredentialsAsync();`
`181`	`181`	`// closeConnectionUtil = new Util("mgt_api_client", httpApiCredentials.Password);`
`182`		`- closeConnectionUtil = new Util();`
	`182`	`+ closeConnectionUtil = new Util(_testOutputHelper);`
`183`	`183`	`await closeConnectionUtil.CloseConnectionAsync(_connection.ClientProvidedName);`
`184`	`184`	`}`
`185`	`185`