From a31734014d6c58f1b8f68708689c7453356f6c7c Mon Sep 17 00:00:00 2001 From: Andrew H <369769+andrew-from-toronto@users.noreply.github.com> Date: Mon, 29 Apr 2024 20:06:24 -0400 Subject: [PATCH] Fix #2693 PageCount doesn't work if the query gets constructed with page number as the first query parameter (#2911) Fix #2693 Co-authored-by: Keegan Campbell --- Octokit.Tests/Http/PaginationTests.cs | 46 +++++++++++++++++++++++++++ Octokit/Helpers/Pagination.cs | 11 +++++-- 2 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 Octokit.Tests/Http/PaginationTests.cs diff --git a/Octokit.Tests/Http/PaginationTests.cs b/Octokit.Tests/Http/PaginationTests.cs new file mode 100644 index 00000000..94657d2b --- /dev/null +++ b/Octokit.Tests/Http/PaginationTests.cs @@ -0,0 +1,46 @@ +using System; +using Octokit.Models.Request.Enterprise; +using Xunit; + +namespace Octokit.Tests.Http +{ + public class PaginationTests + { + public class TheShouldContinueMethod + { + [Fact] + public void HandlesMissingUri() + { + var result = Pagination.ShouldContinue(null, null); + Assert.False(result); + } + + [Fact] + public void HandlesIsDone() + { + var uri = new Uri("http://example.com"); + var options = new ApiOptionsExtended { IsDone = true }; + var result = Pagination.ShouldContinue(uri, options); + Assert.False(result); + } + + [Fact] + public void HandlesPageCountPageFirstParam() + { + var uri = new Uri("http://example.com?page=2"); + var options = new ApiOptions { StartPage = 1, PageCount = 1 }; + var result = Pagination.ShouldContinue(uri, options); + Assert.False(result); + } + + [Fact] + public void HandlesPageCountPageNotFirstParam() + { + var uri = new Uri("http://example.com?page_size=100&page=2"); + var options = new ApiOptions { StartPage = 1, PageCount = 1 }; + var result = Pagination.ShouldContinue(uri, options); + Assert.False(result); + } + } + } +} diff --git a/Octokit/Helpers/Pagination.cs b/Octokit/Helpers/Pagination.cs index fbd6bfe5..01b1196f 100644 --- a/Octokit/Helpers/Pagination.cs +++ b/Octokit/Helpers/Pagination.cs @@ -41,8 +41,7 @@ namespace Octokit { var allValues = ToQueryStringDictionary(uri); - string pageValue; - if (allValues.TryGetValue("page", out pageValue)) + if (allValues.TryGetValue("page", out var pageValue)) { var startPage = options.StartPage ?? 1; var pageCount = options.PageCount.Value; @@ -61,8 +60,14 @@ namespace Octokit static Dictionary ToQueryStringDictionary(Uri uri) { return uri.Query.Split('&') - .Select(keyValue => + .Select((keyValue, i) => { + if (i == 0) + { + // Trim the leading '?' character from the first key-value pair + keyValue = keyValue.Substring(1); + } + var indexOf = keyValue.IndexOf('='); if (indexOf > 0) {