From 87a72ddc530d8393803bc459a654b871ed513232 Mon Sep 17 00:00:00 2001 From: Stephen Horgan Date: Fri, 30 Jun 2023 16:24:26 +0100 Subject: [PATCH 1/2] PR review changes --- .../java/org/kohsuke/github/GHCommit.java | 22 +---- .../kohsuke/github/GHCommitFileIterable.java | 99 +++++++++++++++++++ .../org/kohsuke/github/GHCommitFilesPage.java | 9 +- .../org/kohsuke/github/GHCommitIterable.java | 72 -------------- .../java/org/kohsuke/github/CommitTest.java | 4 +- 5 files changed, 112 insertions(+), 94 deletions(-) create mode 100644 src/main/java/org/kohsuke/github/GHCommitFileIterable.java delete mode 100644 src/main/java/org/kohsuke/github/GHCommitIterable.java diff --git a/src/main/java/org/kohsuke/github/GHCommit.java b/src/main/java/org/kohsuke/github/GHCommit.java index f025f859c6..250dbe7755 100644 --- a/src/main/java/org/kohsuke/github/GHCommit.java +++ b/src/main/java/org/kohsuke/github/GHCommit.java @@ -24,12 +24,6 @@ @SuppressFBWarnings(value = { "NP_UNWRITTEN_FIELD", "UWF_UNWRITTEN_FIELD" }, justification = "JSON API") public class GHCommit { - /** - * Number of files returned in the commit response. If there are more files than this, the response will include - * pagination link headers for the remaining files. - */ - private static final int GH_FILE_LIMIT_PER_COMMIT_PAGE = 300; - private GHRepository owner; private ShortInfo commit; @@ -426,7 +420,7 @@ public URL getUrl() { */ @Deprecated public List getFiles() throws IOException { - return listFiles(); + return listFiles().toList(); } /** @@ -439,21 +433,11 @@ public List getFiles() throws IOException { * @throws IOException * on error */ - public List listFiles() throws IOException { + public PagedIterable listFiles() throws IOException { populate(); - if (files != null && files.size() < GH_FILE_LIMIT_PER_COMMIT_PAGE) { - return Collections.unmodifiableList(files); - } - - PagedIterable filesIterable = new GHCommitIterable(owner, sha); - if (files == null) { - files = new ArrayList<>(); - } - files.clear(); - files.addAll(filesIterable.toList()); - return Collections.unmodifiableList(files); + return new GHCommitFileIterable(owner, sha, files); } /** diff --git a/src/main/java/org/kohsuke/github/GHCommitFileIterable.java b/src/main/java/org/kohsuke/github/GHCommitFileIterable.java new file mode 100644 index 0000000000..1bcd4bb99d --- /dev/null +++ b/src/main/java/org/kohsuke/github/GHCommitFileIterable.java @@ -0,0 +1,99 @@ +package org.kohsuke.github; + +import org.kohsuke.github.GHCommit.File; + +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +import javax.annotation.Nonnull; + +/** + * Iterable for commit listing. + * + * @author Stephen Horgan + */ +class GHCommitFileIterable extends PagedIterable { + + /** + * Number of files returned in the commit response. If there are more files than this, the response will include + * pagination link headers for the remaining files. + */ + private static final int GH_FILE_LIMIT_PER_COMMIT_PAGE = 300; + + private final GHRepository owner; + private final String sha; + private GHCommitFilesPage result; + + /** + * Instantiates a new GH commit iterable. + * + * @param owner + * the owner + * @param sha + * the SHA of the commit + * @param files + * the list of files initially populated + */ + public GHCommitFileIterable(GHRepository owner, String sha, List files) { + this.owner = owner; + this.sha = sha; + this.result = new GHCommitFilesPage(files != null ? files.toArray(new File[0]) : new File[0]); + } + + /** + * Iterator. + * + * @param pageSize + * the page size + * @return the paged iterator + */ + @Nonnull + @Override + public PagedIterator _iterator(int pageSize) { + + Iterator pageIterator; + + if (result.getFiles().length < GH_FILE_LIMIT_PER_COMMIT_PAGE) { + // create a page iterator that only provides one page + pageIterator = Collections.singleton(result.getFiles()).iterator(); + } else { + // page size is controlled by the server for this iterator, do not allow it to be set by the caller + pageSize = 0; + + GitHubRequest request = owner.root() + .createRequest() + .withUrlPath(owner.getApiTailUrl("commits/" + sha)) + .build(); + + pageIterator = adapt( + GitHubPageIterator.create(owner.root().getClient(), GHCommitFilesPage.class, request, pageSize)); + } + + return new PagedIterator<>(pageIterator, null); + } + + /** + * Adapt. + * + * @param base + * the base commit page + * @return the iterator + */ + protected Iterator adapt(final Iterator base) { + return new Iterator() { + + public boolean hasNext() { + return base.hasNext(); + } + + public GHCommit.File[] next() { + GHCommitFilesPage v = base.next(); + if (result == null) { + result = v; + } + return v.getFiles(); + } + }; + } +} diff --git a/src/main/java/org/kohsuke/github/GHCommitFilesPage.java b/src/main/java/org/kohsuke/github/GHCommitFilesPage.java index fcbfb8ce74..8ce482df12 100644 --- a/src/main/java/org/kohsuke/github/GHCommitFilesPage.java +++ b/src/main/java/org/kohsuke/github/GHCommitFilesPage.java @@ -10,7 +10,14 @@ class GHCommitFilesPage { private File[] files; - /** + public GHCommitFilesPage() { + } + + public GHCommitFilesPage(File[] files) { + this.files = files; + } + + /** * Gets the files. * * @param owner diff --git a/src/main/java/org/kohsuke/github/GHCommitIterable.java b/src/main/java/org/kohsuke/github/GHCommitIterable.java deleted file mode 100644 index 2afa61346d..0000000000 --- a/src/main/java/org/kohsuke/github/GHCommitIterable.java +++ /dev/null @@ -1,72 +0,0 @@ -package org.kohsuke.github; - -import java.util.Iterator; - -import javax.annotation.Nonnull; - -/** - * Iterable for commit listing. - * - * @author Stephen Horgan - */ -class GHCommitIterable extends PagedIterable { - - private final GHRepository owner; - private final String sha; - private GHCommitFilesPage result; - - /** - * Instantiates a new GH commit iterable. - * - * @param owner - * the owner - * @param sha - * the SHA of the commit - */ - public GHCommitIterable(GHRepository owner, String sha) { - this.owner = owner; - this.sha = sha; - } - - /** - * Iterator. - * - * @param pageSize - * the page size - * @return the paged iterator - */ - @Nonnull - @Override - public PagedIterator _iterator(int pageSize) { - - GitHubRequest request = owner.root().createRequest().withUrlPath(owner.getApiTailUrl("commits/" + sha)).build(); - - return new PagedIterator<>( - adapt(GitHubPageIterator.create(owner.root().getClient(), GHCommitFilesPage.class, request, pageSize)), - null); - } - - /** - * Adapt. - * - * @param base - * the base commit page - * @return the iterator - */ - protected Iterator adapt(final Iterator base) { - return new Iterator() { - - public boolean hasNext() { - return base.hasNext(); - } - - public GHCommit.File[] next() { - GHCommitFilesPage v = base.next(); - if (result == null) { - result = v; - } - return v.getFiles(); - } - }; - } -} diff --git a/src/test/java/org/kohsuke/github/CommitTest.java b/src/test/java/org/kohsuke/github/CommitTest.java index 23b408659b..257e681cca 100644 --- a/src/test/java/org/kohsuke/github/CommitTest.java +++ b/src/test/java/org/kohsuke/github/CommitTest.java @@ -58,7 +58,7 @@ public void listFilesWhereCommitHasSmallChange() throws Exception { GHRepository repo = getRepository(); GHCommit commit = repo.getCommit("dabf0e89fe7107d6e294a924561533ecf80f2384"); - assertThat(commit.listFiles().size(), equalTo(28)); + assertThat(commit.listFiles().toList().size(), equalTo(28)); } /** @@ -72,7 +72,7 @@ public void listFilesWhereCommitHasLargeChange() throws Exception { GHRepository repo = getRepository(); GHCommit commit = repo.getCommit("b83812aa76bb7c3c43da96fbf8aec1e45db87624"); - assertThat(commit.listFiles().size(), equalTo(691)); + assertThat(commit.listFiles().toList().size(), equalTo(691)); } /** From 7d98e84c38676ff86031fb7ee22c6f0295e64430 Mon Sep 17 00:00:00 2001 From: Stephen Horgan Date: Fri, 30 Jun 2023 16:25:38 +0100 Subject: [PATCH 2/2] Add required default constructor --- .../java/org/kohsuke/github/GHCommitFilesPage.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHCommitFilesPage.java b/src/main/java/org/kohsuke/github/GHCommitFilesPage.java index 8ce482df12..d2ab10dc98 100644 --- a/src/main/java/org/kohsuke/github/GHCommitFilesPage.java +++ b/src/main/java/org/kohsuke/github/GHCommitFilesPage.java @@ -11,13 +11,13 @@ class GHCommitFilesPage { private File[] files; public GHCommitFilesPage() { - } - + } + public GHCommitFilesPage(File[] files) { - this.files = files; - } + this.files = files; + } - /** + /** * Gets the files. * * @param owner