Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Git tags #191 #192

Merged
merged 3 commits into from
May 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@
<artifactId>org.eclipse.jgit</artifactId>
<version>4.8.0.201706111038-r</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.10.19</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
62 changes: 45 additions & 17 deletions src/main/java/org/commonwl/view/git/GitService.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.commonwl.view.researchobject.HashableAgent;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.RevCommit;
Expand All @@ -32,6 +33,7 @@
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -67,7 +69,7 @@ public GitService(@Value("${gitStorage}") Path gitStorage,
* Gets a repository, cloning into a local directory or
* @param gitDetails The details of the Git repository
* @param reuseDir Whether the cached repository can be used
* @returns The git object for the repository
* @return The git object for the repository
*/
public Git getRepository(GitDetails gitDetails, boolean reuseDir)
throws GitAPIException, IOException {
Expand All @@ -84,34 +86,45 @@ public Git getRepository(GitDetails gitDetails, boolean reuseDir)
} else {
// Create a folder and clone repository into it
Files.createDirectory(repoDir);
repo = Git.cloneRepository()
.setCloneSubmodules(cloneSubmodules)
.setURI(gitDetails.getRepoUrl())
.setDirectory(repoDir.toFile())
.setCloneAllBranches(true)
.call();
repo = cloneRepo(gitDetails.getRepoUrl(), repoDir.toFile());
}
} else {
// Another thread is already using the existing folder
// Must create another temporary one
repo = Git.cloneRepository()
.setCloneSubmodules(cloneSubmodules)
.setURI(gitDetails.getRepoUrl())
.setDirectory(createTempDir())
.setCloneAllBranches(true)
.call();
repo = cloneRepo(gitDetails.getRepoUrl(), createTempDir());
}

// Checkout the specific branch or commit ID
if (repo != null) {
// Create a new local branch if it does not exist and not a commit ID
String branchOrCommitId = gitDetails.getBranch();
if (!ObjectId.isId(branchOrCommitId)) {
final boolean isId = ObjectId.isId(branchOrCommitId);
if (!isId) {
branchOrCommitId = "refs/remotes/origin/" + branchOrCommitId;
}
repo.checkout()
.setName(branchOrCommitId)
.call();
try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree on the logic of trying branch before tag, that's the logic used at GitHub as well - see https://github.com/stain/branchandtagtest/blob/test/README.md where I tried to make a branch and a tag both called test (but the branch further ahead)

repo.checkout()
.setName(branchOrCommitId)
.call();
}
catch (Exception ex) {
// Maybe it was a tag
if (!isId && ex instanceof RefNotFoundException) {
final String tag = gitDetails.getBranch();
try {
repo.checkout()
.setName(tag)
.call();
}
catch (Exception ex2) {
// Throw the first exception, to keep the same behavior as before.
throw ex;
}
}
else {
throw ex;
}
}
}

return repo;
Expand Down Expand Up @@ -184,4 +197,19 @@ public GitDetails transferPathToBranch(GitDetails githubInfo) {
}
}

/**
* Clones a Git repository
* @param repoUrl the url of the Git repository
* @param directory the directory to clone the repo into
* @return a Git instance
* @throws GitAPIException if any error occurs cloning the repo
*/
protected Git cloneRepo(String repoUrl, File directory) throws GitAPIException {
return Git.cloneRepository()
.setCloneSubmodules(cloneSubmodules)
.setURI(repoUrl)
.setDirectory(directory)
.setCloneAllBranches(true)
.call();
}
}
70 changes: 70 additions & 0 deletions src/test/java/org/commonwl/view/git/GitServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,51 @@

package org.commonwl.view.git;

import java.io.File;
import java.io.IOException;

import org.eclipse.jgit.api.CheckoutCommand;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

public class GitServiceTest {

private final RefNotFoundException branchNotFoundException = new RefNotFoundException("Branch not found");
private final RefNotFoundException tagNotFoundException = new RefNotFoundException("Tag not found");

private GitService spyGitService;
private Git mockGit;
private CheckoutCommand mockGoodCheckoutCommand;
private CheckoutCommand mockBranchNotFoundCommand;
private CheckoutCommand mockTagNotFoundCommand;
private CheckoutCommand mockCheckoutCommand;

@Before
public void setup() throws GitAPIException {
GitService gitService = new GitService(null, false);
this.spyGitService = spy(gitService);
this.mockGit = mock(Git.class);
this.mockGoodCheckoutCommand = mock(CheckoutCommand.class);
this.mockBranchNotFoundCommand = mock(CheckoutCommand.class);
this.mockTagNotFoundCommand = mock(CheckoutCommand.class);
this.mockCheckoutCommand = mock(CheckoutCommand.class);
when(this.mockGit.checkout()).thenReturn(this.mockCheckoutCommand);
when(this.mockBranchNotFoundCommand.call()).thenThrow(branchNotFoundException);
when(this.mockTagNotFoundCommand.call()).thenThrow(tagNotFoundException);
}

@Test
public void transferPathToBranch() throws Exception {
GitService gitService = new GitService(null, false);
Expand All @@ -39,4 +78,35 @@ public void transferPathToBranch() throws Exception {
assertEquals("branchpart1/branchpart2/branchpart3", step2.getBranch());
assertEquals("workflowInRoot.cwl", step2.getPath());
}

@Test
public void checksOutTag() throws Exception {
when(mockCheckoutCommand.setName("refs/remotes/origin/mytag")).thenReturn(this.mockBranchNotFoundCommand);
when(mockCheckoutCommand.setName("mytag")).thenReturn(this.mockGoodCheckoutCommand);
doReturn(this.mockGit).when(this.spyGitService).cloneRepo(Mockito.any(String.class), Mockito.any(File.class));
assertNotNull(this.spyGitService.getRepository(new GitDetails(null, "mytag", "foo"), false));
}

@Test
public void checksOutBranch() throws GitAPIException, IOException {
when(mockCheckoutCommand.setName("refs/remotes/origin/mybranch")).thenReturn(this.mockGoodCheckoutCommand);
when(mockCheckoutCommand.setName("mytag")).thenReturn(this.mockTagNotFoundCommand);
doReturn(this.mockGit).when(this.spyGitService).cloneRepo(Mockito.any(String.class), Mockito.any(File.class));
assertNotNull(this.spyGitService.getRepository(new GitDetails(null, "mybranch", "foo"), false));
}

@Test()
public void throwsFirstExceptionIfTagAndBranchFail() throws GitAPIException {
when(mockCheckoutCommand.setName("refs/remotes/origin/mytag")).thenReturn(this.mockBranchNotFoundCommand);
when(mockCheckoutCommand.setName("mytag")).thenReturn(this.mockTagNotFoundCommand);
doReturn(this.mockGit).when(this.spyGitService).cloneRepo(Mockito.any(String.class), Mockito.any(File.class));
boolean thrown = false;
try {
this.spyGitService.getRepository(new GitDetails(null, "mytag", "foo"), false);
} catch (Exception e) {
// Make sure it's not this.tagNotFoundException
thrown = (e == this.branchNotFoundException);
}
assertTrue(thrown);
}
}