Refactor dependency management for retriable operations

Passing around WaitGroups everywhere isn't all that nice.

Author: deiwin

main.go

@@ -21,6 +21,8 @@ const (
 	githubStatusPeerReviewContext = "review/peer"
 )
 
+type retryGithubOperation func(func() asyncResponse) MaybeSyncResponse
+
 func main() {
 	conf := NewConfig()
 	githubClient := initGithubClient(conf.AccessToken)
@@ -48,8 +50,13 @@ func main() {
 	asyncOperationWg.Wait()
 }
 
-func CreateHandler(conf Config, gitRepos git.Repos, asyncOperationWg *sync.WaitGroup, pullRequests PullRequests,
-	repositories Repositories, issues Issues, search Search) Handler {
+func CreateHandler(conf Config, gitRepos git.Repos, asyncOperationWg *sync.WaitGroup,
+	pullRequests PullRequests, repositories Repositories, issues Issues, search Search) Handler {
+
+	retry := func(operation func() asyncResponse) MaybeSyncResponse {
+		return delayWithRetries(conf.GithubAPITryDeltas, operation, asyncOperationWg)
+	}
+
 	return func(w http.ResponseWriter, r *http.Request) Response {
 		body, err := ioutil.ReadAll(r.Body)
 		if err != nil {
@@ -61,19 +68,18 @@ func CreateHandler(conf Config, gitRepos git.Repos, asyncOperationWg *sync.WaitG
 		eventType := r.Header.Get("X-Github-Event")
 		switch eventType {
 		case "issue_comment":
-			return handleIssueComment(body, conf, asyncOperationWg, gitRepos, pullRequests, repositories, issues)
+			return handleIssueComment(body, retry, gitRepos, pullRequests, repositories, issues)
 		case "pull_request":
-			return handlePullRequestEvent(body, conf, asyncOperationWg, pullRequests, repositories)
+			return handlePullRequestEvent(body, retry, pullRequests, repositories)
 		case "status":
-			return handleStatusEvent(body, conf, asyncOperationWg, gitRepos, search, issues, pullRequests)
+			return handleStatusEvent(body, retry, gitRepos, search, issues, pullRequests)
 		}
 		return SuccessResponse{"Not an event I understand. Ignoring."}
 	}
 }
 
-func handleIssueComment(body []byte, conf Config, asyncOperationWg *sync.WaitGroup,
-	gitRepos git.Repos, pullRequests PullRequests, repositories Repositories,
-	issues Issues) Response {
+func handleIssueComment(body []byte, retry retryGithubOperation, gitRepos git.Repos,
+	pullRequests PullRequests, repositories Repositories, issues Issues) Response {
 
 	issueComment, err := parseIssueComment(body)
 	if err != nil {
@@ -97,35 +103,36 @@ func handleIssueComment(body []byte, conf Config, asyncOperationWg *sync.WaitGro
 	case mergeCommand:
 		return handleMergeCommand(issueComment, issues, pullRequests, repositories, gitRepos)
 	case checkCommand:
-		return checkForFixupCommitsOnIssueComment(issueComment, pullRequests, repositories, conf, asyncOperationWg)
+		return checkForFixupCommitsOnIssueComment(issueComment, pullRequests, repositories, retry)
 	}
 	return ErrorResponse{
 		Code:         http.StatusInternalServerError,
 		ErrorMessage: fmt.Sprintf("Unhandled comment type: %v", commentCategory),
 	}
 }
 
-func handlePullRequestEvent(body []byte, conf Config, asyncOperationWg *sync.WaitGroup,
-	pullRequests PullRequests, repositories Repositories) Response {
+func handlePullRequestEvent(body []byte, retry retryGithubOperation, pullRequests PullRequests,
+	repositories Repositories) Response {
 
 	pullRequestEvent, err := parsePullRequestEvent(body)
 	if err != nil {
 		return ErrorResponse{err, http.StatusInternalServerError, "Failed to parse the request's body"}
 	} else if !(pullRequestEvent.Action == "opened" || pullRequestEvent.Action == "synchronize") {
 		return SuccessResponse{"PR not opened or synchronized. Ignoring."}
 	}
-	return checkForFixupCommitsOnPREvent(pullRequestEvent, pullRequests, repositories, conf, asyncOperationWg)
+	return checkForFixupCommitsOnPREvent(pullRequestEvent, pullRequests, repositories, retry)
 }
 
-func handleStatusEvent(body []byte, conf Config, asyncOperationWg *sync.WaitGroup, gitRepos git.Repos, search Search,
+func handleStatusEvent(body []byte, retry retryGithubOperation, gitRepos git.Repos, search Search,
 	issues Issues, pullRequests PullRequests) Response {
+
 	statusEvent, err := parseStatusEvent(body)
 	if err != nil {
 		return ErrorResponse{err, http.StatusInternalServerError, "Failed to parse the request's body"}
 	} else if newPullRequestsPossiblyReadyForMerging(statusEvent) {
-		maybeSyncResponse := delayWithRetries(conf.GithubAPITryDeltas, func() asyncResponse {
+		maybeSyncResponse := retry(func() asyncResponse {
 			return mergePullRequestsReadyForMerging(statusEvent, gitRepos, search, issues, pullRequests)
-		}, asyncOperationWg)
+		})
 		if maybeSyncResponse.OperationFinishedSynchronously {
 			return maybeSyncResponse.Response
 		}

squash.go

@@ -6,7 +6,6 @@ import (
 	"log"
 	"net/http"
 	"strings"
-	"sync"
 
 	"github.com/google/go-github/github"
 	"github.com/salemove/github-review-helper/git"
@@ -31,19 +30,19 @@ func handleSquashCommand(issueComment IssueComment, gitRepos git.Repos, pullRequ
 }
 
 func checkForFixupCommitsOnPREvent(pullRequestEvent PullRequestEvent, pullRequests PullRequests,
-	repositories Repositories, conf Config, asyncOperationWg *sync.WaitGroup) Response {
+	repositories Repositories, retry retryGithubOperation) Response {
 
 	isExpectedHead := func(head string) bool {
 		return head == pullRequestEvent.Head.SHA
 	}
 	setStatus := func(status *github.RepoStatus) *ErrorResponse {
 		return setStatusForPREvent(pullRequestEvent, status, repositories)
 	}
-	return checkForFixupCommits(pullRequestEvent, isExpectedHead, setStatus, pullRequests, conf, asyncOperationWg)
+	return checkForFixupCommits(pullRequestEvent, isExpectedHead, setStatus, pullRequests, retry)
 }
 
 func checkForFixupCommitsOnIssueComment(issueComment IssueComment, pullRequests PullRequests,
-	repositories Repositories, conf Config, asyncOperationWg *sync.WaitGroup) Response {
+	repositories Repositories, retry retryGithubOperation) Response {
 
 	isExpectedHead := func(string) bool { return true }
 	setStatus := func(status *github.RepoStatus) *ErrorResponse {
@@ -53,15 +52,15 @@ func checkForFixupCommitsOnIssueComment(issueComment IssueComment, pullRequests
 		}
 		return setStatusForPR(pr, status, repositories)
 	}
-	return checkForFixupCommits(issueComment, isExpectedHead, setStatus, pullRequests, conf, asyncOperationWg)
+	return checkForFixupCommits(issueComment, isExpectedHead, setStatus, pullRequests, retry)
 }
 
 func checkForFixupCommits(issueable Issueable, isExpectedHead func(string) bool,
 	setStatus func(*github.RepoStatus) *ErrorResponse, pullRequests PullRequests,
-	conf Config, asyncOperationWg *sync.WaitGroup) Response {
+	retry retryGithubOperation) Response {
 
 	log.Printf("Checking for fixup commits for PR %s.\n", issueable.Issue().FullName())
-	maybeSyncResponse := delayWithRetries(conf.GithubAPITryDeltas, func() asyncResponse {
+	maybeSyncResponse := retry(func() asyncResponse {
 		commits, asyncErrResp := getCommits(issueable, isExpectedHead, pullRequests)
 		if asyncErrResp != nil {
 			return asyncErrResp.toAsyncResponse()
@@ -78,7 +77,7 @@ func checkForFixupCommits(issueable Issueable, isExpectedHead func(string) bool,
 			return nonRetriable(errResp)
 		}
 		return nonRetriable(SuccessResponse{})
-	}, asyncOperationWg)
+	})
 	if maybeSyncResponse.OperationFinishedSynchronously {
 		return maybeSyncResponse.Response
 	}