From 4f57df1d8d489f53b6d5acd37d7aa3e952faea51 Mon Sep 17 00:00:00 2001 From: Christian Findlay Date: Sat, 9 Apr 2022 13:12:07 +1000 Subject: [PATCH 1/2] Release 5.1.0 --- .gitignore | 2 + README.md | 1 + src/.editorconfig | 7 +- src/Directory.Build.props | 7 +- .../GlobalSuppressions.cs | 8 ++ .../HeadersExtensions.cs | 2 - .../GlobalSuppressions.cs | 8 ++ .../RestClient.Net.DependencyInjection.xml | 17 +++ .../RestClientExtensions.cs | 37 +++++- src/RestClient.Net.UnitTests/Helpers.cs | 14 ++ .../Http.Server/HttpServer.cs | 5 +- .../Http.Server/ServerExtensions.cs | 10 +- .../InMemoryHttpServer.cs | 2 +- .../LogCheckExtensions.cs | 2 +- src/RestClient.Net.UnitTests/MainUnitTests.cs | 48 ++++++- .../MicrosoftDependencyInjectionTests.cs | 2 +- .../MicrosoftDependencyInjectionTests2.cs | 123 ++++++++++++++++++ .../Model/GeoPlugin.cs | 2 + .../PollySendHttpRequestMessage.cs | 49 +++++++ src/RestClient.Net.UnitTests/PollyTests.cs | 65 +++------ src/RestClient.Net/Client.cs | 9 +- src/RestClient.Net/ClientFactory.cs | 25 ++-- 22 files changed, 353 insertions(+), 92 deletions(-) create mode 100644 src/RestClient.Net.Abstractions/GlobalSuppressions.cs create mode 100644 src/RestClient.Net.DependencyInjection/GlobalSuppressions.cs create mode 100644 src/RestClient.Net.UnitTests/Helpers.cs create mode 100644 src/RestClient.Net.UnitTests/PollySendHttpRequestMessage.cs diff --git a/.gitignore b/.gitignore index 17b552af..655a4e3e 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ src/.idea/* src/RestClient.Net.Abstractions/RestClient.Net.Abstractions.xml src/RestClient.Net/RestClient.Net.xml + +.ionide/ diff --git a/README.md b/README.md index 70da53f3..8cf5253f 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,7 @@ member this.TestComposition () = Assert.AreEqual("http://username:password@host.com:5000/pathpart1/pathpart2?fieldname1=field%3C%3EValue1&FieldName2=field%3C%3EValue2#frag",uri.ToString()); ``` + ## Donate | Coin | Address | diff --git a/src/.editorconfig b/src/.editorconfig index 9caf6c82..121c7f00 100644 --- a/src/.editorconfig +++ b/src/.editorconfig @@ -63,11 +63,14 @@ dotnet_diagnostic.CA1825.severity = none # CA1032: Implement standard exception constructors dotnet_diagnostic.CA1032.severity = None -######################## Warnings ################# - # CS1591: Missing XML comment for publicly visible type or member dotnet_diagnostic.CS1591.severity = none + +######################## Warnings ################# + +dotnet_diagnostic.CA1848.severity = warning + ######################################### dotnet_diagnostic.IDE0059.severity = error diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 00214cfa..772a0898 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -1,5 +1,6 @@ + CA1848 true Latest true @@ -14,9 +15,9 @@ true true enable - 5.0.7 - 5.0.7.0 - 5.0.7.0 + 5.1.0 + 5.1.0.0 + 5.1.0.0 true true Christian Findlay diff --git a/src/RestClient.Net.Abstractions/GlobalSuppressions.cs b/src/RestClient.Net.Abstractions/GlobalSuppressions.cs new file mode 100644 index 00000000..1c05a5e2 --- /dev/null +++ b/src/RestClient.Net.Abstractions/GlobalSuppressions.cs @@ -0,0 +1,8 @@ +// This file is used by Code Analysis to maintain SuppressMessage +// attributes that are applied to this project. +// Project-level suppressions either have no target or are given +// a specific target and scoped to a namespace, type, member, etc. + +using System.Diagnostics.CodeAnalysis; + +[assembly: SuppressMessage("Style", "IDE0130:Namespace does not match folder structure", Justification = "", Scope = "namespace", Target = "~N:RestClient.Net")] diff --git a/src/RestClient.Net.Abstractions/HeadersExtensions.cs b/src/RestClient.Net.Abstractions/HeadersExtensions.cs index b4f1f59f..4b4d5587 100644 --- a/src/RestClient.Net.Abstractions/HeadersExtensions.cs +++ b/src/RestClient.Net.Abstractions/HeadersExtensions.cs @@ -9,7 +9,6 @@ namespace RestClient.Net { public static class HeadersExtensions { - #region Internal Fields internal const string Authorization = "Authorization"; @@ -163,6 +162,5 @@ public static IHeadersCollection WithJsonContentTypeHeader(this IHeadersCollecti public static IHeadersCollection JsonContentTypeHeaders { get; } = new HeadersCollection(ContentTypeHeaderName, JsonMediaType); #endregion Public Methods - } } diff --git a/src/RestClient.Net.DependencyInjection/GlobalSuppressions.cs b/src/RestClient.Net.DependencyInjection/GlobalSuppressions.cs new file mode 100644 index 00000000..1c05a5e2 --- /dev/null +++ b/src/RestClient.Net.DependencyInjection/GlobalSuppressions.cs @@ -0,0 +1,8 @@ +// This file is used by Code Analysis to maintain SuppressMessage +// attributes that are applied to this project. +// Project-level suppressions either have no target or are given +// a specific target and scoped to a namespace, type, member, etc. + +using System.Diagnostics.CodeAnalysis; + +[assembly: SuppressMessage("Style", "IDE0130:Namespace does not match folder structure", Justification = "", Scope = "namespace", Target = "~N:RestClient.Net")] diff --git a/src/RestClient.Net.DependencyInjection/RestClient.Net.DependencyInjection.xml b/src/RestClient.Net.DependencyInjection/RestClient.Net.DependencyInjection.xml index 6231c39d..438dc65e 100644 --- a/src/RestClient.Net.DependencyInjection/RestClient.Net.DependencyInjection.xml +++ b/src/RestClient.Net.DependencyInjection/RestClient.Net.DependencyInjection.xml @@ -4,5 +4,22 @@ RestClient.Net.DependencyInjection + + + Adds CreateClient to the container so you can mint IClients. Use this overload if you need to configure a singleton IClient + + + Specify this to configure a singleton IClient + + + + + Adds CreateClient to the container so you can mint IClients. Use this overload if you don't need to configure a singleton IClient (i.e. you will use CreateClient instead) + + + Specify this if you need to configure a singleton IClient + Specify this if you need to take control of construction + + diff --git a/src/RestClient.Net.DependencyInjection/RestClientExtensions.cs b/src/RestClient.Net.DependencyInjection/RestClientExtensions.cs index 9d950885..09a60486 100644 --- a/src/RestClient.Net.DependencyInjection/RestClientExtensions.cs +++ b/src/RestClient.Net.DependencyInjection/RestClientExtensions.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using System; using System.Net.Http; @@ -7,7 +7,29 @@ namespace RestClient.Net { public static class RestClientExtensions { - public static IServiceCollection AddRestClient(this IServiceCollection serviceCollection, Action? configureClient = null) + /// + /// Adds CreateClient to the container so you can mint IClients. Use this overload if you need to configure a singleton IClient + /// + /// + /// Specify this to configure a singleton IClient + /// + public static IServiceCollection AddRestClient( + this IServiceCollection serviceCollection, + Action configureSingletonClient) + => AddRestClient(serviceCollection, configureSingletonClient: configureSingletonClient, createClient: null); + + /// + /// Adds CreateClient to the container so you can mint IClients. Use this overload if you don't need to configure a singleton IClient (i.e. you will use CreateClient instead) + /// + /// + /// Specify this if you need to configure a singleton IClient + /// Specify this if you need to take control of construction + /// + public static IServiceCollection AddRestClient( + this IServiceCollection serviceCollection, + Action? configureSingletonClient = null, + Func? createClient = null + ) { _ = serviceCollection .AddSingleton((sp) => @@ -19,11 +41,16 @@ public static IServiceCollection AddRestClient(this IServiceCollection serviceCo { var clientFactory = new ClientFactory( sp.GetRequiredService(), - sp.GetService()); + sp.GetService(), + createClient != null ? (name, options) => + createClient.Invoke(name, options, sp) : null + ); return clientFactory.CreateClient; - }) - .AddSingleton((sp) => sp.GetRequiredService()("RestClient", configureClient)); + }); + + _ = serviceCollection.AddSingleton((sp) => + sp.GetRequiredService()("RestClient", configureSingletonClient)); return serviceCollection; } diff --git a/src/RestClient.Net.UnitTests/Helpers.cs b/src/RestClient.Net.UnitTests/Helpers.cs new file mode 100644 index 00000000..53c71a86 --- /dev/null +++ b/src/RestClient.Net.UnitTests/Helpers.cs @@ -0,0 +1,14 @@ +using System.Net.Http; +using System.Reflection; + +namespace RestClient.Net.UnitTests +{ + public static class TestHelpers + { +#pragma warning disable CS8601 // Possible null reference assignment. + public static readonly FieldInfo HttpClientHandlerField = typeof(HttpMessageInvoker).GetField("_handler", BindingFlags.Instance | BindingFlags.NonPublic); + public static readonly FieldInfo HttpClientDisposedField = typeof(HttpClient).GetField("_disposed", BindingFlags.Instance | BindingFlags.NonPublic); +#pragma warning restore CS8601 // Possible null reference assignment. + + } +} diff --git a/src/RestClient.Net.UnitTests/Http.Server/HttpServer.cs b/src/RestClient.Net.UnitTests/Http.Server/HttpServer.cs index 776d8d0a..ad208b77 100644 --- a/src/RestClient.Net.UnitTests/Http.Server/HttpServer.cs +++ b/src/RestClient.Net.UnitTests/Http.Server/HttpServer.cs @@ -8,7 +8,7 @@ //Thanks! https://gist.github.com/yetanotherchris/fb50071bced8bf0849ecd2cbbc3e9dce -namespace Http.Server +namespace RestClient.Net.UnitTests.Http.Server { public class HttpServer : IDisposable { @@ -24,8 +24,7 @@ public HttpServer(AbsoluteUrl url, Func func) listener.Prefixes.Add(url.ToString() + "/"); listener.Start(); - _ = Task.Run(() => - func(listener.GetContext())); + _ = Task.Run(async () => func(await listener.GetContextAsync())); } public void Dispose() diff --git a/src/RestClient.Net.UnitTests/Http.Server/ServerExtensions.cs b/src/RestClient.Net.UnitTests/Http.Server/ServerExtensions.cs index b6400545..33179766 100644 --- a/src/RestClient.Net.UnitTests/Http.Server/ServerExtensions.cs +++ b/src/RestClient.Net.UnitTests/Http.Server/ServerExtensions.cs @@ -7,18 +7,12 @@ #pragma warning disable CA1835 // Prefer the 'Memory'-based overloads for 'ReadAsync' and 'WriteAsync' -namespace Http.Server +namespace RestClient.Net.UnitTests.Http.Server { public static class ServerExtensions { public static Task ServeAsync(this HttpServer listener, Func func) - { - return Task.Run(() => - { - var context = listener.listener.GetContext(); - return func(context); - }); - } + => Task.Run(async () => func(await listener.listener.GetContextAsync())); public static AbsoluteUrl GetLocalhostAddress() { diff --git a/src/RestClient.Net.UnitTests/InMemoryHttpServer.cs b/src/RestClient.Net.UnitTests/InMemoryHttpServer.cs index e4cea358..5bc24926 100644 --- a/src/RestClient.Net.UnitTests/InMemoryHttpServer.cs +++ b/src/RestClient.Net.UnitTests/InMemoryHttpServer.cs @@ -1,8 +1,8 @@ #if !NET45 -using Http.Server; using Microsoft.VisualStudio.TestTools.UnitTesting; using Newtonsoft.Json; +using RestClient.Net.UnitTests.Http.Server; using System; using System.Net; using System.Text; diff --git a/src/RestClient.Net.UnitTests/LogCheckExtensions.cs b/src/RestClient.Net.UnitTests/LogCheckExtensions.cs index 13b73ecc..89460d3e 100644 --- a/src/RestClient.Net.UnitTests/LogCheckExtensions.cs +++ b/src/RestClient.Net.UnitTests/LogCheckExtensions.cs @@ -76,7 +76,7 @@ public static bool GetValue(this object state, string key, out T value) { var keyValuePairList = (IReadOnlyList>)state; - var keyValuePair = keyValuePairList.FirstOrDefault(kvp => string.Compare(kvp.Key, key, StringComparison.Ordinal) == 0); + var keyValuePair = keyValuePairList.FirstOrDefault(kvp => string.Equals(kvp.Key, key, StringComparison.Ordinal)); value = (T)keyValuePair.Value; diff --git a/src/RestClient.Net.UnitTests/MainUnitTests.cs b/src/RestClient.Net.UnitTests/MainUnitTests.cs index 2b3cf6d9..8eb4c099 100644 --- a/src/RestClient.Net.UnitTests/MainUnitTests.cs +++ b/src/RestClient.Net.UnitTests/MainUnitTests.cs @@ -152,7 +152,7 @@ public class MainUnitTests : IDisposable #if NET45 NullLoggerFactory.Instance; - private const string LocalBaseUriString = "https://localhost:44337"; + public const string LocalBaseUriString = "https://localhost:44337"; #else LoggerFactory.Create(builder => _ = builder.AddConsole().SetMinimumLevel(LogLevel.Trace)); private static readonly TestServer _testServer; @@ -1458,6 +1458,52 @@ public async Task TestLocalPatchBodyUriCancellationTokenContentType() #endregion #region Misc + [TestMethod] + public void TestHeadersCollectionKeyValueConstruction() + { + const string Key = "test"; + const string Value = "test2"; + var headersCollection = new HeadersCollection(Key, Value); + var keyValuePair = headersCollection.First(); + Assert.AreEqual(1, keyValuePair.Value.Count()); + Assert.AreEqual(Value, keyValuePair.Value.First()); + Assert.AreEqual(Key, keyValuePair.Key); + } + + [TestMethod] + public void TestDisposeDisposesHttpClient() + { + using var client = new Client(new NewtonsoftSerializationAdapter()); + var httpClient = client.lazyHttpClient.Value; + client.Dispose(); +#pragma warning disable CS8605 // Unboxing a possibly null value. + var isHttpClientDisposed = (bool)TestHelpers.HttpClientDisposedField.GetValue(httpClient); +#pragma warning restore CS8605 // Unboxing a possibly null value. + Assert.AreEqual(true, client.Disposed); + Assert.AreEqual(true, isHttpClientDisposed); + } + + [TestMethod] + public void TestDisposeDoesntHappenTwice() + { + using var client = new Client(new NewtonsoftSerializationAdapter()); + //Trick the client in to thinking it has been disposed. + client.Disposed = true; + var httpClient = client.lazyHttpClient.Value; + client.Dispose(); +#pragma warning disable CS8605 // Unboxing a possibly null value. + var isHttpClientDisposed = (bool)TestHelpers.HttpClientDisposedField.GetValue(httpClient); +#pragma warning restore CS8605 // Unboxing a possibly null value. + Assert.AreEqual(false, isHttpClientDisposed); + } + + [TestMethod] + public void TestDefaultThrowExceptionOnFailure() + { + using var client = new Client(new NewtonsoftSerializationAdapter()); + Assert.AreEqual(true, client.ThrowExceptionOnFailure); + } + [TestMethod] public void TestJsonHeaders() { diff --git a/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests.cs b/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests.cs index 6ca2b9ec..7f74579d 100644 --- a/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests.cs +++ b/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests.cs @@ -117,7 +117,7 @@ public void TestStructureMap() _ = s.WithDefaultConventions(); }); - _ = c.For().Use(con => new ClientFactory(con.GetInstance(), null).CreateClient); + _ = c.For().Use(con => new ClientFactory(con.GetInstance(), null, null).CreateClient); _ = c.For().Use(con => new DefaultHttpClientFactory(null, null).CreateClient); _ = c.For().Use(); }); diff --git a/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests2.cs b/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests2.cs index 29862058..4f81a294 100644 --- a/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests2.cs +++ b/src/RestClient.Net.UnitTests/MicrosoftDependencyInjectionTests2.cs @@ -2,8 +2,10 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; using System; using System.Net.Http; +using Urls; namespace RestClient.Net.UnitTests { @@ -75,6 +77,127 @@ public void TestDIMapping2() Assert.AreEqual("test", client.Name); Assert.AreEqual("Hi", client.BaseUrl.Host); } + + [TestMethod] + public void DIConfigureWithInjectedService() + { + const string clientName = "tim"; + const int secondsTimeout = 123; + var testUrl = "http://example.org"; + + var urlProvider = new Mock(); + urlProvider.Setup(x => x.GetUrl()) + .Returns(testUrl) + .Verifiable(); + + var serviceCollection = new ServiceCollection() + .AddRestClient(createClient: (n, o, sp) => + new Client( + new AbsoluteUrl(sp.GetRequiredService().GetUrl()), + createHttpClient: o.CreateHttpClient, + name: n)) + .AddSingleton(urlProvider.Object); + + _ = serviceCollection.AddHttpClient(clientName, new Action((c) => + c.Timeout = new TimeSpan(0, 0, secondsTimeout) + )); + + var serviceProvider = serviceCollection.BuildServiceProvider(); + + var createClient = serviceProvider.GetRequiredService()(clientName); + + var expectedHttpClient = serviceProvider.GetRequiredService().CreateClient(clientName); ; + + if (createClient is not Client client) + { + throw new InvalidOperationException("Nah"); + } + + Assert.AreEqual(clientName, client.Name); + Assert.AreEqual(new AbsoluteUrl(testUrl), client.BaseUrl); + + //Make sure the correct http client handler gets used in the Client + Assert.IsTrue(ReferenceEquals( + TestHelpers.HttpClientHandlerField.GetValue(expectedHttpClient), + TestHelpers.HttpClientHandlerField.GetValue(client.lazyHttpClient.Value))); + + Assert.AreEqual(secondsTimeout, client.lazyHttpClient.Value.Timeout.TotalSeconds); + + urlProvider.VerifyAll(); + } + + + [TestMethod] + public void TestCreateClientFunc() + { + var newtonsoftSerializationAdapter = new NewtonsoftSerializationAdapter(); + var baseUrl = new AbsoluteUrl("http://www.di.com"); + const string clientName = "tim"; + + var serviceCollection = new ServiceCollection() + .AddSingleton(baseUrl) + .AddRestClient(createClient: (name, options, sp) => + new Client( + serializationAdapter: options.SerializationAdapter, + sp.GetRequiredService(), + createHttpClient: options.CreateHttpClient, + name: name)); + + _ = serviceCollection.AddHttpClient(clientName); + + var sp = serviceCollection.BuildServiceProvider(); + + var expectedHttpClient = sp.GetRequiredService().CreateClient(clientName); ; + + var createClient = sp.GetRequiredService(); + + var client = (Client)createClient(clientName, (o) => { o.SerializationAdapter = newtonsoftSerializationAdapter; }); + + Assert.IsTrue(ReferenceEquals(newtonsoftSerializationAdapter, client.SerializationAdapter)); + + //Make sure the correct http client handler gets used in the Client + Assert.IsTrue(ReferenceEquals( + TestHelpers.HttpClientHandlerField.GetValue(expectedHttpClient), + TestHelpers.HttpClientHandlerField.GetValue(client.lazyHttpClient.Value))); + + Assert.AreEqual(baseUrl, client.BaseUrl); + } + + [TestMethod] + public void TestSingletonClient() + { + var newtonsoftSerializationAdapter = new NewtonsoftSerializationAdapter(); + var baseUrl = new AbsoluteUrl("http://www.di.com"); + + var serviceCollection = new ServiceCollection() + .AddSingleton(baseUrl) + .AddRestClient((o) => o.SerializationAdapter = newtonsoftSerializationAdapter); + + const string clientName = "RestClient"; + + _ = serviceCollection.AddHttpClient(clientName); + + var sp = serviceCollection.BuildServiceProvider(); + + using var expectedHttpClient = sp.GetRequiredService().CreateClient(clientName); + + var client = (Client)sp.GetRequiredService(); + + Assert.IsTrue(ReferenceEquals(newtonsoftSerializationAdapter, client.SerializationAdapter)); + + //Make sure the correct http client handler gets used in the Client + Assert.IsTrue(ReferenceEquals( + TestHelpers.HttpClientHandlerField.GetValue(expectedHttpClient), + TestHelpers.HttpClientHandlerField.GetValue(client.lazyHttpClient.Value))); + } + } + public interface IUrlProvider + { +#pragma warning disable CA1055 // URI-like return values should not be strings + string GetUrl(); +#pragma warning restore CA1055 // URI-like return values should not be strings + } + } #endif \ No newline at end of file diff --git a/src/RestClient.Net.UnitTests/Model/GeoPlugin.cs b/src/RestClient.Net.UnitTests/Model/GeoPlugin.cs index c53e0a6d..99bd3643 100644 --- a/src/RestClient.Net.UnitTests/Model/GeoPlugin.cs +++ b/src/RestClient.Net.UnitTests/Model/GeoPlugin.cs @@ -3,7 +3,9 @@ using System.Xml.Serialization; +#pragma warning disable IDE0130 // Namespace does not match folder structure namespace Xml2CSharp +#pragma warning restore IDE0130 // Namespace does not match folder structure { [XmlRoot(ElementName = "a")] public class A diff --git a/src/RestClient.Net.UnitTests/PollySendHttpRequestMessage.cs b/src/RestClient.Net.UnitTests/PollySendHttpRequestMessage.cs new file mode 100644 index 00000000..47fdb6da --- /dev/null +++ b/src/RestClient.Net.UnitTests/PollySendHttpRequestMessage.cs @@ -0,0 +1,49 @@ +using Microsoft.Extensions.Logging; +using Polly.Retry; +using System; +using System.Net.Http; +using System.Threading.Tasks; +using Urls; + +namespace RestClient.Net.UnitTests +{ + //It sucks that we have to create a class in this way. The old version was far less verbose. + //TODO: Look in to another way to achieve this + + public class PollySendHttpRequestMessage : ISendHttpRequestMessage + { + + private readonly AsyncRetryPolicy policy; + + public PollySendHttpRequestMessage(AsyncRetryPolicy policy) => this.policy = policy; + + public int Tries { get; private set; } + + public Task SendHttpRequestMessage( + HttpClient httpClient, + IGetHttpRequestMessage httpRequestMessageFunc, + IRequest request, + ILogger logger, + ISerializationAdapter serializationAdapter) => + policy.ExecuteAsync(() => + { + if (httpRequestMessageFunc == null) throw new ArgumentNullException(nameof(httpRequestMessageFunc)); + if (httpClient == null) throw new ArgumentNullException(nameof(httpClient)); + if (request == null) throw new ArgumentNullException(nameof(request)); + + var httpRequestMessage = httpRequestMessageFunc.GetHttpRequestMessage(request, logger, serializationAdapter); + + //On the third try change the Url to a the correct one + if (Tries == 2) + { + httpRequestMessage.RequestUri = + new AbsoluteUrl(MainUnitTests.LocalBaseUriString) + .WithRelativeUrl(new RelativeUrl("Person")) + .ToUri(); + } + + Tries++; + return httpClient.SendAsync(httpRequestMessage, request.CancellationToken); + }); + } +} diff --git a/src/RestClient.Net.UnitTests/PollyTests.cs b/src/RestClient.Net.UnitTests/PollyTests.cs index 235cd09f..a60b8d4a 100644 --- a/src/RestClient.Net.UnitTests/PollyTests.cs +++ b/src/RestClient.Net.UnitTests/PollyTests.cs @@ -1,60 +1,17 @@  using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Polly; using Polly.Extensions.Http; -using Polly.Retry; -using RestClient.Net.UnitTests.Model; using RestClientApiSamples; using System; -using System.Collections.Generic; using System.Net; using System.Net.Http; using System.Threading.Tasks; -using Urls; namespace RestClient.Net.UnitTests { - //It sucks that we have to create a class in this way. The old version was far less verbose. - //TODO: Look in to another way to achieve this - - public class PollySendHttpRequestMessage : ISendHttpRequestMessage - { - private readonly AsyncRetryPolicy policy; - - public PollySendHttpRequestMessage(AsyncRetryPolicy policy) => this.policy = policy; - - public int Tries { get; private set; } - - public Task SendHttpRequestMessage( - HttpClient httpClient, - IGetHttpRequestMessage httpRequestMessageFunc, - IRequest request, - ILogger logger, - ISerializationAdapter serializationAdapter) => - policy.ExecuteAsync(() => - { - if (httpRequestMessageFunc == null) throw new ArgumentNullException(nameof(httpRequestMessageFunc)); - if (httpClient == null) throw new ArgumentNullException(nameof(httpClient)); - if (request == null) throw new ArgumentNullException(nameof(request)); - - var httpRequestMessage = httpRequestMessageFunc.GetHttpRequestMessage(request, logger, serializationAdapter); - - //On the third try change the Url to a the correct one - if (Tries == 2) - { - httpRequestMessage.RequestUri = - new AbsoluteUrl(MainUnitTests.LocalBaseUriString) - .WithRelativeUrl(new RelativeUrl("Person")) - .ToUri(); - } - - Tries++; - return httpClient.SendAsync(httpRequestMessage, request.CancellationToken); - }); - } [TestClass] public class PollyTests @@ -87,8 +44,10 @@ public async Task TestPollyManualIncorrectUri() [TestMethod] - public async Task TestPollyWithDependencyInjection() + public void TestPollyWithDependencyInjection() { + const string httpClientName = "rc"; + //Configure a Polly policy var policy = HttpPolicyExtensions .HandleTransientHttpError() @@ -100,7 +59,7 @@ public async Task TestPollyWithDependencyInjection() _ = serviceCollection.AddSingleton(typeof(ISerializationAdapter), typeof(NewtonsoftSerializationAdapter)) .AddLogging() //Add the Polly policy to the named HttpClient instance - .AddHttpClient("rc"). + .AddHttpClient(httpClientName). SetHandlerLifetime(TimeSpan.FromMinutes(5)). AddPolicyHandler(policy); @@ -110,13 +69,19 @@ public async Task TestPollyWithDependencyInjection() var serviceProvider = serviceCollection.BuildServiceProvider(); var clientFactory = serviceProvider.GetRequiredService(); - //Create a Rest Client that will get the HttpClient by the name of rc - var client = clientFactory("rc", (o) => o.BaseUrl = new("https://restcountries.eu/rest/v2/")); + //Get the client + var client = (Client)clientFactory(httpClientName); + + //Get the Http client from the Microsoft IHttpClientFactory implementation wihch will have the Polly handler + var expectedHttpClient = serviceProvider.GetRequiredService().CreateClient(httpClientName); + + //Get the actual HttpClient inside the Client + var actualHttpClient = client.lazyHttpClient.Value; - //Make the call - _ = await client.GetAsync>(); + var handler1 = TestHelpers.HttpClientHandlerField.GetValue(expectedHttpClient); + var handler2 = TestHelpers.HttpClientHandlerField.GetValue(actualHttpClient); - //TODO: Implement this completely to ensure that the policy is being applied + Assert.IsTrue(ReferenceEquals(handler1, handler2)); } } diff --git a/src/RestClient.Net/Client.cs b/src/RestClient.Net/Client.cs index c8a66529..c6df8f89 100644 --- a/src/RestClient.Net/Client.cs +++ b/src/RestClient.Net/Client.cs @@ -44,7 +44,8 @@ public sealed class Client : IClient, IDisposable internal readonly Lazy lazyHttpClient; - private bool disposed; + //Set is for testing purposes only. A little nasty TBH + internal bool Disposed { get; set; } #endregion Private Fields @@ -206,7 +207,7 @@ public Client( /// /// Specifies whether or not the client will throw an exception when non-successful status codes are returned in the http response. The default is true /// - public bool ThrowExceptionOnFailure { get; init; } = true; + public bool ThrowExceptionOnFailure { get; init; } #endregion Public Properties @@ -214,9 +215,9 @@ public Client( public void Dispose() { - if (disposed) return; + if (Disposed) return; - disposed = true; + Disposed = true; lazyHttpClient.Value?.Dispose(); } diff --git a/src/RestClient.Net/ClientFactory.cs b/src/RestClient.Net/ClientFactory.cs index 0595fb0c..d75e2cdb 100644 --- a/src/RestClient.Net/ClientFactory.cs +++ b/src/RestClient.Net/ClientFactory.cs @@ -3,15 +3,14 @@ using Microsoft.Extensions.Logging.Abstractions; using System; using System.Collections.Concurrent; -using System.Threading; namespace RestClient.Net { public class ClientFactory { #region Fields - private readonly Func?, Lazy> createClientFunc; - private readonly ConcurrentDictionary> clients; + private readonly Func createClientFunc; + private readonly ConcurrentDictionary clients; private readonly CreateHttpClient createHttpClient; private readonly ILoggerFactory loggerFactory; #endregion @@ -19,29 +18,33 @@ public class ClientFactory #region Constructor public ClientFactory( CreateHttpClient createHttpClient, - ILoggerFactory? loggerFactory = null) + ILoggerFactory? loggerFactory = null, + Func? createClientFunc = null) { this.createHttpClient = createHttpClient; this.loggerFactory = loggerFactory ?? NullLoggerFactory.Instance; - clients = new ConcurrentDictionary>(); + clients = new ConcurrentDictionary(); - createClientFunc = (name, baseUri) => new Lazy(() => MintClient(name, baseUri), LazyThreadSafetyMode.ExecutionAndPublication); + this.createClientFunc = createClientFunc ?? new Func((name, options) => MintClient(name, options)); } #endregion #region Implementation public IClient CreateClient(string name, Action? configureClient = null) - => name == null ? throw new ArgumentNullException(nameof(name)) : clients.GetOrAdd(name, createClientFunc(name, configureClient)).Value; + => name == null ? throw new ArgumentNullException(nameof(name)) : + clients.GetOrAdd(name, (n) => + { + var options = new CreateClientOptions(createHttpClient); + configureClient?.Invoke(options); + return createClientFunc(n, options); + }); #endregion #region Private Methods - private IClient MintClient(string name, Action? configureClient = null) + private IClient MintClient(string name, CreateClientOptions createClientOptions) { - var createClientOptions = new CreateClientOptions(createHttpClient); - - configureClient?.Invoke(createClientOptions); #if NET45 if (createClientOptions.SerializationAdapter == null) throw new InvalidOperationException("You must specify a SerializationAdapter"); From a3c1fffd808f411d1a42bedc16ccda0a48b8e790 Mon Sep 17 00:00:00 2001 From: Christian Findlay <16697547+MelbourneDeveloper@users.noreply.github.com> Date: Sat, 4 Jun 2022 16:09:59 +1000 Subject: [PATCH 2/2] break it --- src/RestClient.NET/ClientCallExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RestClient.NET/ClientCallExtensions.cs b/src/RestClient.NET/ClientCallExtensions.cs index a03091a6..6a7b5d66 100644 --- a/src/RestClient.NET/ClientCallExtensions.cs +++ b/src/RestClient.NET/ClientCallExtensions.cs @@ -12,7 +12,7 @@ public static class ClientCallExtensions this AbsoluteUrl url, IHeadersCollection? headersCollection = null, CancellationToken cancellationToken = default) - { + {dgdfgdfg if (url == null) throw new ArgumentNullException(nameof(url)); var client = new Client(url.WithRelativeUrl(RelativeUrl.Empty)); var response = await client.GetAsync(