From 63a8c1d70a139b6a8a7ff8f171274879c4aa2f02 Mon Sep 17 00:00:00 2001 From: Alexander Efremov Date: Fri, 29 Apr 2016 11:16:07 +0700 Subject: [PATCH] Add ApiOption overloads to methods on IRepoCollaboratorsClient (#1282) --- .../IObservableRepoCollaboratorsClient.cs | 11 +- .../ObservableRepoCollaboratorsClient.cs | 25 ++- .../RepositoryCollaboratorClientTests.cs | 90 ++++++++- .../Octokit.Tests.Integration.csproj | 1 + ...rvableRepositoryCollaboratorClientTests.cs | 139 +++++++++++++ .../Clients/RepoCollaboratorsClientTests.cs | 25 ++- Octokit.Tests/Octokit.Tests.csproj | 1 + .../ObservableRepoCollaboratorsClientTests.cs | 182 ++++++++++++++++++ Octokit/Clients/IRepoCollaboratorsClient.cs | 15 ++ Octokit/Clients/RepoCollaboratorsClient.cs | 25 ++- 10 files changed, 501 insertions(+), 13 deletions(-) create mode 100644 Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs create mode 100644 Octokit.Tests/Reactive/ObservableRepoCollaboratorsClientTests.cs diff --git a/Octokit.Reactive/Clients/IObservableRepoCollaboratorsClient.cs b/Octokit.Reactive/Clients/IObservableRepoCollaboratorsClient.cs index 07a2ebda..3d5d09d0 100644 --- a/Octokit.Reactive/Clients/IObservableRepoCollaboratorsClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepoCollaboratorsClient.cs @@ -10,9 +10,18 @@ namespace Octokit.Reactive /// /// The owner of the repository /// The name of the repository - /// + /// The list of s for the specified repository. IObservable GetAll(string owner, string repo); + /// + /// Gets all the available collaborators on this repo. + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// The list of s for the specified repository. + IObservable GetAll(string owner, string repo, ApiOptions options); + /// /// Checks to see if a user is an assignee for a repository. /// diff --git a/Octokit.Reactive/Clients/ObservableRepoCollaboratorsClient.cs b/Octokit.Reactive/Clients/ObservableRepoCollaboratorsClient.cs index 6a605fc6..800b976f 100644 --- a/Octokit.Reactive/Clients/ObservableRepoCollaboratorsClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepoCollaboratorsClient.cs @@ -23,14 +23,29 @@ namespace Octokit.Reactive /// /// The owner of the repository /// The name of the repository - /// + /// The list of s for the specified repository. public IObservable GetAll(string owner, string repo) { - Ensure.ArgumentNotNull(owner, "owner"); - Ensure.ArgumentNotNull(repo, "repo"); + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(repo, "repo"); - var endpoint = ApiUrls.RepoCollaborators(owner, repo); - return _connection.GetAndFlattenAllPages(endpoint); + return GetAll(owner, repo, ApiOptions.None); + } + + /// + /// Gets all the available collaborators on this repo. + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// The list of s for the specified repository. + public IObservable GetAll(string owner, string repo, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(repo, "repo"); + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.RepoCollaborators(owner, repo), options); } /// diff --git a/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs index beee926b..abfff650 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryCollaboratorClientTests.cs @@ -1,7 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; +using System.Threading.Tasks; using Octokit; using Octokit.Tests.Integration; using Xunit; @@ -29,6 +26,91 @@ public class RepositoryCollaboratorClientTests Assert.Equal(2, collaborators.Count); } } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfCollaboratorsWithoutStart() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = github.Repository.Collaborator; + + // add some collaborators + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var options = new ApiOptions + { + PageSize = 1, + PageCount = 1 + }; + + var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, options); + Assert.NotNull(collaborators); + Assert.Equal(1, collaborators.Count); + } + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfCollaboratorsWithStart() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = github.Repository.Collaborator; + + // add some collaborators + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var options = new ApiOptions + { + PageSize = 1, + PageCount = 1, + StartPage = 2 + }; + + var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, options); + Assert.NotNull(collaborators); + Assert.Equal(1, collaborators.Count); + } + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStartPage() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = github.Repository.Collaborator; + + // add some collaborators + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var startOptions = new ApiOptions + { + PageSize = 1, + PageCount = 1 + }; + + var firstPage = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, startOptions); + + var skipStartOptions = new ApiOptions + { + PageSize = 1, + PageCount = 1, + StartPage = 2 + }; + + var secondPage = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, skipStartOptions); + + Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); + } + } } public class TheIsCollaboratorMethod diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index d790c35a..54e5e040 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -161,6 +161,7 @@ + diff --git a/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs new file mode 100644 index 00000000..c6fc5464 --- /dev/null +++ b/Octokit.Tests.Integration/Reactive/ObservableRepositoryCollaboratorClientTests.cs @@ -0,0 +1,139 @@ +using System.Reactive.Linq; +using System.Threading.Tasks; +using Octokit; +using Octokit.Reactive; +using Octokit.Tests.Integration; +using Xunit; +using Octokit.Tests.Integration.Helpers; + +public class ObservableRepositoryCollaboratorClientTests +{ + public class TheGetAllMethod + { + [IntegrationTest] + public async Task ReturnsAllCollaborators() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = new ObservableRepoCollaboratorsClient(github); + + // add a collaborator + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName).ToList(); + Assert.NotNull(collaborators); + Assert.Equal(2, collaborators.Count); + } + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfCollaboratorsWithoutStart() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = new ObservableRepoCollaboratorsClient(github); + + // add some collaborators + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var options = new ApiOptions + { + PageSize = 1, + PageCount = 1 + }; + + var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, options).ToList(); + Assert.NotNull(collaborators); + Assert.Equal(1, collaborators.Count); + } + } + + [IntegrationTest] + public async Task ReturnsCorrectCountOfCollaboratorsWithStart() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = new ObservableRepoCollaboratorsClient(github); + + // add some collaborators + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var options = new ApiOptions + { + PageSize = 1, + PageCount = 1, + StartPage = 2 + }; + + var collaborators = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, options).ToList(); + Assert.NotNull(collaborators); + Assert.Equal(1, collaborators.Count); + } + } + + [IntegrationTest] + public async Task ReturnsDistinctResultsBasedOnStartPage() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = new ObservableRepoCollaboratorsClient(github); + + // add some collaborators + await fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var startOptions = new ApiOptions + { + PageSize = 1, + PageCount = 1 + }; + + var firstPage = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, startOptions).ToList(); + + var skipStartOptions = new ApiOptions + { + PageSize = 1, + PageCount = 1, + StartPage = 2 + }; + + var secondPage = await fixture.GetAll(context.RepositoryOwner, context.RepositoryName, skipStartOptions).ToList(); + + Assert.NotEqual(firstPage[0].Id, secondPage[0].Id); + } + } + } + + public class TheIsCollaboratorMethod + { + [IntegrationTest] + public async Task ReturnsTrueIfUserIsCollaborator() + { + var github = Helper.GetAuthenticatedClient(); + var repoName = Helper.MakeNameWithTimestamp("public-repo"); + + using (var context = await github.CreateRepositoryContext(new NewRepository(repoName))) + { + var fixture = new ObservableRepoCollaboratorsClient(github); + + // add a collaborator + fixture.Add(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + var isCollab = await fixture.IsCollaborator(context.RepositoryOwner, context.RepositoryName, "m-zuber-octokit-integration-tests"); + + Assert.True(isCollab); + } + } + } +} \ No newline at end of file diff --git a/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs b/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs index 5fb84fc9..5c9921d4 100644 --- a/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs +++ b/Octokit.Tests/Clients/RepoCollaboratorsClientTests.cs @@ -32,7 +32,26 @@ namespace Octokit.Tests.Clients var client = new RepoCollaboratorsClient(connection); client.GetAll("owner", "test"); - connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators")); + connection.Received().GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators"), Args.ApiOptions); + } + + [Fact] + public void RequestsCorrectUrlWithApiOptions() + { + var connection = Substitute.For(); + var client = new RepoCollaboratorsClient(connection); + + var options = new ApiOptions + { + PageSize = 1, + PageCount = 1, + StartPage = 1 + }; + + client.GetAll("owner", "test", options); + + connection.Received() + .GetAll(Arg.Is(u => u.ToString() == "repos/owner/test/collaborators"), options); } [Fact] @@ -44,6 +63,10 @@ namespace Octokit.Tests.Clients await Assert.ThrowsAsync(() => client.GetAll("", "test")); await Assert.ThrowsAsync(() => client.GetAll("owner", null)); await Assert.ThrowsAsync(() => client.GetAll("owner", "")); + + await Assert.ThrowsAsync(() => client.GetAll(null, "test", ApiOptions.None)); + await Assert.ThrowsAsync(() => client.GetAll("owner", null, ApiOptions.None)); + await Assert.ThrowsAsync(() => client.GetAll("owner", "test", null)); } } diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index 4d36e03a..b2f56c40 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -204,6 +204,7 @@ + diff --git a/Octokit.Tests/Reactive/ObservableRepoCollaboratorsClientTests.cs b/Octokit.Tests/Reactive/ObservableRepoCollaboratorsClientTests.cs new file mode 100644 index 00000000..5190c271 --- /dev/null +++ b/Octokit.Tests/Reactive/ObservableRepoCollaboratorsClientTests.cs @@ -0,0 +1,182 @@ +using System; +using System.Collections.Generic; +using System.Reactive.Linq; +using System.Threading.Tasks; +using NSubstitute; +using Octokit.Reactive; +using Octokit.Tests.Helpers; +using Xunit; + +namespace Octokit.Tests.Reactive +{ + public class ObservableRepoCollaboratorsClientTests + { + public class TheGetAllMethod + { + private readonly IGitHubClient _githubClient; + private readonly IObservableRepoCollaboratorsClient _client; + private const string owner = "owner"; + private const string name = "name"; + + public TheGetAllMethod() + { + _githubClient = Substitute.For(); + _client = new ObservableRepoCollaboratorsClient(_githubClient); + } + + [Fact] + public void EnsuresNonNullArguments() + { + Assert.Throws(() => _client.GetAll(null, name)); + Assert.Throws(() => _client.GetAll(owner, null)); + Assert.Throws(() => _client.GetAll(owner, name, null)); + } + + [Fact] + public void EnsuresNonEmptyArguments() + { + Assert.Throws(() => _client.GetAll("", name)); + Assert.Throws(() => _client.GetAll(owner, "")); + } + + [Fact] + public async Task EnsuresNonWhitespaceArguments() + { + await AssertEx.ThrowsWhenGivenWhitespaceArgument( + async whitespace => await _client.GetAll(whitespace, name)); + await AssertEx.ThrowsWhenGivenWhitespaceArgument( + async whitespace => await _client.GetAll(owner, whitespace)); + } + + [Fact] + public void RequestsCorrectUrl() + { + var expectedUrl = string.Format("repos/{0}/{1}/collaborators", owner, name); + + _client.GetAll(owner, name); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 0), + Arg.Any()); + } + + [Fact] + public void RequestsCorrectUrlWithApiOptions() + { + var expectedUrl = string.Format("repos/{0}/{1}/collaborators", owner, name); + + // all properties are setted => only 2 options (StartPage, PageSize) in dictionary + var options = new ApiOptions + { + StartPage = 1, + PageCount = 1, + PageSize = 1 + }; + + _client.GetAll(owner, name, options); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 2), + null); + + // StartPage is setted => only 1 option (StartPage) in dictionary + options = new ApiOptions + { + StartPage = 1 + }; + + _client.GetAll(owner, name, options); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 1), + null); + + // PageCount is setted => none of options in dictionary + options = new ApiOptions + { + PageCount = 1 + }; + + _client.GetAll(owner, name, options); + _githubClient.Connection.Received(1) + .Get>(Arg.Is(u => u.ToString() == expectedUrl), + Arg.Is>(dictionary => dictionary.Count == 0), + null); + } + } + + public class TheAddMethod + { + private readonly IGitHubClient _githubClient; + private IObservableRepoCollaboratorsClient _client; + + public TheAddMethod() + { + _githubClient = Substitute.For(); + } + + private void SetupWithoutNonReactiveClient() + { + _client = new ObservableRepoCollaboratorsClient(_githubClient); + } + + private void SetupWithNonReactiveClient() + { + var deploymentsClient = new RepoCollaboratorsClient(Substitute.For()); + _githubClient.Repository.Collaborator.Returns(deploymentsClient); + _client = new ObservableRepoCollaboratorsClient(_githubClient); + } + + [Fact] + public void EnsuresNonNullArguments() + { + SetupWithNonReactiveClient(); + + Assert.Throws(() => _client.Add(null, "repo", "user")); + Assert.Throws(() => _client.Add("owner", null, "user")); + Assert.Throws(() => _client.Add("owner", "repo", null)); + } + + [Fact] + public void EnsuresNonEmptyArguments() + { + SetupWithNonReactiveClient(); + + Assert.Throws(() => _client.Add("", "repo", "user")); + Assert.Throws(() => _client.Add("owner", "", "user")); + } + + [Fact] + public async Task EnsuresNonWhitespaceArguments() + { + SetupWithNonReactiveClient(); + + await AssertEx.ThrowsWhenGivenWhitespaceArgument( + async whitespace => await _client.Add(whitespace, "repo", "user")); + await AssertEx.ThrowsWhenGivenWhitespaceArgument( + async whitespace => await _client.Add("owner", whitespace, "user")); + } + + [Fact] + public void CallsCreateOnRegularDeploymentsClient() + { + SetupWithoutNonReactiveClient(); + + _client.Add("owner", "repo", "user"); + + _githubClient.Repository.Collaborator.Received(1).Add(Arg.Is("owner"), + Arg.Is("repo"), + Arg.Is("user")); + } + } + + public class TheCtor + { + [Fact] + public void EnsuresNonNullArguments() + { + Assert.Throws(() => new ObservableRepoCollaboratorsClient(null)); + } + } + } +} diff --git a/Octokit/Clients/IRepoCollaboratorsClient.cs b/Octokit/Clients/IRepoCollaboratorsClient.cs index 85c06812..d0513cc5 100644 --- a/Octokit/Clients/IRepoCollaboratorsClient.cs +++ b/Octokit/Clients/IRepoCollaboratorsClient.cs @@ -19,10 +19,25 @@ namespace Octokit /// /// See the API documentation for more information. /// + /// The owner of the repository + /// The name of the repository /// Thrown when a general API error occurs. /// A of . Task> GetAll(string owner, string repo); + /// + /// Gets all the collaborators on a repository. + /// + /// + /// See the API documentation for more information. + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// Thrown when a general API error occurs. + /// A of . + Task> GetAll(string owner, string repo, ApiOptions options); + /// /// Checks if a user is a collaborator on a repo /// diff --git a/Octokit/Clients/RepoCollaboratorsClient.cs b/Octokit/Clients/RepoCollaboratorsClient.cs index db0d41d0..73ac70b6 100644 --- a/Octokit/Clients/RepoCollaboratorsClient.cs +++ b/Octokit/Clients/RepoCollaboratorsClient.cs @@ -28,6 +28,8 @@ namespace Octokit /// /// See the API documentation for more information. /// + /// The owner of the repository + /// The name of the repository /// Thrown when a general API error occurs. /// A of . public Task> GetAll(string owner, string repo) @@ -35,8 +37,27 @@ namespace Octokit Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); Ensure.ArgumentNotNullOrEmptyString(repo, "repo"); - var endpoint = ApiUrls.RepoCollaborators(owner, repo); - return ApiConnection.GetAll(endpoint); + return GetAll(owner, repo, ApiOptions.None); + } + + /// + /// Gets all the collaborators on a repository. + /// + /// + /// See the API documentation for more information. + /// + /// The owner of the repository + /// The name of the repository + /// Options for changing the API response + /// Thrown when a general API error occurs. + /// A of . + public Task> GetAll(string owner, string repo, ApiOptions options) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(repo, "repo"); + Ensure.ArgumentNotNull(options, "options"); + + return ApiConnection.GetAll(ApiUrls.RepoCollaborators(owner, repo), options); } ///