From 57a29e90d0dd60a8a1f6905d8e6c7301eaf81492 Mon Sep 17 00:00:00 2001 From: Pavel Kindruk Date: Fri, 22 Sep 2023 17:09:37 +0300 Subject: [PATCH] [Bug]Fix pagination when ApiOptions.StartPage is explicitly set. --- Octokit.Tests/Helpers/UriExtensionsTests.cs | 58 +++++++++++++++++++++ Octokit/Helpers/UriExtensions.cs | 9 +++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/Octokit.Tests/Helpers/UriExtensionsTests.cs b/Octokit.Tests/Helpers/UriExtensionsTests.cs index 1463ffb6..370781cd 100644 --- a/Octokit.Tests/Helpers/UriExtensionsTests.cs +++ b/Octokit.Tests/Helpers/UriExtensionsTests.cs @@ -161,6 +161,64 @@ namespace Octokit.Tests.Helpers Uri uri = null; Assert.Throws(() => uri.ApplyParameters(new Dictionary())); } + + [Fact] + public void AppendsStartPageIfNone() + { + var uri = new Uri("https://api.github.com/repositories/1/labels"); + + var parameters = new Dictionary(); + Pagination.Setup(parameters, new ApiOptions { StartPage = 3, PageSize = 7 }); + + var actual = uri.ApplyParameters(parameters); + + Assert.Equal( + new Uri("https://api.github.com/repositories/1/labels?per_page=7&page=3"), + actual); + } + + [Fact] + public void AppendsStartPageIfNoneForRelativeUri() + { + var uri = new Uri("/repositories/1/labels", UriKind.Relative); + + var parameters = new Dictionary(); + Pagination.Setup(parameters, new ApiOptions { StartPage = 3, PageSize = 7 }); + + var actual = uri.ApplyParameters(parameters); + + Assert.Equal( + new Uri("/repositories/1/labels?per_page=7&page=3", UriKind.Relative), + actual); + } + + [Fact] + public void DoesNotChangePageNumberInNextPageUriWithStartPage() + { + const string nextPageUriString = "https://api.github.com/repositories/1/labels?per_page=5&page=2"; + var nextPageUri = new Uri(nextPageUriString); + + var parameters = new Dictionary(); + Pagination.Setup(parameters, new ApiOptions { StartPage = 1, PageSize = 5 }); + + var actual = nextPageUri.ApplyParameters(parameters); + + Assert.Equal(new Uri(nextPageUriString), actual); + } + + [Fact] + public void DoesNotChangePageNumberInNextPageUriWithStartPageForRelativeUri() + { + const string nextPageUriString = "/repositories/1/labels?per_page=5&page=2"; + var nextPageUri = new Uri(nextPageUriString, UriKind.Relative); + + var parameters = new Dictionary(); + Pagination.Setup(parameters, new ApiOptions { StartPage = 1, PageSize = 5 }); + + var actual = nextPageUri.ApplyParameters(parameters); + + Assert.Equal(new Uri(nextPageUriString, UriKind.Relative), actual); + } } } } diff --git a/Octokit/Helpers/UriExtensions.cs b/Octokit/Helpers/UriExtensions.cs index b708f707..6f890216 100644 --- a/Octokit/Helpers/UriExtensions.cs +++ b/Octokit/Helpers/UriExtensions.cs @@ -76,7 +76,14 @@ namespace Octokit foreach (var existing in existingParameters) { - if (!p.ContainsKey(existing.Key)) + if (existing.Key == "page") + { + // See https://github.com/octokit/octokit.net/issues/1955 + // See https://github.com/octokit/octokit.net/issues/2602 + // Desired behavior: don't replace page number in nextPageUri + p[existing.Key] = existing.Value; + } + else if (!p.ContainsKey(existing.Key)) { p.Add(existing); }