From 084c513e066c76a531002dd42dbd76ba19c0f8c1 Mon Sep 17 00:00:00 2001 From: Devesh Khandelwal Date: Wed, 23 Mar 2016 13:20:43 +0530 Subject: [PATCH 1/4] Add ApiOptions overloads to Teams client + tests. --- Octokit.Tests/Clients/TeamsClientTests.cs | 20 ++++--- Octokit/Clients/ITeamsClient.cs | 40 ++++++++++++++ Octokit/Clients/TeamsClient.cs | 67 +++++++++++++++++++++-- 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/Octokit.Tests/Clients/TeamsClientTests.cs b/Octokit.Tests/Clients/TeamsClientTests.cs index dfa09458..6452c346 100644 --- a/Octokit.Tests/Clients/TeamsClientTests.cs +++ b/Octokit.Tests/Clients/TeamsClientTests.cs @@ -46,7 +46,9 @@ namespace Octokit.Tests.Clients client.GetAll("orgName"); - connection.Received().GetAll(Arg.Is(u => u.ToString() == "orgs/orgName/teams")); + connection.Received().GetAll( + Arg.Is(u => u.ToString() == "orgs/orgName/teams"), + Args.ApiOptions); } [Fact] @@ -55,6 +57,7 @@ namespace Octokit.Tests.Clients var teams = new TeamsClient(Substitute.For()); await Assert.ThrowsAsync(() => teams.GetAll(null)); + await Assert.ThrowsAsync(() => teams.GetAll("orgName", null)); } } @@ -68,7 +71,9 @@ namespace Octokit.Tests.Clients client.GetAllMembers(1); - connection.Received().GetAll(Arg.Is(u => u.ToString() == "teams/1/members")); + connection.Received().GetAll( + Arg.Is(u => u.ToString() == "teams/1/members"), + Args.ApiOptions); } } @@ -189,7 +194,9 @@ namespace Octokit.Tests.Clients client.GetAllForCurrent(); - connection.Received().GetAll(Arg.Is(u => u.ToString() == "user/teams")); + connection.Received().GetAll( + Arg.Is(u => u.ToString() == "user/teams"), + Args.ApiOptions); } } @@ -244,13 +251,12 @@ namespace Octokit.Tests.Clients { var connection = Substitute.For(); var client = new TeamsClient(connection); - client.GetAllRepositories(1); - - connection.Received().GetAll(Arg.Is(u => u.ToString() == "teams/1/repos")); client.GetAllRepositories(1); - connection.Received().GetAll(Arg.Is(u => u.ToString() == "teams/1/repos")); + connection.Received().GetAll( + Arg.Is(u => u.ToString() == "teams/1/repos"), + Args.ApiOptions); } } diff --git a/Octokit/Clients/ITeamsClient.cs b/Octokit/Clients/ITeamsClient.cs index 45342768..ec832db3 100644 --- a/Octokit/Clients/ITeamsClient.cs +++ b/Octokit/Clients/ITeamsClient.cs @@ -30,10 +30,20 @@ namespace Octokit /// /// Returns all s for the current org. /// + /// Organization for which to list all teams. /// Thrown when a general API error occurs. /// A list of the orgs's teams s. Task> GetAll(string org); + /// + /// Returns all s for the current org. + /// + /// Organization for which to list all teams. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the orgs's teams s. + Task> GetAll(string org, ApiOptions options); + /// /// Returns all s for the current user. /// @@ -42,6 +52,15 @@ namespace Octokit [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] Task> GetAllForCurrent(); + /// + /// Returns all s for the current user. + /// + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the user's s. + [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] + Task> GetAllForCurrent(ApiOptions options); + /// /// Returns all members of the given team. /// @@ -52,6 +71,17 @@ namespace Octokit /// A list of the team's member s. Task> GetAllMembers(int id); + /// + /// Returns all members of the given team. + /// + /// The team identifier + /// Options to change API behaviour. + /// + /// https://developer.github.com/v3/orgs/teams/#list-team-members + /// + /// A list of the team's member s. + Task> GetAllMembers(int id, ApiOptions options); + /// /// Returns newly created for the current org. /// @@ -118,10 +148,20 @@ namespace Octokit /// /// Returns all team's repositories. /// + /// Team Id to list repos. /// Thrown when a general API error occurs. /// The team's repositories Task> GetAllRepositories(int id); + /// + /// Returns all team's repositories. + /// + /// Team Id to list repos. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// The team's repositories + Task> GetAllRepositories(int id, ApiOptions options); + /// /// Add a repository to the team /// diff --git a/Octokit/Clients/TeamsClient.cs b/Octokit/Clients/TeamsClient.cs index e001d4cc..0b12ab8e 100644 --- a/Octokit/Clients/TeamsClient.cs +++ b/Octokit/Clients/TeamsClient.cs @@ -42,14 +42,28 @@ namespace Octokit /// /// Returns all s for the current org. /// + /// Organization to list teams of. /// Thrown when a general API error occurs. /// A list of the orgs's teams s. public Task> GetAll(string org) + { + return GetAll(org, ApiOptions.None); + } + + /// + /// Returns all s for the current org. + /// + /// Organization to list teams of. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the orgs's teams s. + public Task> GetAll(string org, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(org, "org"); + Ensure.ArgumentNotNull(options, "options"); var endpoint = ApiUrls.OrganizationTeams(org); - return ApiConnection.GetAll(endpoint); + return ApiConnection.GetAll(endpoint, options); } /// @@ -59,8 +73,22 @@ namespace Octokit /// A list of the user's s. public Task> GetAllForCurrent() { + return GetAllForCurrent(ApiOptions.None); + } + + /// + /// Returns all s for the current user. + /// + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the user's s. + public Task> GetAllForCurrent(ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + var endpoint = ApiUrls.UserTeams(); - return ApiConnection.GetAll(endpoint); + + return ApiConnection.GetAll(endpoint, options); } /// @@ -73,9 +101,25 @@ namespace Octokit /// A list of the team's member s. public Task> GetAllMembers(int id) { + return GetAllMembers(id, ApiOptions.None); + } + + /// + /// Returns all members of the given team. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#list-team-members + /// + /// The team identifier + /// Options to change API behaviour. + /// A list of the team's member s. + public Task> GetAllMembers(int id, ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + var endpoint = ApiUrls.TeamMembers(id); - return ApiConnection.GetAll(endpoint); + return ApiConnection.GetAll(endpoint, options); } /// @@ -236,13 +280,28 @@ namespace Octokit /// /// Returns all team's repositories. /// + /// Team Id. /// Thrown when a general API error occurs. /// The team's repositories public Task> GetAllRepositories(int id) { + return GetAllRepositories(id, ApiOptions.None); + } + + /// + /// Returns all team's repositories. + /// + /// Team Id. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// The team's repositories + public Task> GetAllRepositories(int id, ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + var endpoint = ApiUrls.TeamRepositories(id); - return ApiConnection.GetAll(endpoint); + return ApiConnection.GetAll(endpoint, options); } /// From 486e315c02b52d9ab517a0dfcbb2b2ff690d3ca5 Mon Sep 17 00:00:00 2001 From: Devesh Khandelwal Date: Wed, 23 Mar 2016 14:57:50 +0530 Subject: [PATCH 2/4] Add ApiOptions overload to Reactive Teams client. --- .../Clients/IObservableTeamsClient.cs | 43 +++++++++++- .../Clients/ObservableTeamsClient.cs | 66 +++++++++++++++++-- .../Reactive/ObservableTeamsClientTests.cs | 2 +- 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/Octokit.Reactive/Clients/IObservableTeamsClient.cs b/Octokit.Reactive/Clients/IObservableTeamsClient.cs index e6671028..dc17b6f4 100644 --- a/Octokit.Reactive/Clients/IObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/IObservableTeamsClient.cs @@ -27,10 +27,20 @@ namespace Octokit.Reactive /// /// Returns all s for the current org. /// + /// Organization to list all teams of. /// Thrown when a general API error occurs. /// A list of the orgs's teams s. IObservable GetAll(string org); + /// + /// Returns all s for the current org. + /// + /// Organization to list all teams of. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the orgs's teams s. + IObservable GetAll(string org, ApiOptions options); + /// /// Returns all s for the current user. /// @@ -39,17 +49,38 @@ namespace Octokit.Reactive [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] IObservable GetAllForCurrent(); + /// + /// Returns all s for the current user. + /// + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the user's s. + [SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")] + IObservable GetAllForCurrent(ApiOptions options); + /// /// Returns all members of the given team. /// - /// The team identifier /// /// https://developer.github.com/v3/orgs/teams/#list-team-members /// + /// The team identifier /// Thrown when a general API error occurs. /// A list of the team's member s. IObservable GetAllMembers(int id); + /// + /// Returns all members of the given team. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#list-team-members + /// + /// The team identifier + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the team's member s. + IObservable GetAllMembers(int id, ApiOptions options); + /// /// Returns newly created for the current org. /// @@ -116,10 +147,20 @@ namespace Octokit.Reactive /// /// Returns all team's repositories. /// + /// Team Id. /// Thrown when a general API error occurs. /// The team's repositories IObservable GetAllRepositories(int id); + /// + /// Returns all team's repositories. + /// + /// Team Id. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// The team's repositories + IObservable GetAllRepositories(int id, ApiOptions options); + /// /// Remove a repository from the team /// diff --git a/Octokit.Reactive/Clients/ObservableTeamsClient.cs b/Octokit.Reactive/Clients/ObservableTeamsClient.cs index c4d44985..cf21e0c9 100644 --- a/Octokit.Reactive/Clients/ObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/ObservableTeamsClient.cs @@ -46,9 +46,23 @@ namespace Octokit.Reactive /// Thrown when a general API error occurs. /// A list of the orgs's teams s. public IObservable GetAll(string org) + { + return GetAll(org, ApiOptions.None); + } + + /// + /// Returns all s for the current org. + /// + /// Organization to list all teams of. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the orgs's teams s. + public IObservable GetAll(string org, ApiOptions options) { Ensure.ArgumentNotNullOrEmptyString(org, "org"); - return _connection.GetAndFlattenAllPages(ApiUrls.OrganizationTeams(org)); + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.OrganizationTeams(org), options); } /// @@ -58,7 +72,20 @@ namespace Octokit.Reactive /// A list of the user's s. public IObservable GetAllForCurrent() { - return _connection.GetAndFlattenAllPages(ApiUrls.UserTeams()); + return GetAllForCurrent(ApiOptions.None); + } + + /// + /// Returns all s for the current user. + /// + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the user's s. + public IObservable GetAllForCurrent(ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.UserTeams(), options); } /// @@ -72,7 +99,24 @@ namespace Octokit.Reactive /// A list of the team's member s. public IObservable GetAllMembers(int id) { - return _connection.GetAndFlattenAllPages(ApiUrls.TeamMembers(id)); + return GetAllMembers(id, ApiOptions.None); + } + + /// + /// Returns all members of the given team. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#list-team-members + /// + /// The team identifier + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// A list of the team's member s. + public IObservable GetAllMembers(int id, ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.TeamMembers(id), options); } /// @@ -166,7 +210,21 @@ namespace Octokit.Reactive /// The team's repositories public IObservable GetAllRepositories(int id) { - return _connection.GetAndFlattenAllPages(ApiUrls.TeamRepositories(id)); + return GetAllRepositories(id, ApiOptions.None); + } + + /// + /// Returns all team's repositories. + /// + /// Team Id. + /// Options to change API behaviour. + /// Thrown when a general API error occurs. + /// The team's repositories + public IObservable GetAllRepositories(int id, ApiOptions options) + { + Ensure.ArgumentNotNull(options, "options"); + + return _connection.GetAndFlattenAllPages(ApiUrls.TeamRepositories(id), options); } /// diff --git a/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs index bf8dc203..68b7dc9d 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs @@ -26,7 +26,7 @@ public class ObservableTeamsClientTests var client = new ObservableTeamsClient(github); - var member = await client.GetAllMembers(team.Id); + var member = await client.GetAllMembers(team.Id, ApiOptions.None); Assert.Equal(Helper.UserName, member.Login); } From 27d6e3bfc9db27745f8ed476054d86491fdce9bf Mon Sep 17 00:00:00 2001 From: Devesh Khandelwal Date: Wed, 30 Mar 2016 20:56:33 +0530 Subject: [PATCH 3/4] Tests added. --- .../Clients/TeamsClientTests.cs | 2 +- .../Reactive/ObservableTeamsClientTests.cs | 6 +-- Octokit.Tests/Octokit.Tests.csproj | 1 + .../Reactive/ObservableTeamsClientTests.cs | 47 +++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 Octokit.Tests/Reactive/ObservableTeamsClientTests.cs diff --git a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs index 1e653c79..a80a6da0 100644 --- a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs @@ -48,7 +48,7 @@ public class TeamsClientTests public class TheGetAllForCurrentMethod { [IntegrationTest] - public async Task GetsIsMemberWhenAuthenticated() + public async Task GetsAllForCurrentWhenAuthenticated() { var github = Helper.GetAuthenticatedClient(); var teams = await github.Organization.Team.GetAllForCurrent(); diff --git a/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs b/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs index 68b7dc9d..d6743e8c 100644 --- a/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs +++ b/Octokit.Tests.Integration/Reactive/ObservableTeamsClientTests.cs @@ -10,13 +10,13 @@ public class ObservableTeamsClientTests { public class TheGetMembersMethod { - readonly Team team; + readonly Team _team; public TheGetMembersMethod() { var github = Helper.GetAuthenticatedClient(); - team = github.Organization.Team.GetAll(Helper.Organization).Result.First(); + _team = github.Organization.Team.GetAll(Helper.Organization).Result.First(); } [OrganizationTest] @@ -26,7 +26,7 @@ public class ObservableTeamsClientTests var client = new ObservableTeamsClient(github); - var member = await client.GetAllMembers(team.Id, ApiOptions.None); + var member = await client.GetAllMembers(_team.Id, ApiOptions.None); Assert.Equal(Helper.UserName, member.Login); } diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index 8fd268a0..4bb75c60 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -218,6 +218,7 @@ + diff --git a/Octokit.Tests/Reactive/ObservableTeamsClientTests.cs b/Octokit.Tests/Reactive/ObservableTeamsClientTests.cs new file mode 100644 index 00000000..eebd3491 --- /dev/null +++ b/Octokit.Tests/Reactive/ObservableTeamsClientTests.cs @@ -0,0 +1,47 @@ +using System; +using System.Reactive.Threading.Tasks; +using System.Threading.Tasks; +using NSubstitute; +using Octokit.Reactive; +using Xunit; + +namespace Octokit.Tests.Reactive +{ + public class ObservableTeamsClientTests + { + public class TheCreateMethod + { + [Fact] + public void PostsToCorrectUrl() + { + var team = new NewTeam("avengers"); + var github = Substitute.For(); + var client = new ObservableTeamsClient(github); + + client.Create("shield", team); + + github.Organization.Team.Received().Create("shield", team); + } + + [Fact] + public void EnsuresNotNullAndNonEmptyArguments() + { + var github = Substitute.For(); + var client = new ObservableTeamsClient(github); + + Assert.ThrowsAsync(() => client.Create("shield", null).ToTask()); + Assert.ThrowsAsync(() => client.Create(null, new NewTeam("avengers")).ToTask()); + Assert.ThrowsAsync(() => client.Create("", new NewTeam("avengers")).ToTask()); + } + } + + public class TheCtor + { + [Fact] + public void EnsuresNotNullGitHubClient() + { + Assert.Throws(() => new ObservableTeamsClient(null)); + } + } + } +} \ No newline at end of file From a1aa51a6bf05a7916c60a4a0086bc7c83e8e242d Mon Sep 17 00:00:00 2001 From: Devesh Khandelwal Date: Sat, 2 Apr 2016 01:01:00 +0530 Subject: [PATCH 4/4] Add Ensures in ObservableTeamsClient. --- .../Clients/ObservableTeamsClient.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Octokit.Reactive/Clients/ObservableTeamsClient.cs b/Octokit.Reactive/Clients/ObservableTeamsClient.cs index cf21e0c9..1711f78c 100644 --- a/Octokit.Reactive/Clients/ObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/ObservableTeamsClient.cs @@ -47,6 +47,8 @@ namespace Octokit.Reactive /// A list of the orgs's teams s. public IObservable GetAll(string org) { + Ensure.ArgumentNotNullOrEmptyString(org, "org"); + return GetAll(org, ApiOptions.None); } @@ -126,6 +128,9 @@ namespace Octokit.Reactive /// Newly created public IObservable Create(string org, NewTeam team) { + Ensure.ArgumentNotNullOrEmptyString(org, "org"); + Ensure.ArgumentNotNull(team, "team"); + return _client.Create(org, team).ToObservable(); } @@ -136,6 +141,8 @@ namespace Octokit.Reactive /// Updated public IObservable Update(int id, UpdateTeam team) { + Ensure.ArgumentNotNull(team, "team"); + return _client.Update(id, team).ToObservable(); } @@ -161,6 +168,8 @@ namespace Octokit.Reactive /// A result indicating the membership status public IObservable AddMembership(int id, string login) { + Ensure.ArgumentNotNullOrEmptyString(login, "login"); + return _client.AddMembership(id, login).ToObservable(); } @@ -175,6 +184,8 @@ namespace Octokit.Reactive /// if the user was removed from the team; otherwise. public IObservable RemoveMembership(int id, string login) { + Ensure.ArgumentNotNullOrEmptyString(login, "login"); + return _client.RemoveMembership(id, login).ToObservable(); } @@ -188,6 +199,8 @@ namespace Octokit.Reactive [Obsolete("Use GetMembership(id, login) to detect pending memberships")] public IObservable IsMember(int id, string login) { + Ensure.ArgumentNotNullOrEmptyString(login, "login"); + return _client.IsMember(id, login).ToObservable(); } @@ -200,6 +213,8 @@ namespace Octokit.Reactive /// A result indicating the membership status public IObservable GetMembership(int id, string login) { + Ensure.ArgumentNotNullOrEmptyString(login, "login"); + return _client.GetMembership(id, login).ToObservable(); } @@ -240,6 +255,9 @@ namespace Octokit.Reactive /// if the repository was added to the team; otherwise. public IObservable AddRepository(int id, string organization, string repoName) { + Ensure.ArgumentNotNullOrEmptyString(organization, "organization"); + Ensure.ArgumentNotNullOrEmptyString(repoName, "repoName"); + return _client.AddRepository(id, organization, repoName).ToObservable(); } @@ -251,6 +269,9 @@ namespace Octokit.Reactive /// public IObservable RemoveRepository(int id, string organization, string repoName) { + Ensure.ArgumentNotNullOrEmptyString(organization, "organization"); + Ensure.ArgumentNotNullOrEmptyString(repoName, "repoName"); + return _client.RemoveRepository(id, organization, repoName).ToObservable(); } @@ -266,6 +287,8 @@ namespace Octokit.Reactive /// if the repository is managed by the given team; otherwise. public IObservable IsRepositoryManagedByTeam(int id, string owner, string repo) { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(repo, "repo"); return _client.IsRepositoryManagedByTeam(id, owner, repo).ToObservable(); } }