From e659f13c1107361a1d076859983934159cfa2c17 Mon Sep 17 00:00:00 2001 From: Arin Ghazarian Date: Fri, 4 Oct 2024 12:47:34 -0700 Subject: [PATCH 1/2] Add GetAllAsync overload that accepts a result collection selector --- src/Octoshift/Services/GithubClient.cs | 16 ++++++- .../Octoshift/Services/GithubClientTests.cs | 42 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/Octoshift/Services/GithubClient.cs b/src/Octoshift/Services/GithubClient.cs index 2cae6402d..ca620ef55 100644 --- a/src/Octoshift/Services/GithubClient.cs +++ b/src/Octoshift/Services/GithubClient.cs @@ -47,13 +47,25 @@ public GithubClient(OctoLogger log, HttpClient httpClient, IVersionProvider vers public virtual async Task GetAsync(string url, Dictionary customHeaders = null) => (await GetWithRetry(url, customHeaders)).Content; - public virtual async IAsyncEnumerable GetAllAsync(string url, Dictionary customHeaders = null) + public virtual IAsyncEnumerable GetAllAsync(string url, Dictionary customHeaders = null) => + GetAllAsync(url, jToken => (JArray)jToken, customHeaders); + + public virtual async IAsyncEnumerable GetAllAsync( + string url, + Func resultCollectionSelector, + Dictionary customHeaders = null) { + if (resultCollectionSelector is null) + { + throw new ArgumentNullException(nameof(resultCollectionSelector)); + } + var nextUrl = url; do { var (content, headers) = await GetWithRetry(nextUrl, customHeaders: customHeaders); - foreach (var jToken in JArray.Parse(content)) + var jContent = JToken.Parse(content); + foreach (var jToken in resultCollectionSelector(jContent)) { yield return jToken; } diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubClientTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubClientTests.cs index fec7f8bd6..9a2c97965 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubClientTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubClientTests.cs @@ -1351,6 +1351,48 @@ public async Task GetAllAsync_Retries_On_Non_Success() results.Should().BeEquivalentTo(firstItem, secondItem); } + [Fact] + public async Task GetAllAsync_Should_Use_Result_Collection_Selector() + { + // Arrange + const string url = "https://api.github.com/orgs/foo/external-groups"; + + const string firstGroupId = "123"; + const string firstGroupName = "Octocat readers"; + const string secondGroupId = "456"; + const string secondGroupName = "Octocat admins"; + const string response = $@" + {{ + ""groups"": [ + {{ + ""group_id"": ""{firstGroupId}"", + ""group_name"": ""{firstGroupName}"", + ""updated_at"": ""2021-01-24T11:31:04-06:00"" + }}, + {{ + ""group_id"": ""{secondGroupId}"", + ""group_name"": ""{secondGroupName}"", + ""updated_at"": ""2021-03-24T11:31:04-06:00"" + }}, + ] + }}"; + + var handlerMock = MockHttpHandler(req => req.Method == HttpMethod.Get && req.RequestUri.ToString() == url, CreateHttpResponseFactory(content: response)); + + using var httpClient = new HttpClient(handlerMock.Object); + var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN); + + // Act + var results = await githubClient.GetAllAsync(url, data => (JArray)data["groups"]).ToListAsync(); + + // Assert + results.Should().HaveCount(2); + results[0]["group_id"].Value().Should().Be(firstGroupId); + results[0]["group_name"].Value().Should().Be(firstGroupName); + results[1]["group_id"].Value().Should().Be(secondGroupId); + results[1]["group_name"].Value().Should().Be(secondGroupName); + } + [Fact] public async Task PostGraphQLWithPaginationAsync_Should_Return_All_Pages() { From a20fabdce272b66e7573e6877661ddfe794bcfd6 Mon Sep 17 00:00:00 2001 From: Arin Ghazarian Date: Fri, 4 Oct 2024 12:48:56 -0700 Subject: [PATCH 2/2] Rewrite GetIdpGroupId to use the new GetAllAsync overload by passing a result collection selector --- src/Octoshift/Services/GithubApi.cs | 7 ++-- .../Octoshift/Services/GithubApiTests.cs | 36 ++++++++++--------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/Octoshift/Services/GithubApi.cs b/src/Octoshift/Services/GithubApi.cs index baca7a80f..e523998ed 100644 --- a/src/Octoshift/Services/GithubApi.cs +++ b/src/Octoshift/Services/GithubApi.cs @@ -574,11 +574,10 @@ public virtual async Task GetIdpGroupId(string org, string groupName) { var url = $"{_apiUrl}/orgs/{org.EscapeDataString()}/external-groups"; - // TODO: Need to implement paging - var response = await _client.GetAsync(url); - var data = JObject.Parse(response); + var group = await _client.GetAllAsync(url, data => (JArray)data["groups"]) + .SingleAsync(x => string.Equals((string)x["group_name"], groupName, StringComparison.OrdinalIgnoreCase)); - return (int)data["groups"].Children().Single(x => ((string)x["group_name"]).ToUpper() == groupName.ToUpper())["group_id"]; + return (int)group["group_id"]; } public virtual async Task GetTeamSlug(string org, string teamName) diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs index 962922ded..826305389 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs @@ -1606,25 +1606,27 @@ public async Task GetIdpGroupId_Returns_The_Idp_Group_Id() var url = $"https://api.github.com/orgs/{GITHUB_ORG}/external-groups"; const int expectedGroupId = 123; - var response = $@" - {{ - ""groups"": [ - {{ - ""group_id"": ""{expectedGroupId}"", - ""group_name"": ""{groupName}"", - ""updated_at"": ""2021-01-24T11:31:04-06:00"" - }}, - {{ - ""group_id"": ""456"", - ""group_name"": ""Octocat admins"", - ""updated_at"": ""2021-03-24T11:31:04-06:00"" - }}, - ] - }}"; + + var group1 = new + { + group_id = expectedGroupId, + group_name = groupName, + updated_at = DateTime.Parse("2021-01-24T11:31:04-06:00") + }; + var group2 = new + { + group_id = "456", + group_name = "Octocat admins", + updated_at = DateTime.Parse("2021-03-24T11:31:04-06:00") + }; _githubClientMock - .Setup(m => m.GetAsync(url, null)) - .ReturnsAsync(response); + .Setup(m => m.GetAllAsync(url, It.IsAny>(), null)) + .Returns(new[] + { + JToken.FromObject(group1), + JToken.FromObject(group2) + }.ToAsyncEnumerable()); // Act var actualGroupId = await _githubApi.GetIdpGroupId(GITHUB_ORG, groupName);