Configurable GitHub API limits in gerrit.config. GitHub API invocation allowance is < 50,000 API / hour which means 13 TPS: quite a low threshold for a production system. This change allows to limit the number of repositories and pull requests returned for a GitHub API in order to avoid the risk on eating up your allowance by "clicking" the wrong option in the GUI. Change-Id: I0bc5abfd5d29a86a214979e8cd8e3ff2d59e39d4
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java index 9857e5b..0512d50 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/GitHubConfig.java
@@ -35,9 +35,19 @@ private static final String FROM_TO_REDIRECT_SEPARATOR = "R>"; private static final String CONF_JOB_POOL_LIMIT = "jobPoolLimit"; private static final String CONF_JOB_EXEC_TIMEOUT = "jobExecTimeout"; + private static final String CONF_PULL_REQUEST_LIST_LIMIT = + "pullRequestListLimit"; + private static final String CONF_REPOSITORY_LIST_PAGE_SIZE = + "repositoryListPageSize"; + private static final String CONF_REPOSITORY_LIST_LIMIT = + "repositoryListLimit"; + public final File gitDir; - private int jobPoolLimit; - private int jobExecTimeout; + public final int jobPoolLimit; + public final int jobExecTimeout; + public final int pullRequestListLimit; + public final int repositoryListPageSize; + public final int repositoryListLimit; public static class NextPage { public final String uri; @@ -64,11 +74,17 @@ new NextPage(fromTo.substring( sepPos + getSeparator(redirect).length() + 1).trim(), redirect); wizardFromTo.put(fromPage, toPage); - - jobPoolLimit = config.getInt(CONF_SECTION, CONF_JOB_POOL_LIMIT, 5); - jobExecTimeout = config.getInt(CONF_SECTION, CONF_JOB_EXEC_TIMEOUT, 10); } + jobPoolLimit = config.getInt(CONF_SECTION, CONF_JOB_POOL_LIMIT, 5); + jobExecTimeout = config.getInt(CONF_SECTION, CONF_JOB_EXEC_TIMEOUT, 10); + pullRequestListLimit = + config.getInt(CONF_SECTION, CONF_PULL_REQUEST_LIST_LIMIT, 50); + repositoryListPageSize = + config.getInt(CONF_SECTION, CONF_REPOSITORY_LIST_PAGE_SIZE, 50); + repositoryListLimit = + config.getInt(CONF_SECTION, CONF_REPOSITORY_LIST_LIMIT, 50); + gitDir = site.resolve(config.getString("gerrit", null, "basePath")); if (gitDir == null) { throw new IllegalStateException("gerrit.basePath must be configured"); @@ -92,12 +108,4 @@ public NextPage getNextPage(String sourcePage) { return wizardFromTo.get(sourcePage); } - - public int getJobPoolLimit() { - return jobPoolLimit; - } - - public int getJobExecTimeout() { - return jobExecTimeout; - } }
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/PullRequestListController.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/PullRequestListController.java index 29eff9c..704d2b8 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/PullRequestListController.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/PullRequestListController.java
@@ -28,7 +28,6 @@ import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHPerson; @@ -56,6 +55,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; +import com.googlesource.gerrit.plugins.github.GitHubConfig; import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin; @Singleton @@ -63,17 +63,20 @@ private static final Logger LOG = LoggerFactory .getLogger(PullRequestListController.class); private static final String DATE_FMT = "yyyy-MM-dd HH:mm z"; - private static final int MAX_PULL_REQUESTS = 20; - private ProjectCache projectsCache; - private GitRepositoryManager repoMgr; + + private final GitHubConfig config; + private final ProjectCache projectsCache; + private final GitRepositoryManager repoMgr; private final Provider<ReviewDb> schema; @Inject - public PullRequestListController(ProjectCache projectsCache, - GitRepositoryManager repoMgr, Provider<ReviewDb> schema) { + public PullRequestListController(final ProjectCache projectsCache, + final GitRepositoryManager repoMgr, final Provider<ReviewDb> schema, + final GitHubConfig config) { this.projectsCache = projectsCache; this.repoMgr = repoMgr; this.schema = schema; + this.config = config; } @Override @@ -145,7 +148,7 @@ String ghRepoName = gerritRepoName.get().split("/")[1]; List<GHPullRequest> repoPullRequests = Lists.newArrayList(); - if (numPullRequests < MAX_PULL_REQUESTS) { + if (numPullRequests < config.pullRequestListLimit) { for (GHPullRequest ghPullRequest : GHRepository.listPullRequests( login.hub, ghOwner, ghRepoName, GHIssueState.OPEN)) {
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/RepositoriesListController.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/RepositoriesListController.java index f254b16..42beed0 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/RepositoriesListController.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/wizard/RepositoriesListController.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.github.wizard; import java.io.IOException; -import java.util.List; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -22,6 +21,8 @@ import org.kohsuke.github.GHOrganization; import org.kohsuke.github.GHRepository; +import org.kohsuke.github.PagedIterable; +import org.kohsuke.github.PagedIterator; import com.google.common.base.Strings; import com.google.gerrit.reviewdb.client.Project; @@ -32,17 +33,20 @@ import com.google.gson.JsonPrimitive; import com.google.inject.Inject; import com.google.inject.Singleton; +import com.googlesource.gerrit.plugins.github.GitHubConfig; import com.googlesource.gerrit.plugins.github.oauth.GitHubLogin; @Singleton public class RepositoriesListController implements VelocityController { - private static final int PAGE_SIZE = 100; - private ProjectCache projects; + private final ProjectCache projects; + private final GitHubConfig config; @Inject - public RepositoriesListController(ProjectCache projects) { + public RepositoriesListController(final ProjectCache projects, + final GitHubConfig config) { this.projects = projects; + this.config = config; } @Override @@ -52,7 +56,12 @@ String organisation = req.getParameter("organisation"); JsonArray jsonRepos = new JsonArray(); - for (GHRepository ghRepository : getRepositories(hubLogin, organisation)) { + int numRepos = 0; + PagedIterator<GHRepository> repoIter = + getRepositories(hubLogin, organisation).iterator(); + + while (repoIter.hasNext() && numRepos < config.repositoryListLimit) { + GHRepository ghRepository = repoIter.next(); JsonObject repository = new JsonObject(); String projectName = organisation + "/" + ghRepository.getName(); if (projects.get(Project.NameKey.parse(projectName)) == null) { @@ -63,20 +72,21 @@ new JsonPrimitive( Strings.nullToEmpty(ghRepository.getDescription()))); jsonRepos.add(repository); + numRepos++; } } resp.getWriter().println(jsonRepos.toString()); } - private List<GHRepository> getRepositories(GitHubLogin hubLogin, + private PagedIterable<GHRepository> getRepositories(GitHubLogin hubLogin, String organisation) throws IOException { if (organisation.equals(hubLogin.getMyself().getLogin())) { - return hubLogin.getMyself().listRepositories(PAGE_SIZE).asList(); + return hubLogin.getMyself().listRepositories(config.repositoryListPageSize); } else { GHOrganization ghOrganisation = hubLogin.getMyself().getOrganizations().byLogin(organisation); - return ghOrganisation.listRepositories(PAGE_SIZE).asList(); + return ghOrganisation.listRepositories(config.repositoryListPageSize); } } }
diff --git a/github-plugin/src/main/java/com/googlesrouce/gerrit/plugins/github/git/JobExecutor.java b/github-plugin/src/main/java/com/googlesrouce/gerrit/plugins/github/git/JobExecutor.java index 3625b50..7462fec 100644 --- a/github-plugin/src/main/java/com/googlesrouce/gerrit/plugins/github/git/JobExecutor.java +++ b/github-plugin/src/main/java/com/googlesrouce/gerrit/plugins/github/git/JobExecutor.java
@@ -35,7 +35,7 @@ this.requestScopePropagator = requestScopePropagator; this.config = config; this.executor = Executors - .newScheduledThreadPool(config.getJobPoolLimit()); + .newScheduledThreadPool(config.jobPoolLimit); } public void exec(GitJob job) { @@ -45,6 +45,6 @@ private int getRandomExecutionDelay(GitJob job) { Random rnd = new Random(System.currentTimeMillis() + job.hashCode()); - return rnd.nextInt(config.getJobExecTimeout()); + return rnd.nextInt(config.jobExecTimeout); } }