Merge pull request #196 from jgangemi/jae/pr-trigger-retry

- retry failed requests to retrieve pull-request info

Author: KostyaSha

github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/generic/utils/RetryableGitHubOperation.java

@@ -0,0 +1,61 @@
+package com.github.kostyasha.github.integration.generic.utils;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+/**
+ * Used to wrap and retry calls to GitHub in the event an error is thrown.
+ */
+public class RetryableGitHubOperation {
+
+    private static final Logger LOG = LoggerFactory.getLogger(RetryableGitHubOperation.class);
+
+    public interface GitOperation<T> {
+        T execute() throws IOException;
+    }
+
+    private RetryableGitHubOperation() {
+    }
+
+    /**
+     * Executes a GitHub operation up to 3 times with a 2 second delay.
+     *
+     * @param operation GitHub operation to execute
+     * @return result of operation
+     */
+    public static <T> T execute(GitOperation<T> operation) throws IOException {
+        return execute(3, 2000, operation);
+    }
+
+    public static <T> T execute(int retries, long delay, GitOperation<T> operation) throws IOException {
+        T result = null;
+
+        int count = 0;
+        while (count++ < retries) {
+            try {
+                result = operation.execute();
+                break;
+            } catch (IOException e) {
+                if (count == retries) {
+                    throw e;
+                }
+
+                LOG.debug("Failed retrieving pull request(s), retrying...", e);
+                sleep(delay);
+            }
+        }
+
+        return result;
+    }
+
+    private static void sleep(long delay) {
+        try {
+            Thread.sleep(delay);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            LOG.warn("Interrupted while delaying git operation", e);
+        }
+    }
+}

github-pullrequest-plugin/src/main/java/org/jenkinsci/plugins/github/pullrequest/GitHubPRPullRequest.java

@@ -19,8 +19,10 @@
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
+import static com.github.kostyasha.github.integration.generic.utils.RetryableGitHubOperation.execute;
 import static org.apache.commons.lang3.builder.ToStringStyle.SHORT_PREFIX_STYLE;
 import static org.jenkinsci.plugins.github.pullrequest.utils.ObjectsUtil.isNull;
 
@@ -69,7 +71,8 @@ public GitHubPRPullRequest(GHPullRequest pr) throws IOException {
 
         try {
             Date maxDate = new Date(0);
-            for (GHIssueComment comment : pr.getComments()) {
+            List<GHIssueComment> comments = execute(() -> pr.getComments());
+            for (GHIssueComment comment : comments) {
                 if (comment.getCreatedAt().compareTo(maxDate) > 0) {
                     maxDate = comment.getCreatedAt();
                 }
@@ -81,7 +84,7 @@ public GitHubPRPullRequest(GHPullRequest pr) throws IOException {
         }
 
         try {
-            userEmail = pr.getUser().getEmail();
+            userEmail = execute(() -> pr.getUser().getEmail());
         } catch (Exception e) {
             LOGGER.error("Can't get GitHub user email.", e);
             userEmail = "";
@@ -90,15 +93,15 @@ public GitHubPRPullRequest(GHPullRequest pr) throws IOException {
         GHRepository remoteRepo = pr.getRepository();
 
         try {
-            updateLabels(remoteRepo.getIssue(number).getLabels());
+            updateLabels(execute(() -> remoteRepo.getIssue(number).getLabels()));
         } catch (IOException e) {
             LOGGER.error("Can't retrieve label list: {}", e);
             inBadState = true;
         }
 
         // see https://github.com/kohsuke/github-api/issues/111
         try {
-            mergeable = pr.getMergeable();
+            mergeable = execute(() -> pr.getMergeable());
         } catch (IOException e) {
             LOGGER.error("Can't get mergeable status.", e);
             mergeable = false;

github-pullrequest-plugin/src/main/java/org/jenkinsci/plugins/github/pullrequest/GitHubPRTrigger.java

@@ -44,6 +44,7 @@
 import static java.text.DateFormat.getDateTimeInstance;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singleton;
+import static com.github.kostyasha.github.integration.generic.utils.RetryableGitHubOperation.execute;
 import static org.jenkinsci.plugins.github.pullrequest.GitHubPRTriggerMode.LIGHT_HOOKS;
 import static org.jenkinsci.plugins.github.pullrequest.trigger.check.BranchRestrictionFilter.withBranchRestriction;
 import static org.jenkinsci.plugins.github.pullrequest.trigger.check.LocalRepoUpdater.updateLocalRepo;
@@ -175,6 +176,7 @@ public void run() {
 
 
     @CheckForNull
+    @Override
     public GitHubPRPollingLogAction getPollingLogAction() {
         if (isNull(pollingLogAction) && nonNull(job)) {
             pollingLogAction = new GitHubPRPollingLogAction(job);
@@ -319,9 +321,9 @@ private static Set<GHPullRequest> pullRequestsToCheck(@Nullable Integer prNumber
                                                           @Nonnull GHRepository remoteRepo,
                                                           @Nonnull GitHubPRRepository localRepo) throws IOException {
         if (prNumber != null) {
-            return singleton(remoteRepo.getPullRequest(prNumber));
+            return execute(() -> singleton(remoteRepo.getPullRequest(prNumber)));
         } else {
-            List<GHPullRequest> remotePulls = remoteRepo.getPullRequests(GHIssueState.OPEN);
+            List<GHPullRequest> remotePulls = execute(() -> remoteRepo.getPullRequests(GHIssueState.OPEN));
 
             Set<Integer> remotePRNums = from(remotePulls).transform(extractPRNumber()).toSet();
 

github-pullrequest-plugin/src/test/java/com/github/kostyasha/github/integration/generic/utils/RetryableGitHubOperationTest.java

@@ -0,0 +1,91 @@
+package com.github.kostyasha.github.integration.generic.utils;
+
+import com.github.kostyasha.github.integration.generic.utils.RetryableGitHubOperation.GitOperation;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class RetryableGitHubOperationTest {
+
+    private Object actualResult;
+
+    @Mock
+    private GitOperation<Object> mockOperation;
+
+    private Object mockResult;
+
+    private int retries;
+
+    private Exception thrownException;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void testNoRetryRequired() throws Exception {
+        givenOperationSucceeds();
+        whenExecuteOperation();
+        thenOperationIsSuccessful();
+    }
+
+    @Test
+    public void testRetryWasExceeded() throws Exception {
+        givenOperationExceedsRetry();
+        whenExecuteOperation();
+        thenOperationWasNotSuccessful();
+    }
+
+    @Test
+    public void testRetryWasRequired() throws Exception {
+        givenOperationRequiresRetry();
+        whenExecuteOperation();
+        thenOperationIsSuccessful();
+    }
+
+    private void givenOperationExceedsRetry() throws IOException {
+        retries = 2;
+        when(mockOperation.execute()).thenThrow(new FileNotFoundException());
+    }
+
+    private void givenOperationRequiresRetry() throws IOException {
+        retries = 2;
+        when(mockOperation.execute()).thenThrow(new FileNotFoundException())
+                .thenReturn(mockResult);
+    }
+
+    private void givenOperationSucceeds() throws IOException {
+        retries = 1;
+        when(mockOperation.execute()).thenReturn(mockResult);
+    }
+
+    private void thenOperationIsSuccessful() throws IOException {
+        assertThat(actualResult, is(mockResult));
+        verify(mockOperation, times(retries)).execute();
+    }
+
+    private void thenOperationWasNotSuccessful() {
+        assertThat(thrownException, instanceOf(FileNotFoundException.class));
+    }
+
+    private void whenExecuteOperation() {
+        try {
+            actualResult = RetryableGitHubOperation.execute(retries, 1L, mockOperation);
+        } catch (Exception e) {
+            thrownException = e;
+        }
+    }
+}