Skip to content

Commit c2d4ba1

Browse files
sarlamsarlam
committed
feat(GitHubPages): introduce per pages (bisect: bad)
Summary ======= implements the per pages limit
1 parent 0041341 commit c2d4ba1

File tree

5 files changed

+86
-22
lines changed

5 files changed

+86
-22
lines changed

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

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
package com.spotify.github.v3.clients;
2222

2323
import static java.util.Arrays.stream;
24+
import static java.util.Objects.isNull;
2425
import static java.util.Objects.nonNull;
2526
import static java.util.function.Function.identity;
2627
import static java.util.stream.Collectors.toMap;
@@ -30,12 +31,18 @@
3031
import com.spotify.github.http.ImmutablePagination;
3132
import com.spotify.github.http.Link;
3233
import com.spotify.github.http.Pagination;
34+
import java.net.URI;
3335
import java.util.Iterator;
3436
import java.util.List;
3537
import java.util.Map;
3638
import java.util.NoSuchElementException;
39+
import java.util.Objects;
3740
import java.util.Optional;
3841
import java.util.concurrent.CompletableFuture;
42+
import java.util.regex.Matcher;
43+
import java.util.regex.Pattern;
44+
import javax.ws.rs.core.UriBuilder;
45+
import okhttp3.HttpUrl;
3946

4047
/**
4148
* Async page implementation for github resources
@@ -44,12 +51,32 @@
4451
*/
4552
public class GithubPage<T> implements AsyncPage<T> {
4653

54+
static final int ITEM_PER_PAGE_DEFAULT = 30;
4755
private final GitHubClient github;
4856
private final String path;
4957
private final TypeReference<List<T>> typeReference;
58+
private final int itemsPerPage;
59+
60+
private String formatPath(String path) {
61+
String fullURL = github.urlFor(path);
62+
HttpUrl inputUrl = HttpUrl.parse(fullURL);
63+
64+
assert inputUrl != null;
65+
if (isNull(inputUrl.queryParameter("per_page"))) {
66+
return inputUrl
67+
.newBuilder()
68+
.addQueryParameter("per_page", Integer.toString(itemsPerPage))
69+
.build()
70+
.encodedPath();
71+
72+
73+
}
74+
75+
return path;
76+
}
5077

5178
/**
52-
* C'tor.
79+
* Constructor.
5380
*
5481
* @param github github client
5582
* @param path resource page path
@@ -58,8 +85,27 @@ public class GithubPage<T> implements AsyncPage<T> {
5885
GithubPage(
5986
final GitHubClient github, final String path, final TypeReference<List<T>> typeReference) {
6087
this.github = github;
61-
this.path = path;
88+
this.path = formatPath(path);
6289
this.typeReference = typeReference;
90+
this.itemsPerPage = ITEM_PER_PAGE_DEFAULT;
91+
}
92+
93+
/**
94+
* Constructor.
95+
*
96+
* @param github github client
97+
* @param path resource page path
98+
* @param typeReference type reference for deserialization
99+
*/
100+
GithubPage(
101+
final GitHubClient github,
102+
final String path,
103+
final TypeReference<List<T>> typeReference,
104+
final int itemsPerPage) {
105+
this.github = github;
106+
this.path = formatPath(path);
107+
this.typeReference = typeReference;
108+
this.itemsPerPage = itemsPerPage;
63109
}
64110

65111
/** {@inheritDoc} */
@@ -153,19 +199,21 @@ private CompletableFuture<Map<String, Link>> linkMapAsync() {
153199
return github
154200
.request(path)
155201
.thenApply(
156-
response -> {
157-
return Optional.ofNullable(response.header("Link"))
158-
.stream()
159-
.flatMap(linkHeader -> stream(linkHeader.split(",")))
160-
.map(linkString -> Link.from(linkString.split(";")))
161-
.filter(link -> link.rel().isPresent())
162-
.collect(toMap(link -> link.rel().get(), identity()));
163-
});
202+
response ->
203+
Optional.ofNullable(response.header("Link")).stream()
204+
.flatMap(linkHeader -> stream(linkHeader.split(",")))
205+
.map(linkString -> Link.from(linkString.split(";")))
206+
.filter(link -> link.rel().isPresent())
207+
.collect(toMap(link -> link.rel().get(), identity())));
164208
}
165209

166210
private Optional<Integer> pageNumberFromUri(final String uri) {
167-
return Optional.ofNullable(uri.replaceAll(".*\\?page=", "").replaceAll("&.*", ""))
168-
.filter(string -> string.matches("\\d+"))
169-
.map(Integer::parseInt);
211+
Pattern pageInQueryPattern = Pattern.compile("page=(\\d+)");
212+
Matcher matcher = pageInQueryPattern.matcher(URI.create(uri).getQuery());
213+
try {
214+
return Optional.of(Integer.parseInt(matcher.group(1)));
215+
} catch (NumberFormatException e) {
216+
return Optional.empty();
217+
}
170218
}
171219
}

‎src/main/java/com/spotify/github/v3/clients/PullRequestClient.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static com.spotify.github.v3.clients.GitHubClient.LIST_REVIEW_REQUEST_TYPE_REFERENCE;
2828
import static com.spotify.github.v3.clients.GitHubClient.LIST_REVIEW_TYPE_REFERENCE;
2929
import static java.util.Objects.isNull;
30+
import static java.lang.Math.toIntExact;
3031

3132
import com.google.common.base.Strings;
3233
import com.google.common.collect.ImmutableMap;
@@ -193,6 +194,12 @@ public CompletableFuture<List<CommitItem>> listCommits(final long prNumber) {
193194
response -> Json.create().fromJsonUncheckedNotNull(response.bodyString(), LIST_COMMIT_TYPE_REFERENCE));
194195
}
195196

197+
public Iterator<AsyncPage<CommitItem>> listCommits(final long prNumber, final int itemsPerPage) {
198+
final String path = "";
199+
200+
return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_COMMIT_TYPE_REFERENCE, itemsPerPage));
201+
}
202+
196203
/**
197204
* List pull request reviews. Reviews are returned in chronological order.
198205
*
@@ -238,10 +245,9 @@ public Iterator<AsyncPage<Review>> listReviews(final int prNumber, final int ite
238245
* @return iterator of reviews
239246
*/
240247
public Iterator<AsyncPage<Review>> listReviews(final long prNumber, final long itemsPerPage) {
241-
// FIXME Use itemsPerPage property
242248
final String path = String.format(PR_REVIEWS_TEMPLATE, owner, repo, prNumber);
243249
log.debug("Fetching pull request reviews from " + path);
244-
return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_REVIEW_TYPE_REFERENCE));
250+
return new GithubPageIterator<>(new GithubPage<>(github, path, LIST_REVIEW_TYPE_REFERENCE, toIntExact(itemsPerPage)));
245251
}
246252

247253
/**

‎src/test/java/com/spotify/github/v3/clients/IssueClientTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,10 @@ public void testCommentPaginationForeach() throws IOException {
121121
Resources.toString(getResource(this.getClass(), "comments_page2.json"), defaultCharset());
122122
final HttpResponse lastPageResponse = createMockResponse(lastPageLink, lastPageBody);
123123

124-
when(github.request(format(COMMENTS_URI_NUMBER_TEMPLATE, "someowner", "somerepo", "123")))
124+
when(github.request(format(COMMENTS_URI_NUMBER_TEMPLATE + "?per_page=30", "someowner", "somerepo", "123")))
125125
.thenReturn(completedFuture(firstPageResponse));
126126
when(github.request(
127-
format(COMMENTS_URI_NUMBER_TEMPLATE + "?page=2", "someowner", "somerepo", "123")))
127+
format(COMMENTS_URI_NUMBER_TEMPLATE + "?page=2&per_page=30", "someowner", "somerepo", "123")))
128128
.thenReturn(completedFuture(lastPageResponse));
129129

130130
final List<Comment> listComments = Lists.newArrayList();

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@
7777
import org.mockito.ArgumentCaptor;
7878

7979
public class PullRequestClientTest {
80+
private static final String MOCK_GITHUB_HOST = "bogus.host";
81+
private static final URI MOCK_GITHUB_URI = URI.create(String.format("http://%s/", MOCK_GITHUB_HOST));
82+
private static final URI MOCK_GITHUB_URI_GQL = MOCK_GITHUB_URI.resolve("/graphql");
8083

8184
private static final String PR_CHANGED_FILES_TEMPLATE = "/repos/%s/%s/pulls/%s/files";
8285
private GitHubClient github;
@@ -92,7 +95,7 @@ public void setUp() {
9295
client = mock(OkHttpClient.class);
9396
github =
9497
GitHubClient.create(
95-
client, URI.create("http://bogus"), URI.create("https://bogus/graphql"), "token");
98+
client,MOCK_GITHUB_URI, MOCK_GITHUB_URI_GQL, "token");
9699
mockGithub = mock(GitHubClient.class);
97100
}
98101

@@ -446,8 +449,10 @@ void testChangedFiles() throws IOException {
446449
}
447450

448451
@Test
449-
public void listCommits() throws Exception {
452+
public void listCommitsWithoutSpecifyingPages() throws Exception {
450453
// Given
454+
final int COMMIT_PER_PAGE = 30;
455+
451456
final String firstPageLink =
452457
"<https://api.github.com/repositories/10270250/pulls/20463/commits?page=2>; rel=\"next\", <https://api.github.com/repositories/10270250/pulls/20463/commits?page=4>; rel=\"last\"";
453458
final String firstPageBody =
@@ -478,6 +483,11 @@ public void listCommits() throws Exception {
478483
final PullRequestClient pullRequestClient = PullRequestClient.create(mockGithub, "owner", "repo");
479484

480485
final List<CommitItem> commits = pullRequestClient.listCommits(1L).get();
481-
assertThat(commits.size(), is(1));
486+
assertThat(commits.size(), is(COMMIT_PER_PAGE));
487+
assertThat(
488+
commits.get(COMMIT_PER_PAGE - 1).commit().tree().sha(), is("219cb4c1ffada21259876d390df1a85767481617"));
489+
// make this test works for the current situation
490+
// make another test for a new method/signature that would return the full list
491+
// code the logic to follow the links
482492
}
483493
}

‎src/test/java/com/spotify/github/v3/clients/TeamClientTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,9 @@ public void listTeamMembersPaged() throws Exception {
184184

185185
final HttpResponse lastPageResponse = createMockResponse(lastPageLink, lastPageBody);
186186

187-
when(github.request(endsWith("/orgs/github/teams/1/members?per_page=1")))
187+
when(github.request(endsWith("/orgs/github/teams/1/members?per_page=30")))
188188
.thenReturn(completedFuture(firstPageResponse));
189-
when(github.request(endsWith("/orgs/github/teams/1/members?page=2")))
189+
when(github.request(endsWith("/orgs/github/teams/1/members?page=2&per_page=30")))
190190
.thenReturn(completedFuture(lastPageResponse));
191191

192192
final Iterable<AsyncPage<User>> pageIterator = () -> teamClient.listTeamMembers("1", 1);

0 commit comments

Comments
 (0)