Skip to content

Commit

Permalink
Merge pull request #163 from salesforce/additional-fork-wait-before-c…
Browse files Browse the repository at this point in the history 
…reating-branch

Additional wait on fork creation before creating branch
  • Loading branch information
@justinharringa
justinharringa authored Jun 14, 2020
2 parents b396482 + bb36158 commit 1ae1dd2
Show file tree
Hide file tree
Showing 6 changed files with 252 additions and 16 deletions.
2 changes: 1 addition & 1 deletion dockerfile-image-update/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
<dependency>
<groupId>org.kohsuke</groupId>
<artifactId>github-api</artifactId>
<version>1.112</version>
<version>1.114</version>
</dependency>
</dependencies>

Expand Down
28 changes: 21 additions & 7 deletions ...update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void modifyAllOnGithub(GHRepository repo, String branch,
break;
} catch (FileNotFoundException e1) {
log.warn("Content in repository not created yet. Retrying connection to fork...");
Thread.sleep(TimeUnit.SECONDS.toMillis(1));
getGitHubUtil().waitFor(TimeUnit.SECONDS.toMillis(1));
}
}
for (GHContent con : tree) {
Expand Down Expand Up @@ -246,15 +246,29 @@ public Optional<GHPullRequest> getPullRequestForImageBranch(GHRepository reposit
return Optional.empty();
}

public void createOrUpdateForkBranchToParentDefault(GHRepository parent, GHRepository fork, GitForkBranch gitForkBranch) throws IOException {
/**
* Create or update the desired {@code gitForkBranch} in {@code fork} based off of the {@code parent} repo's
* default branch to ensure that {@code gitForkBranch} is on the latest commit.
*
* You must have branches in a repo in order to create a ref per:
* https://developer.github.com/enterprise/2.19/v3/git/refs/#create-a-reference
*
* Generally, we can assume that a fork should have branches so if it does not have branches, we're still
* waiting for GitHub to finish replicating the tree behind the scenes.
*
* @param parent parent repo to base from
* @param fork fork repo where we'll create or modify the {@code gitForkBranch}
* @param gitForkBranch desired branch to create or update based on the parent's default branch
*/
public void createOrUpdateForkBranchToParentDefault(GHRepository parent, GHRepository fork, GitForkBranch gitForkBranch) throws IOException, InterruptedException {
GHBranch parentBranch = parent.getBranch(parent.getDefaultBranch());
String sha1 = parentBranch.getSHA1();
GHBranch forkBranch = fork.getBranches().get(gitForkBranch.getBranchName());
gitHubUtil.tryRetrievingBranch(fork, parent.getDefaultBranch());
String branchRefName = String.format("refs/heads/%s", gitForkBranch.getBranchName());
if (forkBranch == null) {
fork.createRef(branchRefName, sha1);
} else {
if (gitHubUtil.repoHasBranch(fork, gitForkBranch.getBranchName())) {
fork.getRef(branchRefName).updateTo(sha1, true);
} else {
fork.createRef(branchRefName, sha1);
}
}

Expand Down Expand Up @@ -289,7 +303,7 @@ public Optional<PagedSearchIterable<GHContent>> getGHContents(String org, String
if (contentsWithImage.getTotalCount() > 0) {
break;
} else {
Thread.sleep(TimeUnit.SECONDS.toMillis(1));
getGitHubUtil().waitFor(TimeUnit.SECONDS.toMillis(1));
}
}

Expand Down
56 changes: 52 additions & 4 deletions ...ile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/GitHubUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
*/
public class GitHubUtil {
private static final Logger log = LoggerFactory.getLogger(GitHubUtil.class);
public static final String NO_BRANCH_WARN_FORMAT = "Couldn't find branch `%s` in repo `%s`. Waiting a second...";

private final GitHub github;

Expand Down Expand Up @@ -116,12 +117,39 @@ public int createPullReq(GHRepository origRepo, String branch,
} else {
// TODO: THIS WILL LOOP FOREVVEEEEEERRRR
log.warn("An error occurred in pull request: {} Trying again...", error);
Thread.sleep(3000);
waitFor(TimeUnit.SECONDS.toMillis(3));
return -1;
}
}
}

/**
* Attempt to find the provided {@code branch}. Generally, github-api will retry for us without
* the need to do this. This particular process is useful when we need more time such as when
* we are waiting for a repository to be forked.
*
* @param repo - wait until we can retrieve {@code branch from this repo}
* @param branchName - the branch to wait for
*/
protected GHBranch tryRetrievingBranch(GHRepository repo, String branchName) throws InterruptedException {
for (int i = 0; i < 10; i++) {
try {
GHBranch branch = repo.getBranch(branchName);
if (branch != null) {
return branch;
}
log.warn(String.format(NO_BRANCH_WARN_FORMAT, branchName, repo.getName()));
} catch (IOException exception) {
// Keep waiting - this is expected rather than a null branch but we'll handle
// both scenarios as neither would indicate that the repo branch is ready
log.warn(String.format(NO_BRANCH_WARN_FORMAT + " Exception was: %s",
branchName, repo.getName(), exception.getMessage()));
}
waitFor(TimeUnit.SECONDS.toMillis(1));
}
return null;
}

public GHRepository tryRetrievingRepository(String repoName) throws InterruptedException {
GHRepository repo = null;
for (int i = 0; i < 10; i++) {
Expand All @@ -130,7 +158,7 @@ public GHRepository tryRetrievingRepository(String repoName) throws InterruptedE
break;
} catch (IOException e1) {
log.warn("Repository not created yet. Retrying connection to repository...");
Thread.sleep(TimeUnit.SECONDS.toMillis(1));
waitFor(TimeUnit.SECONDS.toMillis(1));
}
}
return repo;
Expand All @@ -147,7 +175,7 @@ public GHContent tryRetrievingContent(GHRepository repo, String path, String bra
break;
} catch (IOException e1) {
log.warn("Content in repository not created yet. Retrying connection to fork...");
Thread.sleep(TimeUnit.SECONDS.toMillis(1));
waitFor(TimeUnit.SECONDS.toMillis(1));
}
}
return content;
Expand Down Expand Up @@ -184,12 +212,32 @@ public List<GHRepository> getGHRepositories(Multimap<String, String> pathToDocke
break;
} else {
log.info("Waiting for GitHub API cache to clear...");
Thread.sleep(TimeUnit.MINUTES.toMillis(1));
waitFor(TimeUnit.MINUTES.toMillis(1));
}
}
return listOfRepos;
}

protected void waitFor(long millis) throws InterruptedException {
Thread.sleep(millis);
}

/**
* Check to see if the provided {@code repo} has the {@code branchName}
*
* @param repo - repo to check
* @param branchName - branchName we're looking for in {@code repo}
* @return {@code repo} has branch with name {@code branchName}
* @throws IOException - there was some exception that we couldn't overcome
*/
public boolean repoHasBranch(GHRepository repo, String branchName) throws IOException {
try {
GHBranch branch = repo.getBranch(branchName);
return branch != null;
} catch (GHFileNotFoundException exception) {
return false;
}
}
/**
* Returns a <code>java.util.Map</code> of GitHub repositories owned by a user. Returned Map's keys are the repository
* names and values are their corresponding GitHub repository objects.
Expand Down
73 changes: 73 additions & 0 deletions ...te/src/test/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

import static org.mockito.Matchers.*;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -534,10 +535,82 @@ public void testGHContentsNoOutput() throws Exception {
PagedSearchIterable<GHContent> contentsWithImage = mock(PagedSearchIterable.class);
when(contentsWithImage.getTotalCount()).thenReturn(0);

GitHubUtil gitHubUtil = mock(GitHubUtil.class);
DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class);
when(dockerfileGitHubUtil.getGitHubUtil()).thenReturn(gitHubUtil);
when(dockerfileGitHubUtil.findFilesWithImage(anyString(), eq("org"))).thenReturn(contentsWithImage);
when(dockerfileGitHubUtil.getGHContents("org", "image")).thenCallRealMethod();

assertEquals(dockerfileGitHubUtil.getGHContents("org", "image"), Optional.empty());
}

@Test
public void testThisUserIsOwner() throws IOException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
DockerfileGitHubUtil dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil);
String me = "me";
GHRepository repo = mock(GHRepository.class);
when(repo.getOwnerName()).thenReturn(me);
GHMyself ghMyself = mock(GHMyself.class);
when(ghMyself.getLogin()).thenReturn(me);
when(gitHubUtil.getMyself()).thenReturn(ghMyself);

assertTrue(dockerfileGitHubUtil.thisUserIsOwner(repo));
}

@Test(expectedExceptions = IOException.class)
public void testThisUserIsOwnerCantFindMyself() throws IOException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
DockerfileGitHubUtil dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil);
String me = "me";
GHRepository repo = mock(GHRepository.class);
when(repo.getOwnerName()).thenReturn(me);
GHMyself ghMyself = mock(GHMyself.class);
when(ghMyself.getLogin()).thenReturn(me);
when(gitHubUtil.getMyself()).thenReturn(null);

dockerfileGitHubUtil.thisUserIsOwner(repo);
}

@Test
public void testCreateOrUpdateForkBranchToParentDefaultHasBranch() throws IOException, InterruptedException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
DockerfileGitHubUtil dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil);
GHRepository parent = mock(GHRepository.class);
String defaultBranch = "default";
when(parent.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
String sha = "abcdef";
when(parentBranch.getSHA1()).thenReturn(sha);
when(parent.getBranch(defaultBranch)).thenReturn(parentBranch);
GHRepository fork = mock(GHRepository.class);
GitForkBranch gitForkBranch = new GitForkBranch("imageName", "imageTag", null);
when(gitHubUtil.repoHasBranch(fork, gitForkBranch.getBranchName())).thenReturn(true);
GHRef returnedRef = mock(GHRef.class);
when(fork.getRef(anyString())).thenReturn(returnedRef);

dockerfileGitHubUtil.createOrUpdateForkBranchToParentDefault(parent, fork, gitForkBranch);

verify(returnedRef, times(1)).updateTo(sha, true);
}

@Test
public void testCreateOrUpdateForkBranchToParentDefaultDoesNotHaveBranch() throws IOException, InterruptedException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
DockerfileGitHubUtil dockerfileGitHubUtil = new DockerfileGitHubUtil(gitHubUtil);
GHRepository parent = mock(GHRepository.class);
String defaultBranch = "default";
when(parent.getDefaultBranch()).thenReturn(defaultBranch);
GHBranch parentBranch = mock(GHBranch.class);
String sha = "abcdef";
when(parentBranch.getSHA1()).thenReturn(sha);
when(parent.getBranch(defaultBranch)).thenReturn(parentBranch);
GHRepository fork = mock(GHRepository.class);
GitForkBranch gitForkBranch = new GitForkBranch("imageName", "imageTag", null);
when(gitHubUtil.repoHasBranch(fork, gitForkBranch.getBranchName())).thenReturn(false);

dockerfileGitHubUtil.createOrUpdateForkBranchToParentDefault(parent, fork, gitForkBranch);

verify(fork, times(1)).createRef(anyString(), matches(sha));
}
}
107 changes: 104 additions & 3 deletions ...image-update/src/test/java/com/salesforce/dockerfileimageupdate/utils/GitHubUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;

import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.*;
Expand Down Expand Up @@ -178,7 +179,7 @@ public void testGetGHRepositories() throws Exception {
expectedList.add(repo2);
expectedList.add(repo3);
expectedList.add(repo4);
assertTrue(actualList.size() == expectedList.size());
assertEquals(expectedList.size(), actualList.size());
assertTrue(actualList.containsAll(expectedList));
assertTrue(expectedList.containsAll(actualList));
}
Expand Down Expand Up @@ -207,7 +208,7 @@ public void testGetReposForUserAtCurrentInstant() throws Exception {
GitHub github = mock(GitHub.class);
GitHubUtil gitHubUtil = new GitHubUtil(github);
Map<String, GHRepository> repoByName = gitHubUtil.getReposForUserAtCurrentInstant(currentUser);
assertTrue(repoByName.size() == 4);
assertEquals(repoByName.size(), 4);
assertTrue(repoByName.containsKey("test1") && repoByName.get("test1") == repo1);
assertTrue(repoByName.containsKey("test2") && repoByName.get("test2") == repo2);
assertTrue(repoByName.containsKey("test3") && repoByName.get("test3") == repo3);
Expand All @@ -219,6 +220,106 @@ public void testGetReposForUserAtCurrentInstantWithNullUser() throws Exception {
GitHub github = mock(GitHub.class);
GitHubUtil gitHubUtil = new GitHubUtil(github);
Map<String, GHRepository> repoByName = gitHubUtil.getReposForUserAtCurrentInstant(null);
assertTrue(repoByName.size() == 0);
assertEquals(repoByName.size(), 0);
}

@Test
public void testTryRetrievingBranchWaits10SecondsForExceptions() throws IOException, InterruptedException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
GHRepository fork = mock(GHRepository.class);
when(gitHubUtil.tryRetrievingBranch(any(), any())).thenCallRealMethod();
when(fork.getBranch(any())).thenThrow(new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException(),
new GHFileNotFoundException());

assertNull(gitHubUtil.tryRetrievingBranch(fork, "somebranch"));
verify(gitHubUtil, times(10)).waitFor(TimeUnit.SECONDS.toMillis(1));
}

@Test
public void testTryRetrievingBranchReturnsAfterFound() throws IOException, InterruptedException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
GHRepository fork = mock(GHRepository.class);
when(gitHubUtil.tryRetrievingBranch(any(), any())).thenCallRealMethod();
GHBranch branch = mock(GHBranch.class);
when(fork.getBranch(any())).thenReturn(null, branch);

assertEquals(gitHubUtil.tryRetrievingBranch(fork, "somebranch"), branch);
verify(gitHubUtil, times(1)).waitFor(TimeUnit.SECONDS.toMillis(1));
}

@Test
public void testTryRetrievingBranchReturnsImmediately() throws IOException, InterruptedException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
when(gitHubUtil.tryRetrievingBranch(any(), any())).thenCallRealMethod();
GHRepository fork = mock(GHRepository.class);
GHBranch branch = mock(GHBranch.class);
when(fork.getBranch(any())).thenReturn(branch);

assertEquals(gitHubUtil.tryRetrievingBranch(fork, "somebranch"), branch);
verify(gitHubUtil, times(0)).waitFor(TimeUnit.SECONDS.toMillis(1));
}

@Test
public void testTryRetrievingBranchWaits10SecondsForNullBranch() throws IOException, InterruptedException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
when(gitHubUtil.tryRetrievingBranch(any(), any())).thenCallRealMethod();
GHRepository fork = mock(GHRepository.class);
when(fork.getBranch(any())).thenReturn(null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null);

assertNull(gitHubUtil.tryRetrievingBranch(fork, "somebranch"));
verify(gitHubUtil, times(10)).waitFor(TimeUnit.SECONDS.toMillis(1));
}

@Test
public void testRepoHasBranchTrue() throws IOException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
when(gitHubUtil.repoHasBranch(any(), any())).thenCallRealMethod();
GHRepository repo = mock(GHRepository.class);
String branchName = "some-branch";

when(repo.getBranch(branchName)).thenReturn(mock(GHBranch.class));
assertTrue(gitHubUtil.repoHasBranch(repo, branchName));
}

@Test
public void testRepoHasBranchFalseIfNoBranchReturned() throws IOException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
when(gitHubUtil.repoHasBranch(any(), any())).thenCallRealMethod();
GHRepository repo = mock(GHRepository.class);
String branchName = "some-branch";

when(repo.getBranch(branchName)).thenReturn(null);
assertFalse(gitHubUtil.repoHasBranch(repo, branchName));
}

@Test
public void testRepoHasBranchFalseForGHFileNotFoundException() throws IOException {
GitHubUtil gitHubUtil = mock(GitHubUtil.class);
when(gitHubUtil.repoHasBranch(any(), any())).thenCallRealMethod();
GHRepository repo = mock(GHRepository.class);
String branchName = "some-branch";

when(repo.getBranch(branchName)).thenThrow(new GHFileNotFoundException());
assertFalse(gitHubUtil.repoHasBranch(repo, branchName));
}
}
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>3.2.3</version>
<version>3.2.4</version>
</plugin>

<plugin>
Expand Down

0 comments on commit 1ae1dd2

Please sign in to comment.