Merge branch 'cwltool' into ro-workflow-description

MarkRobbo · MarkRobbo · commit dd36123f7f0f · 2017-08-08T16:52:33.000+01:00
# Conflicts:
#	src/main/java/org/commonwl/view/researchobject/ROBundleService.java
#	src/test/java/org/commonwl/view/researchobject/ROBundleServiceTest.java
diff --git a/Dockerfile b/Dockerfile
@@ -1,4 +1,4 @@
-xFROM maven:3.5-jdk-8-alpine
+FROM maven:3.5-jdk-8-alpine
 MAINTAINER Stian Soiland-Reyes <stain@apache.org>
 
 # Build-time metadata as defined at http://label-schema.org
diff --git a/README.md b/README.md
@@ -135,6 +135,7 @@ Developers and [contributors](https://github.com/common-workflow-language/cwlvie
 * **Mark Robinson** http://orcid.org/0000-0002-8184-7507 
 * Stian Soiland-Reyes http://orcid.org/0000-0001-9842-9718
 * Michael Crusoe http://orcid.org/0000-0002-2961-9670
+* Carole Goble http://orcid.org/0000-0003-1219-2137
 
 Thanks to:  
 
diff --git a/src/main/java/org/commonwl/view/git/GitSemaphore.java b/src/main/java/org/commonwl/view/git/GitSemaphore.java
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.commonwl.view.git;
+
+import org.springframework.stereotype.Component;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Object to manage concurrent access to Git repositories
+ */
+@Component
+public class GitSemaphore {
+
+    private static Map<String, Integer> currentRepos = new HashMap<>();
+
+    /**
+     * Note that a thread will be accessing the repository
+     * @param repoUrl The url of the repository
+     * @return Whether the resource can be accessed safely
+     * (no other threads are using it)
+     */
+    public synchronized boolean acquire(String repoUrl) {
+        if (currentRepos.containsKey(repoUrl)) {
+            currentRepos.put(repoUrl, currentRepos.get(repoUrl) + 1);
+            return false;
+        } else {
+            currentRepos.put(repoUrl, 1);
+            return true;
+        }
+    }
+
+    /**
+     * Release use of the shared resource
+     * @param repoUrl The url of the repository
+     */
+    public synchronized void release(String repoUrl) {
+        if (currentRepos.containsKey(repoUrl)) {
+            int threadCountUsing = currentRepos.get(repoUrl);
+            if (threadCountUsing > 1) {
+                currentRepos.put(repoUrl, threadCountUsing - 1);
+            } else {
+                currentRepos.remove(repoUrl);
+            }
+        }
+    }
+
+}
diff --git a/src/main/java/org/commonwl/view/git/GitService.java b/src/main/java/org/commonwl/view/git/GitService.java
@@ -39,6 +39,8 @@
 import java.util.HashSet;
 import java.util.Set;
 
+import static org.apache.jena.ext.com.google.common.io.Files.createTempDir;
+
 /**
  * Handles Git related functionality
  */
@@ -63,31 +65,43 @@ 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
      */
-    public Git getRepository(GitDetails gitDetails)
+    public Git getRepository(GitDetails gitDetails, boolean reuseDir)
             throws GitAPIException {
         Git repo = null;
         try {
-            // Base dir from configuration, name from hash of repository URL
-            File baseDir = new File(gitStorage.toString());
-            String baseName = DigestUtils.shaHex(GitDetails.normaliseUrl(gitDetails.getRepoUrl()));
+            if (reuseDir) {
+                // Base dir from configuration, name from hash of repository URL
+                File baseDir = new File(gitStorage.toString());
+                String baseName = DigestUtils.shaHex(GitDetails.normaliseUrl(gitDetails.getRepoUrl()));
 
-            // Check if folder already exists
-            File repoDir = new File(baseDir, baseName);
-            if (repoDir.exists() && repoDir.isDirectory()) {
+                // Check if folder already exists
+                File repoDir = new File(baseDir, baseName);
+                if (repoDir.exists() && repoDir.isDirectory()) {
                     repo = Git.open(repoDir);
                     repo.fetch().call();
-            } else {
-                // Create a folder and clone repository into it
-                if (repoDir.mkdir()) {
-                    repo = Git.cloneRepository()
-                            .setCloneSubmodules(cloneSubmodules)
-                            .setURI(gitDetails.getRepoUrl())
-                            .setDirectory(repoDir)
-                            .setCloneAllBranches(true)
-                            .call();
+                } else {
+                    // Create a folder and clone repository into it
+                    if (repoDir.mkdir()) {
+                        repo = Git.cloneRepository()
+                                .setCloneSubmodules(cloneSubmodules)
+                                .setURI(gitDetails.getRepoUrl())
+                                .setDirectory(repoDir)
+                                .setCloneAllBranches(true)
+                                .call();
+                    }
                 }
+            } 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();
             }
 
             // Checkout the specific branch or commit ID
diff --git a/src/main/java/org/commonwl/view/researchobject/ROBundleService.java b/src/main/java/org/commonwl/view/researchobject/ROBundleService.java
@@ -30,6 +30,7 @@
 import org.commonwl.view.cwl.CWLValidationException;
 import org.commonwl.view.cwl.RDFService;
 import org.commonwl.view.git.GitDetails;
+import org.commonwl.view.git.GitSemaphore;
 import org.commonwl.view.git.GitService;
 import org.commonwl.view.git.GitType;
 import org.commonwl.view.graphviz.GraphVizService;
@@ -71,6 +72,7 @@ public class ROBundleService {
     private GitService gitService;
     private RDFService rdfService;
     private CWLTool cwlTool;
+    private GitSemaphore gitSemaphore;
 
     // Configuration variables
     private Agent appAgent;
@@ -97,6 +99,7 @@ public ROBundleService(@Value("${bundleStorage}") Path bundleStorage,
                            GraphVizService graphVizService,
                            GitService gitService,
                            RDFService rdfService,
+                           GitSemaphore gitSemaphore,
                            CWLTool cwlTool) throws URISyntaxException {
         this.bundleStorage = bundleStorage;
         this.appAgent = new Agent(appName);
@@ -105,6 +108,7 @@ public ROBundleService(@Value("${bundleStorage}") Path bundleStorage,
         this.graphVizService = graphVizService;
         this.gitService = gitService;
         this.rdfService = rdfService;
+        this.gitSemaphore = gitSemaphore;
         this.cwlTool = cwlTool;
     }
 
@@ -137,10 +141,16 @@ public Bundle createBundle(Workflow workflow, GitDetails gitInfo) throws IOExcep
 
             // Add the files from the repo to this workflow
             Set<HashableAgent> authors = new HashSet<>();
-            Git gitRepo = gitService.getRepository(workflow.getRetrievedFrom());
-            Path relativePath = Paths.get(FilenameUtils.getPath(gitInfo.getPath()));
-            Path gitPath = gitRepo.getRepository().getWorkTree().toPath().resolve(relativePath);
-            addFilesToBundle(gitInfo, bundle, bundlePath, gitRepo, gitPath, authors, workflow);
+
+            boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
+            try {
+                Git gitRepo = gitService.getRepository(workflow.getRetrievedFrom(), safeToAccess);
+                Path relativePath = Paths.get(FilenameUtils.getPath(gitInfo.getPath()));
+                Path gitPath = gitRepo.getRepository().getWorkTree().toPath().resolve(relativePath);
+                addFilesToBundle(gitInfo, bundle, bundlePath, gitRepo, gitPath, authors, workflow);
+            } finally {
+                gitSemaphore.release(gitInfo.getRepoUrl());
+            }
 
             // Add combined authors
             manifest.setAuthoredBy(new ArrayList<>(authors));
@@ -181,7 +191,6 @@ public Bundle createBundle(Workflow workflow, GitDetails gitInfo) throws IOExcep
                 try {
                     addAggregation(bundle, manifestAnnotations,
                             "workflow.ttl", cwlTool.getRDF(rawUrl));
-
                 } catch (CWLValidationException ex) {
                     logger.error("Could not get RDF for workflow from raw URL", ex.getMessage());
                 }
diff --git a/src/main/java/org/commonwl/view/workflow/WorkflowService.java b/src/main/java/org/commonwl/view/workflow/WorkflowService.java
@@ -23,6 +23,7 @@
 import org.commonwl.view.cwl.CWLToolRunner;
 import org.commonwl.view.cwl.CWLToolStatus;
 import org.commonwl.view.git.GitDetails;
+import org.commonwl.view.git.GitSemaphore;
 import org.commonwl.view.git.GitService;
 import org.commonwl.view.graphviz.GraphVizService;
 import org.commonwl.view.researchobject.ROBundleFactory;
@@ -56,6 +57,7 @@ public class WorkflowService {
     private final ROBundleFactory ROBundleFactory;
     private final GraphVizService graphVizService;
     private final CWLToolRunner cwlToolRunner;
+    private final GitSemaphore gitSemaphore;
     private final int cacheDays;
 
     @Autowired
@@ -66,6 +68,7 @@ public WorkflowService(GitService gitService,
                            ROBundleFactory ROBundleFactory,
                            GraphVizService graphVizService,
                            CWLToolRunner cwlToolRunner,
+                           GitSemaphore gitSemaphore,
                            @Value("${cacheDays}") int cacheDays) {
         this.gitService = gitService;
         this.cwlService = cwlService;
@@ -75,6 +78,7 @@ public WorkflowService(GitService gitService,
         this.graphVizService = graphVizService;
         this.cwlToolRunner = cwlToolRunner;
         this.cacheDays = cacheDays;
+        this.gitSemaphore = gitSemaphore;
     }
 
     /**
@@ -217,50 +221,58 @@ public File getROBundle(GitDetails gitDetails) throws ROBundleNotFoundException
      */
     public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
             throws GitAPIException, WorkflowNotFoundException, IOException {
+        QueuedWorkflow queuedWorkflow;
 
-        // Clone repository to temporary folder
-        Git repo = null;
-        while (repo == null) {
-            try {
-                repo = gitService.getRepository(gitInfo);
-            } catch (RefNotFoundException ex) {
-                // Attempt slashes in branch fix
-                GitDetails correctedForSlash = transferPathToBranch(gitInfo);
-                if (correctedForSlash != null) {
-                    gitInfo = correctedForSlash;
-                } else {
-                    throw ex;
+        try {
+            // Clone repository to temporary folder
+            Git repo = null;
+            while (repo == null) {
+                try {
+                    boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
+                    repo = gitService.getRepository(gitInfo, safeToAccess);
+                } catch (RefNotFoundException ex) {
+                    // Attempt slashes in branch fix
+                    GitDetails correctedForSlash = transferPathToBranch(gitInfo);
+                    if (correctedForSlash != null) {
+                        gitSemaphore.release(gitInfo.getRepoUrl());
+                        gitInfo = correctedForSlash;
+                    } else {
+                        throw ex;
+                    }
                 }
             }
-        }
-        File localPath = repo.getRepository().getWorkTree();
-        String latestCommit = gitService.getCurrentCommitID(repo);
+            File localPath = repo.getRepository().getWorkTree();
+            String latestCommit = gitService.getCurrentCommitID(repo);
 
-        Path pathToWorkflowFile = localPath.toPath().resolve(gitInfo.getPath()).normalize().toAbsolutePath();
-        // Prevent path traversal attacks
-        if (!pathToWorkflowFile.startsWith(localPath.toPath().normalize().toAbsolutePath())) {
-            throw new WorkflowNotFoundException();
-        }
+            Path pathToWorkflowFile = localPath.toPath().resolve(gitInfo.getPath()).normalize().toAbsolutePath();
+            // Prevent path traversal attacks
+            if (!pathToWorkflowFile.startsWith(localPath.toPath().normalize().toAbsolutePath())) {
+                throw new WorkflowNotFoundException();
+            }
 
-        File workflowFile = new File(pathToWorkflowFile.toString());
-        Workflow basicModel = cwlService.parseWorkflowNative(workflowFile);
+            File workflowFile = new File(pathToWorkflowFile.toString());
+            Workflow basicModel = cwlService.parseWorkflowNative(workflowFile);
 
-        // Set origin details
-        basicModel.setRetrievedOn(new Date());
-        basicModel.setRetrievedFrom(gitInfo);
-        basicModel.setLastCommit(latestCommit);
+            // Set origin details
+            basicModel.setRetrievedOn(new Date());
+            basicModel.setRetrievedFrom(gitInfo);
+            basicModel.setLastCommit(latestCommit);
 
-        // Save the queued workflow to database
-        QueuedWorkflow queuedWorkflow = new QueuedWorkflow();
-        queuedWorkflow.setTempRepresentation(basicModel);
-        queuedWorkflowRepository.save(queuedWorkflow);
+            // Save the queued workflow to database
+            queuedWorkflow = new QueuedWorkflow();
+            queuedWorkflow.setTempRepresentation(basicModel);
+            queuedWorkflowRepository.save(queuedWorkflow);
 
-        // ASYNC OPERATIONS
-        // Parse with cwltool and update model
-        try {
-            cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, workflowFile);
-        } catch (Exception e) {
-            logger.error("Could not update workflow with cwltool", e);
+            // ASYNC OPERATIONS
+            // Parse with cwltool and update model
+            try {
+                cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, workflowFile);
+            } catch (Exception e) {
+                logger.error("Could not update workflow with cwltool", e);
+            }
+
+        } finally {
+            gitSemaphore.release(gitInfo.getRepoUrl());
         }
 
         // Return this model to be displayed
@@ -275,14 +287,17 @@ public void retryCwltool(QueuedWorkflow queuedWorkflow) {
         queuedWorkflow.setMessage(null);
         queuedWorkflow.setCwltoolStatus(CWLToolStatus.RUNNING);
         queuedWorkflowRepository.save(queuedWorkflow);
+        GitDetails gitDetails = queuedWorkflow.getTempRepresentation().getRetrievedFrom();
+        boolean safeToAccess = gitSemaphore.acquire(gitDetails.getRepoUrl());
         try {
-            GitDetails gitDetails = queuedWorkflow.getTempRepresentation().getRetrievedFrom();
-            Git repo = gitService.getRepository(gitDetails);
+            Git repo = gitService.getRepository(gitDetails, safeToAccess);
             File localPath = repo.getRepository().getWorkTree();
             Path pathToWorkflowFile = localPath.toPath().resolve(gitDetails.getPath()).normalize().toAbsolutePath();
             cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, new File(pathToWorkflowFile.toString()));
         } catch (Exception e) {
             logger.error("Could not update workflow with cwltool", e);
+        } finally {
+            gitSemaphore.release(gitDetails.getRepoUrl());
         }
     }
 
@@ -343,8 +358,14 @@ private boolean cacheExpired(Workflow workflow) {
                     // Cache expiry time has elapsed
                     // Check current head of the branch with the cached head
                     logger.info("Time has expired for caching, checking commits...");
-                    Git repo = gitService.getRepository(workflow.getRetrievedFrom());
-                    String currentHead = gitService.getCurrentCommitID(repo);
+                    String currentHead;
+                    boolean safeToAccess = gitSemaphore.acquire(workflow.getRetrievedFrom().getRepoUrl());
+                    try {
+                        Git repo = gitService.getRepository(workflow.getRetrievedFrom(), safeToAccess);
+                        currentHead = gitService.getCurrentCommitID(repo);
+                    } finally {
+                        gitSemaphore.release(workflow.getRetrievedFrom().getRepoUrl());
+                    }
                     logger.info("Current: " + workflow.getLastCommit() + ", HEAD: " + currentHead);
 
                     // Reset date in database if there are still no changes
diff --git a/src/main/resources/static/css/main-20170616.css b/src/main/resources/static/css/main-20170616.css
@@ -262,6 +262,10 @@ body {
 .legend .defaults { background-color: #D5AEFC; }
 .legend .selectednode { background-color: #B9FFBB; }
 
+.recommendation {
+    font-weight: bold;
+}
+
 tr.selected, .table-hover tbody tr.selected:hover td, .table-hover tbody tr.selected:hover th {
     background-color: #b9ffbb !important;
 }
diff --git a/src/test/java/org/commonwl/view/git/GitSemaphoreTest.java b/src/test/java/org/commonwl/view/git/GitSemaphoreTest.java
diff --git a/src/test/java/org/commonwl/view/researchobject/ROBundleServiceTest.java b/src/test/java/org/commonwl/view/researchobject/ROBundleServiceTest.java
diff --git a/src/test/java/org/commonwl/view/workflow/WorkflowServiceTest.java b/src/test/java/org/commonwl/view/workflow/WorkflowServiceTest.java

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-xFROM maven:3.5-jdk-8-alpine`
	`1`	`+FROM maven:3.5-jdk-8-alpine`
`2`	`2`	`MAINTAINER Stian Soiland-Reyes <[email protected]>`
`3`	`3`
`4`	`4`	`# Build-time metadata as defined at http://label-schema.org`