From 0e74097ddef31ff18cffaff705a255c532a470f5 Mon Sep 17 00:00:00 2001 From: Haacked Date: Fri, 7 Aug 2015 10:41:02 -0700 Subject: [PATCH 1/5] Handle NoContent response for queued operations We didn't handle the 204 response properly for the `StatisticsClient`. Now we do. Fixes #836 --- .../Clients/StatisticsClientTests.cs | 31 ++++++++++++------- .../Clients/StatisticsClientTests.cs | 25 ++++++++++++--- Octokit.Tests/Http/ApiConnectionTests.cs | 16 ++++++++++ Octokit/Clients/IStatisticsClient.cs | 4 +-- Octokit/Clients/StatisticsClient.cs | 9 ++++-- Octokit/Http/ApiConnection.cs | 2 ++ Octokit/Http/IApiConnection.cs | 4 +-- 7 files changed, 68 insertions(+), 23 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/StatisticsClientTests.cs b/Octokit.Tests.Integration/Clients/StatisticsClientTests.cs index ce6288682e..a6b21bc5bf 100644 --- a/Octokit.Tests.Integration/Clients/StatisticsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/StatisticsClientTests.cs @@ -1,5 +1,4 @@ using System.Linq; -using System.Net.Http.Headers; using System.Threading.Tasks; using Xunit; @@ -18,21 +17,31 @@ public StatisticsClientTests() } [IntegrationTest] - public async Task CanCreateAndRetrieveCommit() + public async Task CanCreateAndRetrieveContributors() { var repository = await CreateRepository(); await CommitToRepository(repository); var contributors = await _client.Repository.Statistics.GetContributors(repository.Owner, repository.Name); Assert.NotNull(contributors); - Assert.True(contributors.Count() == 1); + Assert.Equal(1, contributors.Count); var soleContributor = contributors.First(); Assert.NotNull(soleContributor.Author); Assert.True(soleContributor.Author.Login == repository.Owner); - Assert.True(soleContributor.Weeks.Count() == 1); - Assert.True(soleContributor.Total == 1); + Assert.Equal(1, soleContributor.Weeks.Count); + Assert.Equal(1, soleContributor.Total); + } + + [IntegrationTest] + public async Task CanCreateAndRetrieveEmptyContributors() + { + var repository = await CreateRepository(autoInit: false); + var contributors = await _client.Repository.Statistics.GetContributors(repository.Owner, repository.Name); + + Assert.NotNull(contributors); + Assert.Empty(contributors); } [IntegrationTest] @@ -42,10 +51,10 @@ public async Task CanGetCommitActivityForTheLastYear() await CommitToRepository(repository); var commitActivities = await _client.Repository.Statistics.GetCommitActivity(repository.Owner, repository.Name); Assert.NotNull(commitActivities); - Assert.True(commitActivities.Activity.Count() == 52); + Assert.Equal(52, commitActivities.Activity.Count); var thisWeek = commitActivities.Activity.Last(); - Assert.True(thisWeek.Total == 1); + Assert.Equal(1, thisWeek.Total); Assert.NotNull(thisWeek.Days); } @@ -65,9 +74,7 @@ public async Task CanGetParticipationStatistics() var repository = await CreateRepository(); await CommitToRepository(repository); var weeklyCommitCounts = await _client.Repository.Statistics.GetParticipation(repository.Owner, repository.Name); - Assert.NotNull(weeklyCommitCounts); - Assert.NotNull(weeklyCommitCounts.All); - Assert.NotNull(weeklyCommitCounts.Owner); + Assert.Equal(52, weeklyCommitCounts.All.Count); } [IntegrationTest] @@ -80,10 +87,10 @@ public async Task CanGetPunchCardForRepository() Assert.NotNull(punchCard.PunchPoints); } - async Task CreateRepository() + async Task CreateRepository(bool autoInit = true) { var repoName = Helper.MakeNameWithTimestamp("public-repo"); - var repository = await _client.Repository.Create(new NewRepository(repoName) { AutoInit = true }); + var repository = await _client.Repository.Create(new NewRepository(repoName) { AutoInit = autoInit }); return new RepositorySummary { Owner = repository.Owner.Login, diff --git a/Octokit.Tests/Clients/StatisticsClientTests.cs b/Octokit.Tests/Clients/StatisticsClientTests.cs index 9a1aba7662..102647a051 100644 --- a/Octokit.Tests/Clients/StatisticsClientTests.cs +++ b/Octokit.Tests/Clients/StatisticsClientTests.cs @@ -1,8 +1,8 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Threading.Tasks; using NSubstitute; -using Octokit.Tests.Helpers; using Xunit; namespace Octokit.Tests.Clients @@ -21,16 +21,33 @@ public void DoesThrowOnBadArguments() public class TheGetContributorsMethod { [Fact] - public void RequestsCorrectUrl() + public async Task RetrievesContributorsForCorrectUrl() { var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/contributors", UriKind.Relative); + var client = Substitute.For(); + IReadOnlyList contributors = new ReadOnlyCollection(new[] { new Contributor() }); + client.GetQueuedOperation>(expectedEndPoint, Args.CancellationToken) + .Returns(Task.FromResult(contributors)); + var statisticsClient = new StatisticsClient(client); + var result = await statisticsClient.GetContributors("username","repositoryName"); + + Assert.Equal(1, result.Count); + } + + [Fact] + public async Task RetrievesEmptyContributorsCollectionWhenNoContentReturned() + { + var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/contributors", UriKind.Relative); var client = Substitute.For(); + IReadOnlyList contributors = null; + client.GetQueuedOperation>(expectedEndPoint, Args.CancellationToken) + .Returns(Task.FromResult(contributors)); var statisticsClient = new StatisticsClient(client); - statisticsClient.GetContributors("username","repositoryName"); + var result = await statisticsClient.GetContributors("username", "repositoryName"); - client.Received().GetQueuedOperation>(expectedEndPoint,Args.CancellationToken); + Assert.Empty(result); } [Fact] diff --git a/Octokit.Tests/Http/ApiConnectionTests.cs b/Octokit.Tests/Http/ApiConnectionTests.cs index 909c2b1030..11d1aa0971 100644 --- a/Octokit.Tests/Http/ApiConnectionTests.cs +++ b/Octokit.Tests/Http/ApiConnectionTests.cs @@ -361,6 +361,22 @@ public async Task WhenGetReturnsOkThenBodyAsObjectIsReturned() Assert.Same(actualResult,result); } + [Fact] + public async Task WhenGetReturnsNoContentThenReturnsNull() + { + var queuedOperationUrl = new Uri("anything", UriKind.Relative); + + var result = new object(); + const HttpStatusCode statusCode = HttpStatusCode.NoContent; + IApiResponse response = new ApiResponse(new Response(statusCode, null, new Dictionary(), "application/json"), result); + var connection = Substitute.For(); + connection.GetResponse(queuedOperationUrl, Args.CancellationToken).Returns(Task.FromResult(response)); + var apiConnection = new ApiConnection(connection); + + var actualResult = await apiConnection.GetQueuedOperation(queuedOperationUrl, Args.CancellationToken); + Assert.Null(actualResult); + } + [Fact] public async Task GetIsRepeatedUntilHttpStatusCodeOkIsReturned() { diff --git a/Octokit/Clients/IStatisticsClient.cs b/Octokit/Clients/IStatisticsClient.cs index 1ea3fad933..ff97aca9ef 100644 --- a/Octokit/Clients/IStatisticsClient.cs +++ b/Octokit/Clients/IStatisticsClient.cs @@ -18,7 +18,7 @@ public interface IStatisticsClient /// The owner of the repository /// The name of the repository /// A list of - Task> GetContributors(string owner, string repositoryName); + Task> GetContributors(string owner, string repositoryName); /// /// Returns a list of for the given repository @@ -27,7 +27,7 @@ public interface IStatisticsClient /// The name of the repository /// A token used to cancel this potentially long running request /// A list of - Task> GetContributors(string owner, string repositoryName, CancellationToken cancellationToken); + Task> GetContributors(string owner, string repositoryName, CancellationToken cancellationToken); /// /// Returns the last year of commit activity grouped by week. diff --git a/Octokit/Clients/StatisticsClient.cs b/Octokit/Clients/StatisticsClient.cs index 5ac2cd809d..5cd132bf3a 100644 --- a/Octokit/Clients/StatisticsClient.cs +++ b/Octokit/Clients/StatisticsClient.cs @@ -1,4 +1,6 @@ using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -26,7 +28,7 @@ public StatisticsClient(IApiConnection apiConnection) : base(apiConnection) /// The owner of the repository /// The name of the repository /// A list of - public Task> GetContributors(string owner, string repositoryName) + public Task> GetContributors(string owner, string repositoryName) { return GetContributors(owner, repositoryName, CancellationToken.None); } @@ -38,13 +40,14 @@ public Task> GetContributors(string owner, string repos /// The name of the repository /// A token used to cancel this potentially long running request /// A list of - public async Task> GetContributors(string owner, string repositoryName, CancellationToken cancellationToken) + public async Task> GetContributors(string owner, string repositoryName, CancellationToken cancellationToken) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); var endpoint = "/repos/{0}/{1}/stats/contributors".FormatUri(owner, repositoryName); - return await ApiConnection.GetQueuedOperation>(endpoint, cancellationToken); + return await ApiConnection.GetQueuedOperation>(endpoint, cancellationToken) + ?? new ReadOnlyCollection(new Contributor[] {}); } /// diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index 18991d3a2d..64d312d1ad 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -444,6 +444,8 @@ public async Task GetQueuedOperation(Uri uri, CancellationToken cancellati { case HttpStatusCode.Accepted: continue; + case HttpStatusCode.NoContent: + return default(T); case HttpStatusCode.OK: return response.Body; } diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 8a7c36bd1f..0c06b130ae 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -262,6 +262,6 @@ public interface IApiConnection /// A token used to cancel this potentially long running request /// The updated API resource. /// Thrown when an API error occurs. - Task GetQueuedOperation(Uri uri,CancellationToken cancellationToken); + Task GetQueuedOperation(Uri uri, CancellationToken cancellationToken); } -} \ No newline at end of file +} From 6179361ed925a16b7584244b3a4a43799870cf96 Mon Sep 17 00:00:00 2001 From: Haacked Date: Fri, 7 Aug 2015 16:02:20 -0700 Subject: [PATCH 2/5] Add tests for UnixTimestampExtensions Also refactored the code to be a bit easier to understand and have less calculations. --- .../Helpers/UnixTimestampExtensionsTests.cs | 46 +++++++++++++++++++ Octokit.Tests/Octokit.Tests.csproj | 1 + Octokit/Helpers/UnixTimeStampExtensions.cs | 7 ++- 3 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 Octokit.Tests/Helpers/UnixTimestampExtensionsTests.cs diff --git a/Octokit.Tests/Helpers/UnixTimestampExtensionsTests.cs b/Octokit.Tests/Helpers/UnixTimestampExtensionsTests.cs new file mode 100644 index 0000000000..1f1b379839 --- /dev/null +++ b/Octokit.Tests/Helpers/UnixTimestampExtensionsTests.cs @@ -0,0 +1,46 @@ +using System; +using Octokit.Helpers; +using Xunit; + +public class UnixTimestampExtensionsTests +{ + public class TheToUnixTimeMethod + { + [Fact] + public void ReturnsUnixEpochCorrectly() + { + var epoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero); + Assert.Equal(0, epoch.ToUnixTime()); + } + + [Fact] + public void ReturnsRandomDateCorrectly() + { + var epoch = new DateTimeOffset(1975, 1, 23, 1, 1, 1, TimeSpan.Zero); + Assert.Equal(159670861, epoch.ToUnixTime()); + } + } + + public class TheFromUnixTimeMethod + { + [Fact] + public void ReturnsDateFromUnixEpochCorrectly() + { + var epoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero); + + var result = ((long)0).FromUnixTime(); + + Assert.Equal(epoch, result); + } + + [Fact] + public void ReturnsDateFromRandomTimeCorrectly() + { + var expected = new DateTimeOffset(1975, 1, 23, 1, 1, 2, TimeSpan.Zero); + + var result = ((long)159670862).FromUnixTime(); + + Assert.Equal(expected, result); + } + } +} diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index c9ae7fde8c..4307109be5 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -123,6 +123,7 @@ + diff --git a/Octokit/Helpers/UnixTimeStampExtensions.cs b/Octokit/Helpers/UnixTimeStampExtensions.cs index 70ec92b32b..60a74b8d97 100644 --- a/Octokit/Helpers/UnixTimeStampExtensions.cs +++ b/Octokit/Helpers/UnixTimeStampExtensions.cs @@ -7,8 +7,7 @@ namespace Octokit.Helpers /// public static class UnixTimestampExtensions { - // Unix Epoch is January 1, 1970 00:00 -0:00 - const long unixEpochTicks = 621355968000000000; + static readonly DateTimeOffset epoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero); /// /// Convert a Unix tick to a with UTC offset @@ -16,7 +15,7 @@ public static class UnixTimestampExtensions /// UTC tick public static DateTimeOffset FromUnixTime(this long unixTime) { - return new DateTimeOffset(unixTime * TimeSpan.TicksPerSecond + unixEpochTicks, TimeSpan.Zero); + return epoch.AddSeconds(unixTime); } /// @@ -25,7 +24,7 @@ public static DateTimeOffset FromUnixTime(this long unixTime) /// Date Time with UTC offset public static long ToUnixTime(this DateTimeOffset date) { - return (date.Ticks - unixEpochTicks) / TimeSpan.TicksPerSecond; + return Convert.ToInt64((date.ToUniversalTime() - epoch).TotalSeconds); } } } From 890f852c90c3585fd46cc9b509dd9f2d1f237761 Mon Sep 17 00:00:00 2001 From: Haacked Date: Fri, 7 Aug 2015 16:20:45 -0700 Subject: [PATCH 3/5] :art: Refactor GetQueuedOperation to reduce duplication Most calls to `GetQueuedOperation` are used to queue up an operation that'll return a collection in a subsequent call. In the case that the API returns No Content, we want to return an empty collection. This refactoring embeds that last bit of logic into `GetQueuedOperation` rather than making every caller have to do it. --- .../Clients/StatisticsClientTests.cs | 67 ++++++++++++------- Octokit.Tests/Http/ApiConnectionTests.cs | 60 +++++++---------- Octokit/Clients/StatisticsClient.cs | 13 ++-- Octokit/Http/ApiConnection.cs | 18 ++--- Octokit/Http/IApiConnection.cs | 10 +-- 5 files changed, 88 insertions(+), 80 deletions(-) diff --git a/Octokit.Tests/Clients/StatisticsClientTests.cs b/Octokit.Tests/Clients/StatisticsClientTests.cs index 102647a051..bcf4d57813 100644 --- a/Octokit.Tests/Clients/StatisticsClientTests.cs +++ b/Octokit.Tests/Clients/StatisticsClientTests.cs @@ -3,6 +3,7 @@ using System.Collections.ObjectModel; using System.Threading.Tasks; using NSubstitute; +using Octokit.Helpers; using Xunit; namespace Octokit.Tests.Clients @@ -26,7 +27,7 @@ public async Task RetrievesContributorsForCorrectUrl() var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/contributors", UriKind.Relative); var client = Substitute.For(); IReadOnlyList contributors = new ReadOnlyCollection(new[] { new Contributor() }); - client.GetQueuedOperation>(expectedEndPoint, Args.CancellationToken) + client.GetQueuedOperation(expectedEndPoint, Args.CancellationToken) .Returns(Task.FromResult(contributors)); var statisticsClient = new StatisticsClient(client); @@ -35,21 +36,6 @@ public async Task RetrievesContributorsForCorrectUrl() Assert.Equal(1, result.Count); } - [Fact] - public async Task RetrievesEmptyContributorsCollectionWhenNoContentReturned() - { - var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/contributors", UriKind.Relative); - var client = Substitute.For(); - IReadOnlyList contributors = null; - client.GetQueuedOperation>(expectedEndPoint, Args.CancellationToken) - .Returns(Task.FromResult(contributors)); - var statisticsClient = new StatisticsClient(client); - - var result = await statisticsClient.GetContributors("username", "repositoryName"); - - Assert.Empty(result); - } - [Fact] public async Task ThrowsIfGivenNullOwner() { @@ -68,16 +54,24 @@ public async Task ThrowsIfGivenNullRepositoryName() public class TheGetCommitActivityForTheLastYearMethod { [Fact] - public void RequestsCorrectUrl() + public async Task RequestsCorrectUrl() { var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/commit_activity", UriKind.Relative); + var data = new WeeklyCommitActivity(new[] { 1, 2 }, 100, 42); + IReadOnlyList response = new ReadOnlyCollection(new[] { data }); var client = Substitute.For(); + client.GetQueuedOperation(expectedEndPoint, Args.CancellationToken) + .Returns(Task.FromResult(response)); var statisticsClient = new StatisticsClient(client); - statisticsClient.GetCommitActivity("username", "repositoryName"); + var result = await statisticsClient.GetCommitActivity("username", "repositoryName"); - client.Received().GetQueuedOperation>(expectedEndPoint, Args.CancellationToken); + Assert.Equal(2, result.Activity[0].Days.Count); + Assert.Equal(1, result.Activity[0].Days[0]); + Assert.Equal(2, result.Activity[0].Days[1]); + Assert.Equal(100, result.Activity[0].Total); + Assert.Equal(42, result.Activity[0].Week); } [Fact] @@ -98,16 +92,31 @@ public async Task ThrowsIfGivenNullRepositoryName() public class TheGetAdditionsAndDeletionsPerWeekMethod { [Fact] - public void RequestsCorrectUrl() + public async Task RequestsCorrectUrl() { var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/code_frequency", UriKind.Relative); + long firstTimestamp = 159670861; + long secondTimestamp = 0; + IReadOnlyList data = new ReadOnlyCollection(new[] + { + new[] { firstTimestamp, 10, 52 }, + new[] { secondTimestamp, 0, 9 } + }); var client = Substitute.For(); + client.GetQueuedOperation(expectedEndPoint, Args.CancellationToken) + .Returns(Task.FromResult(data)); var statisticsClient = new StatisticsClient(client); - statisticsClient.GetCodeFrequency("username", "repositoryName"); + var codeFrequency = await statisticsClient.GetCodeFrequency("username", "repositoryName"); - client.Received().GetQueuedOperation>(expectedEndPoint, Args.CancellationToken); + Assert.Equal(2, codeFrequency.AdditionsAndDeletionsByWeek.Count); + Assert.Equal(firstTimestamp.FromUnixTime(), codeFrequency.AdditionsAndDeletionsByWeek[0].Timestamp); + Assert.Equal(10, codeFrequency.AdditionsAndDeletionsByWeek[0].Additions); + Assert.Equal(52, codeFrequency.AdditionsAndDeletionsByWeek[0].Deletions); + Assert.Equal(secondTimestamp.FromUnixTime(), codeFrequency.AdditionsAndDeletionsByWeek[1].Timestamp); + Assert.Equal(0, codeFrequency.AdditionsAndDeletionsByWeek[1].Additions); + Assert.Equal(9, codeFrequency.AdditionsAndDeletionsByWeek[1].Deletions); } [Fact] @@ -158,16 +167,22 @@ public async Task ThrowsIfGivenNullRepositoryName() public class TheGetHourlyCommitCountsMethod { [Fact] - public void RequestsCorrectUrl() + public async Task RetrievesPunchCard() { var expectedEndPoint = new Uri("/repos/username/repositoryName/stats/punch_card", UriKind.Relative); var client = Substitute.For(); + IReadOnlyList data = new ReadOnlyCollection(new[] { new[] { 2, 8, 42 } }); + client.GetQueuedOperation(expectedEndPoint, Args.CancellationToken) + .Returns(Task.FromResult(data)); var statisticsClient = new StatisticsClient(client); - statisticsClient.GetPunchCard("username", "repositoryName"); + var result = await statisticsClient.GetPunchCard("username", "repositoryName"); - client.Received().GetQueuedOperation>(expectedEndPoint, Args.CancellationToken); + Assert.Equal(1, result.PunchPoints.Count); + Assert.Equal(DayOfWeek.Tuesday, result.PunchPoints[0].DayOfWeek); + Assert.Equal(8, result.PunchPoints[0].HourOfTheDay); + Assert.Equal(42, result.PunchPoints[0].CommitCount); } [Fact] @@ -185,4 +200,4 @@ public async Task ThrowsIfGivenNullRepositoryName() } } } -} \ No newline at end of file +} diff --git a/Octokit.Tests/Http/ApiConnectionTests.cs b/Octokit.Tests/Http/ApiConnectionTests.cs index 11d1aa0971..1e767bd800 100644 --- a/Octokit.Tests/Http/ApiConnectionTests.cs +++ b/Octokit.Tests/Http/ApiConnectionTests.cs @@ -6,7 +6,6 @@ using System.Threading.Tasks; using NSubstitute; using Octokit.Internal; -using Octokit.Tests.Helpers; using Xunit; namespace Octokit.Tests.Http @@ -315,22 +314,6 @@ public async Task EnsuresArgumentNotNull() public class TheGetQueuedOperationMethod { - [Fact] - public async Task MakesGetRequest() - { - var queuedOperationUrl = new Uri("anything", UriKind.Relative); - - const HttpStatusCode statusCode = HttpStatusCode.OK; - IApiResponse response = new ApiResponse(new Response(statusCode, null, new Dictionary(), "application/json"), new object()); - var connection = Substitute.For(); - connection.GetResponse(queuedOperationUrl,Args.CancellationToken).Returns(Task.FromResult(response)); - var apiConnection = new ApiConnection(connection); - - await apiConnection.GetQueuedOperation(queuedOperationUrl,CancellationToken.None); - - connection.Received().GetResponse(queuedOperationUrl, Args.CancellationToken); - } - [Fact] public async Task WhenGetReturnsNotOkOrAcceptedApiExceptionIsThrown() { @@ -350,31 +333,36 @@ public async Task WhenGetReturnsOkThenBodyAsObjectIsReturned() { var queuedOperationUrl = new Uri("anything", UriKind.Relative); - var result = new object(); + var result = new[] { new object() }; const HttpStatusCode statusCode = HttpStatusCode.OK; - IApiResponse response = new ApiResponse(new Response(statusCode, null, new Dictionary(), "application/json"), result); + var httpResponse = new Response(statusCode, null, new Dictionary(), "application/json"); + IApiResponse> response = new ApiResponse>(httpResponse, result); var connection = Substitute.For(); - connection.GetResponse(queuedOperationUrl, Args.CancellationToken).Returns(Task.FromResult(response)); + connection.GetResponse>(queuedOperationUrl, Args.CancellationToken) + .Returns(Task.FromResult(response)); var apiConnection = new ApiConnection(connection); var actualResult = await apiConnection.GetQueuedOperation(queuedOperationUrl, Args.CancellationToken); - Assert.Same(actualResult,result); + Assert.Same(actualResult, result); } [Fact] - public async Task WhenGetReturnsNoContentThenReturnsNull() + public async Task WhenGetReturnsNoContentThenReturnsEmptyCollection() { var queuedOperationUrl = new Uri("anything", UriKind.Relative); - var result = new object(); + var result = new [] { new object() }; const HttpStatusCode statusCode = HttpStatusCode.NoContent; - IApiResponse response = new ApiResponse(new Response(statusCode, null, new Dictionary(), "application/json"), result); + var httpResponse = new Response(statusCode, null, new Dictionary(), "application/json"); + IApiResponse> response = new ApiResponse>( + httpResponse, result); var connection = Substitute.For(); - connection.GetResponse(queuedOperationUrl, Args.CancellationToken).Returns(Task.FromResult(response)); + connection.GetResponse>(queuedOperationUrl, Args.CancellationToken) + .Returns(Task.FromResult(response)); var apiConnection = new ApiConnection(connection); var actualResult = await apiConnection.GetQueuedOperation(queuedOperationUrl, Args.CancellationToken); - Assert.Null(actualResult); + Assert.Empty(actualResult); } [Fact] @@ -382,11 +370,13 @@ public async Task GetIsRepeatedUntilHttpStatusCodeOkIsReturned() { var queuedOperationUrl = new Uri("anything", UriKind.Relative); - var result = new object(); - IApiResponse firstResponse = new ApiResponse(new Response(HttpStatusCode.Accepted, null, new Dictionary(), "application/json"), result); - IApiResponse completedResponse = new ApiResponse(new Response(HttpStatusCode.OK, null, new Dictionary(), "application/json"), result); + var result = new [] { new object() }; + IApiResponse> firstResponse = new ApiResponse>( + new Response(HttpStatusCode.Accepted, null, new Dictionary(), "application/json"), result); + IApiResponse> completedResponse = new ApiResponse>( + new Response(HttpStatusCode.OK, null, new Dictionary(), "application/json"), result); var connection = Substitute.For(); - connection.GetResponse(queuedOperationUrl, Args.CancellationToken) + connection.GetResponse>(queuedOperationUrl, Args.CancellationToken) .Returns(x => Task.FromResult(firstResponse), x => Task.FromResult(firstResponse), x => Task.FromResult(completedResponse)); @@ -395,17 +385,19 @@ public async Task GetIsRepeatedUntilHttpStatusCodeOkIsReturned() await apiConnection.GetQueuedOperation(queuedOperationUrl, CancellationToken.None); - connection.Received(3).GetResponse(queuedOperationUrl, Args.CancellationToken); + connection.Received(3).GetResponse>(queuedOperationUrl, Args.CancellationToken); } public async Task CanCancelQueuedOperation() { var queuedOperationUrl = new Uri("anything", UriKind.Relative); - var result = new object(); - IApiResponse accepted = new ApiResponse(new Response(HttpStatusCode.Accepted, null, new Dictionary(), "application/json"), result); + var result = new [] { new object() }; + IApiResponse> accepted = new ApiResponse>( + new Response(HttpStatusCode.Accepted, null, new Dictionary(), "application/json"), result); var connection = Substitute.For(); - connection.GetResponse(queuedOperationUrl, Args.CancellationToken).Returns(x => Task.FromResult(accepted)); + connection.GetResponse>(queuedOperationUrl, Args.CancellationToken) + .Returns(x => Task.FromResult(accepted)); var apiConnection = new ApiConnection(connection); diff --git a/Octokit/Clients/StatisticsClient.cs b/Octokit/Clients/StatisticsClient.cs index 5cd132bf3a..72107cf9c8 100644 --- a/Octokit/Clients/StatisticsClient.cs +++ b/Octokit/Clients/StatisticsClient.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -46,8 +45,7 @@ public async Task> GetContributors(string owner, stri Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); var endpoint = "/repos/{0}/{1}/stats/contributors".FormatUri(owner, repositoryName); - return await ApiConnection.GetQueuedOperation>(endpoint, cancellationToken) - ?? new ReadOnlyCollection(new Contributor[] {}); + return await ApiConnection.GetQueuedOperation(endpoint, cancellationToken); } /// @@ -74,7 +72,7 @@ public async Task GetCommitActivity(string owner, string reposit Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); var endpoint = "/repos/{0}/{1}/stats/commit_activity".FormatUri(owner, repositoryName); - var activity = await ApiConnection.GetQueuedOperation>(endpoint,cancellationToken); + var activity = await ApiConnection.GetQueuedOperation(endpoint,cancellationToken); return new CommitActivity(activity); } @@ -102,7 +100,7 @@ public async Task GetCodeFrequency(string owner, string repositor Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); var endpoint = "/repos/{0}/{1}/stats/code_frequency".FormatUri(owner, repositoryName); - var rawFrequencies = await ApiConnection.GetQueuedOperation>(endpoint,cancellationToken); + var rawFrequencies = await ApiConnection.GetQueuedOperation(endpoint,cancellationToken); return new CodeFrequency(rawFrequencies); } @@ -130,7 +128,8 @@ public async Task GetParticipation(string owner, string repositor Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); var endpoint = "/repos/{0}/{1}/stats/participation".FormatUri(owner, repositoryName); - return await ApiConnection.GetQueuedOperation(endpoint,cancellationToken); + var result = await ApiConnection.GetQueuedOperation(endpoint,cancellationToken); + return result.FirstOrDefault(); } /// @@ -157,7 +156,7 @@ public async Task GetPunchCard(string owner, string repositoryName, C Ensure.ArgumentNotNullOrEmptyString(repositoryName, "repositoryName"); var endpoint = "/repos/{0}/{1}/stats/punch_card".FormatUri(owner, repositoryName); - var punchCardData = await ApiConnection.GetQueuedOperation>(endpoint, cancellationToken); + var punchCardData = await ApiConnection.GetQueuedOperation(endpoint, cancellationToken); return new PunchCard(punchCardData); } } diff --git a/Octokit/Http/ApiConnection.cs b/Octokit/Http/ApiConnection.cs index 64d312d1ad..a23ecbbfe3 100644 --- a/Octokit/Http/ApiConnection.cs +++ b/Octokit/Http/ApiConnection.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Net; using System.Threading; using System.Threading.Tasks; @@ -422,35 +423,36 @@ public async Task GetRedirect(Uri uri) } /// - /// Executes a GET to the API object at the specified URI. This operation is appropriate for - /// API calls which queue long running calculations. - /// It expects the API to respond with an initial 202 Accepted, and queries again until a - /// 200 OK is received. + /// Executes a GET to the API object at the specified URI. This operation is appropriate for API calls which + /// queue long running calculations and return a collection of a resource. + /// It expects the API to respond with an initial 202 Accepted, and queries again until a 200 OK is received. + /// It returns an empty collection if it receives a 204 No Content response. /// /// The API resource's type. /// URI of the API resource to update /// A token used to cancel this potentially long running request /// The updated API resource. /// Thrown when an API error occurs. - public async Task GetQueuedOperation(Uri uri, CancellationToken cancellationToken) + public async Task> GetQueuedOperation(Uri uri, CancellationToken cancellationToken) { while (true) { Ensure.ArgumentNotNull(uri, "uri"); - var response = await Connection.GetResponse(uri, cancellationToken); + var response = await Connection.GetResponse>(uri, cancellationToken); switch (response.HttpResponse.StatusCode) { case HttpStatusCode.Accepted: continue; case HttpStatusCode.NoContent: - return default(T); + return new ReadOnlyCollection(new T[] {}); case HttpStatusCode.OK: return response.Body; } - throw new ApiException("Queued Operations expect status codes of Accepted or OK.", response.HttpResponse.StatusCode); + throw new ApiException("Queued Operations expect status codes of Accepted, No Content, or OK.", + response.HttpResponse.StatusCode); } } diff --git a/Octokit/Http/IApiConnection.cs b/Octokit/Http/IApiConnection.cs index 0c06b130ae..b3f5befeb2 100644 --- a/Octokit/Http/IApiConnection.cs +++ b/Octokit/Http/IApiConnection.cs @@ -252,16 +252,16 @@ public interface IApiConnection Task GetRedirect(Uri uri); /// - /// Executes a GET to the API object at the specified URI. This operation is appropriate for - /// API calls which queue long running calculations. - /// It expects the API to respond with an initial 202 Accepted, and queries again until a - /// 200 OK is received. + /// Executes a GET to the API object at the specified URI. This operation is appropriate for API calls which + /// queue long running calculations and return a collection of a resource. + /// It expects the API to respond with an initial 202 Accepted, and queries again until a 200 OK is received. + /// It returns an empty collection if it receives a 204 No Content response. /// /// The API resource's type. /// URI of the API resource to update /// A token used to cancel this potentially long running request /// The updated API resource. /// Thrown when an API error occurs. - Task GetQueuedOperation(Uri uri, CancellationToken cancellationToken); + Task> GetQueuedOperation(Uri uri, CancellationToken cancellationToken); } } From 375f83e031404b87f2c7198af038fe6b8239d3f6 Mon Sep 17 00:00:00 2001 From: Haacked Date: Fri, 7 Aug 2015 16:28:23 -0700 Subject: [PATCH 4/5] Add timestamp tests to platform unit tests --- Octokit.Tests/OctoKit.Tests-NetCore45.csproj | 1 + Octokit.Tests/Octokit.Tests-Portable.csproj | 1 + 2 files changed, 2 insertions(+) diff --git a/Octokit.Tests/OctoKit.Tests-NetCore45.csproj b/Octokit.Tests/OctoKit.Tests-NetCore45.csproj index 6f584c0246..3050eb69c5 100644 --- a/Octokit.Tests/OctoKit.Tests-NetCore45.csproj +++ b/Octokit.Tests/OctoKit.Tests-NetCore45.csproj @@ -109,6 +109,7 @@ + diff --git a/Octokit.Tests/Octokit.Tests-Portable.csproj b/Octokit.Tests/Octokit.Tests-Portable.csproj index 776fc864e3..e64b0f2796 100644 --- a/Octokit.Tests/Octokit.Tests-Portable.csproj +++ b/Octokit.Tests/Octokit.Tests-Portable.csproj @@ -107,6 +107,7 @@ + From 4e09d71a7364c4e85e9e665186ec086460c33a8a Mon Sep 17 00:00:00 2001 From: Haacked Date: Fri, 7 Aug 2015 16:37:54 -0700 Subject: [PATCH 5/5] Omit the IStatisticsClient from these convention tests It doesn't follow the pattern of most of our other clients. --- Octokit.Tests.Conventions/ModelTests.cs | 10 ++++++++-- Octokit.Tests.Conventions/SyncObservableClients.cs | 13 ++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/Octokit.Tests.Conventions/ModelTests.cs b/Octokit.Tests.Conventions/ModelTests.cs index df87d961a8..926485fa7e 100644 --- a/Octokit.Tests.Conventions/ModelTests.cs +++ b/Octokit.Tests.Conventions/ModelTests.cs @@ -71,7 +71,8 @@ public void CheckPaginationGetAllMethodNames(Type clientInterface) .Where(x => x.Name.StartsWith("Get")); var invalidMethods = methodsThatCanPaginate - .Where(x => !x.Name.StartsWith("GetAll")); + .Where(x => !x.Name.StartsWith("GetAll")) + .ToList(); if (invalidMethods.Any()) { @@ -81,7 +82,12 @@ public void CheckPaginationGetAllMethodNames(Type clientInterface) public static IEnumerable GetClientInterfaces() { - return typeof(IEventsClient).Assembly.ExportedTypes.Where(TypeExtensions.IsClientInterface).Select(type => new[] { type }); + return typeof(IEventsClient) + .Assembly + .ExportedTypes + .Where(TypeExtensions.IsClientInterface) + .Where(t => t != typeof(IStatisticsClient)) // This convention doesn't apply to this one type. + .Select(type => new[] { type }); } public static IEnumerable ModelTypes diff --git a/Octokit.Tests.Conventions/SyncObservableClients.cs b/Octokit.Tests.Conventions/SyncObservableClients.cs index b1d8df2ab8..b222a51764 100644 --- a/Octokit.Tests.Conventions/SyncObservableClients.cs +++ b/Octokit.Tests.Conventions/SyncObservableClients.cs @@ -1,12 +1,10 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Linq; using System.Reactive; using System.Reflection; using Octokit.Tests.Helpers; using Xunit; -using Xunit.Extensions; namespace Octokit.Tests.Conventions { @@ -22,13 +20,13 @@ public void CheckObservableClients(Type clientInterface) var mainNames = Array.ConvertAll(mainMethods, m => m.Name); var observableNames = Array.ConvertAll(observableMethods, m => m.Name); - var methodsMissingOnReactiveClient = mainNames.Except(observableNames); + var methodsMissingOnReactiveClient = mainNames.Except(observableNames).ToList(); if (methodsMissingOnReactiveClient.Any()) { throw new InterfaceMissingMethodsException(observableClient, methodsMissingOnReactiveClient); } - var additionalMethodsOnReactiveClient = observableNames.Except(mainNames); + var additionalMethodsOnReactiveClient = observableNames.Except(mainNames).ToList(); if (additionalMethodsOnReactiveClient.Any()) { throw new InterfaceHasAdditionalMethodsException(observableClient, additionalMethodsOnReactiveClient); @@ -122,7 +120,12 @@ private static void CheckParameters(MethodInfo mainMethod, MethodInfo observable public static IEnumerable GetClientInterfaces() { - return typeof(IEventsClient).Assembly.ExportedTypes.Where(TypeExtensions.IsClientInterface).Select(type => new[] { type }); + return typeof(IEventsClient) + .Assembly + .ExportedTypes + .Where(TypeExtensions.IsClientInterface) + .Where(t => t != typeof(IStatisticsClient)) // This convention doesn't apply to this one type. + .Select(type => new[] { type }); } } } \ No newline at end of file