diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3961fe99c..c8b88cc74 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -4,3 +4,5 @@ - Show a generic error message instead of the actual one for unhandled exceptions in non-verbose mode. - Exit code is now 1 instead of 0 in case of an error. - Errors are written to std error instead of std out. +- Adding Support to get multi page results from Github API. +- The Github to Github migrations are no longer limited to 30 repos. \ No newline at end of file diff --git a/src/Octoshift/GithubApi.cs b/src/Octoshift/GithubApi.cs index f29430ffe..498b6e827 100644 --- a/src/Octoshift/GithubApi.cs +++ b/src/Octoshift/GithubApi.cs @@ -38,22 +38,16 @@ public virtual async Task CreateTeam(string org, string teamName) public virtual async Task> GetTeamMembers(string org, string teamName) { - var url = $"https://api.github.com/orgs/{org}/teams/{teamName}/members"; + var url = $"https://api.github.com/orgs/{org}/teams/{teamName}/members?per_page=100"; - var response = await _client.GetAsync(url); - var data = JArray.Parse(response); - - return data.Children().Select(x => (string)x["login"]).ToList(); + return await _client.GetAllAsync(url).Select(x => (string)x["login"]).ToListAsync(); } public virtual async Task> GetRepos(string org) { - var url = $"https://api.github.com/orgs/{org}/repos"; - - var response = await _client.GetAsync(url); - var data = JArray.Parse(response); + var url = $"https://api.github.com/orgs/{org}/repos?per_page=100"; - return data.Children().Select(x => (string)x["name"]).ToList(); + return await _client.GetAllAsync(url).Select(x => (string)x["name"]).ToListAsync(); } public virtual async Task RemoveTeamMember(string org, string teamName, string member) diff --git a/src/Octoshift/GithubClient.cs b/src/Octoshift/GithubClient.cs index f68b36016..fda4e7792 100644 --- a/src/Octoshift/GithubClient.cs +++ b/src/Octoshift/GithubClient.cs @@ -1,7 +1,11 @@ using System; +using System.Collections.Generic; +using System.Linq; using System.Net.Http; using System.Net.Http.Headers; +using System.Text.RegularExpressions; using System.Threading.Tasks; +using Newtonsoft.Json.Linq; using OctoshiftCLI.Extensions; namespace OctoshiftCLI @@ -25,20 +29,36 @@ public GithubClient(OctoLogger log, HttpClient httpClient, string personalAccess } } - public virtual async Task GetAsync(string url) => await SendAsync(HttpMethod.Get, url); + public virtual async Task GetAsync(string url) => (await SendAsync(HttpMethod.Get, url)).Content; + + public virtual async IAsyncEnumerable GetAllAsync(string url) + { + var nextUrl = url; + do + { + var (content, headers) = await SendAsync(HttpMethod.Get, nextUrl); + foreach (var jToken in JArray.Parse(content)) + { + yield return jToken; + } + + nextUrl = GetNextUrl(headers); + } while (nextUrl != null); + } public virtual async Task PostAsync(string url, object body) => - await SendAsync(HttpMethod.Post, url, body); + (await SendAsync(HttpMethod.Post, url, body)).Content; public virtual async Task PutAsync(string url, object body) => - await SendAsync(HttpMethod.Put, url, body); + (await SendAsync(HttpMethod.Put, url, body)).Content; public virtual async Task PatchAsync(string url, object body) => - await SendAsync(HttpMethod.Patch, url, body); + (await SendAsync(HttpMethod.Patch, url, body)).Content; - public virtual async Task DeleteAsync(string url) => await SendAsync(HttpMethod.Delete, url); + public virtual async Task DeleteAsync(string url) => (await SendAsync(HttpMethod.Delete, url)).Content; - private async Task SendAsync(HttpMethod httpMethod, string url, object body = null) + private async Task<(string Content, KeyValuePair>[] ResponseHeaders)> SendAsync( + HttpMethod httpMethod, string url, object body = null) { url = url?.Replace(" ", "%20"); @@ -50,7 +70,7 @@ private async Task SendAsync(HttpMethod httpMethod, string url, object b } using var payload = body?.ToJson().ToStringContent(); - var response = httpMethod.ToString() switch + using var response = httpMethod.ToString() switch { "GET" => await _httpClient.GetAsync(url), "DELETE" => await _httpClient.DeleteAsync(url), @@ -64,7 +84,29 @@ private async Task SendAsync(HttpMethod httpMethod, string url, object b response.EnsureSuccessStatusCode(); - return content; + return (content, response.Headers.ToArray()); + } + + private string GetNextUrl(KeyValuePair>[] headers) + { + var linkHeaderValue = ExtractLinkHeader(headers); + + var nextUrl = linkHeaderValue? + .Split(",", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries) + .Select(link => + { + var rx = new Regex(@"<(?.+)>;\s*rel=""(?.+)"""); + var url = rx.Match(link).Groups["url"].Value; + var rel = rx.Match(link).Groups["rel"].Value; // first, next, last, prev + + return (Url: url, Rel: rel); + }) + .FirstOrDefault(x => x.Rel == "next").Url; + + return nextUrl; } + + private string ExtractLinkHeader(KeyValuePair>[] headers) => + headers.SingleOrDefault(kvp => kvp.Key == "Link").Value?.FirstOrDefault(); } } \ No newline at end of file diff --git a/src/Octoshift/Octoshift.csproj b/src/Octoshift/Octoshift.csproj index 0d2a4c0d6..15289a99e 100644 --- a/src/Octoshift/Octoshift.csproj +++ b/src/Octoshift/Octoshift.csproj @@ -7,6 +7,7 @@ + diff --git a/src/OctoshiftCLI.Tests/GithubApiTests.cs b/src/OctoshiftCLI.Tests/GithubApiTests.cs index 4650bbac0..7d73eb0b2 100644 --- a/src/OctoshiftCLI.Tests/GithubApiTests.cs +++ b/src/OctoshiftCLI.Tests/GithubApiTests.cs @@ -1,7 +1,10 @@ +using System.Collections.Generic; +using System.Linq; using System.Net.Http; using System.Threading.Tasks; using FluentAssertions; using Moq; +using Newtonsoft.Json.Linq; using OctoshiftCLI.Extensions; using Xunit; @@ -69,10 +72,11 @@ public async Task GetTeamMembers_Returns_Team_Members() const string org = "ORG"; const string teamName = "TEAM_NAME"; - var url = $"https://api.github.com/orgs/{org}/teams/{teamName}/members"; + var url = $"https://api.github.com/orgs/{org}/teams/{teamName}/members?per_page=100"; + const string teamMember1 = "TEAM_MEMBER_1"; const string teamMember2 = "TEAM_MEMBER_2"; - var response = $@" + var responsePage1 = $@" [ {{ ""login"": ""{teamMember1}"", @@ -84,17 +88,45 @@ public async Task GetTeamMembers_Returns_Team_Members() }} ]"; + const string teamMember3 = "TEAM_MEMBER_3"; + const string teamMember4 = "TEAM_MEMBER_4"; + var responsePage2 = $@" + [ + {{ + ""login"": ""{teamMember3}"", + ""id"": 3 + }}, + {{ + ""login"": ""{teamMember4}"", + ""id"": 4 + }} + ]"; + + async IAsyncEnumerable GetAllPages() + { + var jArrayPage1 = JArray.Parse(responsePage1); + yield return jArrayPage1[0]; + yield return jArrayPage1[1]; + + var jArrayPage2 = JArray.Parse(responsePage2); + yield return jArrayPage2[0]; + yield return jArrayPage2[1]; + + await Task.CompletedTask; + } + var githubClientMock = new Mock(null, null, null); githubClientMock - .Setup(m => m.GetAsync(url)) - .ReturnsAsync(response); + .Setup(m => m.GetAllAsync(url)) + .Returns(GetAllPages); // Act var githubApi = new GithubApi(githubClientMock.Object); - var result = await githubApi.GetTeamMembers(org, teamName); + var result = (await githubApi.GetTeamMembers(org, teamName)).ToArray(); // Assert - result.Should().Equal(teamMember1, teamMember2); + result.Should().HaveCount(4); + result.Should().Equal(teamMember1, teamMember2, teamMember3, teamMember4); } [Fact] @@ -102,11 +134,11 @@ public async Task GetRepos_Returns_Names_Of_All_Repositories() { // Arrange const string org = "ORG"; - var url = $"https://api.github.com/orgs/{org}/repos"; + var url = $"https://api.github.com/orgs/{org}/repos?per_page=100"; const string repoName1 = "FOO"; const string repoName2 = "BAR"; - var response = $@" + var responsePage1 = $@" [ {{ ""id"": 1, @@ -118,18 +150,45 @@ public async Task GetRepos_Returns_Names_Of_All_Repositories() }} ]"; + const string repoName3 = "BAZ"; + const string repoName4 = "QUX"; + var responsePage2 = $@" + [ + {{ + ""id"": 3, + ""name"": ""{repoName3}"" + }}, + {{ + ""id"": 4, + ""name"": ""{repoName4}"" + }} + ]"; + + async IAsyncEnumerable GetAllPages() + { + var jArrayPage1 = JArray.Parse(responsePage1); + yield return jArrayPage1[0]; + yield return jArrayPage1[1]; + + var jArrayPage2 = JArray.Parse(responsePage2); + yield return jArrayPage2[0]; + yield return jArrayPage2[1]; + + await Task.CompletedTask; + } + var githubClientMock = new Mock(null, null, null); githubClientMock - .Setup(m => m.GetAsync(url)) - .ReturnsAsync(response); + .Setup(m => m.GetAllAsync(url)) + .Returns(GetAllPages); // Act var githubApi = new GithubApi(githubClientMock.Object); - var result = await githubApi.GetRepos(org); + var result = (await githubApi.GetRepos(org)).ToArray(); // Assert - result.Should().HaveCount(2); - result.Should().Equal(repoName1, repoName2); + result.Should().HaveCount(4); + result.Should().Equal(repoName1, repoName2, repoName3, repoName4); } [Fact] diff --git a/src/OctoshiftCLI.Tests/GithubClientTests.cs b/src/OctoshiftCLI.Tests/GithubClientTests.cs index 2383cf70f..44fda6971 100644 --- a/src/OctoshiftCLI.Tests/GithubClientTests.cs +++ b/src/OctoshiftCLI.Tests/GithubClientTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Net; using System.Net.Http; using System.Threading; @@ -6,6 +7,7 @@ using FluentAssertions; using Moq; using Moq.Protected; +using Newtonsoft.Json.Linq; using Xunit; namespace OctoshiftCLI.Tests @@ -584,6 +586,248 @@ await FluentActions .ThrowExactlyAsync(); } + [Fact] + public async Task GetAllAsync_Should_Get_All_Pages() + { + // Arrange + const string url = "https://api.github.com/search/code?q=addClass+user%3Amozilla"; + + const string firstItem = "first"; + const string secondItem = "second"; + using var firstResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent($"[\"{firstItem}\", \"{secondItem}\"]"), + }; + firstResponse.Headers.Add("Link", new[] + { + $"<{url}&page=2>; rel=\"next\", " + + $"<{url}&page=4>; rel=\"last\"" + }); + + const string thirdItem = "third"; + const string fourthItem = "fourth"; + using var secondResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent($"[\"{thirdItem}\", \"{fourthItem}\"]") + }; + secondResponse.Headers.Add("Link", new[] + { + $"<{url}&page=1>; rel=\"prev\", " + + $"<{url}&page=3>; rel=\"next\", " + + $"<{url}&page=3>; rel=\"last\", " + + $"<{url}&page=1>; rel=\"first\"" + }); + + const string fifthItem = "fifth"; + using var thirdResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent($"[\"{fifthItem}\"]") + }; + + var handlerMock = new Mock(); + handlerMock // first request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == url), + ItExpr.IsAny()) + .ReturnsAsync(firstResponse); + handlerMock // second request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == $"{url}&page=2"), + ItExpr.IsAny()) + .ReturnsAsync(secondResponse); + handlerMock // third request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == $"{url}&page=3"), + ItExpr.IsAny()) + .ReturnsAsync(thirdResponse); + + using var httpClient = new HttpClient(handlerMock.Object); + var githubClient = new GithubClient(_loggerMock.Object, httpClient, PERSONAL_ACCESS_TOKEN); + + // Act + var results = new List(); + await foreach (var result in githubClient.GetAllAsync(url)) + { + results.Add(result); + } + + // Assert + results.Should().HaveCount(5); + results[0].Value().Should().Be(firstItem); + results[1].Value().Should().Be(secondItem); + results[2].Value().Should().Be(thirdItem); + results[3].Value().Should().Be(fourthItem); + results[4].Value().Should().Be(fifthItem); + } + + [Fact] + public async Task GetAllAsync_Logs_The_Url_Per_Each_Page_Request() + { + // Arrange + const string url = "https://example.com/resource"; + + using var firstResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("[\"first\"]"), + }; + firstResponse.Headers.Add("Link", new[] + { + $"<{url}&page=2>; rel=\"next\", " + + $"<{url}&page=2>; rel=\"last\"" + }); + + using var secondResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("[\"second\"]"), + }; + + var handlerMock = new Mock(); + handlerMock // first request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == url), + ItExpr.IsAny()) + .ReturnsAsync(firstResponse); + + handlerMock // second request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == $"{url}&page=2"), + ItExpr.IsAny()) + .ReturnsAsync(secondResponse); + + using var httpClient = new HttpClient(handlerMock.Object); + var githubClient = new GithubClient(_loggerMock.Object, httpClient, PERSONAL_ACCESS_TOKEN); + + // Act + await foreach (var _ in githubClient.GetAllAsync(url)) { } + + // Assert + _loggerMock.Verify(m => m.LogVerbose(It.Is(actual => actual == $"HTTP GET: {url}"))); + _loggerMock.Verify(m => m.LogVerbose(It.Is(actual => actual == $"HTTP GET: {url}&page=2"))); + } + + [Fact] + public async Task GetAllAsync_Logs_The_Response_Per_Each_Page_Request() + { + // Arrange + const string url = "https://example.com/resource"; + + var firstResponseContent = "[\"firs\"]"; + using var firstResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(firstResponseContent), + }; + firstResponse.Headers.Add("Link", new[] + { + $"<{url}&page=2>; rel=\"next\", " + + $"<{url}&page=2>; rel=\"last\"" + }); + + var secondResponseContent = "[\"second\"]"; + using var secondResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(secondResponseContent), + }; + + var handlerMock = new Mock(); + handlerMock // first request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == url), + ItExpr.IsAny()) + .ReturnsAsync(firstResponse); + + handlerMock // second request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == $"{url}&page=2"), + ItExpr.IsAny()) + .ReturnsAsync(secondResponse); + + using var httpClient = new HttpClient(handlerMock.Object); + var githubClient = new GithubClient(_loggerMock.Object, httpClient, PERSONAL_ACCESS_TOKEN); + + // Act + await foreach (var _ in githubClient.GetAllAsync(url)) { } + + // Assert + _loggerMock.Verify(m => + m.LogVerbose( + It.Is(actual => + actual == $"RESPONSE ({HttpStatusCode.OK}): {firstResponseContent}"))); + _loggerMock.Verify(m => + m.LogVerbose(It.Is(actual => + actual == $"RESPONSE ({HttpStatusCode.OK}): {secondResponseContent}"))); + } + + [Fact] + public async Task GetAllAsync_Throws_HttpRequestException_On_Non_Success_Response() + { + // Arrange + const string url = "https://example.com/resource"; + + using var firstResponse = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("[\"first\"]"), + }; + firstResponse.Headers.Add("Link", new[] + { + $"<{url}&page=2>; rel=\"next\", " + + $"<{url}&page=2>; rel=\"last\"" + }); + + using var failureResponse = new HttpResponseMessage(HttpStatusCode.InternalServerError); + + var handlerMock = new Mock(); + handlerMock // first request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == url), + ItExpr.IsAny()) + .ReturnsAsync(firstResponse); + + handlerMock // second request + .Protected() + .Setup>( + "SendAsync", + ItExpr.Is(req => req.Method == HttpMethod.Get && + req.RequestUri.ToString() == $"{url}&page=2"), + ItExpr.IsAny()) + .ReturnsAsync(failureResponse); + + using var httpClient = new HttpClient(handlerMock.Object); + var githubClient = new GithubClient(_loggerMock.Object, httpClient, PERSONAL_ACCESS_TOKEN); + + // Act, Assert + await FluentActions + .Invoking(async () => + { + await foreach (var _ in githubClient.GetAllAsync(url)) { } + }) + .Should() + .ThrowExactlyAsync(); + } + private Mock MockHttpHandlerForGet() => MockHttpHandler(req => req.Method == HttpMethod.Get);