fix: page extraction regex issue

Author: sarlam

pom.xml

@@ -227,6 +227,12 @@
             <version>2.2</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.github.npathai</groupId>
+            <artifactId>hamcrest-optional</artifactId>
+            <version>2.0.0</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.junit.jupiter</groupId>
             <artifactId>junit-jupiter-engine</artifactId>

src/main/java/com/spotify/github/v3/clients/GithubPage.java

@@ -30,7 +30,6 @@
 import com.spotify.github.http.ImmutablePagination;
 import com.spotify.github.http.Link;
 import com.spotify.github.http.Pagination;
-import java.net.URI;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -54,7 +53,7 @@ public class GithubPage<T> implements AsyncPage<T> {
   private final TypeReference<List<T>> typeReference;
   private final int itemsPerPage;
 
-  private static String formatPath(final String path, final int itemsPerPage) {
+  protected static String formatPath(final String path, final int itemsPerPage) {
     try {
       URIBuilder uriBuilder = new URIBuilder(path);
       if (uriBuilder.getQueryParams().stream().anyMatch(p -> p.getName().equals("per_page"))) {
@@ -199,12 +198,13 @@ private CompletableFuture<Map<String, Link>> linkMapAsync() {
                     .collect(toMap(link -> link.rel().get(), identity())));
   }
 
-  private Optional<Integer> pageNumberFromUri(final String uri) {
-    Pattern pageInQueryPattern = Pattern.compile("page=(\\d+)");
-    Matcher matcher = pageInQueryPattern.matcher(URI.create(uri).getQuery());
+  protected static Optional<Integer> pageNumberFromUri(final String uri) {
+    Pattern pageInQueryPattern = Pattern.compile("(^|\\?|&)page=(?<page>\\d+)", Pattern.CASE_INSENSITIVE);
     try {
-      return Optional.of(Integer.parseInt(matcher.group(1)));
-    } catch (NumberFormatException e) {
+      String query = new URIBuilder(uri).build().getQuery();
+      Matcher matcher = pageInQueryPattern.matcher(query);
+      return matcher.find() ? Optional.of(Integer.parseInt(matcher.group("page"))) : Optional.empty();
+    } catch (Exception e) {
       return Optional.empty();
     }
   }

src/test/java/com/spotify/github/v3/clients/GitHubPageTest.java

@@ -0,0 +1,53 @@
+/*-
+ * -\-\-
+ * github-api
+ * --
+ * Copyright (C) 2016 - 2025 Spotify AB
+ * --
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * -/-/-
+ */
+
+package com.spotify.github.v3.clients;
+
+import static org.hamcrest.core.Is.is;
+
+import java.net.URI;
+import org.junit.jupiter.api.Test;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static com.github.npathai.hamcrestopt.OptionalMatchers.isPresentAndIs;
+import static com.github.npathai.hamcrestopt.OptionalMatchers.isEmpty;
+
+public class GitHubPageTest {
+  private static final String MOCK_GITHUB_HOST = "bogus.host";
+  private static final URI MOCK_GITHUB_URI = URI.create(String.format("http://%s/api/v3/", MOCK_GITHUB_HOST));
+
+  @Test
+  public void testFormatPathWithPerPage() {
+    assertThat(GithubPage.formatPath("/commits?page=2", 3), is("/commits?page=2&per_page=3"));
+    assertThat(GithubPage.formatPath("NOT_A_CORRECT PATH ", 3), is("NOT_A_CORRECT PATH "));
+    assertThat(GithubPage.formatPath("/commits", 3), is("/commits?per_page=3"));
+    assertThat(GithubPage.formatPath("/commits?page=2&per_page=7", 3), is("/commits?page=2&per_page=7"));
+  }
+
+  @Test
+  public void testPageNumberFromURI() {
+    assertThat(GithubPage.pageNumberFromUri(MOCK_GITHUB_URI.resolve("/commits?page=5").toString()), isPresentAndIs(5));
+    assertThat(GithubPage.pageNumberFromUri(MOCK_GITHUB_URI.resolve("/commits").toString()), isEmpty());
+    assertThat(GithubPage.pageNumberFromUri(MOCK_GITHUB_URI.resolve("commits").toString()), isEmpty());
+    assertThat(GithubPage.pageNumberFromUri("/commits?page=2"), isPresentAndIs(2));
+    assertThat(GithubPage.pageNumberFromUri("/commits?per_page=4&page=2"), isPresentAndIs(2));
+    assertThat(GithubPage.pageNumberFromUri("NOT_A_CORRECT PATH "), isEmpty());
+    assertThat(GithubPage.pageNumberFromUri("/commits"), isEmpty());
+  }
+}

src/test/java/com/spotify/github/v3/clients/PullRequestClientTest.java

@@ -21,14 +21,10 @@
 package com.spotify.github.v3.clients;
 
 import static com.google.common.io.Resources.getResource;
-import static com.spotify.github.MockHelper.createMockHttpResponse;
 import static com.spotify.github.MockHelper.createMockResponse;
-import static com.spotify.github.v3.UserTest.assertUser;
-import static com.spotify.github.v3.clients.GitHubClient.LIST_COMMIT_TYPE_REFERENCE;
 import static java.lang.String.format;
 import static java.nio.charset.Charset.defaultCharset;
 import static java.util.concurrent.CompletableFuture.completedFuture;
-import static java.util.concurrent.CompletableFuture.completedStage;
 import static java.util.stream.Collectors.toList;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
@@ -45,7 +41,6 @@
 import com.spotify.github.async.AsyncPage;
 import com.spotify.github.http.HttpResponse;
 import com.spotify.github.jackson.Json;
-import com.spotify.github.v3.User;
 import com.spotify.github.v3.exceptions.RequestNotOkException;
 import com.spotify.github.v3.git.FileItem;
 import com.spotify.github.v3.git.ImmutableFileItem;
@@ -63,7 +58,6 @@
 import java.net.URI;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CompletionStage;
 import java.util.concurrent.ExecutionException;
 import okhttp3.Call;
 import okhttp3.Callback;
@@ -80,7 +74,7 @@
 
 public class PullRequestClientTest {
   private static final String MOCK_GITHUB_HOST = "bogus.host";
-  private static final URI MOCK_GITHUB_URI = URI.create(String.format("http://%s/", MOCK_GITHUB_HOST));
+  private static final URI MOCK_GITHUB_URI = URI.create(String.format("http://%s/api/v3", MOCK_GITHUB_HOST));
   private static final URI MOCK_GITHUB_URI_GQL = MOCK_GITHUB_URI.resolve("/graphql");
 
   private static final String PR_CHANGED_FILES_TEMPLATE = "/repos/%s/%s/pulls/%s/files";