diff --git a/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs b/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs index 29d4d55e..12a4ebf8 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoryCommitsClients.cs @@ -35,6 +35,15 @@ namespace Octokit.Reactive /// IObservable GetAll(string owner, string name); + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// + IObservable GetAll(string owner, string name, ApiOptions options); + /// /// Gets all commits for a given repository /// @@ -44,6 +53,16 @@ namespace Octokit.Reactive /// IObservable GetAll(string owner, string name, CommitRequest request); + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Used to filter list of commits returned + /// Options for changing the API response + /// + IObservable GetAll(string owner, string name, CommitRequest request, ApiOptions options); + /// /// Get the SHA-1 of a commit reference /// diff --git a/Octokit.Reactive/Clients/ObservableCommitStatusClient.cs b/Octokit.Reactive/Clients/ObservableCommitStatusClient.cs index cc43773e..e34ed7a9 100644 --- a/Octokit.Reactive/Clients/ObservableCommitStatusClient.cs +++ b/Octokit.Reactive/Clients/ObservableCommitStatusClient.cs @@ -52,7 +52,7 @@ namespace Octokit.Reactive Ensure.ArgumentNotNullOrEmptyString(reference, "reference"); Ensure.ArgumentNotNull(options, "options"); - return _connection.GetAndFlattenAllPages(ApiUrls.CommitStatuses(owner, name, reference),options); + return _connection.GetAndFlattenAllPages(ApiUrls.CommitStatuses(owner, name, reference), options); } /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs b/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs index 629142d7..3304e261 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryCommitsClients.cs @@ -54,7 +54,26 @@ namespace Octokit.Reactive /// public IObservable GetAll(string owner, string name) { - return GetAll(owner, name, new CommitRequest()); + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + + return GetAll(owner, name, new CommitRequest(), ApiOptions.None); + } + + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// + public IObservable GetAll(string owner, string name, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNull(options, "options"); + + return GetAll(owner, name, new CommitRequest(), options); } /// @@ -70,8 +89,24 @@ namespace Octokit.Reactive Ensure.ArgumentNotNullOrEmptyString(name, "name"); Ensure.ArgumentNotNull(request, "request"); - return _connection.GetAndFlattenAllPages(ApiUrls.RepositoryCommits(owner, name), - request.ToParametersDictionary()); + return GetAll(owner, name, request, ApiOptions.None); + } + + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Used to filter list of commits returned + /// Options for changing the API response + /// + public IObservable GetAll(string owner, string name, CommitRequest request, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNull(request, "request"); + + return _connection.GetAndFlattenAllPages(ApiUrls.RepositoryCommits(owner, name), request.ToParametersDictionary(), options); } /// diff --git a/Octokit.Reactive/Helpers/ConnectionExtensions.cs b/Octokit.Reactive/Helpers/ConnectionExtensions.cs index becbf01c..5f2aec15 100644 --- a/Octokit.Reactive/Helpers/ConnectionExtensions.cs +++ b/Octokit.Reactive/Helpers/ConnectionExtensions.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.Linq; using System.Reactive.Linq; using System.Reactive.Threading.Tasks; @@ -16,11 +15,7 @@ namespace Octokit.Reactive.Internal public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, ApiOptions options) { - return GetPagesWithOptions(url, options, (pageUrl, o) => - { - var parameters = Pagination.Setup(new Dictionary(), options); - return connection.Get>(pageUrl, parameters, null).ToObservable(); - }); + return connection.GetAndFlattenAllPages(url, null, options); } public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, IDictionary parameters) @@ -28,6 +23,15 @@ namespace Octokit.Reactive.Internal return GetPages(url, parameters, (pageUrl, pageParams) => connection.Get>(pageUrl, pageParams, null).ToObservable()); } + public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, IDictionary parameters, ApiOptions options) + { + return GetPagesWithOptions(url, parameters, options, (pageUrl, pageParams, o) => + { + var passingParameters = Pagination.Setup(parameters, options); + return connection.Get>(pageUrl, passingParameters, null).ToObservable(); + }); + } + public static IObservable GetAndFlattenAllPages(this IConnection connection, Uri url, IDictionary parameters, string accepts) { return GetPages(url, parameters, (pageUrl, pageParams) => connection.Get>(pageUrl, pageParams, accepts).ToObservable()); @@ -47,19 +51,16 @@ namespace Octokit.Reactive.Internal .SelectMany(resp => resp.Body); } - static IObservable GetPagesWithOptions(Uri uri, ApiOptions options, - Func>>> getPageFunc) + static IObservable GetPagesWithOptions(Uri uri, IDictionary parameters, ApiOptions options, Func, ApiOptions, IObservable>>> getPageFunc) { - return getPageFunc(uri, options).Expand(resp => + return getPageFunc(uri, parameters, options).Expand(resp => { - var nextPageUri = resp.HttpResponse.ApiInfo.GetNextPageUrl(); + var nextPageUrl = resp.HttpResponse.ApiInfo.GetNextPageUrl(); - var shouldContinue = Pagination.ShouldContinue( - nextPageUri, - options); + var shouldContinue = Pagination.ShouldContinue(nextPageUrl, options); - return shouldContinue - ? Observable.Defer(() => getPageFunc(nextPageUri, null)) + return shouldContinue + ? Observable.Defer(() => getPageFunc(nextPageUrl, null, null)) : Observable.Empty>>(); }) .Where(resp => resp != null) diff --git a/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs index 83401a3e..c5a3e3c4 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryCommitsClientTests.cs @@ -42,6 +42,59 @@ public class RepositoryCommitsClientTests Assert.NotEmpty(list); } + [IntegrationTest] + public async Task CanGetCorrectCountOfCommitsWithoutStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var commits = await _fixture.GetAll("shiftkey", "ReactiveGit", options); + Assert.Equal(5, commits.Count); + } + + [IntegrationTest] + public async Task CanGetCorrectCountOfCommitsWithStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var commits = await _fixture.GetAll("shiftkey", "ReactiveGit", options); + Assert.Equal(5, commits.Count); + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStart() + { + var startOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1, + }; + + var skipStartOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var firstCommit = await _fixture.GetAll("shiftkey", "ReactiveGit", startOptions); + var secondCommit = await _fixture.GetAll("shiftkey", "ReactiveGit", skipStartOptions); + + Assert.NotEqual(firstCommit[0].Sha, secondCommit[0].Sha); + Assert.NotEqual(firstCommit[1].Sha, secondCommit[1].Sha); + Assert.NotEqual(firstCommit[2].Sha, secondCommit[2].Sha); + Assert.NotEqual(firstCommit[3].Sha, secondCommit[3].Sha); + Assert.NotEqual(firstCommit[4].Sha, secondCommit[4].Sha); + } + [IntegrationTest] public async Task CanGetListOfCommitsBySha() { diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index 5cd0ff60..3192936c 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -137,6 +137,7 @@ + @@ -151,7 +152,7 @@ - + diff --git a/Octokit.Tests.Integration/Reactive/ObservableRepositoryCommitsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableRepositoryCommitsClientTests.cs new file mode 100644 index 00000000..63995c8c --- /dev/null +++ b/Octokit.Tests.Integration/Reactive/ObservableRepositoryCommitsClientTests.cs @@ -0,0 +1,75 @@ +using System.Linq; +using System.Threading.Tasks; +using Octokit.Reactive; +using System.Reactive.Linq; +using Xunit; + +namespace Octokit.Tests.Integration.Reactive +{ + public class ObservableRepositoryCommitsClientTests + { + public class TheGetAllMethod + { + readonly ObservableRepositoryCommitsClient _repositoryCommitsClient; + + public TheGetAllMethod() + { + var client = Helper.GetAuthenticatedClient(); + _repositoryCommitsClient = new ObservableRepositoryCommitsClient(client); + } + + [IntegrationTest] + public async Task CanGetCorrectCountOfCommitsWithoutStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1 + }; + + var commits = await _repositoryCommitsClient.GetAll("shiftkey", "ReactiveGit", options).ToList(); + Assert.Equal(5, commits.Count); + } + + [IntegrationTest] + public async Task CanGetCorrectCountOfCommitsWithStart() + { + var options = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var commits = await _repositoryCommitsClient.GetAll("shiftkey", "ReactiveGit", options).ToList(); + Assert.Equal(5, commits.Count); + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStart() + { + var startOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1, + }; + + var skipStartOptions = new ApiOptions + { + PageSize = 5, + PageCount = 1, + StartPage = 2 + }; + + var firstCommit = await _repositoryCommitsClient.GetAll("shiftkey", "ReactiveGit", startOptions).ToList(); + var secondCommit = await _repositoryCommitsClient.GetAll("shiftkey", "ReactiveGit", skipStartOptions).ToList(); + + Assert.NotEqual(firstCommit[0].Sha, secondCommit[0].Sha); + Assert.NotEqual(firstCommit[1].Sha, secondCommit[1].Sha); + Assert.NotEqual(firstCommit[2].Sha, secondCommit[2].Sha); + Assert.NotEqual(firstCommit[3].Sha, secondCommit[3].Sha); + Assert.NotEqual(firstCommit[4].Sha, secondCommit[4].Sha); + } + } + } +} diff --git a/Octokit.Tests.Integration/Reactive/ObservableRepositoryDeployKeysClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableRepositoryDeployKeysClientTests.cs index 3df43c17..19fe4a83 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableRepositoryDeployKeysClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableRepositoryDeployKeysClientTests.cs @@ -6,7 +6,7 @@ using Octokit.Reactive; using Octokit.Tests.Integration; using Xunit; -public class ObservableRepositoryDeployKeysClientTests : IDisposable +public class ObservableRespositoryDeployKeysClientTests : IDisposable { const string _key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDB8IE5+RppLpeW+6lqo0fpfvMunKg6W4bhYCfVJIOYbpKoHP95nTUMZPBT++9NLeB4/YsuNTCrrpnpjc4f2IVpGvloRiVXjAzoJk9QIL6uzn1zRFdvaxSJ3Urhe9LcLHcIgccgZgSdWGzaZI3xtMvGC4diwWNsPjvVc/RyDM/MPqAim0X5XVOQwEFsSsUSraezJ+VgYMYzLYBcKWW0B86HVVhL4ZtmcY/RN2544bljnzw2M3aQvXNPTvkuiUoqLOI+5/qzZ8PfkruO55YtweEd0lkY6oZvrBPMD6dLODEqMHb4tD6htx60wSipNqjPwpOMpzp0Bk3G909unVXi6Fw5"; const string _keyTitle = "octokit@github"; @@ -14,7 +14,7 @@ public class ObservableRepositoryDeployKeysClientTests : IDisposable Repository _repository; string _owner; - public ObservableRepositoryDeployKeysClientTests() + public ObservableRespositoryDeployKeysClientTests() { var github = Helper.GetAuthenticatedClient(); diff --git a/Octokit.Tests/Clients/RepositoriesClientTests.cs b/Octokit.Tests/Clients/RepositoriesClientTests.cs index cae96871..6789f048 100644 --- a/Octokit.Tests/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests/Clients/RepositoriesClientTests.cs @@ -740,7 +740,7 @@ namespace Octokit.Tests.Clients await Assert.ThrowsAsync(() => client.GetAll("owner", null)); await Assert.ThrowsAsync(() => client.GetAll("owner", "")); - await Assert.ThrowsAsync(() => client.GetAll("owner", "repo", null)); + await Assert.ThrowsAsync(() => client.GetAll("owner", "repo", null, ApiOptions.None)); } [Fact] @@ -752,8 +752,7 @@ namespace Octokit.Tests.Clients client.GetAll("owner", "name"); connection.Received() - .GetAll(Arg.Is(u => u.ToString() == "repos/owner/name/commits"), - Arg.Any>()); + .GetAll(Arg.Is(u => u.ToString() == "repos/owner/name/commits"), Args.EmptyDictionary, Args.ApiOptions); } } diff --git a/Octokit.Tests/Clients/RespositoryCommitsClientTests.cs b/Octokit.Tests/Clients/RespositoryCommitsClientTests.cs new file mode 100644 index 00000000..fe0ba30f --- /dev/null +++ b/Octokit.Tests/Clients/RespositoryCommitsClientTests.cs @@ -0,0 +1,50 @@ +using NSubstitute; +using System; +using System.Collections.Generic; +using Xunit; + +namespace Octokit.Tests.Clients +{ + public class RespositoryCommitsClientTests + { + public class TheGetAllMethod + { + [Fact] + public void EnsuresNonEmptyArguments() + { + var connection = Substitute.For(); + var client = new RepositoryCommitsClient(connection); + var options = new ApiOptions(); + var request = new CommitRequest(); + + Assert.ThrowsAsync(() => client.GetAll("", "name", request, options)); + Assert.ThrowsAsync(() => client.GetAll("owner", "", request, options)); + } + + [Fact] + public void EnsuresNonNullArguments() + { + var connection = Substitute.For(); + var client = new RepositoryCommitsClient(connection); + var options = new ApiOptions(); + var request = new CommitRequest(); + + Assert.ThrowsAsync(() => client.GetAll(null, "name", request, options)); + Assert.ThrowsAsync(() => client.GetAll("owner", null, request, options)); + Assert.ThrowsAsync(() => client.GetAll("owner", "name", null, options)); + Assert.ThrowsAsync(() => client.GetAll("owner", "name", request, null)); + + } + + [Fact] + public void GetsCorrectUrl() + { + var connection = Substitute.For(); + var client = new RepositoryCommitsClient(connection); + + client.GetAll("fake", "repo", new CommitRequest(), new ApiOptions()); + connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/fake/repo/commits"), Args.EmptyDictionary, Args.ApiOptions); + } + } + } +} diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index 1ba8aca1..0cc589e2 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -99,6 +99,7 @@ + diff --git a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs index a2f0e31f..9825cd74 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoriesClientTests.cs @@ -269,13 +269,13 @@ namespace Octokit.Tests.Reactive public class TheGetAllCommitsMethod { [Fact] - public void EnsuresArguments() + public void EnsuresNonNullArguments() { var client = new ObservableRepositoriesClient(Substitute.For()); Assert.Throws(() => client.Commit.GetAll(null, "repo")); Assert.Throws(() => client.Commit.GetAll("owner", null)); - Assert.Throws(() => client.Commit.GetAll("owner", "repo", null)); + Assert.Throws(() => client.Commit.GetAll("owner", "repo", null, ApiOptions.None)); Assert.Throws(() => client.Commit.GetAll("", "repo")); Assert.Throws(() => client.Commit.GetAll("owner", "")); } diff --git a/Octokit.Tests/Reactive/ObservableRepositoryCommitsClientTests.cs b/Octokit.Tests/Reactive/ObservableRepositoryCommitsClientTests.cs index bcd91f37..07d15de3 100644 --- a/Octokit.Tests/Reactive/ObservableRepositoryCommitsClientTests.cs +++ b/Octokit.Tests/Reactive/ObservableRepositoryCommitsClientTests.cs @@ -18,6 +18,48 @@ namespace Octokit.Tests.Reactive } } + public class TheGetAllMethod + { + [Fact] + public void EnsuresNonEmptyArguments() + { + var githubClient = Substitute.For(); + var client = new ObservableRepositoryCommitsClient(githubClient); + var options = new ApiOptions(); + var request = new CommitRequest(); + + Assert.ThrowsAsync(() => client.GetAll("", "name", request, options).ToTask()); + Assert.ThrowsAsync(() => client.GetAll("owner", "", request, options).ToTask()); + } + + [Fact] + public void EnsuresNonNullArguments() + { + var githubClient = Substitute.For(); + var client = new ObservableRepositoryCommitsClient(githubClient); + var options = new ApiOptions(); + var request = new CommitRequest(); + + Assert.ThrowsAsync(() => client.GetAll(null, "name", request, options).ToTask()); + Assert.ThrowsAsync(() => client.GetAll("owner", null, request, options).ToTask()); + Assert.ThrowsAsync(() => client.GetAll("owner", "name", null, options).ToTask()); + Assert.ThrowsAsync(() => client.GetAll("owner", "name", request, null).ToTask()); + + } + + [Fact] + public void GetsCorrectUrl() + { + var githubClient = Substitute.For(); + var client = new ObservableRepositoryCommitsClient(githubClient); + var options = new ApiOptions(); + var request = new CommitRequest(); + + client.GetAll("fake", "repo", request, options); + githubClient.Received().Repository.Commit.GetAll("fake", "repo", request, options); + } + } + public class TheGetSha1Method { [Fact] diff --git a/Octokit/Clients/IRepositoryCommitsClient.cs b/Octokit/Clients/IRepositoryCommitsClient.cs index bd486192..33c7b877 100644 --- a/Octokit/Clients/IRepositoryCommitsClient.cs +++ b/Octokit/Clients/IRepositoryCommitsClient.cs @@ -42,6 +42,15 @@ namespace Octokit /// Task> GetAll(string owner, string name); + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// + Task> GetAll(string owner, string name, ApiOptions options); + /// /// Gets all commits for a given repository /// @@ -51,6 +60,15 @@ namespace Octokit /// Task> GetAll(string owner, string name, CommitRequest request); + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Used to filter list of commits returned + /// Options for changing the API response + /// + Task> GetAll(string owner, string name, CommitRequest request, ApiOptions options); /// /// Get the SHA-1 of a commit reference /// diff --git a/Octokit/Clients/RepositoryCommitsClient.cs b/Octokit/Clients/RepositoryCommitsClient.cs index e5fa9613..f2ea2ccd 100644 --- a/Octokit/Clients/RepositoryCommitsClient.cs +++ b/Octokit/Clients/RepositoryCommitsClient.cs @@ -60,9 +60,27 @@ namespace Octokit /// public Task> GetAll(string owner, string name) { - return GetAll(owner, name, new CommitRequest()); + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + + return GetAll(owner, name, new CommitRequest(), ApiOptions.None); } + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// + public Task> GetAll(string owner, string name, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + + return GetAll(owner, name, new CommitRequest(), options); + } + /// /// Gets all commits for a given repository /// @@ -76,8 +94,24 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(name, "name"); Ensure.ArgumentNotNull(request, "request"); - return _apiConnection.GetAll(ApiUrls.RepositoryCommits(owner, name), - request.ToParametersDictionary()); + return GetAll(owner, name, request, ApiOptions.None); + } + + /// + /// Gets all commits for a given repository + /// + /// The owner of the repository + /// The name of the repository + /// Used to filter list of commits returned + /// Options for changing the API response + /// + public Task> GetAll(string owner, string name, CommitRequest request, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + Ensure.ArgumentNotNull(request, "request"); + + return _apiConnection.GetAll(ApiUrls.RepositoryCommits(owner, name), request.ToParametersDictionary(), options); } ///