Refactor ROBundle into service with dependency injection and add test

Mark Robinson · Mark Robinson · commit f6119088b422 · 2017-03-18T19:47:09.000Z
diff --git a/src/main/java/org/commonwl/view/researchobject/ROBundleFactory.java b/src/main/java/org/commonwl/view/researchobject/ROBundleFactory.java
@@ -20,14 +20,13 @@
 package org.commonwl.view.researchobject;
 
 import org.apache.commons.io.FilenameUtils;
-import org.commonwl.view.github.GitHubService;
+import org.apache.taverna.robundle.Bundle;
 import org.commonwl.view.github.GithubDetails;
 import org.commonwl.view.workflow.Workflow;
 import org.commonwl.view.workflow.WorkflowRepository;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.scheduling.annotation.Async;
 import org.springframework.scheduling.annotation.EnableAsync;
 import org.springframework.stereotype.Component;
@@ -47,26 +46,14 @@ public class ROBundleFactory {
 
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
 
-    private final String applicationName;
-    private final String applicationURL;
-    private final int singleFileSizeLimit;
-    private final Path storageLocation;
     private final WorkflowRepository workflowRepository;
-    private final GitHubService githubService;
+    private final ROBundleService roBundleService;
 
     @Autowired
-    public ROBundleFactory(@Value("${applicationName}") String applicationName,
-                           @Value("${applicationURL}") String applicationURL,
-                           @Value("${graphvizStorage}") Path graphvizStorage,
-                           @Value("${singleFileSizeLimit}") int singleFileSizeLimit,
-                           GitHubService gitHubService,
+    public ROBundleFactory(ROBundleService roBundleService,
                            WorkflowRepository workflowRepository) {
-        this.applicationName = applicationName;
-        this.applicationURL = applicationURL;
-        this.storageLocation = graphvizStorage;
         this.workflowRepository = workflowRepository;
-        this.githubService = gitHubService;
-        this.singleFileSizeLimit = singleFileSizeLimit;
+        this.roBundleService = roBundleService;
     }
 
     /**
@@ -85,16 +72,15 @@ public void workflowROFromGithub(GithubDetails githubInfo)
                 githubInfo.getBranch(), FilenameUtils.getPath(githubInfo.getPath()));
 
         // Create a new Research Object Bundle with Github contents
-        ROBundle bundle = new ROBundle(githubService, roDetails,
-                applicationName, applicationURL, singleFileSizeLimit);
+        Bundle bundle = roBundleService.newBundleFromGithub(roDetails);
 
         // Save the bundle to the storage location in properties
-        Path bundleLocation = bundle.saveToFile(storageLocation);
+        Path bundleLocation = roBundleService.saveToFile(bundle);
 
         // Add the path to the bundle to the bundle
         Workflow workflow = workflowRepository.findByRetrievedFrom(githubInfo);
 
-        // Chance that this thread could be created before workflow model is saved
+        // Chance that this thread could be done before workflow model is saved
         int attempts = 5;
         while (attempts > 0 && workflow == null) {
             // Delay this thread by 0.5s and try again until success or too many attempts
diff --git a/src/main/java/org/commonwl/view/researchobject/ROBundleService.java b/src/main/java/org/commonwl/view/researchobject/ROBundleService.java
@@ -34,6 +34,9 @@
 import org.eclipse.egit.github.core.User;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.stereotype.Service;
 
 import java.io.IOException;
 import java.net.URI;
@@ -48,54 +51,66 @@
 import java.util.regex.Pattern;
 
 /**
- * Represents a Workflow Research Object Bundle
+ * Service handling Research Object Bundles
  */
-public class ROBundle {
+@Service
+public class ROBundleService {
 
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
 
+    // Services
     private GitHubService githubService;
 
-    private Bundle bundle;
-    private GithubDetails githubInfo;
-    private Agent thisApp;
+    // Configuration variables
+    private Agent appAgent;
     private int singleFileSizeLimit;
-    private Set<HashableAgent> authors = new HashSet<>();
+    private Path bundleStorage;
 
     // Pattern for extracting version from a cwl file
     private final String CWL_VERSION_REGEX = "cwlVersion:\\s*\"?(?:cwl:)?([^\\s\"]+)\"?";
     private final Pattern cwlVersionPattern = Pattern.compile(CWL_VERSION_REGEX);
 
     /**
-     * Creates a new research object bundle for a workflow from a Github repository
-     * @param githubService The service for handling Github functionality
-     * @param githubInfo The information necessary to access the Github directory associated with the RO
+     * Creates an instance of this service which handles Research Object Bundles
+     * @param bundleStorage The configured storage location for bundles
      * @param appName The name of the application from properties, for attribution
      * @param appURL The URL of the application from properties, for attribution
-     * @throws IOException Any API errors which may have occurred
+     * @param singleFileSizeLimit The file size limit for the RO bundle
+     * @param githubService The service for handling Github functionality
+     * @throws URISyntaxException Error in creating URI for appURL
      */
-    public ROBundle(GitHubService githubService, GithubDetails githubInfo,
-                    String appName, String appURL, int singleFileSizeLimit) throws IOException {
-        // File size limits
+    @Autowired
+    public ROBundleService(@Value("${bundleStorage}") Path bundleStorage,
+                           @Value("${applicationName}") String appName,
+                           @Value("${applicationURL}") String appURL,
+                           @Value("${singleFileSizeLimit}") int singleFileSizeLimit,
+                           GitHubService githubService) throws URISyntaxException {
+        this.bundleStorage = bundleStorage;
+        this.appAgent = new Agent(appName);
+        appAgent.setUri(new URI(appURL));
         this.singleFileSizeLimit = singleFileSizeLimit;
-
-        // Create a new RO bundle
-        this.bundle = Bundles.createBundle();
-        this.githubInfo = githubInfo;
         this.githubService = githubService;
+    }
+
+    /**
+     * Creates a new research object bundle for a workflow from a Github repository
+     * @param githubInfo The information to access the repository
+     * @return The constructed bundle
+     */
+    public Bundle newBundleFromGithub(GithubDetails githubInfo) throws IOException {
 
+        // Create a new RO bundle
+        Bundle bundle = Bundles.createBundle();
         Manifest manifest = bundle.getManifest();
 
         // Simplified attribution for RO bundle
         try {
             // Tool attribution in createdBy
-            thisApp = new Agent(appName);
-            thisApp.setUri(new URI(appURL));
-            manifest.setCreatedBy(thisApp);
+            manifest.setCreatedBy(appAgent);
 
             // Retrieval Info
             // TODO: Make this importedBy/On/From
-            manifest.setRetrievedBy(thisApp);
+            manifest.setRetrievedBy(appAgent);
             manifest.setRetrievedOn(manifest.getCreatedOn());
             manifest.setRetrievedFrom(new URI(githubInfo.getURL()));
 
@@ -109,18 +124,40 @@ public ROBundle(GitHubService githubService, GithubDetails githubInfo,
 
         // Add the files from the Github repo to this workflow
         List<RepositoryContents> repoContents = githubService.getContents(githubInfo);
-        addFiles(repoContents, bundleFiles);
+        Set<HashableAgent> authors = new HashSet<HashableAgent>();
+        addFilesToBundle(bundle, githubInfo, repoContents, bundleFiles, authors);
 
         // Add combined authors
         manifest.setAuthoredBy(new ArrayList<>(authors));
+
+        // Return the completed bundle
+        return bundle;
+
+    }
+
+    /**
+     * Save the Research Object bundle to disk
+     * @param roBundle The bundle to be saved
+     * @return The path to the research object
+     * @throws IOException Any errors in saving
+     */
+    public Path saveToFile(Bundle roBundle) throws IOException {
+        String fileName = "bundle-" + java.util.UUID.randomUUID() + ".zip";
+        Path bundleLocation = Files.createFile(bundleStorage.resolve(fileName));
+        Bundles.closeAndSaveBundle(roBundle, bundleLocation);
+        return bundleLocation;
     }
 
     /**
      * Add files to this bundle from a list of Github repository contents
+     * @param bundle The RO bundle to add files/directories to
+     * @param githubInfo The information used to access the repository
      * @param repoContents The contents of the Github repository
      * @param path The path in the Research Object to add the files
      */
-    private void addFiles(List<RepositoryContents> repoContents, Path path) throws IOException {
+    private void addFilesToBundle(Bundle bundle, GithubDetails githubInfo,
+                                  List<RepositoryContents> repoContents, Path path,
+                                  Set<HashableAgent> authors) throws IOException {
 
         // Loop through repo contents and add them
         for (RepositoryContents repoContent : repoContents) {
@@ -138,7 +175,7 @@ private void addFiles(List<RepositoryContents> repoContents, Path path) throws I
                 Files.createDirectory(subdirPath);
 
                 // Add the files in the subdirectory to this new folder
-                addFiles(subdirectory, subdirPath);
+                addFilesToBundle(bundle, githubInfo, subdirectory, subdirPath, authors);
 
                 // Otherwise this is a file so add to the bundle
             } else if (repoContent.getType().equals(GitHubService.TYPE_FILE)) {
@@ -214,7 +251,7 @@ private void addFiles(List<RepositoryContents> repoContents, Path path) throws I
 
                     // Set retrieved information for this file in the manifest
                     aggregation.setRetrievedFrom(rawURI);
-                    aggregation.setRetrievedBy(thisApp);
+                    aggregation.setRetrievedBy(appAgent);
                     aggregation.setRetrievedOn(aggregation.getCreatedOn());
 
                 } catch (URISyntaxException ex) {
@@ -224,24 +261,4 @@ private void addFiles(List<RepositoryContents> repoContents, Path path) throws I
         }
     }
 
-    /**
-     * Save the Research Object bundle to disk
-     * @param directory The directory in which the RO will be saved
-     * @return The path to the research object
-     * @throws IOException Any errors in saving
-     */
-    public Path saveToFile(Path directory) throws IOException {
-        String fileName = "bundle-" + java.util.UUID.randomUUID() + ".zip";
-        Path bundleLocation = Files.createFile(directory.resolve(fileName));
-        Bundles.closeAndSaveBundle(bundle, bundleLocation);
-        return bundleLocation;
-    }
-
-    /**
-     * Getter method for the bundle
-     * @return The RO bundle
-     */
-    public Bundle getBundle() {
-        return bundle;
-    }
 }
diff --git a/src/test/java/org/commonwl/view/researchobject/ROBundleFactoryTest.java b/src/test/java/org/commonwl/view/researchobject/ROBundleFactoryTest.java
@@ -0,0 +1,52 @@
+package org.commonwl.view.researchobject;
+
+import org.commonwl.view.github.GithubDetails;
+import org.commonwl.view.workflow.Workflow;
+import org.commonwl.view.workflow.WorkflowRepository;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.nio.file.Paths;
+import java.util.HashMap;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.anyObject;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test the separate async method to call the ROBundle constructor
+ */
+public class ROBundleFactoryTest {
+
+    /**
+     * Simulate creation of a valid workflow
+     */
+    @Test
+    public void bundleForValidWorkflow() throws Exception {
+
+        Workflow validWorkflow = new Workflow("Valid Workflow", "Doc for Valid Workflow",
+                new HashMap<>(), new HashMap<>(), new HashMap<>(), null);
+
+        // Mocked path to a RO bundle
+        ROBundleService mockROBundleService = Mockito.mock(ROBundleService.class);
+        when(mockROBundleService.saveToFile(anyObject()))
+                .thenReturn(Paths.get("test/path/to/check/for.zip"));
+
+        // Test method retries multiple times to get workflow model before success
+        WorkflowRepository mockRepository = Mockito.mock(WorkflowRepository.class);
+        when(mockRepository.findByRetrievedFrom(anyObject()))
+                .thenReturn(null)
+                .thenReturn(null)
+                .thenReturn(validWorkflow);
+
+        // Create factory under test
+        ROBundleFactory factory = new ROBundleFactory(mockROBundleService, mockRepository);
+
+        // Attempt to add RO to workflow
+        factory.workflowROFromGithub(Mockito.mock(GithubDetails.class));
+
+        assertEquals("test/path/to/check/for.zip", validWorkflow.getRoBundle());
+
+    }
+
+}
diff --git a/src/test/java/org/commonwl/view/researchobject/ROBundleServiceTest.java b/src/test/java/org/commonwl/view/researchobject/ROBundleServiceTest.java
@@ -24,7 +24,7 @@
 import static org.mockito.Matchers.anyObject;
 import static org.mockito.Mockito.when;
 
-public class ROBundleTest {
+public class ROBundleServiceTest {
 
     /**
      * Use a temporary directory for testing
@@ -44,15 +44,16 @@ public void generateAndSaveROBundle() throws Exception {
         // Create new RO bundle
         GithubDetails lobSTRv1Details = new GithubDetails("common-workflow-language", "workflows",
                 "933bf2a1a1cce32d88f88f136275535da9df0954", "workflows/lobSTR");
-        ROBundle bundle = new ROBundle(mockGithubService, lobSTRv1Details, "CWL Viewer",
-                "https://view.commonwl.org", 5242880);
-        Path bundleRoot = bundle.getBundle().getRoot().resolve("workflow");
+        ROBundleService bundleService = new ROBundleService(roBundleFolder.getRoot().toPath(),
+                "CWL Viewer", "https://view.commonwl.org", 5242880, mockGithubService);
+        Bundle bundle = bundleService.newBundleFromGithub(lobSTRv1Details);
+        Path bundleRoot = bundle.getRoot().resolve("workflow");
 
         // Check bundle exists
-        assertNotNull(bundle.getBundle());
+        assertNotNull(bundle);
 
         // Check basic manifest metadata
-        Manifest manifest = bundle.getBundle().getManifest();
+        Manifest manifest = bundle.getManifest();
         assertEquals("CWL Viewer", manifest.getCreatedBy().getName());
         assertEquals("https://view.commonwl.org", manifest.getCreatedBy().getUri().toString());
         assertEquals("Mark Robinson", manifest.getAuthoredBy().get(0).getName());
@@ -69,7 +70,7 @@ public void generateAndSaveROBundle() throws Exception {
         assertEquals("https://w3id.org/cwl/v1.0", cwlAggregate.getConformsTo().toString());
 
         // Save and check it exists in the temporary folder
-        bundle.saveToFile(roBundleFolder.getRoot().toPath());
+        bundleService.saveToFile(bundle);
         File[] fileList =  roBundleFolder.getRoot().listFiles();
         assertTrue(fileList.length == 1);
         for (File ro : fileList) {
@@ -92,13 +93,14 @@ public void filesOverLimit() throws Exception {
         // Create new RO bundle where all files are external
         GithubDetails lobSTRv1Details = new GithubDetails("common-workflow-language", "workflows",
                 "933bf2a1a1cce32d88f88f136275535da9df0954", "workflows/lobSTR");
-        ROBundle bundle = new ROBundle(mockGithubService, lobSTRv1Details, "CWL Viewer",
-                "https://view.commonwl.org", 0);
+        ROBundleService bundleService = new ROBundleService(roBundleFolder.getRoot().toPath(),
+                "CWL Viewer", "https://view.commonwl.org", 0, mockGithubService);
+        Bundle bundle = bundleService.newBundleFromGithub(lobSTRv1Details);
 
-        Manifest manifest = bundle.getBundle().getManifest();
+        Manifest manifest = bundle.getManifest();
 
         // Check files are externally linked in the aggregate
-        Path bundleRoot = bundle.getBundle().getRoot().resolve("workflow");
+        Path bundleRoot = bundle.getRoot().resolve("workflow");
         PathMetadata urlAggregate = manifest.getAggregation(
                 bundleRoot.resolve("lobSTR-demo.url"));
         assertEquals("Mark Robinson", urlAggregate.getAuthoredBy().get(0).getName());
