From f8ee4f94a143a1174235e2e211eec8639b29853f Mon Sep 17 00:00:00 2001 From: Mark Taylor Date: Fri, 31 Jul 2015 10:35:51 +0100 Subject: [PATCH 1/8] WIP checkin for getting last ApiInfo object --- Octokit.Reactive/IObservableGitHubClient.cs | 2 +- Octokit.Reactive/ObservableGitHubClient.cs | 6 ++ .../Clients/GitHubClientTests.cs | 32 ++++++++ .../Octokit.Tests.Integration.csproj | 1 + Octokit.Tests/GitHubClientTests.cs | 67 ++++++++++++++++ Octokit.Tests/Http/ConnectionTests.cs | 80 +++++++++++++++++++ Octokit/GitHubClient.cs | 6 ++ Octokit/Http/Connection.cs | 6 ++ Octokit/Http/HttpClientAdapter.cs | 12 ++- Octokit/Http/IApiInfo.cs | 20 +++++ Octokit/Http/IConnection.cs | 2 +- Octokit/Http/IHttpClient.cs | 2 +- Octokit/IGitHubClient.cs | 2 +- Octokit/Octokit-Mono.csproj | 1 + Octokit/Octokit-MonoAndroid.csproj | 3 +- Octokit/Octokit-Monotouch.csproj | 3 +- Octokit/Octokit-Portable.csproj | 1 + Octokit/Octokit-netcore45.csproj | 1 + Octokit/Octokit.csproj | 1 + 19 files changed, 241 insertions(+), 7 deletions(-) create mode 100644 Octokit.Tests.Integration/Clients/GitHubClientTests.cs create mode 100644 Octokit/Http/IApiInfo.cs diff --git a/Octokit.Reactive/IObservableGitHubClient.cs b/Octokit.Reactive/IObservableGitHubClient.cs index e6374a0c5e..1dc1bb3cba 100644 --- a/Octokit.Reactive/IObservableGitHubClient.cs +++ b/Octokit.Reactive/IObservableGitHubClient.cs @@ -1,6 +1,6 @@ namespace Octokit.Reactive { - public interface IObservableGitHubClient + public interface IObservableGitHubClient : IApiInfo { IConnection Connection { get; } diff --git a/Octokit.Reactive/ObservableGitHubClient.cs b/Octokit.Reactive/ObservableGitHubClient.cs index dc5c3d0326..0ddd519d2c 100644 --- a/Octokit.Reactive/ObservableGitHubClient.cs +++ b/Octokit.Reactive/ObservableGitHubClient.cs @@ -68,5 +68,11 @@ public IConnection Connection public IObservableNotificationsClient Notification { get; private set; } public IObservableGitDatabaseClient GitDatabase { get; private set; } public IObservableSearchClient Search { get; private set; } + + /// + /// Gets the latest API Info - this will be null if no API calls have been made + /// + /// representing the information returned as part of an Api call + public ApiInfo LastApiInfo { get { return _gitHubClient.Connection.LastApiInfo; } } } } diff --git a/Octokit.Tests.Integration/Clients/GitHubClientTests.cs b/Octokit.Tests.Integration/Clients/GitHubClientTests.cs new file mode 100644 index 0000000000..14ca889479 --- /dev/null +++ b/Octokit.Tests.Integration/Clients/GitHubClientTests.cs @@ -0,0 +1,32 @@ +using Octokit.Tests.Integration; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +public class GitHubClientTests +{ + public class TheLastApiInfoProperty + { + [IntegrationTest] + public async Task CanRetrieveLastApiInfo() + { + var github = Helper.GetAuthenticatedClient(); + + // Doesn't matter which API gets called + await github.Miscellaneous.GetRateLimits(); + + var result = github.LastApiInfo; + + //Assert.True(result.Links.Count > 0); + //Assert.True(result.AcceptedOauthScopes.Count > 0); + //Assert.True(result.OauthScopes.Count > 0); + //Assert.False(String.IsNullOrEmpty(result.Etag)); + Assert.True(result.RateLimit.Limit > 0); + Assert.True(result.RateLimit.Remaining > -1); + Assert.NotNull(result.RateLimit.Reset); + } + } +} diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index 87dd6b7a9d..45b1fb1e2e 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -75,6 +75,7 @@ + diff --git a/Octokit.Tests/GitHubClientTests.cs b/Octokit.Tests/GitHubClientTests.cs index 1b0fc24293..b02b4ac7aa 100644 --- a/Octokit.Tests/GitHubClientTests.cs +++ b/Octokit.Tests/GitHubClientTests.cs @@ -5,6 +5,7 @@ using Octokit.Internal; using Xunit; using Xunit.Extensions; +using System.Collections.Generic; namespace Octokit.Tests { @@ -104,5 +105,71 @@ public void IsRetrievedFromCredentialStore() Assert.Equal("bar", client.Credentials.Password); } } + + public class TheLastApiInfoProperty + { + [Fact] + public async Task ReturnsNullIfNew() + { + var connection = Substitute.For(); + connection.LastApiInfo.Returns((ApiInfo)null); + var client = new GitHubClient(connection); + + var result = client.LastApiInfo; + + Assert.Null(result); + + var temp = connection.Received(1).LastApiInfo; + } + + [Fact] + public async Task ReturnsObjectIfNotNew() + { + var apiInfo = new ApiInfo( + new Dictionary + { + { + "next", + new Uri("https://api.github.com/repos/rails/rails/issues?page=4&per_page=5") + }, + { + "last", + new Uri("https://api.github.com/repos/rails/rails/issues?page=131&per_page=5") + }, + { + "first", + new Uri("https://api.github.com/repos/rails/rails/issues?page=1&per_page=5") + }, + { + "prev", + new Uri("https://api.github.com/repos/rails/rails/issues?page=2&per_page=5") + } + }, + new List + { + "user", + }, + new List + { + "user", + "public_repo", + "repo", + "gist" + }, + "5634b0b187fd2e91e3126a75006cc4fa", + new RateLimit(100, 75, 1372700873) + ); + var connection = Substitute.For(); + connection.LastApiInfo.Returns(apiInfo); + var client = new GitHubClient(connection); + + var result = client.LastApiInfo; + + Assert.NotNull(result); + + var temp = connection.Received(1).LastApiInfo; + } + } + } } diff --git a/Octokit.Tests/Http/ConnectionTests.cs b/Octokit.Tests/Http/ConnectionTests.cs index 02f1601ee7..ad335e4409 100644 --- a/Octokit.Tests/Http/ConnectionTests.cs +++ b/Octokit.Tests/Http/ConnectionTests.cs @@ -564,5 +564,85 @@ public void CreatesConnectionWithBaseAddress() Assert.True(connection.UserAgent.StartsWith("OctokitTests (")); } } + + public class TheLastAPiInfoProperty + { + [Fact] + public async Task ReturnsNullIfNew() + { + var httpClient = Substitute.For(); + httpClient.LastApiInfo.Returns((ApiInfo)null); + var connection = new Connection(new ProductHeaderValue("OctokitTests"), + _exampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + + var result = connection.LastApiInfo; + + Assert.Null(result); + + var temp = httpClient.Received(1).LastApiInfo; + } + + [Fact] + public async Task ReturnsObjectIfNotNew() + { + var apiInfo = new ApiInfo( + new Dictionary + { + { + "next", + new Uri("https://api.github.com/repos/rails/rails/issues?page=4&per_page=5") + }, + { + "last", + new Uri("https://api.github.com/repos/rails/rails/issues?page=131&per_page=5") + }, + { + "first", + new Uri("https://api.github.com/repos/rails/rails/issues?page=1&per_page=5") + }, + { + "prev", + new Uri("https://api.github.com/repos/rails/rails/issues?page=2&per_page=5") + } + }, + new List + { + "user", + }, + new List + { + "user", + "public_repo", + "repo", + "gist" + }, + "5634b0b187fd2e91e3126a75006cc4fa", + new RateLimit(100, 75, 1372700873) + ); + + var httpClient = Substitute.For(); + httpClient.LastApiInfo.Returns(apiInfo); + var connection = new Connection(new ProductHeaderValue("OctokitTests"), + _exampleUri, + Substitute.For(), + httpClient, + Substitute.For()); + + var result = connection.LastApiInfo; + + // No point checking all of the values as they are tested elsewhere + // Just provde that the ApiInfo is populated + Assert.Equal(4, result.Links.Count); + Assert.Equal(1, result.OauthScopes.Count); + Assert.Equal(4, result.AcceptedOauthScopes.Count); + Assert.Equal("5634b0b187fd2e91e3126a75006cc4fa", result.Etag); + Assert.Equal(100, result.RateLimit.Limit); + + var temp = httpClient.Received(1).LastApiInfo; + } + } } } diff --git a/Octokit/GitHubClient.cs b/Octokit/GitHubClient.cs index d88a625720..b79640af75 100644 --- a/Octokit/GitHubClient.cs +++ b/Octokit/GitHubClient.cs @@ -100,6 +100,12 @@ public GitHubClient(IConnection connection) Deployment = new DeploymentsClient(apiConnection); } + /// + /// Gets the latest API Info - this will be null if no API calls have been made + /// + /// representing the information returned as part of an Api call + public ApiInfo LastApiInfo { get { return Connection.LastApiInfo; } } + /// /// Convenience property for getting and setting credentials. /// diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 846923d3d8..532c9e717a 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -136,6 +136,12 @@ public Connection( _jsonPipeline = new JsonHttpPipeline(); } + /// + /// Gets the latest API Info - this will be null if no API calls have been made + /// + /// representing the information returned as part of an Api call + public ApiInfo LastApiInfo { get { return _httpClient.LastApiInfo; } } + public Task> Get(Uri uri, IDictionary parameters, string accepts) { Ensure.ArgumentNotNull(uri, "uri"); diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 59dd87c025..bf33df39cb 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -28,6 +28,12 @@ public HttpClientAdapter(Func getHandler) _http = new HttpClient(new RedirectHandler { InnerHandler = getHandler() }); } + /// + /// Gets the latest API Info - this will be null if no API calls have been made + /// + /// representing the information returned as part of an Api call + public ApiInfo LastApiInfo { get; private set; } + /// /// Sends the specified request and returns a response. /// @@ -45,7 +51,11 @@ public async Task Send(IRequest request, CancellationToken cancellati // Make the request var responseMessage = await _http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest) .ConfigureAwait(false); - return await BuildResponse(responseMessage).ConfigureAwait(false); + var response = await BuildResponse(responseMessage).ConfigureAwait(false); + + LastApiInfo = response.ApiInfo; + + return response; } } diff --git a/Octokit/Http/IApiInfo.cs b/Octokit/Http/IApiInfo.cs new file mode 100644 index 0000000000..04a2cf892e --- /dev/null +++ b/Octokit/Http/IApiInfo.cs @@ -0,0 +1,20 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Octokit +{ + /// + /// Provides a property for the Last recorded API infomation + /// + public interface IApiInfo + { + /// + /// Gets the latest API Info - this will be null if no API calls have been made + /// + /// representing the information returned as part of an Api call + ApiInfo LastApiInfo { get; } + } +} diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index 7deec1ccb8..dd756b1096 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -10,7 +10,7 @@ namespace Octokit /// /// A connection for making HTTP requests against URI endpoints. /// - public interface IConnection + public interface IConnection : IApiInfo { /// /// Performs an asynchronous HTTP GET request that expects a containing HTML. diff --git a/Octokit/Http/IHttpClient.cs b/Octokit/Http/IHttpClient.cs index f554d750dc..db9de47a23 100644 --- a/Octokit/Http/IHttpClient.cs +++ b/Octokit/Http/IHttpClient.cs @@ -10,7 +10,7 @@ namespace Octokit.Internal /// /// Most folks won't ever need to swap this out. But if you're trying to run this on Windows Phone, you might. /// - public interface IHttpClient : IDisposable + public interface IHttpClient : IDisposable, IApiInfo { /// /// Sends the specified request and returns a response. diff --git a/Octokit/IGitHubClient.cs b/Octokit/IGitHubClient.cs index a828c83e42..1ae593cceb 100644 --- a/Octokit/IGitHubClient.cs +++ b/Octokit/IGitHubClient.cs @@ -5,7 +5,7 @@ namespace Octokit /// /// A Client for the GitHub API v3. You can read more about the api here: http://developer.github.com. /// - public interface IGitHubClient + public interface IGitHubClient : IApiInfo { /// /// Provides a client connection to make rest requests to HTTP endpoints. diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 64daa00563..4db1ff4c5e 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -399,6 +399,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 9002587a63..66103c5755 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -1,4 +1,4 @@ - + Debug @@ -415,6 +415,7 @@ + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 607ec59518..bde1203d5b 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -1,4 +1,4 @@ - + Debug @@ -408,6 +408,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 74f211dfd1..4080a4852b 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -398,6 +398,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 944bb68159..65c2b6f0d1 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -402,6 +402,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index 60a9607f7c..943d7bca20 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -84,6 +84,7 @@ + From a6a6fdf13f627988cda3b1cb2cd7b45b104dde24 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Fri, 31 Jul 2015 12:08:16 +0200 Subject: [PATCH 2/8] Renamed IApiInfo -> IApiInfoProvider --- Octokit.Reactive/IObservableGitHubClient.cs | 2 +- Octokit/Http/{IApiInfo.cs => IApiInfoProvider.cs} | 10 ++-------- Octokit/Http/IConnection.cs | 2 +- Octokit/Http/IHttpClient.cs | 2 +- Octokit/IGitHubClient.cs | 2 +- Octokit/Octokit-Mono.csproj | 2 +- Octokit/Octokit-MonoAndroid.csproj | 2 +- Octokit/Octokit-Monotouch.csproj | 2 +- Octokit/Octokit-Portable.csproj | 2 +- Octokit/Octokit-netcore45.csproj | 2 +- Octokit/Octokit.csproj | 2 +- 11 files changed, 12 insertions(+), 18 deletions(-) rename Octokit/Http/{IApiInfo.cs => IApiInfoProvider.cs} (70%) diff --git a/Octokit.Reactive/IObservableGitHubClient.cs b/Octokit.Reactive/IObservableGitHubClient.cs index 1dc1bb3cba..be317491fb 100644 --- a/Octokit.Reactive/IObservableGitHubClient.cs +++ b/Octokit.Reactive/IObservableGitHubClient.cs @@ -1,6 +1,6 @@ namespace Octokit.Reactive { - public interface IObservableGitHubClient : IApiInfo + public interface IObservableGitHubClient : IApiInfoProvider { IConnection Connection { get; } diff --git a/Octokit/Http/IApiInfo.cs b/Octokit/Http/IApiInfoProvider.cs similarity index 70% rename from Octokit/Http/IApiInfo.cs rename to Octokit/Http/IApiInfoProvider.cs index 04a2cf892e..eba8439a31 100644 --- a/Octokit/Http/IApiInfo.cs +++ b/Octokit/Http/IApiInfoProvider.cs @@ -1,15 +1,9 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; - -namespace Octokit +namespace Octokit { /// /// Provides a property for the Last recorded API infomation /// - public interface IApiInfo + public interface IApiInfoProvider { /// /// Gets the latest API Info - this will be null if no API calls have been made diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index dd756b1096..bde7885afb 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -10,7 +10,7 @@ namespace Octokit /// /// A connection for making HTTP requests against URI endpoints. /// - public interface IConnection : IApiInfo + public interface IConnection : IApiInfoProvider { /// /// Performs an asynchronous HTTP GET request that expects a containing HTML. diff --git a/Octokit/Http/IHttpClient.cs b/Octokit/Http/IHttpClient.cs index db9de47a23..f0ac5b09ae 100644 --- a/Octokit/Http/IHttpClient.cs +++ b/Octokit/Http/IHttpClient.cs @@ -10,7 +10,7 @@ namespace Octokit.Internal /// /// Most folks won't ever need to swap this out. But if you're trying to run this on Windows Phone, you might. /// - public interface IHttpClient : IDisposable, IApiInfo + public interface IHttpClient : IDisposable, IApiInfoProvider { /// /// Sends the specified request and returns a response. diff --git a/Octokit/IGitHubClient.cs b/Octokit/IGitHubClient.cs index 1ae593cceb..7db4a5554c 100644 --- a/Octokit/IGitHubClient.cs +++ b/Octokit/IGitHubClient.cs @@ -5,7 +5,7 @@ namespace Octokit /// /// A Client for the GitHub API v3. You can read more about the api here: http://developer.github.com. /// - public interface IGitHubClient : IApiInfo + public interface IGitHubClient : IApiInfoProvider { /// /// Provides a client connection to make rest requests to HTTP endpoints. diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 4db1ff4c5e..51aebfb91d 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -399,7 +399,7 @@ - + \ No newline at end of file diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 66103c5755..c84937fe4b 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -415,7 +415,7 @@ - + \ No newline at end of file diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index bde1203d5b..8cdea025f8 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -408,7 +408,7 @@ - + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 4080a4852b..5d043c7996 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -398,7 +398,7 @@ - + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index 65c2b6f0d1..623dbd10bd 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -402,7 +402,7 @@ - + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index 943d7bca20..ff9bcee33c 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -84,7 +84,7 @@ - + From 1acdd54704b020de10c2022bd3ed5dbc358249d9 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Fri, 31 Jul 2015 12:12:46 +0200 Subject: [PATCH 3/8] Removed IApiInfoProvider from IHttpClient --- Octokit/Http/Connection.cs | 3 ++- Octokit/Http/HttpClientAdapter.cs | 12 +----------- Octokit/Http/IHttpClient.cs | 2 +- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 532c9e717a..e373091501 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -140,7 +140,7 @@ public Connection( /// Gets the latest API Info - this will be null if no API calls have been made /// /// representing the information returned as part of an Api call - public ApiInfo LastApiInfo { get { return _httpClient.LastApiInfo; } } + public ApiInfo LastApiInfo { get; private set; } public Task> Get(Uri uri, IDictionary parameters, string accepts) { @@ -531,6 +531,7 @@ async Task RunRequest(IRequest request, CancellationToken cancellatio await _authenticator.Apply(request).ConfigureAwait(false); var response = await _httpClient.Send(request, cancellationToken).ConfigureAwait(false); HandleErrors(response); + LastApiInfo = response.ApiInfo; return response; } diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index bf33df39cb..59dd87c025 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -28,12 +28,6 @@ public HttpClientAdapter(Func getHandler) _http = new HttpClient(new RedirectHandler { InnerHandler = getHandler() }); } - /// - /// Gets the latest API Info - this will be null if no API calls have been made - /// - /// representing the information returned as part of an Api call - public ApiInfo LastApiInfo { get; private set; } - /// /// Sends the specified request and returns a response. /// @@ -51,11 +45,7 @@ public async Task Send(IRequest request, CancellationToken cancellati // Make the request var responseMessage = await _http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest) .ConfigureAwait(false); - var response = await BuildResponse(responseMessage).ConfigureAwait(false); - - LastApiInfo = response.ApiInfo; - - return response; + return await BuildResponse(responseMessage).ConfigureAwait(false); } } diff --git a/Octokit/Http/IHttpClient.cs b/Octokit/Http/IHttpClient.cs index f0ac5b09ae..f554d750dc 100644 --- a/Octokit/Http/IHttpClient.cs +++ b/Octokit/Http/IHttpClient.cs @@ -10,7 +10,7 @@ namespace Octokit.Internal /// /// Most folks won't ever need to swap this out. But if you're trying to run this on Windows Phone, you might. /// - public interface IHttpClient : IDisposable, IApiInfoProvider + public interface IHttpClient : IDisposable { /// /// Sends the specified request and returns a response. From bc5a14a8afa8fc89126adf146286ed4d491ffe38 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Fri, 31 Jul 2015 12:20:34 +0200 Subject: [PATCH 4/8] Fixed failing tests --- Octokit.Tests/Http/ConnectionTests.cs | 20 ++++++++++++++------ Octokit/Http/Response.cs | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Octokit.Tests/Http/ConnectionTests.cs b/Octokit.Tests/Http/ConnectionTests.cs index ad335e4409..254c598d72 100644 --- a/Octokit.Tests/Http/ConnectionTests.cs +++ b/Octokit.Tests/Http/ConnectionTests.cs @@ -4,8 +4,10 @@ using System.Linq; using System.Net; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using NSubstitute; +using NSubstitute.Core.Arguments; using Octokit.Internal; using Octokit.Tests.Helpers; using Xunit; @@ -571,7 +573,6 @@ public class TheLastAPiInfoProperty public async Task ReturnsNullIfNew() { var httpClient = Substitute.For(); - httpClient.LastApiInfo.Returns((ApiInfo)null); var connection = new Connection(new ProductHeaderValue("OctokitTests"), _exampleUri, Substitute.For(), @@ -581,8 +582,6 @@ public async Task ReturnsNullIfNew() var result = connection.LastApiInfo; Assert.Null(result); - - var temp = httpClient.Received(1).LastApiInfo; } [Fact] @@ -624,13 +623,24 @@ public async Task ReturnsObjectIfNotNew() ); var httpClient = Substitute.For(); - httpClient.LastApiInfo.Returns(apiInfo); + + // We really only care about the ApiInfo property... + var expectedResponse = new Response(HttpStatusCode.OK, null, new Dictionary(), "application/json") + { + ApiInfo = apiInfo + }; + + httpClient.Send(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(expectedResponse)); + var connection = new Connection(new ProductHeaderValue("OctokitTests"), _exampleUri, Substitute.For(), httpClient, Substitute.For()); + connection.Get(new Uri("https://example.com"), TimeSpan.MaxValue); + var result = connection.LastApiInfo; // No point checking all of the values as they are tested elsewhere @@ -640,8 +650,6 @@ public async Task ReturnsObjectIfNotNew() Assert.Equal(4, result.AcceptedOauthScopes.Count); Assert.Equal("5634b0b187fd2e91e3126a75006cc4fa", result.Etag); Assert.Equal(100, result.RateLimit.Limit); - - var temp = httpClient.Received(1).LastApiInfo; } } } diff --git a/Octokit/Http/Response.cs b/Octokit/Http/Response.cs index 89b2750b3d..c143cdddc7 100644 --- a/Octokit/Http/Response.cs +++ b/Octokit/Http/Response.cs @@ -43,7 +43,7 @@ public Response(HttpStatusCode statusCode, object body, IDictionary /// Information about the API response parsed from the response headers. /// - public ApiInfo ApiInfo { get; private set; } + public ApiInfo ApiInfo { get; internal set; } // This setter is internal for use in tests. /// /// The response status code. /// From ba365249a73aa440ff31d529da3875b899bb41bc Mon Sep 17 00:00:00 2001 From: Mark Taylor Date: Sun, 2 Aug 2015 16:55:39 +0100 Subject: [PATCH 5/8] Added locking for thread saftey to LastApiInfo --- Octokit/Http/Connection.cs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index e373091501..56ae749993 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -140,7 +140,25 @@ public Connection( /// Gets the latest API Info - this will be null if no API calls have been made /// /// representing the information returned as part of an Api call - public ApiInfo LastApiInfo { get; private set; } + public ApiInfo LastApiInfo + { + get + { + lock (LastApiInfoLocker) + { + return _lastApiInfo; + } + } + private set + { + lock (LastApiInfoLocker) + { + _lastApiInfo = value; + } + } + } + private ApiInfo _lastApiInfo; + private readonly object LastApiInfoLocker = new object(); public Task> Get(Uri uri, IDictionary parameters, string accepts) { @@ -530,8 +548,11 @@ async Task RunRequest(IRequest request, CancellationToken cancellatio request.Headers.Add("User-Agent", UserAgent); await _authenticator.Apply(request).ConfigureAwait(false); var response = await _httpClient.Send(request, cancellationToken).ConfigureAwait(false); + if (response != null) + { + LastApiInfo = response.ApiInfo; + } HandleErrors(response); - LastApiInfo = response.ApiInfo; return response; } From 5bd1f1d6c56fd32835edcddbd8da9241282b94cb Mon Sep 17 00:00:00 2001 From: Mark Taylor Date: Sun, 2 Aug 2015 18:16:11 +0100 Subject: [PATCH 6/8] Added further integration tests for LastApiInfo --- .../Clients/GitHubClientTests.cs | 67 ++++++++++++++++--- Octokit.Tests.Integration/Helper.cs | 10 ++- .../PersonalAccessTokenTestAttribute.cs | 30 +++++++++ .../Octokit.Tests.Integration.csproj | 1 + 4 files changed, 99 insertions(+), 9 deletions(-) create mode 100644 Octokit.Tests.Integration/Helpers/PersonalAccessTokenTestAttribute.cs diff --git a/Octokit.Tests.Integration/Clients/GitHubClientTests.cs b/Octokit.Tests.Integration/Clients/GitHubClientTests.cs index 14ca889479..9ce45f8b87 100644 --- a/Octokit.Tests.Integration/Clients/GitHubClientTests.cs +++ b/Octokit.Tests.Integration/Clients/GitHubClientTests.cs @@ -1,4 +1,5 @@ -using Octokit.Tests.Integration; +using Octokit; +using Octokit.Tests.Integration; using System; using System.Collections.Generic; using System.Linq; @@ -11,22 +12,72 @@ public class GitHubClientTests public class TheLastApiInfoProperty { [IntegrationTest] - public async Task CanRetrieveLastApiInfo() + public async Task CanRetrieveLastApiInfoWithEtag() { + // To check for etag, I'm using a new repository + // As per suggestion here -> https://github.com/octokit/octokit.net/pull/855#issuecomment-126966532 var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); - // Doesn't matter which API gets called - await github.Miscellaneous.GetRateLimits(); + var createdRepository = await github.Repository.Create(new NewRepository(repoName)); + + try + { + var result = github.LastApiInfo; + + Assert.True(result.Links.Count == 0); + Assert.True(result.AcceptedOauthScopes.Count > -1); + Assert.True(result.OauthScopes.Count > -1); + Assert.False(String.IsNullOrEmpty(result.Etag)); + Assert.True(result.RateLimit.Limit > 0); + Assert.True(result.RateLimit.Remaining > -1); + Assert.NotNull(result.RateLimit.Reset); + } + finally + { + Helper.DeleteRepo(createdRepository); + } + } + + [IntegrationTest] + public async Task CanRetrieveLastApiInfoWithLinks() + { + // To check for links, I'm doing a list of all contributors to the octokit.net project + // Adapted from suggestion here -> https://github.com/octokit/octokit.net/pull/855#issuecomment-126966532 + var github = Helper.GetAuthenticatedClient(); + + await github.Repository.GetAllContributors("octokit", "octokit.net"); + + var result = github.LastApiInfo; + + Assert.True(result.Links.Count > 0); + Assert.True(result.AcceptedOauthScopes.Count > -1); + Assert.True(result.OauthScopes.Count > -1); + Assert.False(String.IsNullOrEmpty(result.Etag)); + Assert.True(result.RateLimit.Limit > 0); + Assert.True(result.RateLimit.Remaining > -1); + Assert.NotNull(result.RateLimit.Reset); + } + + [PersonalAccessTokenTest] + public async Task CanRetrieveLastApiInfoAcceptedOauth() + { + // To check for OAuth & AcceptedOAuth I'm getting the octokit user + // Adapted from suggestion here -> https://github.com/octokit/octokit.net/pull/855#issuecomment-126966532 + var github = Helper.GetAuthenticatedClient(); + + await github.User.Get("octokit"); var result = github.LastApiInfo; - //Assert.True(result.Links.Count > 0); - //Assert.True(result.AcceptedOauthScopes.Count > 0); - //Assert.True(result.OauthScopes.Count > 0); - //Assert.False(String.IsNullOrEmpty(result.Etag)); + Assert.True(result.Links.Count == 0); + Assert.True(result.AcceptedOauthScopes.Count > 0); + Assert.True(result.OauthScopes.Count > 0); + Assert.False(String.IsNullOrEmpty(result.Etag)); Assert.True(result.RateLimit.Limit > 0); Assert.True(result.RateLimit.Remaining > -1); Assert.NotNull(result.RateLimit.Reset); } + } } diff --git a/Octokit.Tests.Integration/Helper.cs b/Octokit.Tests.Integration/Helper.cs index 6959652097..8e623528d8 100644 --- a/Octokit.Tests.Integration/Helper.cs +++ b/Octokit.Tests.Integration/Helper.cs @@ -12,7 +12,7 @@ public static class Helper var githubUsername = Environment.GetEnvironmentVariable("OCTOKIT_GITHUBUSERNAME"); UserName = githubUsername; Organization = Environment.GetEnvironmentVariable("OCTOKIT_GITHUBORGANIZATION"); - + var githubToken = Environment.GetEnvironmentVariable("OCTOKIT_OAUTHTOKEN"); if (githubToken != null) @@ -52,6 +52,14 @@ static Helper() public static Credentials ApplicationCredentials { get { return _oauthApplicationCredentials.Value; } } + public static bool IsUsingToken + { + get + { + return !String.IsNullOrWhiteSpace(Environment.GetEnvironmentVariable("OCTOKIT_OAUTHTOKEN")); + } + } + public static bool IsPaidAccount { get diff --git a/Octokit.Tests.Integration/Helpers/PersonalAccessTokenTestAttribute.cs b/Octokit.Tests.Integration/Helpers/PersonalAccessTokenTestAttribute.cs new file mode 100644 index 0000000000..0fd107ec66 --- /dev/null +++ b/Octokit.Tests.Integration/Helpers/PersonalAccessTokenTestAttribute.cs @@ -0,0 +1,30 @@ +using System.Collections.Generic; +using System.Linq; +using Xunit; +using Xunit.Abstractions; +using Xunit.Sdk; + +namespace Octokit.Tests.Integration +{ + public class PersonalAccessTokenTestDiscoverer : IXunitTestCaseDiscoverer + { + readonly IMessageSink diagnosticMessageSink; + + public PersonalAccessTokenTestDiscoverer(IMessageSink diagnosticMessageSink) + { + this.diagnosticMessageSink = diagnosticMessageSink; + } + + public IEnumerable Discover(ITestFrameworkDiscoveryOptions discoveryOptions, ITestMethod testMethod, IAttributeInfo factAttribute) + { + return Helper.IsUsingToken + ? new[] { new XunitTestCase(diagnosticMessageSink, discoveryOptions.MethodDisplayOrDefault(), testMethod) } + : Enumerable.Empty(); + } + } + + [XunitTestCaseDiscoverer("Octokit.Tests.Integration.PersonalAccessTokenTestDiscoverer", "Octokit.Tests.Integration")] + public class PersonalAccessTokenTestAttribute : FactAttribute + { + } +} diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index 45b1fb1e2e..f6154d2829 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -102,6 +102,7 @@ + From b2c7e1c2a7bf2b481c989dfa78f77fdfbd375db7 Mon Sep 17 00:00:00 2001 From: Mark Taylor Date: Sun, 16 Aug 2015 21:27:26 +0100 Subject: [PATCH 7/8] Chnage LastApiInfo to GetLastApiInfo with cloned version --- Octokit.Reactive/ObservableGitHubClient.cs | 5 +- .../Clients/GitHubClientTests.cs | 6 +- Octokit.Tests/GitHubClientTests.cs | 12 +-- Octokit.Tests/Http/ApiInfoTests.cs | 100 ++++++++++++++++++ Octokit.Tests/Http/ConnectionTests.cs | 4 +- Octokit.Tests/Http/RateLimitTests.cs | 24 +++++ Octokit.Tests/Octokit.Tests.csproj | 1 + Octokit/GitHubClient.cs | 5 +- Octokit/Helpers/CollectionExtensions.cs | 36 ++++++- Octokit/Http/ApiInfo.cs | 20 ++++ Octokit/Http/Connection.cs | 27 ++--- Octokit/Http/IApiInfoProvider.cs | 3 +- Octokit/Http/RateLimit.cs | 14 +++ 13 files changed, 224 insertions(+), 33 deletions(-) create mode 100644 Octokit.Tests/Http/ApiInfoTests.cs diff --git a/Octokit.Reactive/ObservableGitHubClient.cs b/Octokit.Reactive/ObservableGitHubClient.cs index 0ddd519d2c..4970eebc9c 100644 --- a/Octokit.Reactive/ObservableGitHubClient.cs +++ b/Octokit.Reactive/ObservableGitHubClient.cs @@ -73,6 +73,9 @@ public IConnection Connection /// Gets the latest API Info - this will be null if no API calls have been made /// /// representing the information returned as part of an Api call - public ApiInfo LastApiInfo { get { return _gitHubClient.Connection.LastApiInfo; } } + public ApiInfo GetLastApiInfo() + { + return _gitHubClient.Connection.GetLastApiInfo(); + } } } diff --git a/Octokit.Tests.Integration/Clients/GitHubClientTests.cs b/Octokit.Tests.Integration/Clients/GitHubClientTests.cs index 9ce45f8b87..aa7c6fcebb 100644 --- a/Octokit.Tests.Integration/Clients/GitHubClientTests.cs +++ b/Octokit.Tests.Integration/Clients/GitHubClientTests.cs @@ -23,7 +23,7 @@ public async Task CanRetrieveLastApiInfoWithEtag() try { - var result = github.LastApiInfo; + var result = github.GetLastApiInfo(); Assert.True(result.Links.Count == 0); Assert.True(result.AcceptedOauthScopes.Count > -1); @@ -48,7 +48,7 @@ public async Task CanRetrieveLastApiInfoWithLinks() await github.Repository.GetAllContributors("octokit", "octokit.net"); - var result = github.LastApiInfo; + var result = github.GetLastApiInfo(); Assert.True(result.Links.Count > 0); Assert.True(result.AcceptedOauthScopes.Count > -1); @@ -68,7 +68,7 @@ public async Task CanRetrieveLastApiInfoAcceptedOauth() await github.User.Get("octokit"); - var result = github.LastApiInfo; + var result = github.GetLastApiInfo(); Assert.True(result.Links.Count == 0); Assert.True(result.AcceptedOauthScopes.Count > 0); diff --git a/Octokit.Tests/GitHubClientTests.cs b/Octokit.Tests/GitHubClientTests.cs index b02b4ac7aa..d3f8cb797d 100644 --- a/Octokit.Tests/GitHubClientTests.cs +++ b/Octokit.Tests/GitHubClientTests.cs @@ -112,14 +112,14 @@ public class TheLastApiInfoProperty public async Task ReturnsNullIfNew() { var connection = Substitute.For(); - connection.LastApiInfo.Returns((ApiInfo)null); + connection.GetLastApiInfo().Returns((ApiInfo)null); var client = new GitHubClient(connection); - var result = client.LastApiInfo; + var result = client.GetLastApiInfo(); Assert.Null(result); - var temp = connection.Received(1).LastApiInfo; + var temp = connection.Received(1).GetLastApiInfo(); } [Fact] @@ -160,14 +160,14 @@ public async Task ReturnsObjectIfNotNew() new RateLimit(100, 75, 1372700873) ); var connection = Substitute.For(); - connection.LastApiInfo.Returns(apiInfo); + connection.GetLastApiInfo().Returns(apiInfo); var client = new GitHubClient(connection); - var result = client.LastApiInfo; + var result = client.GetLastApiInfo(); Assert.NotNull(result); - var temp = connection.Received(1).LastApiInfo; + var temp = connection.Received(1).GetLastApiInfo(); } } diff --git a/Octokit.Tests/Http/ApiInfoTests.cs b/Octokit.Tests/Http/ApiInfoTests.cs new file mode 100644 index 0000000000..5971c58f31 --- /dev/null +++ b/Octokit.Tests/Http/ApiInfoTests.cs @@ -0,0 +1,100 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Octokit.Tests.Http +{ + public class ApiInfoTests + { + public class TheMethods + { + [Fact] + public void CanClone() + { + var original = new ApiInfo( + new Dictionary + { + { + "next", + new Uri("https://api.github.com/repos/rails/rails/issues?page=4&per_page=5") + }, + { + "last", + new Uri("https://api.github.com/repos/rails/rails/issues?page=131&per_page=5") + }, + { + "first", + new Uri("https://api.github.com/repos/rails/rails/issues?page=1&per_page=5") + }, + { + "prev", + new Uri("https://api.github.com/repos/rails/rails/issues?page=2&per_page=5") + } + }, + new List + { + "user", + }, + new List + { + "user", + "public_repo", + "repo", + "gist" + }, + "5634b0b187fd2e91e3126a75006cc4fa", + new RateLimit(100, 75, 1372700873) + ); + + var clone = original.Clone(); + + // Note the use of Assert.NotSame tests for value types - this should continue to test should the underlying + // model are changed to Object types + Assert.NotSame(original, clone); + + Assert.Equal(original.Etag, clone.Etag); + Assert.NotSame(original.Etag, clone.Etag); + + Assert.Equal(original.AcceptedOauthScopes.Count, clone.AcceptedOauthScopes.Count); + Assert.NotSame(original.AcceptedOauthScopes, clone.AcceptedOauthScopes); + for (int i = 0; i < original.AcceptedOauthScopes.Count; i++) + { + Assert.Equal(original.AcceptedOauthScopes[i], clone.AcceptedOauthScopes[i]); + Assert.NotSame(original.AcceptedOauthScopes[i], clone.AcceptedOauthScopes[i]); + } + + Assert.Equal(original.Links.Count, clone.Links.Count); + Assert.NotSame(original.Links, clone.Links); + for (int i = 0; i < original.Links.Count; i++) + { + Assert.Equal(original.Links.Keys.ToArray()[i], clone.Links.Keys.ToArray()[i]); + Assert.NotSame(original.Links.Keys.ToArray()[i], clone.Links.Keys.ToArray()[i]); + Assert.Equal(original.Links.Values.ToArray()[i].ToString(), clone.Links.Values.ToArray()[i].ToString()); + Assert.NotSame(original.Links.Values.ToArray()[i], clone.Links.Values.ToArray()[i]); + } + + Assert.Equal(original.OauthScopes.Count, clone.OauthScopes.Count); + Assert.NotSame(original.OauthScopes, clone.OauthScopes); + for (int i = 0; i < original.OauthScopes.Count; i++) + { + Assert.Equal(original.OauthScopes[i], clone.OauthScopes[i]); + Assert.NotSame(original.OauthScopes[i], clone.OauthScopes[i]); + } + + Assert.NotSame(original.RateLimit, clone.RateLimit); + Assert.Equal(original.RateLimit.Limit, clone.RateLimit.Limit); + Assert.NotSame(original.RateLimit.Limit, clone.RateLimit.Limit); + Assert.Equal(original.RateLimit.Remaining, clone.RateLimit.Remaining); + Assert.NotSame(original.RateLimit.Remaining, clone.RateLimit.Remaining); + Assert.Equal(original.RateLimit.ResetAsUtcEpochSeconds, clone.RateLimit.ResetAsUtcEpochSeconds); + Assert.NotSame(original.RateLimit.ResetAsUtcEpochSeconds, clone.RateLimit.ResetAsUtcEpochSeconds); + Assert.Equal(original.RateLimit.Reset, clone.RateLimit.Reset); + Assert.NotSame(original.RateLimit.Reset, clone.RateLimit.Reset); + } + } + + } +} diff --git a/Octokit.Tests/Http/ConnectionTests.cs b/Octokit.Tests/Http/ConnectionTests.cs index cf310db630..05c4eae1ca 100644 --- a/Octokit.Tests/Http/ConnectionTests.cs +++ b/Octokit.Tests/Http/ConnectionTests.cs @@ -630,7 +630,7 @@ public async Task ReturnsNullIfNew() httpClient, Substitute.For()); - var result = connection.LastApiInfo; + var result = connection.GetLastApiInfo(); Assert.Null(result); } @@ -692,7 +692,7 @@ public async Task ReturnsObjectIfNotNew() connection.Get(new Uri("https://example.com"), TimeSpan.MaxValue); - var result = connection.LastApiInfo; + var result = connection.GetLastApiInfo(); // No point checking all of the values as they are tested elsewhere // Just provde that the ApiInfo is populated diff --git a/Octokit.Tests/Http/RateLimitTests.cs b/Octokit.Tests/Http/RateLimitTests.cs index 3b0db94424..9473d05e63 100644 --- a/Octokit.Tests/Http/RateLimitTests.cs +++ b/Octokit.Tests/Http/RateLimitTests.cs @@ -109,6 +109,30 @@ public void EnsuresHeadersNotNull() { Assert.Throws(() => new RateLimit(null)); } + + } + + public class TheMethods + { + [Fact] + public void CanClone() + { + var original = new RateLimit(100, 42, 1372700873); + + var clone = original.Clone(); + + // Note the use of Assert.NotSame tests for value types - this should continue to test should the underlying + // model are changed to Object types + Assert.NotSame(original, clone); + Assert.Equal(original.Limit, clone.Limit); + Assert.NotSame(original.Limit, clone.Limit); + Assert.Equal(original.Remaining, clone.Remaining); + Assert.NotSame(original.Remaining, clone.Remaining); + Assert.Equal(original.ResetAsUtcEpochSeconds, clone.ResetAsUtcEpochSeconds); + Assert.NotSame(original.ResetAsUtcEpochSeconds, clone.ResetAsUtcEpochSeconds); + Assert.Equal(original.Reset, clone.Reset); + Assert.NotSame(original.Reset, clone.Reset); + } } } } \ No newline at end of file diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index c9ae7fde8c..3c5335c573 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -130,6 +130,7 @@ + Code diff --git a/Octokit/GitHubClient.cs b/Octokit/GitHubClient.cs index b79640af75..6115f282a0 100644 --- a/Octokit/GitHubClient.cs +++ b/Octokit/GitHubClient.cs @@ -104,7 +104,10 @@ public GitHubClient(IConnection connection) /// Gets the latest API Info - this will be null if no API calls have been made /// /// representing the information returned as part of an Api call - public ApiInfo LastApiInfo { get { return Connection.LastApiInfo; } } + public ApiInfo GetLastApiInfo() + { + return Connection.GetLastApiInfo(); + } /// /// Convenience property for getting and setting credentials. diff --git a/Octokit/Helpers/CollectionExtensions.cs b/Octokit/Helpers/CollectionExtensions.cs index 15cc101bef..3481918c1d 100644 --- a/Octokit/Helpers/CollectionExtensions.cs +++ b/Octokit/Helpers/CollectionExtensions.cs @@ -1,4 +1,6 @@ -using System.Collections.Generic; +using System; +using System.Linq; +using System.Collections.Generic; namespace Octokit { @@ -11,5 +13,37 @@ public static TValue SafeGet(this IReadOnlyDictionary Clone(this IReadOnlyList input) + { + List output = null; + if (input == null) + return output; + + output = new List(); + + foreach (var item in input) + { + output.Add(new String(item.ToCharArray())); + } + + return output; + } + + public static IDictionary Clone(this IReadOnlyDictionary input) + { + Dictionary output = null; + if (input == null) + return output; + + output = new Dictionary(); + + foreach (var item in input) + { + output.Add(new String(item.Key.ToCharArray()), new Uri(item.Value.ToString())); + } + + return output; + } } } diff --git a/Octokit/Http/ApiInfo.cs b/Octokit/Http/ApiInfo.cs index 54550cb76e..345cdda3df 100644 --- a/Octokit/Http/ApiInfo.cs +++ b/Octokit/Http/ApiInfo.cs @@ -51,5 +51,25 @@ public ApiInfo(IDictionary links, /// Information about the API rate limit /// public RateLimit RateLimit { get; private set; } + + /// + /// Allows you to clone ApiInfo + /// + /// A clone of + public ApiInfo Clone() + { + // Seem to have to do this to pass a whole bunch of tests (for example Octokit.Tests.Clients.EventsClientTests.DeserializesCommitCommentEventCorrectly) + // I believe this has something to do with the Mocking framework. + if (this == null || this.Links == null || this.OauthScopes == null || this.RateLimit == null || this.Etag == null) + return null; + + return new ApiInfo(this.Links.Clone(), + this.OauthScopes.Clone(), + this.AcceptedOauthScopes.Clone(), + new String(this.Etag.ToCharArray()), + this.RateLimit.Clone()); + } + + } } diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 56ae749993..5b086ca348 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -140,25 +140,15 @@ public Connection( /// Gets the latest API Info - this will be null if no API calls have been made /// /// representing the information returned as part of an Api call - public ApiInfo LastApiInfo - { - get - { - lock (LastApiInfoLocker) - { - return _lastApiInfo; - } - } - private set - { - lock (LastApiInfoLocker) - { - _lastApiInfo = value; - } - } + public ApiInfo GetLastApiInfo() + { + // We've choosen to not wrap the _lastApiInfo in a lock. Originally the code was returning a reference - so there was a danger of + // on thread writing to the object while another was reading. Now we are cloning the ApiInfo on request - thus removing the need (or overhead) + // of putting locks in place. + // See https://github.com/octokit/octokit.net/pull/855#discussion_r36774884 + return _lastApiInfo == null ? null : _lastApiInfo.Clone(); } private ApiInfo _lastApiInfo; - private readonly object LastApiInfoLocker = new object(); public Task> Get(Uri uri, IDictionary parameters, string accepts) { @@ -550,7 +540,8 @@ async Task RunRequest(IRequest request, CancellationToken cancellatio var response = await _httpClient.Send(request, cancellationToken).ConfigureAwait(false); if (response != null) { - LastApiInfo = response.ApiInfo; + // Use the clone method to avoid keeping hold of the original (just in case it effect the lifetime of the whole response + _lastApiInfo = response.ApiInfo.Clone(); } HandleErrors(response); return response; diff --git a/Octokit/Http/IApiInfoProvider.cs b/Octokit/Http/IApiInfoProvider.cs index eba8439a31..c0b5dfb987 100644 --- a/Octokit/Http/IApiInfoProvider.cs +++ b/Octokit/Http/IApiInfoProvider.cs @@ -9,6 +9,7 @@ public interface IApiInfoProvider /// Gets the latest API Info - this will be null if no API calls have been made /// /// representing the information returned as part of an Api call - ApiInfo LastApiInfo { get; } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate"), System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] + ApiInfo GetLastApiInfo(); } } diff --git a/Octokit/Http/RateLimit.cs b/Octokit/Http/RateLimit.cs index 6d30371164..e55e67b9d6 100644 --- a/Octokit/Http/RateLimit.cs +++ b/Octokit/Http/RateLimit.cs @@ -99,5 +99,19 @@ internal string DebuggerDisplay } } + /// + /// Allows you to clone RateLimit + /// + /// A clone of + public RateLimit Clone() + { + var clone = new RateLimit(); + clone.Limit = this.Limit; + clone.Remaining = this.Remaining; + clone.ResetAsUtcEpochSeconds = this.ResetAsUtcEpochSeconds; + + return clone; + } + } } From 07d6a8ad609f60890b7e469b12a5c398af2c45f9 Mon Sep 17 00:00:00 2001 From: Mark Taylor Date: Fri, 28 Aug 2015 15:25:32 +0100 Subject: [PATCH 8/8] Minor changes based on feedback --- Octokit/Http/ApiInfo.cs | 2 +- Octokit/Http/RateLimit.cs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Octokit/Http/ApiInfo.cs b/Octokit/Http/ApiInfo.cs index 345cdda3df..9a47f70de8 100644 --- a/Octokit/Http/ApiInfo.cs +++ b/Octokit/Http/ApiInfo.cs @@ -60,7 +60,7 @@ public ApiInfo Clone() { // Seem to have to do this to pass a whole bunch of tests (for example Octokit.Tests.Clients.EventsClientTests.DeserializesCommitCommentEventCorrectly) // I believe this has something to do with the Mocking framework. - if (this == null || this.Links == null || this.OauthScopes == null || this.RateLimit == null || this.Etag == null) + if (this.Links == null || this.OauthScopes == null || this.RateLimit == null || this.Etag == null) return null; return new ApiInfo(this.Links.Clone(), diff --git a/Octokit/Http/RateLimit.cs b/Octokit/Http/RateLimit.cs index e55e67b9d6..6a3950eecc 100644 --- a/Octokit/Http/RateLimit.cs +++ b/Octokit/Http/RateLimit.cs @@ -105,12 +105,12 @@ internal string DebuggerDisplay /// A clone of public RateLimit Clone() { - var clone = new RateLimit(); - clone.Limit = this.Limit; - clone.Remaining = this.Remaining; - clone.ResetAsUtcEpochSeconds = this.ResetAsUtcEpochSeconds; - - return clone; + return new RateLimit + { + Limit = this.Limit, + Remaining = this.Remaining, + ResetAsUtcEpochSeconds = this.ResetAsUtcEpochSeconds + }; } }