Remove Bundle and Git directories when they are not necessary (e.g. process succeeed and they are not used, or process failed).

Author: kinow

src/main/java/org/commonwl/view/Scheduler.java

@@ -19,13 +19,10 @@
 import java.nio.file.Paths;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 import java.util.stream.Stream;
 
 /**
@@ -43,8 +40,9 @@ public class Scheduler {
     @Value("${tmpDirAgeLimitDays}")
     private Integer TMP_DIR_AGE_LIMIT_DAYS;
 
-    @Value("${bundleStorage}")
-    private String bundleStorage;
+    // We do not want to remove the bundles, as we use the disk as a sort of
+    // cache. Whenever a workflow page is displayed in the browser the UI
+    // fires a request to re-generate it. We skip that by keeping files on disk.
     @Value("${graphvizStorage}")
     private String graphvizStorage;
     @Value("${gitStorage}")
@@ -102,7 +100,7 @@ public void removeOldQueuedWorkflowEntries() {
     public void clearTmpDir() {
         // Temporary files used for graphviz, RO, and git may be stored in different
         // locations, so we will collect all of them here.
-        List<String> temporaryDirectories = Stream.of(bundleStorage, graphvizStorage, gitStorage)
+        List<String> temporaryDirectories = Stream.of(graphvizStorage, gitStorage)
                 .distinct()
                 .toList();
         temporaryDirectories.forEach(this::clearDirectory);
@@ -130,7 +128,7 @@ private void clearDirectory(String temporaryDirectory) {
             // Really unexpected. walkFileTree should throw an IllegalArgumentException for negative maxDepth (clearly
             // not happening here), a SecurityException if the security manager denies access, or this IOException in
             // the cases where an I/O error happened (disk error, OS error, file not found, etc.). So just a warning.
-            logger.warn(String.format("Unexpected I/O error was thrown walking directory [%s]: %s", dir.toString(), e.getMessage()), e);
+            logger.warn(String.format("Unexpected I/O error was thrown walking directory [%s]: %s", dir, e.getMessage()), e);
         }
 
         // Delete the directories accumulated by the visitor.

src/main/java/org/commonwl/view/researchobject/ROBundleFactory.java

@@ -19,8 +19,10 @@
 
 package org.commonwl.view.researchobject;
 
+import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.taverna.robundle.Bundle;
+import org.apache.taverna.robundle.fs.BundleFileSystem;
 import org.commonwl.view.git.GitDetails;
 import org.commonwl.view.workflow.Workflow;
 import org.commonwl.view.workflow.WorkflowRepository;
@@ -31,7 +33,9 @@
 import org.springframework.scheduling.annotation.EnableAsync;
 import org.springframework.stereotype.Component;
 
+import java.io.File;
 import java.io.IOException;
+import java.nio.file.FileSystem;
 import java.nio.file.Path;
 
 /**
@@ -77,6 +81,26 @@ public void createWorkflowRO(Workflow workflow)
 
         // Save the bundle to the storage location in properties
         Path bundleLocation = roBundleService.saveToFile(bundle);
+        // The RoBundleService#saveToFile call above will delegate to Taverna's
+        // Bundles.closeAndSaveBundle, which empties the bundle temporary
+        // directory without deleting the directory itself. The following call is
+        // just for cleaning it up.
+        if (bundle != null) {
+            try {
+                BundleFileSystem fs = (BundleFileSystem) bundle.getFileSystem();
+                File tmpDir = new File(System.getProperty("java.io.tmpdir"));
+                // The file system source will be something like /tmp/bundles_dir/hash/bundle.zip,
+                // and we want /tmp/hash to be deleted. N.B. Taverna uses the temporary directory
+                // for the temporary bundles' directory, not the bundles file specified by Spring.
+                String bundleTmpDirName = fs.getSource().toAbsolutePath().getParent().getFileName().toString();
+                File bundleTmpDir = new File(tmpDir, bundleTmpDirName);
+                if (bundleTmpDir.exists() && bundleTmpDir.isDirectory()) {
+                    FileUtils.forceDelete(bundleTmpDir);
+                }
+            } catch (IOException e) {
+                logger.warn(String.format("Failed to delete temporary directory for bundle [%s]: %s", bundle.getSource(), e.getMessage()), e);
+            }
+        }
 
         // Add RO Bundle to associated workflow model
         workflow.setRoBundlePath(bundleLocation.toString());

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

@@ -163,6 +163,7 @@ public Bundle createBundle(Workflow workflow, GitDetails gitInfo) throws IOExcep
                 throw e;
             } finally {
                 gitSemaphore.release(gitInfo.getRepoUrl());
+                org.commonwl.view.util.FileUtils.deleteGitRepository(gitRepo);
             }
 
             // Add combined authors
@@ -221,6 +222,9 @@ public Bundle createBundle(Workflow workflow, GitDetails gitInfo) throws IOExcep
 
         } catch (URISyntaxException ex) {
             logger.error("Error creating URI for RO Bundle", ex);
+            if (bundlePath != null) {
+                FileUtils.forceDelete(bundlePath.toFile());
+            }
         } catch (GitAPIException ex) {
             logger.error("Error getting repository to create RO Bundle", ex);
             FileUtils.forceDelete(bundlePath.toFile());

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

@@ -2,6 +2,7 @@
 
 import org.eclipse.jgit.api.Git;
 
+import java.io.File;
 import java.io.IOException;
 
 /**
@@ -15,8 +16,16 @@ public class FileUtils {
     private FileUtils() {}
 
     public static void deleteGitRepository(Git repo) throws IOException {
-        if (repo != null && repo.getRepository() != null && repo.getRepository().getDirectory().exists()) {
-            org.apache.commons.io.FileUtils.forceDelete(repo.getRepository().getDirectory());
+        if (
+                repo != null &&
+                repo.getRepository() != null &&
+                repo.getRepository().getDirectory() != null &&
+                repo.getRepository().getDirectory().exists()
+        ) {
+            // This is literally the git directory, i.e. /some/hierarchy/repository/.git,
+            // but we want to delete its parent directory.
+            File gitDirectory = repo.getRepository().getDirectory();
+            org.apache.commons.io.FileUtils.forceDelete(gitDirectory.getParentFile());
         }
     }
 }