Refactor tests for generic git changes

Author: MarkRobbo

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

@@ -36,7 +36,6 @@
 import org.apache.jena.riot.RiotException;
 import org.commonwl.view.docker.DockerService;
 import org.commonwl.view.git.GitDetails;
-import org.commonwl.view.git.GitService;
 import org.commonwl.view.graphviz.ModelDotWriter;
 import org.commonwl.view.graphviz.RDFDotWriter;
 import org.commonwl.view.workflow.Workflow;
@@ -67,7 +66,6 @@ public class CWLService {
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
 
     // Autowired properties/services
-    private final GitService gitService;
     private final RDFService rdfService;
     private final CWLTool cwlTool;
     private final int singleFileSizeLimit;
@@ -98,16 +96,14 @@ public class CWLService {
 
     /**
      * Constructor for the Common Workflow Language service
-     * @param gitService A service for accessing Github functionality
+     * @param rdfService A service for handling RDF queries
      * @param cwlTool Handles cwltool integration
      * @param singleFileSizeLimit The file size limit for single files
      */
     @Autowired
-    public CWLService(GitService gitService,
-                      RDFService rdfService,
+    public CWLService(RDFService rdfService,
                       CWLTool cwlTool,
                       @Value("${singleFileSizeLimit}") int singleFileSizeLimit) {
-        this.gitService = gitService;
         this.rdfService = rdfService;
         this.cwlTool = cwlTool;
         this.singleFileSizeLimit = singleFileSizeLimit;

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

@@ -101,5 +101,15 @@ public Git getRepository(GitDetails gitDetails)
         return repo;
     }
 
+    /**
+     * Gets the commit ID of the HEAD for the given repository
+     * @param repo The Git repository
+     * @return The commit ID of the HEAD for the repository
+     * @throws IOException If the HEAD is detached
+     */
+    public String getCurrentCommitID(Git repo) throws IOException {
+        return repo.getRepository().findRef("HEAD").getObjectId().getName();
+    }
+
 
 }

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

@@ -20,10 +20,8 @@
 package org.commonwl.view.workflow;
 
 import org.commonwl.view.git.GitDetails;
-import org.commonwl.view.git.GitService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 import org.springframework.validation.Errors;
 import org.springframework.validation.ValidationUtils;
@@ -51,16 +49,6 @@ public class WorkflowFormValidator {
     private static final String GIT_REPO_REGEX = "^((git|ssh|http(s)?)|(git@[\\w\\.]+))(:(//)?)([\\w\\.@\\:/\\-~]+)(\\.git)(/)?$";
     private static final Pattern gitRepoPattern = Pattern.compile(GIT_REPO_REGEX);
 
-    /**
-     * Github API service
-     */
-    private final GitService githubService;
-
-    @Autowired
-    public WorkflowFormValidator(GitService githubService) {
-        this.githubService = githubService;
-    }
-
     /**
      * Validates a WorkflowForm to ensure the URL is not empty and links to a cwl file
      * @param form The given WorkflowForm
@@ -72,9 +60,6 @@ public GitDetails validateAndParse(WorkflowForm form, Errors e) {
         // If not null and isn't just whitespace
         if (!e.hasErrors()) {
 
-            // Object to be returned if valid Git details are found
-            GitDetails gitDetails = null;
-
             // Github URL
             Matcher m = githubCwlPattern.matcher(form.getUrl());
             if (m.find()) {
@@ -92,9 +77,11 @@ public GitDetails validateAndParse(WorkflowForm form, Errors e) {
             // General Git details if didn't match the above
             ValidationUtils.rejectIfEmptyOrWhitespace(e, "branch", "branch.emptyOrWhitespace");
             ValidationUtils.rejectIfEmptyOrWhitespace(e, "path", "path.emptyOrWhitespace");
-            m = gitRepoPattern.matcher(form.getUrl());
-            if (m.find()) {
-                return new GitDetails(form.getUrl(), form.getBranch(), form.getPath());
+            if (!e.hasErrors()) {
+                m = gitRepoPattern.matcher(form.getUrl());
+                if (m.find()) {
+                    return new GitDetails(form.getUrl(), form.getBranch(), form.getPath());
+                }
             }
 
         }

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

@@ -22,7 +22,6 @@
 import org.commonwl.view.cwl.CWLService;
 import org.commonwl.view.cwl.CWLToolRunner;
 import org.commonwl.view.cwl.CWLToolStatus;
-import org.commonwl.view.cwl.CWLValidationException;
 import org.commonwl.view.git.GitDetails;
 import org.commonwl.view.git.GitService;
 import org.commonwl.view.graphviz.GraphVizService;
@@ -217,7 +216,7 @@ public File getROBundle(GitDetails gitDetails) throws ROBundleNotFoundException
      * @throws IOException Other file handling exceptions
      */
     public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
-            throws GitAPIException, CWLValidationException, IOException {
+            throws GitAPIException, WorkflowNotFoundException, IOException {
 
         // Clone repository to temporary folder
         Git repo = null;
@@ -235,7 +234,7 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
             }
         }
         File localPath = repo.getRepository().getWorkTree();
-        String latestCommit = repo.getRepository().findRef("HEAD").getObjectId().getName();
+        String latestCommit = gitService.getCurrentCommitID(repo);
 
         Path pathToWorkflowFile = localPath.toPath().resolve(gitInfo.getPath()).normalize().toAbsolutePath();
         // Prevent path traversal attacks
@@ -345,7 +344,7 @@ private boolean cacheExpired(Workflow workflow) {
                     // 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 = repo.getRepository().findRef("HEAD").getObjectId().getName();
+                    String currentHead = gitService.getCurrentCommitID(repo);
                     logger.info("Current: " + workflow.getLastCommit() + ", HEAD: " + currentHead);
 
                     // Reset date in database if there are still no changes

src/main/resources/templates/about.html

@@ -36,7 +36,7 @@
         <div class="col-md-12" role="main" id="main">
             <h1>About</h1>
             <div style="position:relative;float:right;height:0;padding-bottom:30%;width:50%;margin-left:20px;">
-                <iframe src="https://www.youtube.com/embed/_yjhVTmvxLU?ecver=2" style="position:absolute;width:100%;height:100%;left:0" allowfullscreen="true"></iframe>
+                <iframe src="https://www.youtube-nocookie.com/embed/_yjhVTmvxLU?rel=0?ecver=2" style="position:absolute;width:100%;height:100%;left:0" allowfullscreen="true"></iframe>
             </div>
 
             <p>CWL Viewer is a richly featured web visualisation suite for workflows written in the

src/test/java/org/commonwl/view/PageControllerTest.java

@@ -67,7 +67,7 @@ public void homePageWithURL() throws Exception {
                 .andExpect(view().name("index"))
                 .andExpect(model().attributeExists("workflowForm"))
                 .andExpect(model().attribute("workflowForm",
-                        hasProperty("githubURL", is("https://github.com/test/default/link"))));
+                        hasProperty("url", is("https://github.com/test/default/link"))));
     }
 
     /**

src/test/java/org/commonwl/view/cwl/CWLServiceTest.java

@@ -22,10 +22,8 @@
 import org.apache.jena.query.*;
 import org.apache.jena.rdf.model.Model;
 import org.apache.jena.rdf.model.ModelFactory;
-import org.commonwl.view.github.GitDetails;
-import org.commonwl.view.github.GitService;
+import org.commonwl.view.git.GitDetails;
 import org.commonwl.view.workflow.Workflow;
-import org.commonwl.view.workflow.WorkflowOverview;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -63,7 +61,7 @@ public void setUp() throws Exception {
         Model workflowModel = ModelFactory.createDefaultModel();
         workflowModel.read(new ByteArrayInputStream(readFileToString(packedWorkflowRdf).getBytes()), null, "TURTLE");
         Dataset workflowDataset = DatasetFactory.create();
-        workflowDataset.addNamedModel("https://cdn.rawgit.com/common-workflow-language/workflows/master/workflows/make-to-cwl/dna.cwl#main", workflowModel);
+        workflowDataset.addNamedModel("http://localhost:3030/cwlviewer/github.com/common-workflow-language/workflows/blob/549c973ccc01781595ce562dea4cedc6c9540fe0/workflows/make-to-cwl/dna.cwl#main", workflowModel);
 
         Answer queryRdf = new Answer<ResultSet>() {
             @Override
@@ -76,7 +74,7 @@ public ResultSet answer(InvocationOnMock invocation) throws Throwable {
             }
         };
 
-        this.rdfService = Mockito.spy(new RDFService("http://madeup.endpoint/"));
+        this.rdfService = Mockito.spy(new RDFService("http://localhost:3030/cwlviewer/"));
         Mockito.doAnswer(queryRdf).when(rdfService).runQuery(anyObject());
         Mockito.doReturn(true).when(rdfService).graphExists(anyString());
     }
@@ -92,45 +90,21 @@ public ResultSet answer(InvocationOnMock invocation) throws Throwable {
      */
     @Test
     public void parseLobSTRDraft3WorkflowNative() throws Exception {
-
-        // Get mock Github service
-        GitService mockGithubService = getMockGithubService("workflows/lobSTR/",
-                "src/test/resources/cwl/lobstr-draft3/");
-
-        // Test cwl service
-        CWLService cwlService = new CWLService(mockGithubService,
-                rdfService, new CWLTool(), 5242880);
-
-        // Get workflow from community repo by commit ID so it will not change
-        GitDetails lobSTRDraft3Details = new GitDetails("common-workflow-language",
-                "workflows", null, "workflows/lobSTR/lobSTR-workflow.cwl");
-        Workflow lobSTRDraft3 = cwlService.parseWorkflowNative(lobSTRDraft3Details, "920c6be45f08e979e715a0018f22c532b024074f");
-
+        CWLService cwlService = new CWLService(rdfService, Mockito.mock(CWLTool.class), 5242880);
+        Workflow lobSTRDraft3 = cwlService.parseWorkflowNative(
+                new File("src/test/resources/cwl/lobstr-draft3/lobSTR-workflow.cwl"));
         testLobSTRWorkflow(lobSTRDraft3, true);
-
     }
 
     /**
      * Test native loading parsing of a the LobSTR workflow CWL version 1.0
      */
     @Test
     public void parseLobSTRv1WorkflowNative() throws Exception {
-
-        // Get mock Github service
-        GitService mockGithubService = getMockGithubService("workflows/lobSTR/",
-                "src/test/resources/cwl/lobstr-draft3/");
-
-        // Test cwl service
-        CWLService cwlService = new CWLService(mockGithubService,
-                rdfService, new CWLTool(), 5242880);
-
-        // Get workflow from community repo by commit ID so it will not change
-        GitDetails lobSTRv1Details = new GitDetails("common-workflow-language",
-                "workflows", null, "workflows/lobSTR/lobSTR-workflow.cwl");
-        Workflow lobSTRv1 = cwlService.parseWorkflowNative(lobSTRv1Details, "933bf2a1a1cce32d88f88f136275535da9df0954");
-
+        CWLService cwlService = new CWLService(rdfService, new CWLTool(), 5242880);
+        Workflow lobSTRv1 = cwlService.parseWorkflowNative(
+                new File("src/test/resources/cwl/lobstr-v1/lobSTR-workflow.cwl"));
         testLobSTRWorkflow(lobSTRv1, true);
-
     }
 
     /**
@@ -146,15 +120,18 @@ public void parseWorkflowWithCwltool() throws Exception {
                 .thenReturn(readFileToString(packedWorkflowRdf));
 
         // CWLService to test
-        CWLService cwlService = new CWLService(Mockito.mock(GitService.class),
-                rdfService, mockCwlTool, 5242880);
+        CWLService cwlService = new CWLService(rdfService, mockCwlTool, 5242880);
 
-        GitDetails githubInfo = new GitDetails("common-workflow-language",
-                "workflows", "549c973ccc01781595ce562dea4cedc6c9540fe0",
-                "workflows/make-to-cwl/dna.cwl");
+        GitDetails gitInfo = new GitDetails("https://github.com/common-workflow-language/workflows.git",
+                "549c973ccc01781595ce562dea4cedc6c9540fe0", "workflows/make-to-cwl/dna.cwl");
+        Workflow basicModel = new Workflow(null, null, null, null, null, null);
+        basicModel.setRetrievedFrom(gitInfo);
+        basicModel.setPackedWorkflowID("main");
+        basicModel.setLastCommit("549c973ccc01781595ce562dea4cedc6c9540fe0");
 
         // Parse the workflow
-        Workflow workflow = cwlService.parseWorkflowWithCwltool(githubInfo, "master", "main");
+        Workflow workflow = cwlService.parseWorkflowWithCwltool(basicModel,
+                new File("src/test/resources/cwl/make_to_cwl/dna.cwl"));
 
         // Check basic information
         assertNotNull(workflow);
@@ -167,58 +144,18 @@ public void parseWorkflowWithCwltool() throws Exception {
     }
 
     /**
-     * Test retrieval of a workflow overview for hello world example in cwl
+     * Test IOException is thrown when files are over limit
      */
     @Test
-    public void getHelloWorkflowOverview() throws Exception {
-
-        // Mock githubService class
-        GitService mockGithubService = Mockito.mock(GitService.class);
-        File workflowFile = new File("src/test/resources/cwl/hello/hello.cwl");
-        when(mockGithubService.downloadFile(anyObject()))
-                .thenReturn(readFileToString(workflowFile));
-
-        // Test cwl service
-        CWLService cwlService = new CWLService(mockGithubService,
-                rdfService, Mockito.mock(CWLTool.class), 5242880);
-
-        // Run workflow overview
-        GitDetails helloDetails = new GitDetails("common-workflow-language",
-                "workflows", "8296e92d358bb5da4dc3c6e7aabefa89726e3409", "workflows/hello/hello.cwl");
-        WorkflowOverview hello = cwlService.getWorkflowOverview(helloDetails);
-        assertNotNull(hello);
-
-        // No docs for this workflow
-        assertEquals("Hello World", hello.getLabel());
-        assertEquals("Puts a message into a file using echo", hello.getDoc());
-        assertEquals("hello.cwl", hello.getFileName());
-
-    }
-
-    /**
-     * Test IOException is thrown when files are over limit with getWorkflowOverview
-     */
-    @Test
-    public void workflowOverviewOverSingleFileSizeLimitThrowsIOException() throws Exception {
-
-        // Mock githubService class
-        GitService mockGithubService = Mockito.mock(GitService.class);
-        File workflowFile = new File("src/test/resources/cwl/hello/hello.cwl");
-        when(mockGithubService.downloadFile(anyObject()))
-                .thenReturn(readFileToString(workflowFile));
-
-        // Test cwl service with 0 filesize limit
-        CWLService cwlService = new CWLService(mockGithubService,
-                Mockito.mock(RDFService.class), Mockito.mock(CWLTool.class), 0);
-
-        // Run workflow overview
-        GitDetails helloDetails = new GitDetails("common-workflow-language",
-                "workflows", "8296e92d358bb5da4dc3c6e7aabefa89726e3409", "workflows/hello/hello.cwl");
+    public void workflowOverSingleFileSizeLimitThrowsIOException() throws Exception {
 
         // Should throw IOException due to oversized files
         thrown.expect(IOException.class);
-        thrown.expectMessage("File 'workflows/hello/hello.cwl' is over singleFileSizeLimit - 672 bytes/0 bytes");
-        cwlService.getWorkflowOverview(helloDetails);
+        thrown.expectMessage("File 'lobSTR-workflow.cwl' is over singleFileSizeLimit - 2 KB/0 bytes");
+
+        CWLService cwlService = new CWLService(rdfService, Mockito.mock(CWLTool.class), 0);
+        cwlService.parseWorkflowNative(
+                new File("src/test/resources/cwl/lobstr-draft3/lobSTR-workflow.cwl"));
 
     }
 
@@ -265,26 +202,4 @@ private void testLobSTRWorkflow(Workflow lobSTR, boolean nativeParsed) throws Ex
         }
     }
 
-    /**
-     * Get a mock GithubService which redirects downloads to the filesystem
-     */
-    private GitService getMockGithubService(String originalFolder,
-                                            String resourcesFolder) throws IOException {
-        GitService mockGithubService = Mockito.mock(GitService.class);
-        Answer fileAnswer = new Answer<String>() {
-            @Override
-            public String answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                GitDetails details = (GitDetails) args[0];
-                File workflowFile = new File(resourcesFolder
-                        + details.getPath().replace(originalFolder, ""));
-                return readFileToString(workflowFile);
-            }
-        };
-        when(mockGithubService.downloadFile(anyObject())).thenAnswer(fileAnswer);
-        when(mockGithubService.downloadFile(anyObject(), anyObject())).thenAnswer(fileAnswer);
-
-        return mockGithubService;
-    }
-
 }