From fb6adcb92b10db683fcf24bc2e4e79e6108062b6 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 8 Apr 2014 01:25:25 -0300 Subject: [PATCH 1/6] Implemented Get / IsMember for teams Issue #331. Added integration tests and a new attribute that requires a new environment variable with an organization to test. Conflicts: Octokit.Tests.Integration/Octokit.Tests.Integration.csproj --- .../Clients/TeamsClientTests.cs | 123 ++++++++++++++++++ Octokit.Tests.Integration/Helper.cs | 11 ++ .../Octokit.Tests.Integration.csproj | 4 +- .../OrganizationTestAttribute.cs | 19 +++ Octokit/Clients/ITeamsClient.cs | 21 +++ Octokit/Clients/TeamsClient.cs | 42 +++++- Octokit/Helpers/ApiUrls.cs | 13 +- 7 files changed, 228 insertions(+), 5 deletions(-) create mode 100644 Octokit.Tests.Integration/Clients/TeamsClientTests.cs create mode 100644 Octokit.Tests.Integration/OrganizationTestAttribute.cs diff --git a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs new file mode 100644 index 0000000000..2017e3f4c8 --- /dev/null +++ b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs @@ -0,0 +1,123 @@ +using System.Linq; +using System.Net; +using System.Reactive.Linq; +using System.Threading.Tasks; +using Octokit; +using Octokit.Internal; +using Octokit.Tests.Helpers; +using Octokit.Tests.Integration; +using Xunit; +using System; +using System.Collections.Generic; +using Xunit.Sdk; + +public class TeamsClientTests +{ + public class TheCreateMethod + { + [OrganizationTest] + public async Task FailsWhenNotAuthenticated() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")); + var newTeam = new NewTeam("Test"); + + var e = await AssertEx.Throws(async + () => await github.Organization.Team.CreateTeam(Helper.Organization, newTeam)); + + Assert.Equal(HttpStatusCode.Unauthorized, e.StatusCode); + } + + [OrganizationTest] + public async Task FailsWhenAuthenticatedWithBadCredentials() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = new Credentials(Helper.Credentials.Login, "bad-password") + }; + var newTeam = new NewTeam("Test"); + + var e = await AssertEx.Throws(async + () => await github.Organization.Team.CreateTeam(Helper.Organization, newTeam)); + Assert.Equal(HttpStatusCode.Unauthorized, e.StatusCode); + } + + [OrganizationTest] + public async Task SucceedsWhenAuthenticated() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = Helper.Credentials + }; + + var newTeam = new NewTeam(Guid.NewGuid().ToString()); + + var team = await github.Organization.Team.CreateTeam(Helper.Organization, newTeam); + + Assert.Equal(newTeam.Name, team.Name); + } + } + + public class TheIsMemberMethod + { + readonly Team team; + + public TheIsMemberMethod() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) { Credentials = Helper.Credentials }; + + team = github.Organization.Team.GetAllTeams(Helper.Organization).Result.First(); + } + + //TODO: seems like a bug in Github: it's actually returning the membership information! + //Maybe because it's a public organization? + //[OrganizationTest] + public async Task FailsWhenNotAuthenticated() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")); + + var e = await AssertEx.Throws(async + () => await github.Organization.Team.IsMember(team.Id, Helper.UserName)); + + Assert.Equal(HttpStatusCode.Unauthorized, e.StatusCode); + } + + [OrganizationTest] + public async Task FailsWhenAuthenticatedWithBadCredentials() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = new Credentials(Helper.Credentials.Login, "bad-password") + }; + + var e = await AssertEx.Throws(async + () => await github.Organization.Team.IsMember(team.Id, Helper.UserName)); + Assert.Equal(HttpStatusCode.Unauthorized, e.StatusCode); + } + + [OrganizationTest] + public async Task GetsIsMemberWhenAuthenticated() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = Helper.Credentials + }; + + var isMember = await github.Organization.Team.IsMember(team.Id, Helper.UserName); + + Assert.True(isMember); + } + + [OrganizationTest] + public async Task GetsIsMemberFalseForNonMemberWhenAuthenticated() + { + var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) + { + Credentials = Helper.Credentials + }; + + var isMember = await github.Organization.Team.IsMember(team.Id, "foo"); + + Assert.False(isMember); + } + } +} diff --git a/Octokit.Tests.Integration/Helper.cs b/Octokit.Tests.Integration/Helper.cs index caf73046f1..3b44476f03 100644 --- a/Octokit.Tests.Integration/Helper.cs +++ b/Octokit.Tests.Integration/Helper.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Net.Http.Headers; namespace Octokit.Tests.Integration @@ -9,6 +10,7 @@ public static class Helper { var githubUsername = Environment.GetEnvironmentVariable("OCTOKIT_GITHUBUSERNAME"); UserName = githubUsername; + Organization = Environment.GetEnvironmentVariable("OCTOKIT_GITHUBORGANIZATION"); var githubToken = Environment.GetEnvironmentVariable("OCTOKIT_OAUTHTOKEN"); @@ -23,7 +25,16 @@ public static class Helper return new Credentials(githubUsername, githubPassword); }); + static Helper() + { + // Force reading of environment variables. + // This wasn't happening if UserName/Organization were + // retrieved before Credentials. + Debug.WriteIf(Credentials == null, "No credentials specified."); + } + public static string UserName { get; private set; } + public static string Organization { get; private set; } public static Credentials Credentials { get { return _credentialsThunk.Value; }} diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index 31d362838b..dc62d36612 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -78,9 +78,11 @@ + + @@ -121,4 +123,4 @@ --> - \ No newline at end of file + diff --git a/Octokit.Tests.Integration/OrganizationTestAttribute.cs b/Octokit.Tests.Integration/OrganizationTestAttribute.cs new file mode 100644 index 0000000000..61b00cfbaa --- /dev/null +++ b/Octokit.Tests.Integration/OrganizationTestAttribute.cs @@ -0,0 +1,19 @@ +using System.Collections.Generic; +using Xunit.Sdk; + +namespace Octokit.Tests.Integration +{ + public class OrganizationTestAttribute : IntegrationTestAttribute + { + protected override IEnumerable EnumerateTestCommands(IMethodInfo testMethod) + { + if (Helper.Organization == null) + return new[] + { + new SkipCommand(testMethod, MethodUtility.GetDisplayName(testMethod), "Automation settings not configured. Please set the OCTOKIT_GITHUBORGANIZATION environment variable to a GitHub organization owned by the test account specified in OCTOKIT_GITHUBUSERNAME.") + }; + else + return base.EnumerateTestCommands(testMethod); + } + } +} diff --git a/Octokit/Clients/ITeamsClient.cs b/Octokit/Clients/ITeamsClient.cs index 8cafd72b62..be759a8e68 100644 --- a/Octokit/Clients/ITeamsClient.cs +++ b/Octokit/Clients/ITeamsClient.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; #endif using System.Threading.Tasks; +using System.Diagnostics.CodeAnalysis; namespace Octokit { @@ -13,6 +14,18 @@ namespace Octokit /// public interface ITeamsClient { + /// + /// Gets a single by identifier. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#get-team + /// + /// The team identifier. + /// The with the given identifier. + [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get", + Justification = "Method makes a network request")] + Task Get(int id); + /// /// Returns all s for the current org. /// @@ -41,5 +54,13 @@ public interface ITeamsClient /// Task Delete(int id); + /// + /// Gets whether the user with the given + /// is a member of the team with the given . + /// + /// The team to check. + /// The user to check. + /// if the user is a member of the team; otherwise. + Task IsMember(int id, string login); } } diff --git a/Octokit/Clients/TeamsClient.cs b/Octokit/Clients/TeamsClient.cs index d2ae08c9ac..462ea4038c 100644 --- a/Octokit/Clients/TeamsClient.cs +++ b/Octokit/Clients/TeamsClient.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; #endif using System.Threading.Tasks; +using System.Diagnostics.CodeAnalysis; namespace Octokit { @@ -23,6 +24,21 @@ public TeamsClient(IApiConnection apiConnection) { } + /// + /// Gets a single by identifier. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#get-team + /// + /// The team identifier. + /// The with the given identifier. + public Task Get(int id) + { + var endpoint = ApiUrls.Teams(id); + + return ApiConnection.Get(endpoint); + } + /// /// Returns all s for the current org. /// @@ -60,7 +76,7 @@ public Task Update(int id, UpdateTeam team) { Ensure.ArgumentNotNull(team, "team"); - var endpoint = ApiUrls.TeamsUpdateOrDelete(id); + var endpoint = ApiUrls.Teams(id); return ApiConnection.Patch(endpoint, team); } @@ -71,8 +87,30 @@ public Task Update(int id, UpdateTeam team) /// public Task Delete(int id) { - var endpoint = ApiUrls.TeamsUpdateOrDelete(id); + var endpoint = ApiUrls.Teams(id); return ApiConnection.Delete(endpoint); } + + /// + /// Gets whether the user with the given + /// is a member of the team with the given . + /// + /// The team to check. + /// The user to check. + /// if the user is a member of the team; otherwise. + public async Task IsMember(int id, string login) + { + var endpoint = ApiUrls.TeamMember(id, login); + + try + { + var response = await ApiConnection.Connection.GetAsync(endpoint); + return response.StatusCode == System.Net.HttpStatusCode.NoContent; + } + catch (NotFoundException) + { + return false; + } + } } } diff --git a/Octokit/Helpers/ApiUrls.cs b/Octokit/Helpers/ApiUrls.cs index 2e3fe12929..5f87286787 100644 --- a/Octokit/Helpers/ApiUrls.cs +++ b/Octokit/Helpers/ApiUrls.cs @@ -937,15 +937,24 @@ public static Uri OrganizationTeams(string organization) /// /// returns the for teams - /// use for update or deleting a team /// /// /// - public static Uri TeamsUpdateOrDelete(int id) + public static Uri Teams(int id) { return "teams/{0}".FormatUri(id); } + /// + /// returns the for team members + /// + /// The team id + /// The user login. + public static Uri TeamMember(int id, string login) + { + return "teams/{0}/members/{1}".FormatUri(id, login); + } + /// /// returns the for teams /// use for update or deleting a team From 773859f6a5fb06a976b67384978095962dc4e383 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 8 Apr 2014 09:49:21 -0300 Subject: [PATCH 2/6] Update documentation on how to run the integration tests. --- CONTRIBUTING.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4f4939a159..45beba7aaa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -97,13 +97,15 @@ Run this command to confirm all the tests pass: `.\build` Octokit has integration tests that access the GitHub API, but they must be configured before they will be executed. To configure the tests, create a -test GitHub account (i.e., don't use your real GitHub account) and then set -the following two environment variables: +test GitHub account (i.e., don't use your real GitHub account) and a test +organization owned by that account. Then set the following environment variables: `OCTOKIT_GITHUBUSERNAME` (set this to the test account's username) `OCTOKIT_GITHUBPASSWORD` (set this to the test account's password) +`OCTOKIT_GITHUBORGANIZATION` (set this to the test account's organization) -Once both of these are set, the integration tests will be executed both when + +Once these are set, the integration tests will be executed both when running the FullBuild MSBuild target, and when running the Octokit.Tests.Integration assembly through an xUnit.net-friendly test runner. From f76c74bd8097dbc0314942c41e1448798711ccaa Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 22 Apr 2014 17:24:52 -0300 Subject: [PATCH 3/6] Fix build errors from renamed ITeamsClient members. --- Octokit.Tests.Integration/Clients/TeamsClientTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs index 2017e3f4c8..8e2f8d2445 100644 --- a/Octokit.Tests.Integration/Clients/TeamsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/TeamsClientTests.cs @@ -22,7 +22,7 @@ public async Task FailsWhenNotAuthenticated() var newTeam = new NewTeam("Test"); var e = await AssertEx.Throws(async - () => await github.Organization.Team.CreateTeam(Helper.Organization, newTeam)); + () => await github.Organization.Team.Create(Helper.Organization, newTeam)); Assert.Equal(HttpStatusCode.Unauthorized, e.StatusCode); } @@ -37,7 +37,7 @@ public async Task FailsWhenAuthenticatedWithBadCredentials() var newTeam = new NewTeam("Test"); var e = await AssertEx.Throws(async - () => await github.Organization.Team.CreateTeam(Helper.Organization, newTeam)); + () => await github.Organization.Team.Create(Helper.Organization, newTeam)); Assert.Equal(HttpStatusCode.Unauthorized, e.StatusCode); } @@ -51,7 +51,7 @@ public async Task SucceedsWhenAuthenticated() var newTeam = new NewTeam(Guid.NewGuid().ToString()); - var team = await github.Organization.Team.CreateTeam(Helper.Organization, newTeam); + var team = await github.Organization.Team.Create(Helper.Organization, newTeam); Assert.Equal(newTeam.Name, team.Name); } @@ -65,7 +65,7 @@ public TheIsMemberMethod() { var github = new GitHubClient(new ProductHeaderValue("OctokitTests")) { Credentials = Helper.Credentials }; - team = github.Organization.Team.GetAllTeams(Helper.Organization).Result.First(); + team = github.Organization.Team.GetAll(Helper.Organization).Result.First(); } //TODO: seems like a bug in Github: it's actually returning the membership information! From 5f136c926e7342230cf892d4fb724a57a95b673e Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 22 Apr 2014 18:15:44 -0300 Subject: [PATCH 4/6] Added missing reactive members. --- .../Clients/IObservableTeamsClient.cs | 19 ++++++++++++++ .../Clients/ObservableTeamsClient.cs | 26 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableTeamsClient.cs b/Octokit.Reactive/Clients/IObservableTeamsClient.cs index f8ba608b35..e6f0a72f7c 100644 --- a/Octokit.Reactive/Clients/IObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/IObservableTeamsClient.cs @@ -11,6 +11,16 @@ namespace Octokit.Reactive /// public interface IObservableTeamsClient { + /// + /// Gets a single by identifier. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#get-team + /// + /// The team identifier. + /// The with the given identifier. + IObservable Get(int id); + /// /// Returns all s for the current org. /// @@ -38,5 +48,14 @@ public interface IObservableTeamsClient /// Thrown when a general API error occurs. /// IObservable Delete(int id); + + /// + /// Gets whether the user with the given + /// is a member of the team with the given . + /// + /// The team to check. + /// The user to check. + /// if the user is a member of the team; otherwise. + IObservable IsMember(int id, string login); } } diff --git a/Octokit.Reactive/Clients/ObservableTeamsClient.cs b/Octokit.Reactive/Clients/ObservableTeamsClient.cs index 2c2a09915a..b295ac1ad3 100644 --- a/Octokit.Reactive/Clients/ObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/ObservableTeamsClient.cs @@ -27,6 +27,19 @@ public ObservableTeamsClient(IGitHubClient client) _client = client.Organization.Team; } + /// + /// Gets a single by identifier. + /// + /// + /// https://developer.github.com/v3/orgs/teams/#get-team + /// + /// The team identifier. + /// The with the given identifier. + public IObservable Get(int id) + { + return _client.Get(id).ToObservable(); + } + /// /// Returns all s for the current org. /// @@ -67,5 +80,18 @@ public IObservable Delete(int id) { return _client.Delete(id).ToObservable(); } + + /// + /// Gets whether the user with the given + /// is a member of the team with the given . + /// + /// The team to check. + /// The user to check. + /// if the user is a member of the team; otherwise. + public IObservable IsMember(int id, string login) + { + return _client.IsMember(id, login).ToObservable(); + } + } } From e668267e9940f2d4557c967ec2a25656fed6d600 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 22 Apr 2014 18:20:57 -0300 Subject: [PATCH 5/6] Fixed code analysis issue with naming of Get --- Octokit.Reactive/Clients/IObservableTeamsClient.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Octokit.Reactive/Clients/IObservableTeamsClient.cs b/Octokit.Reactive/Clients/IObservableTeamsClient.cs index e6f0a72f7c..67db723dac 100644 --- a/Octokit.Reactive/Clients/IObservableTeamsClient.cs +++ b/Octokit.Reactive/Clients/IObservableTeamsClient.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.Reactive; namespace Octokit.Reactive @@ -19,6 +20,8 @@ public interface IObservableTeamsClient /// /// The team identifier. /// The with the given identifier. + [SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get", + Justification = "Method makes a network request")] IObservable Get(int id); /// From 3c3c6f3bc87d087ec824d95b5bfb661845d12ca2 Mon Sep 17 00:00:00 2001 From: Daniel Cazzulino Date: Tue, 22 Apr 2014 18:21:13 -0300 Subject: [PATCH 6/6] Added missing check for empty/null login and its tests --- Octokit.Tests/Clients/TeamsClientTests.cs | 20 ++++++++++++++++++++ Octokit/Clients/TeamsClient.cs | 2 ++ 2 files changed, 22 insertions(+) diff --git a/Octokit.Tests/Clients/TeamsClientTests.cs b/Octokit.Tests/Clients/TeamsClientTests.cs index a2ae38867e..377dd2a638 100644 --- a/Octokit.Tests/Clients/TeamsClientTests.cs +++ b/Octokit.Tests/Clients/TeamsClientTests.cs @@ -107,5 +107,25 @@ public void RequestsTheCorrectUrl() } } + public class TheIsMemberMethod + { + [Fact] + public void EnsuresNonNullLogin() + { + var connection = Substitute.For(); + var client = new TeamsClient(connection); + + AssertEx.Throws(() => client.IsMember(1, null)); + } + + [Fact] + public void EnsuresNonEmptyLogin() + { + var connection = Substitute.For(); + var client = new TeamsClient(connection); + + AssertEx.Throws(() => client.IsMember(1, "")); + } + } } } diff --git a/Octokit/Clients/TeamsClient.cs b/Octokit/Clients/TeamsClient.cs index 462ea4038c..48310ff5c1 100644 --- a/Octokit/Clients/TeamsClient.cs +++ b/Octokit/Clients/TeamsClient.cs @@ -100,6 +100,8 @@ public Task Delete(int id) /// if the user is a member of the team; otherwise. public async Task IsMember(int id, string login) { + Ensure.ArgumentNotNullOrEmptyString(login, "login"); + var endpoint = ApiUrls.TeamMember(id, login); try