Remove temp repositories (#494)

This commit prevents `/tmp` folder from growing indefinitely large by
implementing two fixes to the git repo conflict management logic:
 - When a conflict is detected, the git repository is cloned on a
   temporary folder directly on the `gitStorage` fs (not in the system
   `/tmp` directory). This prevents using Docker's ephemeral fs under
   the hood.
 - A temporary repository is deleted as soon as the invoking method
   releases the git semaphore. This makes sense because the primary copy
   of each repository will still be preserved in cache.

Author: GlassOfWhiskey

src/main/java/org/commonwl/view/cwl/CWLToolRunner.java

@@ -171,6 +171,7 @@ public void createWorkflowFromQueued(QueuedWorkflow queuedWorkflow)
       FileUtils.deleteGitRepository(repo);
     } finally {
       gitSemaphore.release(repoUrl);
+      FileUtils.deleteTemporaryGitRepository(repo);
       queuedWorkflowRepository.save(queuedWorkflow);
     }
   }

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

@@ -19,8 +19,6 @@
 
 package org.commonwl.view.git;
 
-import static org.apache.jena.ext.com.google.common.io.Files.createTempDir;
-
 import java.io.File;
 import java.io.IOException;
 import java.net.URI;
@@ -29,6 +27,7 @@
 import java.nio.file.Path;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.UUID;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.commonwl.view.researchobject.HashableAgent;
 import org.eclipse.jgit.api.Git;
@@ -52,10 +51,10 @@ public class GitService {
   private final Logger logger = LoggerFactory.getLogger(this.getClass());
 
   // Location to check out git repositories into
-  private Path gitStorage;
+  private final Path gitStorage;
 
   // Whether submodules are also cloned
-  private boolean cloneSubmodules;
+  private final boolean cloneSubmodules;
 
   @Autowired
   public GitService(
@@ -218,4 +217,10 @@ protected Git cloneRepo(String repoUrl, File directory) throws GitAPIException {
         .setCloneAllBranches(true)
         .call();
   }
+
+  protected File createTempDir() throws IOException {
+    Path repoDir = gitStorage.resolve(String.valueOf(UUID.randomUUID()));
+    Files.createDirectory(repoDir);
+    return repoDir.toFile();
+  }
 }

src/main/java/org/commonwl/view/util/FileUtils.java

@@ -3,6 +3,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
+import java.util.regex.Pattern;
+
 import org.apache.taverna.robundle.Bundle;
 import org.apache.taverna.robundle.fs.BundleFileSystem;
 import org.eclipse.jgit.api.Git;
@@ -17,6 +19,9 @@
  */
 public class FileUtils {
 
+  private static final Pattern UUID_REGEX_PATTERN =
+      Pattern.compile("^[{]?[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}[}]?$");
+
   private FileUtils() {}
 
   /**
@@ -47,6 +52,36 @@ public static void deleteGitRepository(Git repo) throws IOException {
     }
   }
 
+  /**
+   * Deletes the directory of a temporary git repository. Note that the <code>Git</code> object
+   * contains a repository with a directory, but this directory points to the
+   *
+   * <pre>.git</pre>
+   *
+   * directory. This method will delete the parent of the
+   *
+   * <pre>.git</pre>
+   *
+   * directory, which corresponds to the cloned folder with the source code from git. Since
+   * temporary folders are generated using <code>UUID.randomUUID()</code> instead of the commit hex
+   * digest, if the folder name is a valid UUID it is identified as temporary.
+   *
+   * @param repo Git repository object
+   * @throws IOException if it fails to delete the Git repository directory
+   * @since 1.4.6
+   */
+  public static void deleteTemporaryGitRepository(Git repo) throws IOException {
+    if (repo != null
+        && repo.getRepository() != null
+        && repo.getRepository().getDirectory() != null
+        && repo.getRepository().getDirectory().getParentFile() != null) {
+      if (UUID_REGEX_PATTERN.matcher(
+          repo.getRepository().getDirectory().getParentFile().getName()).matches()) {
+        deleteGitRepository(repo);
+      }
+    }
+  }
+
   /**
    * Deletes the bundle temporary directory.
    *

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

@@ -216,9 +216,9 @@ public Workflow getWorkflow(GitDetails gitInfo) {
   public List<WorkflowOverview> getWorkflowsFromDirectory(GitDetails gitInfo)
       throws IOException, GitAPIException {
     List<WorkflowOverview> workflowsInDir = new ArrayList<>();
+    Git repo = null;
     try {
       boolean safeToAccess = gitSemaphore.acquire(gitInfo.getRepoUrl());
-      Git repo = null;
       while (repo == null) {
         try {
           repo = gitService.getRepository(gitInfo, safeToAccess);
@@ -260,6 +260,7 @@ public List<WorkflowOverview> getWorkflowsFromDirectory(GitDetails gitInfo)
       }
     } finally {
       gitSemaphore.release(gitInfo.getRepoUrl());
+      FileUtils.deleteTemporaryGitRepository(repo);
     }
     return workflowsInDir;
   }
@@ -389,6 +390,7 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
       throw e;
     } finally {
       gitSemaphore.release(gitInfo.getRepoUrl());
+      FileUtils.deleteTemporaryGitRepository(repo);
     }
 
     // Return this model to be displayed
@@ -563,12 +565,14 @@ private boolean cacheExpired(Workflow workflow) {
           logger.info(
               "Time has expired for caching, checking commits for workflow " + workflow.getID());
           String currentHead;
+          Git repo = null;
           boolean safeToAccess = gitSemaphore.acquire(workflow.getRetrievedFrom().getRepoUrl());
           try {
-            Git repo = gitService.getRepository(workflow.getRetrievedFrom(), safeToAccess);
+            repo = gitService.getRepository(workflow.getRetrievedFrom(), safeToAccess);
             currentHead = gitService.getCurrentCommitID(repo);
           } finally {
             gitSemaphore.release(workflow.getRetrievedFrom().getRepoUrl());
+            FileUtils.deleteTemporaryGitRepository(repo);
           }
           logger.info(
               "Current: "

src/test/java/org/commonwl/view/git/GitServiceTest.java

@@ -29,6 +29,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
 import org.eclipse.jgit.api.CheckoutCommand;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
@@ -53,7 +54,7 @@ public class GitServiceTest {
 
   @BeforeEach
   public void setup() throws GitAPIException {
-    GitService gitService = new GitService(null, false);
+    GitService gitService = new GitService(Path.of(System.getProperty("java.io.tmpdir")), false);
     this.spyGitService = spy(gitService);
     this.mockGit = mock(Git.class);
     this.mockGoodCheckoutCommand = mock(CheckoutCommand.class);

src/test/java/org/commonwl/view/util/FileUtilsTest.java

@@ -8,9 +8,13 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.UUID;
+import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.taverna.robundle.Bundle;
 import org.apache.taverna.robundle.fs.BundleFileSystem;
+import org.commonwl.view.git.GitDetails;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.lib.Repository;
 import org.junit.Rule;
@@ -250,4 +254,33 @@ public void testBundleParentDirectory() throws IOException {
     FileUtils.deleteBundleParentDirectory(bundle);
     assertFalse(bundleTemporaryDirectory.exists());
   }
+
+  @Test
+  public void testRemoveTemporaryRepository() throws IOException {
+    Git tempRepository = mock(Git.class);
+    Repository tempRepository1 = mock(Repository.class);
+    when(tempRepository.getRepository()).thenReturn(tempRepository1);
+    File tempGitRepositoryParent = temporaryFolder.newFolder(String.valueOf(UUID.randomUUID()));
+    File tempGitRepository = tempGitRepositoryParent.toPath().resolve(".git").toFile();
+    Files.createDirectory(tempGitRepository.toPath());
+    when(tempRepository1.getDirectory()).thenReturn(tempGitRepository);
+    assertTrue(tempGitRepository.exists());
+    FileUtils.deleteTemporaryGitRepository(tempRepository);
+    assertFalse(tempGitRepository.exists());
+
+    Git notTempRepository = mock(Git.class);
+    Repository notTempRepository1 = mock(Repository.class);
+    when(notTempRepository.getRepository()).thenReturn(notTempRepository1);
+    File notTempGitRepositoryParent =
+        temporaryFolder.newFolder(
+            DigestUtils.sha1Hex(
+                GitDetails.normaliseUrl(
+                    "https://github.com/common-workflow-language/cwlviewer.git")));
+    File notTempGitRepository = notTempGitRepositoryParent.toPath().resolve(".git").toFile();
+    Files.createDirectory(notTempGitRepository.toPath());
+    when(notTempRepository1.getDirectory()).thenReturn(notTempGitRepository);
+    assertTrue(notTempGitRepository.exists());
+    FileUtils.deleteTemporaryGitRepository(notTempRepository);
+    assertTrue(notTempGitRepository.exists());
+  }
 }