Merge pull request #315 from SeqWare/feature/seqware-2022_runner_bugfix

Feature/seqware 2022 runner bugfix

Author: denis-yuen

seqware-pipeline/src/main/java/net/sourceforge/seqware/pipeline/runner/Runner.java

@@ -28,6 +28,7 @@
 import net.sourceforge.seqware.common.metadata.Metadata;
 import net.sourceforge.seqware.common.metadata.MetadataDB;
 import net.sourceforge.seqware.common.metadata.MetadataFactory;
+import net.sourceforge.seqware.common.model.ParentAccessionModel;
 import net.sourceforge.seqware.common.model.Processing;
 import net.sourceforge.seqware.common.module.FileMetadata;
 import net.sourceforge.seqware.common.module.ReturnValue;
@@ -43,21 +44,21 @@
 import org.apache.commons.io.FileUtils;
 
 // FIXME: auto-adding to rc.data, support "," delimited
-// FIXME: When adding STDOUT/STDERR to metadb, we should add a timestamp or something else to make it easier to merge. Right now, it is hard to tell which stdout message corresponds to which step in stderr 
+// FIXME: When adding STDOUT/STDERR to metadb, we should add a timestamp or something else to make it easier to merge. Right now, it is hard to tell which stdout message corresponds to which step in stderr
 // FIXME: is parent accession comma separated?
 
 /*
  * Run each method for the requested module object. Based on return value, either continue or exit:
  *   Return values > 0 are errors that will cause the runner to exit.
  *   Return of 0 implies success and the runner will continue, assuming all is well.
- *   Return of -1 implies the method was not implemented for that Module, AND IS NOT AN ERROR!!! By default the runner will continue on with steps as if it succeeded!  
+ *   Return of -1 implies the method was not implemented for that Module, AND IS NOT AN ERROR!!! By default the runner will continue on with steps as if it succeeded!
  */
 //Create a main() function here, which will be compiled in, to parse input and run all steps
 /**
  * <p>
  * Runner class.
  * </p>
- * 
+ *
  * @author boconnor
  * @version $Id: $Id
  */
@@ -185,7 +186,7 @@ public Runner() {
      * <p>
      * getSyntax.
      * </p>
-     * 
+     *
      * @param parser
      *            a {@link joptsimple.OptionParser} object.
      * @param errorMessage
@@ -211,19 +212,20 @@ public static void getSyntax(OptionParser parser, String errorMessage) {
         System.exit(-1);
     }
 
-    private void writeProcessingAccessionToFile(File file, boolean append) {
+    private void writeStringToFile(File file, boolean append, String output) {
         int maxTries = (Integer) options.valueOf("metadata-tries-number");
-        for (int i = 0; i < maxTries; i++) {
+        for (int i = 0; i <= maxTries; i++) {
+            Log.fatal("On try " + i + " of " + maxTries);
             // Break on success
-            if (LockingFileTools.lockAndWrite(file, processingAccession + System.getProperty("line.separator"), append)) {
+            if (LockingFileTools.lockAndWrite(file, output, append)) {
                 break;
             } // Sleep if going to try again
             else if (i < maxTries) {
                 ProcessTools.sleep((Integer) options.valueOf("metadata-tries-delay"));
             } // Return error if failed on last try
-            else {
+            if (maxTries == i) {
                 ReturnValue retval = new ReturnValue();
-                retval.printAndAppendtoStderr("Could not write to processingAccession File for metadata");
+                retval.printAndAppendtoStderr("Could not write to " + file.getAbsolutePath() + " for metadata");
                 retval.setExitStatus(ReturnValue.METADATAINVALIDIDCHAIN);
                 meta.update_processing_event(processingID, retval);
                 meta.update_processing_status(processingID, ProcessingStatus.failed);
@@ -236,7 +238,7 @@ else if (i < maxTries) {
      * <p>
      * printAndAppendtoStderr.
      * </p>
-     * 
+     *
      * @param buffer
      *            a {@link java.lang.String} object.
      */
@@ -249,7 +251,7 @@ public void printAndAppendtoStderr(String buffer) {
      * <p>
      * printAndAppendtoStdout.
      * </p>
-     * 
+     *
      * @param buffer
      *            a {@link java.lang.String} object.
      */
@@ -266,7 +268,7 @@ public void printAndAppendtoStdout(String buffer) {
      * <p>
      * evaluateReturn.
      * </p>
-     * 
+     *
      * @param app
      *            a {@link net.sourceforge.seqware.pipeline.module.Module} object.
      * @param methodName
@@ -370,7 +372,7 @@ public void evaluateReturn(Module app, String methodName) {
 
     /**
      * FIXME: this needs to be migrated to something that is ZIP64 aware. Try using the unzip feature of FileTools.java
-     * 
+     *
      * @param zipFile
      *            a {@link java.lang.String} object.
      * @throws java.util.zip.ZipException
@@ -438,7 +440,7 @@ public static void unzipPkg(String zipFile) throws ZipException, IOException {
      * <p>
      * main.
      * </p>
-     * 
+     *
      * @param args
      *            an array of {@link java.lang.String} objects.
      */
@@ -642,24 +644,7 @@ private void preProcessMetadata() {
 
                 try {
                     if ((file.exists() || file.createNewFile()) && file.canWrite()) {
-                        int maxTries = (Integer) options.valueOf("metadata-tries-number");
-                        for (int i = 0; i < maxTries; i++) {
-                            // Break on success
-                            if (LockingFileTools.lockAndAppend(file, workflowRunAccession + System.getProperty("line.separator"))) {
-                                break;
-                            } // Sleep if going to try again
-                            else if (i < maxTries) {
-                                ProcessTools.sleep((Integer) options.valueOf("metadata-tries-delay"));
-                            } // Return error if failed on last try
-                            else {
-                                ReturnValue retval = new ReturnValue();
-                                retval.printAndAppendtoStderr("Could not write to processingID File for metadata");
-                                retval.setExitStatus(ReturnValue.METADATAINVALIDIDCHAIN);
-                                meta.update_processing_event(workflowRunAccession, retval);
-                                meta.update_processing_status(workflowRunAccession, ProcessingStatus.failed);
-                                System.exit(retval.getExitStatus());
-                            }
-                        }
+                        this.writeStringToFile(file, true, workflowRunAccession + System.getProperty("line.separator"));
                     } else {
                         Log.error("Could not create processingAccession File for metadata");
                         System.exit(ReturnValue.METADATAINVALIDIDCHAIN);
@@ -719,16 +704,29 @@ else if (i < maxTries) {
                 BufferedReader r;
                 String line;
                 r = new BufferedReader(new FileReader(file));
-
+                boolean accessionFound = false;
+                // seqware-2023, we need to ensure that there is at least one accession found
                 while ((line = r.readLine()) != null) {
                     try {
-                        parentAccessions.add(Integer.parseInt(line));
+                        int parseInt = Integer.parseInt(line);
+                        parentAccessions.add(parseInt);
+                        int[] accessions = new int[] { parseInt };
+                        List<ParentAccessionModel> viaParentAccessions = meta.getViaParentAccessions(accessions);
+                        if (viaParentAccessions == null) {
+                            Log.error("Invalid accession found when parsing parent accession file '" + line + "'");
+                            System.exit(ReturnValue.METADATAINVALIDIDCHAIN);
+                        }
+                        accessionFound = true;
                     } catch (NumberFormatException ex) {
                         Log.error("Non number found when parsing parent accession file '" + line + "'");
                         System.exit(ReturnValue.METADATAINVALIDIDCHAIN);
                     }
                 }
                 r.close();
+                if (!accessionFound) {
+                    Log.error("No number found when parsing parent accession file '" + file + "'");
+                    System.exit(ReturnValue.METADATAINVALIDIDCHAIN);
+                }
             } catch (Exception e) {
                 Log.error("Could not open parent accession file for metadata-parent-accession-file: " + e.getMessage());
                 System.exit(ReturnValue.METADATAINVALIDIDCHAIN);
@@ -880,35 +878,18 @@ private void postProcessMetadata() {
             // Try to write to each processingIDFile until success or timeout
             for (File file : processingIDFiles) {
                 Log.debug("Writing out accession to " + file.toString());
-                int maxTries = (Integer) options.valueOf("metadata-tries-number");
-                for (int i = 0; i < maxTries; i++) {
-                    // Break on success
-                    if (LockingFileTools.lockAndAppend(file, processingID + System.getProperty("line.separator"))) {
-                        break;
-                    } // Sleep if going to try again
-                    else if (i < maxTries) {
-                        ProcessTools.sleep((Integer) options.valueOf("metadata-tries-delay"));
-                    } // Return error if failed on last try
-                    else {
-                        ReturnValue retval = new ReturnValue();
-                        retval.printAndAppendtoStderr("Could not write to processingID File for metadata");
-                        retval.setExitStatus(ReturnValue.METADATAINVALIDIDCHAIN);
-                        meta.update_processing_event(processingID, retval);
-                        meta.update_processing_status(processingID, ProcessingStatus.failed);
-                        System.exit(retval.getExitStatus());
-                    }
-                }
+                this.writeStringToFile(file, true, processingID + System.getProperty("line.separator"));
             }
             Log.debug("Completed processingIDFiles");
 
             // Try to write to each processingAccessionFile until success or timeout
             for (File file : processingAccessionFiles) {
                 Log.debug("Writing out to " + file.toString());
-                writeProcessingAccessionToFile(file, true);
+                writeStringToFile(file, true, processingAccession + System.getProperty("line.separator"));
             }
             Log.debug("Completed processingAccessionFiles");
             if (processingAccessionFileCheck != null) {
-                writeProcessingAccessionToFile(processingAccessionFileCheck, false);
+                writeStringToFile(processingAccessionFileCheck, true, processingAccession + System.getProperty("line.separator"));
             }
             Log.debug("Completed processingAccessionFileCheck");
             meta.update_processing_status(processingID, ProcessingStatus.success);
@@ -972,7 +953,7 @@ private void invokeModuleMethods() {
      * <p>
      * run.
      * </p>
-     * 
+     *
      * @param args
      *            an array of {@link java.lang.String} objects.
      */

seqware-webservice/pom.xml

@@ -133,14 +133,15 @@
                 </executions>
 
                 <configuration>
-                    <argLine>${argLine} -Xms512m -Xmx512m</argLine>
+                    <!-- <argLine>${argLine} -Xms512m -Xmx512m</argLine> -->
                     <forkMode>pertest</forkMode>
                     <!-- enforce that our WSResourceTests run before the plugin tests, this sucks and our tests shouldn't have state -->
 		    <runOrder>reversealphabetical</runOrder>
                     <includes>
                         <include>**/WSResourceTestSuite.java</include>
                         <include>**/plugins/*.java</include>
                         <include>**/deciders/*.java</include>
+                        <include>**/runner/*.java</include>
                     </includes>
                 </configuration>
             </plugin>

seqware-webservice/src/test/java/net/sourceforge/seqware/pipeline/runner/ModuleRunnerTest.java

@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2012 SeqWare
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package net.sourceforge.seqware.pipeline.runner;
+
+import io.seqware.Reports;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import net.sourceforge.seqware.common.module.ReturnValue;
+import net.sourceforge.seqware.common.util.testtools.BasicTestDatabaseCreator;
+import net.sourceforge.seqware.pipeline.plugins.ExtendedPluginTest;
+import net.sourceforge.seqware.pipeline.plugins.ModuleRunner;
+import org.apache.commons.io.FileUtils;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.contrib.java.lang.system.ExpectedSystemExit;
+
+/**
+ * Runs tests for the ModuleRunner class.
+ *
+ * @author dyuen
+ */
+
+public class ModuleRunnerTest extends ExtendedPluginTest {
+
+    /**
+     * This allows us to intercept and ignore the various System.exit calls within runner so they don't take down the tests as well
+     */
+    @Rule
+    public final ExpectedSystemExit exit = ExpectedSystemExit.none();
+
+    @BeforeClass
+    public static void beforeClass() {
+        BasicTestDatabaseCreator.resetDatabaseWithUsers();
+        Reports.triggerProvenanceReport();
+    }
+
+    @Before
+    @Override
+    public void setUp() {
+        instance = new ModuleRunner();
+        super.setUp();
+    }
+
+    public ModuleRunnerTest() {
+    }
+
+    @Test
+    public void normalParentAccessionRunTest() throws IOException {
+        System.out.println("normalParentAccessionRunTest");
+        Path parentAccession = Files.createTempFile("accession", "test");
+        FileUtils.write(parentAccession.toFile(), "836");
+        exit.expectSystemExitWithStatus(ReturnValue.SUCCESS);
+        launchPlugin("--metadata", "--metadata-parent-accession-file", parentAccession.toAbsolutePath().toString(), "--module",
+                "net.sourceforge.seqware.pipeline.modules.GenericCommandRunner", "--", "--gcr-algorithm", "bash_cp", "--gcr-command",
+                "echo");
+    }
+
+    @Test
+    public void normalAccessionFileRunTest() throws IOException {
+        System.out.println("normalAccessionFileRunTest");
+        Path resultAccession = Files.createTempFile("accession", "test");
+        exit.expectSystemExitWithStatus(ReturnValue.SUCCESS);
+        launchPlugin("--metadata", "--metadata-processing-accession-file", resultAccession.toAbsolutePath().toString(), "--module",
+                "net.sourceforge.seqware.pipeline.modules.GenericCommandRunner", "--", "--gcr-algorithm", "bash_cp", "--gcr-command",
+                "echo");
+    }
+
+    @Test
+    public void testBlankParentAccessionFileFail() throws IOException {
+        System.out.println("testBlankParentAccessionFile");
+        Path parentAccession = Files.createTempFile("accession", "test");
+        exit.expectSystemExitWithStatus(ReturnValue.METADATAINVALIDIDCHAIN);
+        launchPlugin("--metadata", "--metadata-parent-accession-file", parentAccession.toAbsolutePath().toString(), "--module",
+                "net.sourceforge.seqware.pipeline.modules.GenericCommandRunner", "--", "--gcr-algorithm", "bash_cp", "--gcr-command",
+                "echo");
+    }
+
+    @Test
+    public void testInvalidAccessionFileFail() throws IOException {
+        System.out.println("testInvalidAccessionFile");
+        Path parentAccession = Files.createTempFile("accession", "test");
+        FileUtils.write(parentAccession.toFile(), "-1");
+        exit.expectSystemExitWithStatus(ReturnValue.SQLQUERYFAILED);
+        launchPlugin("--metadata", "--metadata-parent-accession-file", parentAccession.toAbsolutePath().toString(), "--module",
+                "net.sourceforge.seqware.pipeline.modules.GenericCommandRunner", "--", "--gcr-algorithm", "bash_cp", "--gcr-command",
+                "echo");
+    }
+
+}