From 66587ee0d19d797a5928c15e801e0dbf03c7fd0d Mon Sep 17 00:00:00 2001 From: Yankai <71268247+reny-gearset@users.noreply.github.com> Date: Tue, 7 Feb 2023 17:35:54 +0000 Subject: [PATCH] [feat] Add Response caching --- Octokit.Tests/Caching/CachedResponseTests.cs | 102 ++++++ .../Caching/CachingHttpClientTests.cs | 331 ++++++++++++++++++ Octokit.Tests/GitHubClientTests.cs | 42 +++ Octokit.Tests/Helpers/ApiInfoComparer.cs | 41 +++ Octokit.Tests/Helpers/CollectionComparer.cs | 23 ++ Octokit.Tests/Helpers/RateLimitComparer.cs | 30 ++ Octokit.Tests/Helpers/ResponseComparer.cs | 37 ++ .../Helpers/StringKeyedDictionaryComparer.cs | 23 ++ Octokit.Tests/Http/ConnectionTests.cs | 43 +++ Octokit/Caching/CachedResponse.cs | 63 ++++ Octokit/Caching/CachingHttpClient.cs | 86 +++++ Octokit/Caching/IResponseCache.cs | 12 + Octokit/GitHubClient.cs | 16 + Octokit/Http/Connection.cs | 18 +- Octokit/Http/IConnection.cs | 17 +- 15 files changed, 879 insertions(+), 5 deletions(-) create mode 100644 Octokit.Tests/Caching/CachedResponseTests.cs create mode 100644 Octokit.Tests/Caching/CachingHttpClientTests.cs create mode 100644 Octokit.Tests/Helpers/ApiInfoComparer.cs create mode 100644 Octokit.Tests/Helpers/CollectionComparer.cs create mode 100644 Octokit.Tests/Helpers/RateLimitComparer.cs create mode 100644 Octokit.Tests/Helpers/ResponseComparer.cs create mode 100644 Octokit.Tests/Helpers/StringKeyedDictionaryComparer.cs create mode 100644 Octokit/Caching/CachedResponse.cs create mode 100644 Octokit/Caching/CachingHttpClient.cs create mode 100644 Octokit/Caching/IResponseCache.cs diff --git a/Octokit.Tests/Caching/CachedResponseTests.cs b/Octokit.Tests/Caching/CachedResponseTests.cs new file mode 100644 index 00000000..1bf57af6 --- /dev/null +++ b/Octokit.Tests/Caching/CachedResponseTests.cs @@ -0,0 +1,102 @@ +using System; +using System.Collections.Generic; +using System.Net; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Octokit.Caching; +using Octokit.Internal; +using Octokit.Tests.Helpers; +using Xunit; + +namespace Octokit.Tests.Caching +{ + public class CachedResponseTests + { + public class V1 + { + public class TheToResponseMethod + { + [Fact] + public void CreatesResponseWithSameProperties() + { + var body = new object(); + var headers = new Dictionary(); + var apiInfo = new ApiInfo(new Dictionary(), new List(), new List(), "etag", new RateLimit()); + const HttpStatusCode httpStatusCode = HttpStatusCode.OK; + const string contentType = "content-type"; + var v1 = new CachedResponse.V1(body, headers, apiInfo, httpStatusCode, contentType); + + var response = v1.ToResponse(); + + Assert.Equal(new Response(httpStatusCode, body, headers, contentType), response, new ResponseComparer()); + } + } + + public class TheCreateMethod + { + [Fact] + public void EnsuresNonNullArguments() + { + Assert.Throws(() => CachedResponse.V1.Create(null)); + } + + [Fact] + public void EnsuresNonNullResponseHeader() + { + var response = Substitute.For(); + response.Headers.ReturnsNull(); + + Assert.Throws(() => CachedResponse.V1.Create(response)); + } + + [Fact] + public void CreatesV1WithSameProperties() + { + var response = Substitute.For(); + response.Headers.Returns(new Dictionary()); + + var v1 = CachedResponse.V1.Create(response); + Assert.Equal(response.Body, v1.Body); + Assert.Equal(response.Headers, v1.Headers); + Assert.Equal(response.ApiInfo, v1.ApiInfo); + Assert.Equal(response.StatusCode, v1.StatusCode); + Assert.Equal(response.ContentType, v1.ContentType); + } + } + + public class TheCtor + { + [Fact] + public void EnsuresNonNullHeaders() + { + Assert.Throws(() => new CachedResponse.V1("body", null, new ApiInfo(new Dictionary(), new List(), new List(), "etag", new RateLimit()), HttpStatusCode.OK, "content-type")); + } + + [Fact] + public void AllowsParametersOtherThanHeadersToBeNull() + { + new CachedResponse.V1(null, new Dictionary(), null, HttpStatusCode.OK, null); + } + + [Fact] + public void SetsProperties() + { + var body = new object(); + var headers = new Dictionary(); + var apiInfo = new ApiInfo(new Dictionary(), new List(), new List(), "etag", new RateLimit()); + const HttpStatusCode httpStatusCode = HttpStatusCode.OK; + const string contentType = "content-type"; + + var v1 = new CachedResponse.V1(body, headers, apiInfo, httpStatusCode, contentType); + + Assert.Equal(body, v1.Body); + Assert.Equal(headers, v1.Headers); + Assert.Equal(apiInfo, v1.ApiInfo); + Assert.Equal(httpStatusCode, v1.StatusCode); + Assert.Equal(contentType, v1.ContentType); + } + } + } + + } +} diff --git a/Octokit.Tests/Caching/CachingHttpClientTests.cs b/Octokit.Tests/Caching/CachingHttpClientTests.cs new file mode 100644 index 00000000..b5bcb69b --- /dev/null +++ b/Octokit.Tests/Caching/CachingHttpClientTests.cs @@ -0,0 +1,331 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using NSubstitute.ReturnsExtensions; +using Octokit.Caching; +using Octokit.Internal; +using Octokit.Tests.Helpers; +using Xunit; + +namespace Octokit.Tests.Caching +{ + public class CachingHttpClientTests + { + public class TheSendMethod + { + [Fact] + public void EnsuresNonNullArguments() + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + assert + Assert.ThrowsAsync(() => cachingHttpClient.Send(null, CancellationToken.None)); + } + + [Theory] + [MemberData(nameof(NonGetHttpMethods))] + public async Task CallsUnderlyingHttpClientMethodForNonGetRequests(HttpMethod method) + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + var request = Substitute.For(); + var cancellationToken = CancellationToken.None; + var expectedResponse = Substitute.For(); + + request.Method.Returns(method); + underlyingClient.Send(request, cancellationToken).Returns(expectedResponse); + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + var response = await cachingHttpClient.Send(request, CancellationToken.None); + + // assert + Assert.Equal(expectedResponse, response); + Assert.Empty(responseCache.ReceivedCalls()); + Assert.Single(underlyingClient.ReceivedCalls()); + underlyingClient.Received(1).Send(request, cancellationToken); + } + + public static IEnumerable NonGetHttpMethods() + { + yield return new object[] { HttpMethod.Delete }; + yield return new object[] { HttpMethod.Post }; + yield return new object[] { HttpMethod.Put }; + yield return new object[] { HttpMethod.Trace }; + yield return new object[] { HttpMethod.Options }; + yield return new object[] { HttpMethod.Head }; + } + + [Fact] + public async Task UsesCachedResponseIfEtagIsPresentAndGithubReturns304() + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + const string etag = "my-etag"; + var request = Substitute.For(); + request.Method.Returns(HttpMethod.Get); + request.Headers.Returns(new Dictionary()); + + var cachedResponse = Substitute.For(); + cachedResponse.Headers.Returns(new Dictionary { { "ETag", etag } }); + + var cachedV1Response = CachedResponse.V1.Create(cachedResponse); + var cancellationToken = CancellationToken.None; + + var githubResponse = Substitute.For(); + githubResponse.StatusCode.Returns(HttpStatusCode.NotModified); + + underlyingClient.Send(Arg.Is(req => req == request && req.Headers["If-None-Matched"] == etag), cancellationToken).ReturnsForAnyArgs(githubResponse); + responseCache.GetAsync(request).Returns(cachedV1Response); + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + var response = await cachingHttpClient.Send(request, cancellationToken); + + // assert + Assert.Equal(cachedV1Response.ToResponse(), response, new ResponseComparer()); + Assert.Single(underlyingClient.ReceivedCalls()); + underlyingClient.Received(1).Send(request, cancellationToken); + Assert.Single(responseCache.ReceivedCalls()); + responseCache.Received(1).GetAsync(request); + } + + [Theory] + [MemberData(nameof(NonNotModifiedHttpStatusCodesWithSetCacheFailure))] + public async Task UsesGithubResponseIfEtagIsPresentAndGithubReturnsNon304(HttpStatusCode httpStatusCode, bool setCacheThrows) + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + const string etag = "my-etag"; + var request = Substitute.For(); + request.Method.Returns(HttpMethod.Get); + request.Headers.Returns(new Dictionary()); + + var cachedResponse = Substitute.For(); + cachedResponse.Headers.Returns(new Dictionary { { "ETag", etag } }); + + var cachedV1Response = CachedResponse.V1.Create(cachedResponse); + var cancellationToken = CancellationToken.None; + + var githubResponse = Substitute.For(); + githubResponse.StatusCode.Returns(httpStatusCode); + + underlyingClient.Send(Arg.Is(req => req == request && req.Headers["If-None-Matched"] == etag), cancellationToken).ReturnsForAnyArgs(githubResponse); + responseCache.GetAsync(request).Returns(cachedV1Response); + if (setCacheThrows) + { + responseCache.SetAsync(request, Arg.Any()).ThrowsForAnyArgs(); + } + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + var response = await cachingHttpClient.Send(request, cancellationToken); + + // assert + Assert.Equal(githubResponse, response, new ResponseComparer()); + Assert.Single(underlyingClient.ReceivedCalls()); + underlyingClient.Received(1).Send(request, cancellationToken); + Assert.Equal(2, responseCache.ReceivedCalls().Count()); + responseCache.Received(1).GetAsync(request); + responseCache.Received(1).SetAsync(request, Arg.Is(v1 => new ResponseComparer().Equals(v1, CachedResponse.V1.Create(githubResponse)))); + } + + public static IEnumerable NonNotModifiedHttpStatusCodesWithSetCacheFailure() + { + foreach (var statusCode in Enum.GetValues(typeof(HttpStatusCode))) + { + if (statusCode.Equals(HttpStatusCode.NotModified)) continue; + yield return new[] { statusCode, true }; + yield return new[] { statusCode, false }; + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UsesGithubResponseIfCachedEntryIsNull(bool setCacheThrows) + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + var request = Substitute.For(); + request.Method.Returns(HttpMethod.Get); + request.Headers.Returns(new Dictionary()); + + var cancellationToken = CancellationToken.None; + + var githubResponse = Substitute.For(); + + underlyingClient.Send(Arg.Is(req => req == request && !req.Headers.Any()), cancellationToken).ReturnsForAnyArgs(githubResponse); + responseCache.GetAsync(request).ReturnsNull(); + if (setCacheThrows) + { + responseCache.SetAsync(request, Arg.Any()).ThrowsForAnyArgs(); + } + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + var response = await cachingHttpClient.Send(request, cancellationToken); + + // assert + Assert.Equal(githubResponse, response, new ResponseComparer()); + Assert.Single(underlyingClient.ReceivedCalls()); + underlyingClient.Received(1).Send(request, cancellationToken); + Assert.Equal(2, responseCache.ReceivedCalls().Count()); + responseCache.Received(1).GetAsync(request); + responseCache.Received(1).SetAsync(request, Arg.Is(v1 => new ResponseComparer().Equals(v1, CachedResponse.V1.Create(githubResponse)))); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task UsesGithubResponseIfGetCachedEntryThrows(bool setCacheThrows) + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + var request = Substitute.For(); + request.Method.Returns(HttpMethod.Get); + request.Headers.Returns(new Dictionary()); + + var cancellationToken = CancellationToken.None; + + var githubResponse = Substitute.For(); + + underlyingClient.Send(Arg.Is(req => req == request && !req.Headers.Any()), cancellationToken).ReturnsForAnyArgs(githubResponse); + responseCache.GetAsync(Args.Request).ThrowsForAnyArgs(); + if (setCacheThrows) + { + responseCache.SetAsync(request, Arg.Any()).ThrowsForAnyArgs(); + } + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + var response = await cachingHttpClient.Send(request, cancellationToken); + + // assert + Assert.Equal(githubResponse, response, new ResponseComparer()); + Assert.Single(underlyingClient.ReceivedCalls()); + underlyingClient.Received(1).Send(request, cancellationToken); + Assert.Equal(2, responseCache.ReceivedCalls().Count()); + responseCache.Received(1).GetAsync(request); + responseCache.Received(1).SetAsync(request, Arg.Is(v1 => new ResponseComparer().Equals(v1, CachedResponse.V1.Create(githubResponse)))); + } + + [Theory] + [InlineData(null, true)] + [InlineData(null, false)] + [InlineData("", true)] + [InlineData("", false)] + public async Task UsesGithubResponseIfCachedEntryEtagIsNullOrEmpty(string etag, bool setCacheThrows) + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + var request = Substitute.For(); + request.Method.Returns(HttpMethod.Get); + request.Headers.Returns(new Dictionary()); + + var cachedResponse = Substitute.For(); + cachedResponse.Headers.Returns(etag == null ? new Dictionary() : new Dictionary { { "ETag", etag } }); + + var cachedV1Response = CachedResponse.V1.Create(cachedResponse); + var cancellationToken = CancellationToken.None; + + var githubResponse = Substitute.For(); + + underlyingClient.Send(Arg.Is(req => req == request && !req.Headers.Any()), cancellationToken).ReturnsForAnyArgs(githubResponse); + responseCache.GetAsync(request).Returns(cachedV1Response); + if (setCacheThrows) + { + responseCache.SetAsync(request, Arg.Any()).ThrowsForAnyArgs(); + } + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + var response = await cachingHttpClient.Send(request, cancellationToken); + + // assert + Assert.Equal(githubResponse, response, new ResponseComparer()); + Assert.Single(underlyingClient.ReceivedCalls()); + underlyingClient.Received(1).Send(request, cancellationToken); + Assert.Equal(2, responseCache.ReceivedCalls().Count()); + responseCache.Received(1).GetAsync(request); + responseCache.Received(1).SetAsync(request, Arg.Is(v1 => new ResponseComparer().Equals(v1, CachedResponse.V1.Create(githubResponse)))); + } + } + + public class TheSetRequestTimeoutMethod + { + [Fact] + public void SetsRequestTimeoutForUnderlyingClient() + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + var timeout = TimeSpan.Zero; + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + cachingHttpClient.SetRequestTimeout(timeout); + + // assert + underlyingClient.Received(1).SetRequestTimeout(timeout); + } + } + + public class TheDisposeMethod + { + [Fact] + public void CallsDisposeForUnderlyingClient() + { + // arrange + var underlyingClient = Substitute.For(); + var responseCache = Substitute.For(); + + var cachingHttpClient = new CachingHttpClient(underlyingClient, responseCache); + + // act + cachingHttpClient.Dispose(); + + // assert + underlyingClient.Received(1).Dispose(); + } + } + + public class TheCtor + { + [Theory] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + public void EnsuresNonNullArguments(bool httpClientIsNull, bool responseCacheIsNull) + { + var httpClient = httpClientIsNull ? null : Substitute.For(); + var responseCache = responseCacheIsNull ? null : Substitute.For(); + + Assert.Throws(() => new CachingHttpClient(httpClient, responseCache)); + } + } + } +} diff --git a/Octokit.Tests/GitHubClientTests.cs b/Octokit.Tests/GitHubClientTests.cs index ccc2e2c4..9be895fa 100644 --- a/Octokit.Tests/GitHubClientTests.cs +++ b/Octokit.Tests/GitHubClientTests.cs @@ -6,6 +6,7 @@ using Xunit; using System.Collections.Generic; using System.Reflection; using System.Linq; +using Octokit.Caching; namespace Octokit.Tests { @@ -134,6 +135,47 @@ namespace Octokit.Tests } } + public class TheResponseCacheProperty + { + [Fact] + public void WhenSetWrapsExistingHttpClientWithCachingHttpClient() + { + var responseCache = Substitute.For(); + var client = new GitHubClient(new ProductHeaderValue("OctokitTests")); + Assert.IsType(client.Connection); + var existingConnection = (Connection) client.Connection; + var existingHttpClient = existingConnection._httpClient; + + client.ResponseCache = responseCache; + + Assert.Equal(existingConnection, client.Connection); + Assert.IsType(existingConnection._httpClient); + var cachingHttpClient = (CachingHttpClient) existingConnection._httpClient; + Assert.Equal(existingHttpClient, cachingHttpClient._httpClient); + Assert.Equal(responseCache, cachingHttpClient._responseCache); + } + + [Fact] + public void WhenResetWrapsUnderlyingHttpClientWithCachingHttpClient() + { + var responseCache = Substitute.For(); + + var client = new GitHubClient(new ProductHeaderValue("OctokitTests")); + Assert.IsType(client.Connection); + var existingConnection = (Connection) client.Connection; + var existingHttpClient = existingConnection._httpClient; + client.ResponseCache = Substitute.For(); + + client.ResponseCache = responseCache; + + Assert.Equal(existingConnection, client.Connection); + Assert.IsType(existingConnection._httpClient); + var cachingHttpClient = (CachingHttpClient) existingConnection._httpClient; + Assert.Equal(existingHttpClient, cachingHttpClient._httpClient); + Assert.Equal(responseCache, cachingHttpClient._responseCache); + } + } + public class TheLastApiInfoProperty { [Fact] diff --git a/Octokit.Tests/Helpers/ApiInfoComparer.cs b/Octokit.Tests/Helpers/ApiInfoComparer.cs new file mode 100644 index 00000000..d15403b7 --- /dev/null +++ b/Octokit.Tests/Helpers/ApiInfoComparer.cs @@ -0,0 +1,41 @@ +using System; +using System.Collections.Generic; + +namespace Octokit.Tests.Helpers +{ + public class ApiInfoComparer : IEqualityComparer + { + private static readonly CollectionComparer _stringCollectionComparer = new CollectionComparer(); + private static readonly StringKeyedDictionaryComparer _stringKeyedDictionaryComparer = new StringKeyedDictionaryComparer(); + private static readonly RateLimitComparer _rateLimitComparer = new RateLimitComparer(); + + public bool Equals(ApiInfo x, ApiInfo y) + { + if (ReferenceEquals(x, y)) return true; + if (ReferenceEquals(x, null)) return false; + if (ReferenceEquals(y, null)) return false; + if (x.GetType() != y.GetType()) return false; + + return _stringCollectionComparer.Equals(x.OauthScopes, y.OauthScopes) && + _stringCollectionComparer.Equals(x.AcceptedOauthScopes, y.AcceptedOauthScopes) && + x.Etag == y.Etag && + _stringKeyedDictionaryComparer.Equals(x.Links, y.Links) && + _rateLimitComparer.Equals(x.RateLimit, y.RateLimit) && + x.ServerTimeDifference.Equals(y.ServerTimeDifference); + } + + public int GetHashCode(ApiInfo obj) + { + unchecked + { + var hashCode = (obj.OauthScopes != null ? _stringCollectionComparer.GetHashCode(obj.OauthScopes) : 0); + hashCode = (hashCode * 397) ^ (obj.AcceptedOauthScopes != null ? _stringCollectionComparer.GetHashCode(obj.AcceptedOauthScopes) : 0); + hashCode = (hashCode * 397) ^ (obj.Etag != null ? obj.Etag.GetHashCode() : 0); + hashCode = (hashCode * 397) ^ (obj.Links != null ? _stringKeyedDictionaryComparer.GetHashCode(obj.Links) : 0); + hashCode = (hashCode * 397) ^ (obj.RateLimit != null ? _rateLimitComparer.GetHashCode(obj.RateLimit) : 0); + hashCode = (hashCode * 397) ^ obj.ServerTimeDifference.GetHashCode(); + return hashCode; + } + } + } +} diff --git a/Octokit.Tests/Helpers/CollectionComparer.cs b/Octokit.Tests/Helpers/CollectionComparer.cs new file mode 100644 index 00000000..3720885c --- /dev/null +++ b/Octokit.Tests/Helpers/CollectionComparer.cs @@ -0,0 +1,23 @@ +using System.Collections.Generic; +using System.Linq; + +namespace Octokit.Tests.Helpers +{ + public class CollectionComparer : IEqualityComparer> + { + public bool Equals(IReadOnlyCollection x, IReadOnlyCollection y) + { + if (ReferenceEquals(x, y)) return true; + if (ReferenceEquals(x, null)) return false; + if (ReferenceEquals(y, null)) return false; + if (x.GetType() != y.GetType()) return false; + + return x.Count == y.Count && x.Intersect(y).Count() == x.Count(); + } + + public int GetHashCode(IReadOnlyCollection obj) + { + return obj.Count; + } + } +} diff --git a/Octokit.Tests/Helpers/RateLimitComparer.cs b/Octokit.Tests/Helpers/RateLimitComparer.cs new file mode 100644 index 00000000..3f3eaa2b --- /dev/null +++ b/Octokit.Tests/Helpers/RateLimitComparer.cs @@ -0,0 +1,30 @@ +using System.Collections.Generic; + +namespace Octokit.Tests.Helpers +{ + public class RateLimitComparer : IEqualityComparer + { + public bool Equals(RateLimit x, RateLimit y) + { + if (ReferenceEquals(x, y)) return true; + if (ReferenceEquals(x, null)) return false; + if (ReferenceEquals(y, null)) return false; + if (x.GetType() != y.GetType()) return false; + + return x.Limit == y.Limit && + x.Remaining == y.Remaining && + x.ResetAsUtcEpochSeconds == y.ResetAsUtcEpochSeconds; + } + + public int GetHashCode(RateLimit obj) + { + unchecked + { + var hashCode = obj.Limit; + hashCode = (hashCode * 397) ^ obj.Remaining; + hashCode = (hashCode * 397) ^ obj.ResetAsUtcEpochSeconds.GetHashCode(); + return hashCode; + } + } + } +} diff --git a/Octokit.Tests/Helpers/ResponseComparer.cs b/Octokit.Tests/Helpers/ResponseComparer.cs new file mode 100644 index 00000000..ee3c2a8e --- /dev/null +++ b/Octokit.Tests/Helpers/ResponseComparer.cs @@ -0,0 +1,37 @@ +using System.Collections.Generic; + +namespace Octokit.Tests.Helpers +{ + public class ResponseComparer : IEqualityComparer + { + private static readonly StringKeyedDictionaryComparer _stringKeyedDictionaryComparer = new StringKeyedDictionaryComparer(); + private static readonly ApiInfoComparer _apiInfoComparer = new ApiInfoComparer(); + + public bool Equals(IResponse x, IResponse y) + { + if (ReferenceEquals(x, y)) return true; + if (ReferenceEquals(x, null)) return false; + if (ReferenceEquals(y, null)) return false; + if (x.GetType() != y.GetType()) return false; + + return Equals(x.Body, y.Body) && + _stringKeyedDictionaryComparer.Equals(x.Headers, y.Headers) && + _apiInfoComparer.Equals(x.ApiInfo, y.ApiInfo) && + x.StatusCode == y.StatusCode && + x.ContentType == y.ContentType; + } + + public int GetHashCode(IResponse obj) + { + unchecked + { + var hashCode = (obj.Body != null ? obj.Body.GetHashCode() : 0); + hashCode = (hashCode * 397) ^ (obj.Headers != null ? _stringKeyedDictionaryComparer.GetHashCode(obj.Headers) : 0); + hashCode = (hashCode * 397) ^ (obj.ApiInfo != null ? _apiInfoComparer.GetHashCode(obj.ApiInfo) : 0); + hashCode = (hashCode * 397) ^ (int)obj.StatusCode; + hashCode = (hashCode * 397) ^ (obj.ContentType != null ? obj.ContentType.GetHashCode() : 0); + return hashCode; + } + } + } +} diff --git a/Octokit.Tests/Helpers/StringKeyedDictionaryComparer.cs b/Octokit.Tests/Helpers/StringKeyedDictionaryComparer.cs new file mode 100644 index 00000000..f8b9309b --- /dev/null +++ b/Octokit.Tests/Helpers/StringKeyedDictionaryComparer.cs @@ -0,0 +1,23 @@ +using System.Collections.Generic; +using System.Linq; + +namespace Octokit.Tests.Helpers +{ + public class StringKeyedDictionaryComparer : IEqualityComparer> + { + public bool Equals(IReadOnlyDictionary x, IReadOnlyDictionary y) + { + if (ReferenceEquals(x, y)) return true; + if (ReferenceEquals(x, null)) return false; + if (ReferenceEquals(y, null)) return false; + if (x.GetType() != y.GetType()) return false; + + return x.Count == y.Count && x.Intersect(y).Count() == x.Count; + } + + public int GetHashCode(IReadOnlyDictionary obj) + { + return obj.Count; + } + } +} diff --git a/Octokit.Tests/Http/ConnectionTests.cs b/Octokit.Tests/Http/ConnectionTests.cs index ced0736d..c915386f 100644 --- a/Octokit.Tests/Http/ConnectionTests.cs +++ b/Octokit.Tests/Http/ConnectionTests.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Threading; using System.Threading.Tasks; using NSubstitute; +using Octokit.Caching; using Octokit.Internal; using Xunit; @@ -859,5 +860,47 @@ namespace Octokit.Tests.Http Assert.Equal(100, result.RateLimit.Limit); } } + + public class TheResponseCacheProperty + { + [Fact] + public void WhenSetWrapsExistingHttpClientWithCachingHttpClient() + { + var responseCache = Substitute.For(); + var httpClient = Substitute.For(); + var connection = new Connection(new ProductHeaderValue("OctokitTests"), + _exampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + + connection.ResponseCache = responseCache; + + Assert.IsType(connection._httpClient); + var cachingHttpClient = (CachingHttpClient) connection._httpClient; + Assert.Equal(httpClient, cachingHttpClient._httpClient); + Assert.Equal(responseCache, cachingHttpClient._responseCache); + } + + [Fact] + public void WhenResetWrapsUnderlyingHttpClientWithCachingHttpClient() + { + var responseCache = Substitute.For(); + var httpClient = Substitute.For(); + var connection = new Connection(new ProductHeaderValue("OctokitTests"), + _exampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + connection.ResponseCache = Substitute.For(); + + connection.ResponseCache = responseCache; + + Assert.IsType(connection._httpClient); + var cachingHttpClient = (CachingHttpClient) connection._httpClient; + Assert.Equal(httpClient, cachingHttpClient._httpClient); + Assert.Equal(responseCache, cachingHttpClient._responseCache); + } + } } } diff --git a/Octokit/Caching/CachedResponse.cs b/Octokit/Caching/CachedResponse.cs new file mode 100644 index 00000000..4d63371a --- /dev/null +++ b/Octokit/Caching/CachedResponse.cs @@ -0,0 +1,63 @@ +using System.Collections.Generic; +using System.Linq; +using System.Net; +using Octokit.Internal; + +namespace Octokit.Caching +{ + /// + /// When implementation details of changes: + /// + /// mark as Obsolete + /// create a V2 + /// update usages of to V2 + /// + /// + public static class CachedResponse + { + public sealed class V1 : IResponse + { + public V1(object body, IReadOnlyDictionary headers, ApiInfo apiInfo, HttpStatusCode statusCode, string contentType) + { + Ensure.ArgumentNotNull(headers, nameof(headers)); + + StatusCode = statusCode; + Body = body; + Headers = headers; + ApiInfo = apiInfo; + ContentType = contentType; + } + + /// + /// Raw response body. Typically a string, but when requesting images, it will be a byte array. + /// + public object Body { get; private set; } + /// + /// Information about the API. + /// + public IReadOnlyDictionary Headers { get; private set; } + /// + /// Information about the API response parsed from the response headers. + /// + public ApiInfo ApiInfo { get; internal set; } // This setter is internal for use in tests. + /// + /// The response status code. + /// + public HttpStatusCode StatusCode { get; private set; } + /// + /// The content type of the response. + /// + public string ContentType { get; private set; } + + internal Response ToResponse() => + new Response(StatusCode, Body, Headers.ToDictionary(kvp => kvp.Key, kvp => kvp.Value), ContentType); + + public static V1 Create(IResponse response) + { + Ensure.ArgumentNotNull(response, nameof(response)); + + return new V1(response.Body, response.Headers, response.ApiInfo, response.StatusCode, response.ContentType); + } + } + } +} diff --git a/Octokit/Caching/CachingHttpClient.cs b/Octokit/Caching/CachingHttpClient.cs new file mode 100644 index 00000000..059c1aef --- /dev/null +++ b/Octokit/Caching/CachingHttpClient.cs @@ -0,0 +1,86 @@ +using System; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using Octokit.Internal; + +namespace Octokit.Caching +{ + public sealed class CachingHttpClient : IHttpClient + { + internal readonly IHttpClient _httpClient; + internal readonly IResponseCache _responseCache; + + public CachingHttpClient(IHttpClient httpClient, IResponseCache responseCache) + { + Ensure.ArgumentNotNull(httpClient, nameof(httpClient)); + Ensure.ArgumentNotNull(responseCache, nameof(responseCache)); + + _httpClient = httpClient is CachingHttpClient cachingHttpClient ? cachingHttpClient._httpClient : httpClient; + _responseCache = responseCache; + } + + public async Task Send(IRequest request, CancellationToken cancellationToken) + { + Ensure.ArgumentNotNull(request, nameof(request)); + + if (request.Method != HttpMethod.Get) + { + return await _httpClient.Send(request, cancellationToken); + } + + var cachedResponse = await TryGetCachedResponse(request); + if (cachedResponse != null && !string.IsNullOrEmpty(cachedResponse.ApiInfo.Etag)) + { + request.Headers["If-None-Match"] = cachedResponse.ApiInfo.Etag; + var conditionalResponse = await _httpClient.Send(request, cancellationToken); + if (conditionalResponse.StatusCode == HttpStatusCode.NotModified) + { + return cachedResponse; + } + + TrySetCachedResponse(request, conditionalResponse); + return conditionalResponse; + } + + var response = await _httpClient.Send(request, cancellationToken); + TrySetCachedResponse(request, response); + return response; + } + + private async Task TryGetCachedResponse(IRequest request) + { + try + { + return (await _responseCache.GetAsync(request))?.ToResponse(); + } + catch (Exception) + { + return null; + } + } + + private async Task TrySetCachedResponse(IRequest request, IResponse response) + { + try + { + await _responseCache.SetAsync(request, CachedResponse.V1.Create(response)); + } + catch (Exception) + { + // ignored + } + } + + public void SetRequestTimeout(TimeSpan timeout) + { + _httpClient.SetRequestTimeout(timeout); + } + + public void Dispose() + { + _httpClient.Dispose(); + } + } +} diff --git a/Octokit/Caching/IResponseCache.cs b/Octokit/Caching/IResponseCache.cs new file mode 100644 index 00000000..0c2b8171 --- /dev/null +++ b/Octokit/Caching/IResponseCache.cs @@ -0,0 +1,12 @@ +using System.Threading.Tasks; +using Octokit.Internal; + +namespace Octokit.Caching +{ + public interface IResponseCache + { + Task GetAsync(IRequest request); + + Task SetAsync(IRequest request, CachedResponse.V1 cachedResponse); + } +} diff --git a/Octokit/GitHubClient.cs b/Octokit/GitHubClient.cs index 77c8dbba..367dbdf5 100644 --- a/Octokit/GitHubClient.cs +++ b/Octokit/GitHubClient.cs @@ -1,4 +1,5 @@ using System; +using Octokit.Caching; using Octokit.Internal; namespace Octokit @@ -163,6 +164,21 @@ namespace Octokit } } + /// + /// Convenience property for setting response cache. + /// + /// + /// Setting this property will wrap existing in . + /// + public IResponseCache ResponseCache + { + set + { + Ensure.ArgumentNotNull(value, nameof(value)); + Connection.ResponseCache = value; + } + } + /// /// The base address of the GitHub API. This defaults to https://api.github.com, /// but you can change it if needed (to talk to a GitHub:Enterprise server for instance). diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 6a9f065c..3ade4de9 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Reflection; using System.Threading; using System.Threading.Tasks; +using Octokit.Caching; using Octokit.Internal; namespace Octokit @@ -23,7 +24,7 @@ namespace Octokit readonly Authenticator _authenticator; readonly JsonHttpPipeline _jsonPipeline; - readonly IHttpClient _httpClient; + internal IHttpClient _httpClient; /// /// Creates a new connection instance used to make requests of the GitHub API. @@ -650,6 +651,21 @@ namespace Octokit } } + /// + /// Sets response cache used by the connection. + /// + /// + /// Setting this property will wrap existing in . + /// + public IResponseCache ResponseCache + { + set + { + Ensure.ArgumentNotNull(value, nameof(value)); + _httpClient = new CachingHttpClient(_httpClient, value); + } + } + async Task> GetHtml(IRequest request) { request.Headers.Add("Accept", AcceptHeaders.StableVersionHtml); diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index 3eacde04..1f0e052f 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Net; using System.Threading; using System.Threading.Tasks; +using Octokit.Caching; using Octokit.Internal; namespace Octokit @@ -302,7 +303,7 @@ namespace Octokit /// The type to map the response to /// URI endpoint to send request to /// The object to serialize as the body of the request - /// Specifies accept response media type + /// Specifies accept response media type Task> Delete(Uri uri, object data, string accepts); /// @@ -319,13 +320,21 @@ namespace Octokit /// Gets or sets the credentials used by the connection. /// /// - /// You can use this property if you only have a single hard-coded credential. Otherwise, pass in an - /// to the constructor. - /// Setting this property will change the to use + /// You can use this property if you only have a single hard-coded credential. Otherwise, pass in an + /// to the constructor. + /// Setting this property will change the to use /// the default with just these credentials. /// Credentials Credentials { get; set; } + /// + /// Sets response cache used by the connection. + /// + /// + /// Setting this property will wrap existing in . + /// + IResponseCache ResponseCache { set; } + /// /// Sets the timeout for the connection between the client and the server. ///