Skip to content

Commit 3d0c86b

Browse files
MarkRobboMarkRobbo
committed
Add semaphore calling to Git repo access
1 parent a67352e commit 3d0c86b

File tree

3 files changed

+76
-46
lines changed

3 files changed

+76
-46
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class GitSemaphore {
3838
* @return Whether the resource can be accessed safely
3939
* (no other threads are using it)
4040
*/
41-
synchronized boolean acquire(String repoUrl) {
41+
public synchronized boolean acquire(String repoUrl) {
4242
if (currentRepos.containsKey(repoUrl)) {
4343
currentRepos.put(repoUrl, currentRepos.get(repoUrl) + 1);
4444
return false;
@@ -52,7 +52,7 @@ synchronized boolean acquire(String repoUrl) {
5252
* Release use of the shared resource
5353
* @param repoUrl The url of the repository
5454
*/
55-
synchronized void release(String repoUrl) {
55+
public synchronized void release(String repoUrl) {
5656
if (currentRepos.containsKey(repoUrl)) {
5757
int threadCountUsing = currentRepos.get(repoUrl);
5858
if (threadCountUsing > 1) {

‎src/main/java/org/commonwl/view/researchobject/ROBundleService.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.commonwl.view.cwl.CWLTool;
2828
import org.commonwl.view.cwl.CWLValidationException;
2929
import org.commonwl.view.git.GitDetails;
30+
import org.commonwl.view.git.GitSemaphore;
3031
import org.commonwl.view.git.GitService;
3132
import org.commonwl.view.git.GitType;
3233
import org.commonwl.view.graphviz.GraphVizService;
@@ -67,6 +68,7 @@ public class ROBundleService {
6768
private GraphVizService graphVizService;
6869
private GitService gitService;
6970
private CWLTool cwlTool;
71+
private GitSemaphore gitSemaphore;
7072

7173
// Configuration variables
7274
private Agent appAgent;
@@ -92,13 +94,15 @@ public ROBundleService(@Value("${bundleStorage}") Path bundleStorage,
9294
@Value("${singleFileSizeLimit}") int singleFileSizeLimit,
9395
GraphVizService graphVizService,
9496
GitService gitService,
97+
GitSemaphore gitSemaphore,
9598
CWLTool cwlTool) throws URISyntaxException {
9699
this.bundleStorage = bundleStorage;
97100
this.appAgent = new Agent(appName);
98101
appAgent.setUri(new URI(appURL));
99102
this.singleFileSizeLimit = singleFileSizeLimit;
100103
this.graphVizService = graphVizService;
101104
this.gitService = gitService;
105+
this.gitSemaphore = gitSemaphore;
102106
this.cwlTool = cwlTool;
103107
}
104108

@@ -131,10 +135,15 @@ public Bundle createBundle(Workflow workflow, GitDetails gitInfo) throws IOExcep
131135

132136
// Add the files from the repo to this workflow
133137
Set<HashableAgent> authors = new HashSet<>();
134-
Git gitRepo = gitService.getRepository(workflow.getRetrievedFrom());
135-
Path relativePath = Paths.get(FilenameUtils.getPath(gitInfo.getPath()));
136-
Path gitPath = gitRepo.getRepository().getWorkTree().toPath().resolve(relativePath);
137-
addFilesToBundle(gitInfo, bundle, bundlePath, gitRepo, gitPath, authors);
138+
boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
139+
try {
140+
Git gitRepo = gitService.getRepository(workflow.getRetrievedFrom(), safeToAccess);
141+
Path relativePath = Paths.get(FilenameUtils.getPath(gitInfo.getPath()));
142+
Path gitPath = gitRepo.getRepository().getWorkTree().toPath().resolve(relativePath);
143+
addFilesToBundle(gitInfo, bundle, bundlePath, gitRepo, gitPath, authors);
144+
} finally {
145+
gitSemaphore.release(gitInfo.getRepoUrl());
146+
}
138147

139148
// Add combined authors
140149
manifest.setAuthoredBy(new ArrayList<>(authors));

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

Lines changed: 61 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.commonwl.view.cwl.CWLToolRunner;
2424
import org.commonwl.view.cwl.CWLToolStatus;
2525
import org.commonwl.view.git.GitDetails;
26+
import org.commonwl.view.git.GitSemaphore;
2627
import org.commonwl.view.git.GitService;
2728
import org.commonwl.view.graphviz.GraphVizService;
2829
import org.commonwl.view.researchobject.ROBundleFactory;
@@ -56,6 +57,7 @@ public class WorkflowService {
5657
private final ROBundleFactory ROBundleFactory;
5758
private final GraphVizService graphVizService;
5859
private final CWLToolRunner cwlToolRunner;
60+
private final GitSemaphore gitSemaphore;
5961
private final int cacheDays;
6062

6163
@Autowired
@@ -66,6 +68,7 @@ public WorkflowService(GitService gitService,
6668
ROBundleFactory ROBundleFactory,
6769
GraphVizService graphVizService,
6870
CWLToolRunner cwlToolRunner,
71+
GitSemaphore gitSemaphore,
6972
@Value("${cacheDays}") int cacheDays) {
7073
this.gitService = gitService;
7174
this.cwlService = cwlService;
@@ -75,6 +78,7 @@ public WorkflowService(GitService gitService,
7578
this.graphVizService = graphVizService;
7679
this.cwlToolRunner = cwlToolRunner;
7780
this.cacheDays = cacheDays;
81+
this.gitSemaphore = gitSemaphore;
7882
}
7983

8084
/**
@@ -217,50 +221,58 @@ public File getROBundle(GitDetails gitDetails) throws ROBundleNotFoundException
217221
*/
218222
public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
219223
throws GitAPIException, WorkflowNotFoundException, IOException {
224+
QueuedWorkflow queuedWorkflow;
220225

221-
// Clone repository to temporary folder
222-
Git repo = null;
223-
while (repo == null) {
224-
try {
225-
repo = gitService.getRepository(gitInfo);
226-
} catch (RefNotFoundException ex) {
227-
// Attempt slashes in branch fix
228-
GitDetails correctedForSlash = transferPathToBranch(gitInfo);
229-
if (correctedForSlash != null) {
230-
gitInfo = correctedForSlash;
231-
} else {
232-
throw ex;
226+
try {
227+
// Clone repository to temporary folder
228+
Git repo = null;
229+
while (repo == null) {
230+
try {
231+
boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
232+
repo = gitService.getRepository(gitInfo, safeToAccess);
233+
} catch (RefNotFoundException ex) {
234+
// Attempt slashes in branch fix
235+
GitDetails correctedForSlash = transferPathToBranch(gitInfo);
236+
if (correctedForSlash != null) {
237+
gitSemaphore.release(gitInfo.getRepoUrl());
238+
gitInfo = correctedForSlash;
239+
} else {
240+
throw ex;
241+
}
233242
}
234243
}
235-
}
236-
File localPath = repo.getRepository().getWorkTree();
237-
String latestCommit = gitService.getCurrentCommitID(repo);
244+
File localPath = repo.getRepository().getWorkTree();
245+
String latestCommit = gitService.getCurrentCommitID(repo);
238246

239-
Path pathToWorkflowFile = localPath.toPath().resolve(gitInfo.getPath()).normalize().toAbsolutePath();
240-
// Prevent path traversal attacks
241-
if (!pathToWorkflowFile.startsWith(localPath.toPath().normalize().toAbsolutePath())) {
242-
throw new WorkflowNotFoundException();
243-
}
247+
Path pathToWorkflowFile = localPath.toPath().resolve(gitInfo.getPath()).normalize().toAbsolutePath();
248+
// Prevent path traversal attacks
249+
if (!pathToWorkflowFile.startsWith(localPath.toPath().normalize().toAbsolutePath())) {
250+
throw new WorkflowNotFoundException();
251+
}
244252

245-
File workflowFile = new File(pathToWorkflowFile.toString());
246-
Workflow basicModel = cwlService.parseWorkflowNative(workflowFile);
253+
File workflowFile = new File(pathToWorkflowFile.toString());
254+
Workflow basicModel = cwlService.parseWorkflowNative(workflowFile);
247255

248-
// Set origin details
249-
basicModel.setRetrievedOn(new Date());
250-
basicModel.setRetrievedFrom(gitInfo);
251-
basicModel.setLastCommit(latestCommit);
256+
// Set origin details
257+
basicModel.setRetrievedOn(new Date());
258+
basicModel.setRetrievedFrom(gitInfo);
259+
basicModel.setLastCommit(latestCommit);
252260

253-
// Save the queued workflow to database
254-
QueuedWorkflow queuedWorkflow = new QueuedWorkflow();
255-
queuedWorkflow.setTempRepresentation(basicModel);
256-
queuedWorkflowRepository.save(queuedWorkflow);
261+
// Save the queued workflow to database
262+
queuedWorkflow = new QueuedWorkflow();
263+
queuedWorkflow.setTempRepresentation(basicModel);
264+
queuedWorkflowRepository.save(queuedWorkflow);
257265

258-
// ASYNC OPERATIONS
259-
// Parse with cwltool and update model
260-
try {
261-
cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, workflowFile);
262-
} catch (Exception e) {
263-
logger.error("Could not update workflow with cwltool", e);
266+
// ASYNC OPERATIONS
267+
// Parse with cwltool and update model
268+
try {
269+
cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, workflowFile);
270+
} catch (Exception e) {
271+
logger.error("Could not update workflow with cwltool", e);
272+
}
273+
274+
} finally {
275+
gitSemaphore.release(gitInfo.getRepoUrl());
264276
}
265277

266278
// Return this model to be displayed
@@ -275,14 +287,17 @@ public void retryCwltool(QueuedWorkflow queuedWorkflow) {
275287
queuedWorkflow.setMessage(null);
276288
queuedWorkflow.setCwltoolStatus(CWLToolStatus.RUNNING);
277289
queuedWorkflowRepository.save(queuedWorkflow);
290+
GitDetails gitDetails = queuedWorkflow.getTempRepresentation().getRetrievedFrom();
291+
boolean safeToAccess = gitSemaphore.acquire(gitDetails.getRepoUrl());
278292
try {
279-
GitDetails gitDetails = queuedWorkflow.getTempRepresentation().getRetrievedFrom();
280-
Git repo = gitService.getRepository(gitDetails);
293+
Git repo = gitService.getRepository(gitDetails, safeToAccess);
281294
File localPath = repo.getRepository().getWorkTree();
282295
Path pathToWorkflowFile = localPath.toPath().resolve(gitDetails.getPath()).normalize().toAbsolutePath();
283296
cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, new File(pathToWorkflowFile.toString()));
284297
} catch (Exception e) {
285298
logger.error("Could not update workflow with cwltool", e);
299+
} finally {
300+
gitSemaphore.release(gitDetails.getRepoUrl());
286301
}
287302
}
288303

@@ -343,8 +358,14 @@ private boolean cacheExpired(Workflow workflow) {
343358
// Cache expiry time has elapsed
344359
// Check current head of the branch with the cached head
345360
logger.info("Time has expired for caching, checking commits...");
346-
Git repo = gitService.getRepository(workflow.getRetrievedFrom());
347-
String currentHead = gitService.getCurrentCommitID(repo);
361+
String currentHead;
362+
boolean safeToAccess = gitSemaphore.acquire(workflow.getRetrievedFrom().getRepoUrl());
363+
try {
364+
Git repo = gitService.getRepository(workflow.getRetrievedFrom(), safeToAccess);
365+
currentHead = gitService.getCurrentCommitID(repo);
366+
} finally {
367+
gitSemaphore.release(workflow.getRetrievedFrom().getRepoUrl());
368+
}
348369
logger.info("Current: " + workflow.getLastCommit() + ", HEAD: " + currentHead);
349370

350371
// Reset date in database if there are still no changes

0 commit comments

Comments
 (0)