Skip to content

Commit 7c99558

Browse files
stainstain
authored
Merge pull request #165 from common-workflow-language/hotfix-branch-checkout
Fix for invalid branch names being accepted in some circumstances
2 parents 15122b2 + fcf9044 commit 7c99558

File tree

2 files changed

+84
-68
lines changed

2 files changed

+84
-68
lines changed

‎src/main/java/org/commonwl/view/git/GitService.java‎

Lines changed: 42 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.commonwl.view.researchobject.HashableAgent;
2424
import org.eclipse.jgit.api.Git;
2525
import org.eclipse.jgit.api.errors.GitAPIException;
26-
import org.eclipse.jgit.api.errors.RefNotFoundException;
2726
import org.eclipse.jgit.lib.ObjectId;
2827
import org.eclipse.jgit.lib.PersonIdent;
2928
import org.eclipse.jgit.revwalk.RevCommit;
@@ -72,59 +71,47 @@ public GitService(@Value("${gitStorage}") Path gitStorage,
7271
*/
7372
public Git getRepository(GitDetails gitDetails, boolean reuseDir)
7473
throws GitAPIException, IOException {
75-
Git repo = null;
76-
while (repo == null) {
77-
try {
78-
if (reuseDir) {
79-
// Base dir from configuration, name from hash of repository URL
80-
String baseName = DigestUtils.sha1Hex(GitDetails.normaliseUrl(gitDetails.getRepoUrl()));
81-
82-
// Check if folder already exists
83-
Path repoDir = gitStorage.resolve(baseName);
84-
if (Files.isReadable(repoDir) && Files.isDirectory(repoDir)) {
85-
repo = Git.open(repoDir.toFile());
86-
repo.fetch().call();
87-
} else {
88-
// Create a folder and clone repository into it
89-
Files.createDirectory(repoDir);
90-
repo = Git.cloneRepository()
91-
.setCloneSubmodules(cloneSubmodules)
92-
.setURI(gitDetails.getRepoUrl())
93-
.setDirectory(repoDir.toFile())
94-
.setCloneAllBranches(true)
95-
.call();
96-
}
97-
} else {
98-
// Another thread is already using the existing folder
99-
// Must create another temporary one
100-
repo = Git.cloneRepository()
101-
.setCloneSubmodules(cloneSubmodules)
102-
.setURI(gitDetails.getRepoUrl())
103-
.setDirectory(createTempDir())
104-
.setCloneAllBranches(true)
105-
.call();
106-
}
74+
Git repo;
75+
if (reuseDir) {
76+
// Base dir from configuration, name from hash of repository URL
77+
String baseName = DigestUtils.sha1Hex(GitDetails.normaliseUrl(gitDetails.getRepoUrl()));
78+
79+
// Check if folder already exists
80+
Path repoDir = gitStorage.resolve(baseName);
81+
if (Files.isReadable(repoDir) && Files.isDirectory(repoDir)) {
82+
repo = Git.open(repoDir.toFile());
83+
repo.fetch().call();
84+
} else {
85+
// Create a folder and clone repository into it
86+
Files.createDirectory(repoDir);
87+
repo = Git.cloneRepository()
88+
.setCloneSubmodules(cloneSubmodules)
89+
.setURI(gitDetails.getRepoUrl())
90+
.setDirectory(repoDir.toFile())
91+
.setCloneAllBranches(true)
92+
.call();
93+
}
94+
} else {
95+
// Another thread is already using the existing folder
96+
// Must create another temporary one
97+
repo = Git.cloneRepository()
98+
.setCloneSubmodules(cloneSubmodules)
99+
.setURI(gitDetails.getRepoUrl())
100+
.setDirectory(createTempDir())
101+
.setCloneAllBranches(true)
102+
.call();
103+
}
107104

108-
// Checkout the specific branch or commit ID
109-
if (repo != null) {
110-
// Create a new local branch if it does not exist and not a commit ID
111-
String branchOrCommitId = gitDetails.getBranch();
112-
if (!ObjectId.isId(branchOrCommitId)) {
113-
branchOrCommitId = "refs/remotes/origin/" + branchOrCommitId;
114-
}
115-
repo.checkout()
116-
.setName(branchOrCommitId)
117-
.call();
118-
}
119-
} catch (RefNotFoundException ex) {
120-
// Attempt slashes in branch fix
121-
GitDetails correctedForSlash = transferPathToBranch(gitDetails);
122-
if (correctedForSlash != null) {
123-
gitDetails = correctedForSlash;
124-
} else {
125-
throw ex;
126-
}
105+
// Checkout the specific branch or commit ID
106+
if (repo != null) {
107+
// Create a new local branch if it does not exist and not a commit ID
108+
String branchOrCommitId = gitDetails.getBranch();
109+
if (!ObjectId.isId(branchOrCommitId)) {
110+
branchOrCommitId = "refs/remotes/origin/" + branchOrCommitId;
127111
}
112+
repo.checkout()
113+
.setName(branchOrCommitId)
114+
.call();
128115
}
129116

130117
return repo;
@@ -189,8 +176,9 @@ public GitDetails transferPathToBranch(GitDetails githubInfo) {
189176
if (firstSlash > 0) {
190177
branch += "/" + path.substring(0, firstSlash);
191178
path = path.substring(firstSlash + 1);
192-
return new GitDetails(githubInfo.getRepoUrl(), branch,
193-
path);
179+
GitDetails newDetails = new GitDetails(githubInfo.getRepoUrl(), branch, path);
180+
newDetails.setPackedId(githubInfo.getPackedId());
181+
return newDetails;
194182
} else {
195183
return null;
196184
}

‎src/main/java/org/commonwl/view/workflow/WorkflowService.java‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,6 @@
1919

2020
package org.commonwl.view.workflow;
2121

22-
import java.io.File;
23-
import java.io.IOException;
24-
import java.nio.file.Files;
25-
import java.nio.file.Path;
26-
import java.nio.file.Paths;
27-
import java.util.ArrayList;
28-
import java.util.Calendar;
29-
import java.util.Date;
30-
import java.util.List;
31-
3222
import org.commonwl.view.cwl.CWLService;
3323
import org.commonwl.view.cwl.CWLToolRunner;
3424
import org.commonwl.view.cwl.CWLToolStatus;
@@ -40,6 +30,7 @@
4030
import org.commonwl.view.researchobject.ROBundleNotFoundException;
4131
import org.eclipse.jgit.api.Git;
4232
import org.eclipse.jgit.api.errors.GitAPIException;
33+
import org.eclipse.jgit.api.errors.RefNotFoundException;
4334
import org.slf4j.Logger;
4435
import org.slf4j.LoggerFactory;
4536
import org.springframework.beans.factory.annotation.Autowired;
@@ -48,6 +39,16 @@
4839
import org.springframework.data.domain.Pageable;
4940
import org.springframework.stereotype.Service;
5041

42+
import java.io.File;
43+
import java.io.IOException;
44+
import java.nio.file.Files;
45+
import java.nio.file.Path;
46+
import java.nio.file.Paths;
47+
import java.util.ArrayList;
48+
import java.util.Calendar;
49+
import java.util.Date;
50+
import java.util.List;
51+
5152
@Service
5253
public class WorkflowService {
5354

@@ -193,9 +194,23 @@ public Workflow getWorkflow(GitDetails gitInfo) {
193194
*/
194195
public List<WorkflowOverview> getWorkflowsFromDirectory(GitDetails gitInfo) throws IOException, GitAPIException {
195196
List<WorkflowOverview> workflowsInDir = new ArrayList<>();
196-
boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
197197
try {
198-
Git repo = gitService.getRepository(gitInfo, safeToAccess);
198+
boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
199+
Git repo = null;
200+
while (repo == null) {
201+
try {
202+
repo = gitService.getRepository(gitInfo, safeToAccess);
203+
} catch (RefNotFoundException ex) {
204+
// Attempt slashes in branch fix
205+
GitDetails correctedForSlash = gitService.transferPathToBranch(gitInfo);
206+
if (correctedForSlash != null) {
207+
gitInfo = correctedForSlash;
208+
} else {
209+
throw ex;
210+
}
211+
}
212+
}
213+
199214
Path localPath = repo.getRepository().getWorkTree().toPath();
200215
Path pathToDirectory = localPath.resolve(gitInfo.getPath()).normalize().toAbsolutePath();
201216
Path root = Paths.get("/").toAbsolutePath();
@@ -267,9 +282,22 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
267282
throws GitAPIException, WorkflowNotFoundException, IOException {
268283
QueuedWorkflow queuedWorkflow;
269284

270-
boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
271285
try {
272-
Git repo = gitService.getRepository(gitInfo, safeToAccess);
286+
boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
287+
Git repo = null;
288+
while (repo == null) {
289+
try {
290+
repo = gitService.getRepository(gitInfo, safeToAccess);
291+
} catch (RefNotFoundException ex) {
292+
// Attempt slashes in branch fix
293+
GitDetails correctedForSlash = gitService.transferPathToBranch(gitInfo);
294+
if (correctedForSlash != null) {
295+
gitInfo = correctedForSlash;
296+
} else {
297+
throw ex;
298+
}
299+
}
300+
}
273301
File localPath = repo.getRepository().getWorkTree();
274302
String latestCommit = gitService.getCurrentCommitID(repo);
275303

0 commit comments

Comments
 (0)