correclty handle input requirements

Author: JensKrumsieck

crates/cwl-execution/src/lib.rs

@@ -82,7 +82,7 @@ pub fn execute_cwlfile(cwlfile: impl AsRef<Path>, raw_inputs: &[String], outdir:
         }
     }
 
-    let output_values = execute(cwlfile, input_values, outdir, None)?;
+    let output_values = execute(cwlfile, &input_values, outdir, None)?;
     let json = serde_json::to_string_pretty(&output_values)?;
     println!("{json}");
 
@@ -91,7 +91,7 @@ pub fn execute_cwlfile(cwlfile: impl AsRef<Path>, raw_inputs: &[String], outdir:
 
 pub fn execute(
     cwlfile: impl AsRef<Path>,
-    input_values: InputObject,
+    input_values: &InputObject,
     outdir: Option<impl AsRef<Path>>,
     cwl_doc: Option<&CWLDocument>,
 ) -> Result<HashMap<String, DefaultValue>, Box<dyn Error>> {
@@ -130,6 +130,11 @@ pub struct InputObject {
     #[serde(rename = "cwl:hints")]
     #[serde(default)]
     pub hints: Vec<Requirement>,
+
+    #[serde(skip)]
+    cwl_requirements: Vec<Requirement>,
+    #[serde(skip)]
+    cwl_hints: Vec<Requirement>,
 }
 
 impl InputObject {
@@ -141,21 +146,56 @@ impl InputObject {
     }
 
     pub fn add_requirement(&mut self, requirement: &Requirement) {
-        if self
-            .requirements
-            .iter()
-            .any(|r| std::mem::discriminant(r) == std::mem::discriminant(requirement))
+        if let Some(r) = self
+            .cwl_requirements
+            .iter_mut()
+            .find(|r| std::mem::discriminant(*r) == std::mem::discriminant(requirement))
         {
-            return;
+            *r = requirement.clone();
+        } else {
+            self.cwl_requirements.push(requirement.clone());
         }
-        self.requirements.push(requirement.clone());
     }
 
     pub fn add_hint(&mut self, hint: &Requirement) {
-        if self.hints.iter().any(|r| std::mem::discriminant(r) == std::mem::discriminant(hint)) {
-            return;
+        if let Some(r) = self
+            .cwl_hints
+            .iter_mut()
+            .find(|r| std::mem::discriminant(*r) == std::mem::discriminant(hint))
+        {
+            *r = hint.clone();
+        } else {
+            self.cwl_hints.push(hint.clone());
+        }
+    }
+
+    pub fn handle_requirements(&self, requirements: &[Requirement], hints: &[Requirement]) -> Self {
+        let mut new_obj = self.clone();
+        for hint in hints {
+            new_obj.add_hint(hint);
+        }
+
+        for req in requirements {
+            new_obj.add_requirement(req);
         }
-        self.hints.push(hint.clone());
+        new_obj
+    }
+
+    pub fn lock(&mut self) {
+        fn merge(dst: &mut Vec<Requirement>, src: &[Requirement]) {
+            for req in src {
+                if let Some(r) = dst.iter_mut().find(|r| std::mem::discriminant(*r) == std::mem::discriminant(req)) {
+                    *r = req.clone();
+                } else {
+                    dst.push(req.clone());
+                }
+            }
+        }
+        merge(&mut self.cwl_requirements, &self.requirements);
+        self.requirements = self.cwl_requirements.clone();
+
+        merge(&mut self.cwl_hints, &self.hints);
+        self.hints = self.cwl_hints.clone();
     }
 }
 
@@ -264,3 +304,26 @@ pub fn set_container_engine(value: ContainerEngine) {
 pub fn container_engine() -> ContainerEngine {
     CONTAINER_ENGINE.with(|engine| *engine.borrow())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use cwl::{requirements::EnvVarRequirement, types::EnviromentDefs};
+
+    #[test]
+    fn test_add_requirement() {
+        let mut input = InputObject::default();
+        let base_req = Requirement::EnvVarRequirement(EnvVarRequirement {
+            env_def: EnviromentDefs::Map(HashMap::from([("MY_ENV".to_string(), "BASE".to_string())])),
+        });
+        input.add_requirement(&base_req);
+        assert_eq!(input.requirements.len(), 1);
+
+        let requirement = Requirement::EnvVarRequirement(EnvVarRequirement {
+            env_def: EnviromentDefs::Map(HashMap::from([("MY_ENV".to_string(), "OVERWRITE".to_string())])),
+        });
+        input.add_requirement(&requirement);
+        assert_eq!(input.requirements.len(), 1);
+        assert_eq!(input.requirements[0], requirement);
+    }
+}

crates/cwl-execution/src/runner.rs

@@ -41,7 +41,7 @@ use wait_timeout::ChildExt;
 
 pub fn run_workflow(
     workflow: &mut Workflow,
-    input_values: InputObject,
+    input_values: &InputObject,
     cwl_path: Option<&PathBuf>,
     out_dir: Option<String>,
 ) -> Result<HashMap<String, DefaultValue>, Box<dyn Error>> {
@@ -59,14 +59,7 @@ pub fn run_workflow(
     };
 
     let workflow_folder = cwl_path.unwrap().parent().unwrap_or(Path::new("."));
-
-    let mut input_values = input_values;
-    for req in &workflow.requirements {
-        input_values.add_requirement(req);
-    }
-    for hint in &workflow.hints {
-        input_values.add_hint(hint);
-    }
+    let input_values = input_values.handle_requirements(&workflow.requirements, &workflow.hints);
 
     //prevent tool from outputting
     set_print_output(false);
@@ -124,21 +117,12 @@ pub fn run_workflow(
                     }
                 }
             }
-            let mut input_values = InputObject {
-                inputs: step_inputs,
-                requirements: input_values.requirements.clone(),
-                hints: input_values.hints.clone(),
-            };
-            for req in &step.requirements {
-                input_values.add_requirement(req);
-            }
-            for hint in &step.hints {
-                input_values.add_hint(hint);
-            }
+            let input_values = input_values.handle_requirements(&step.requirements, &step.hints);
+
             let step_outputs = if let Some(path) = path {
-                execute(&path, input_values, Some(tmp_path.clone()), None)?
+                execute(&path, &input_values, Some(tmp_path.clone()), None)?
             } else if let StringOrDocument::Document(doc) = &step.run {
-                execute(workflow_folder, input_values, Some(tmp_path.clone()), Some(doc))?
+                execute(workflow_folder, &input_values, Some(tmp_path.clone()), Some(doc))?
             } else {
                 unreachable!()
             };
@@ -227,7 +211,7 @@ pub fn run_workflow(
 
 pub fn run_tool(
     tool: &mut CWLDocument,
-    input_values: InputObject,
+    input_values: &InputObject,
     cwl_path: Option<&PathBuf>,
     out_dir: Option<String>,
 ) -> Result<HashMap<String, DefaultValue>, Box<dyn Error>> {
@@ -254,13 +238,8 @@ pub fn run_tool(
     //create runtime tmpdir
     let tmp_dir = tempdir()?;
 
-    let mut input_values = input_values;
-    for req in &tool.requirements {
-        input_values.add_requirement(req);
-    }
-    for hint in &tool.hints {
-        input_values.add_hint(hint);
-    }
+    let mut input_values = input_values.handle_requirements(&tool.requirements, &tool.hints);
+    input_values.lock();
 
     //build runtime object
     let mut runtime = RuntimeEnvironment::initialize(tool, &input_values, dir.path(), tool_path, tmp_dir.path())?;

tests/runner_test.rs

@@ -188,7 +188,7 @@ baseCommand:
 ";
 
     let mut tool: CWLDocument = serde_yaml::from_str(cwl).expect("Tool parsing failed");
-    let result = run_tool(&mut tool, Default::default(), None, None);
+    let result = run_tool(&mut tool, &Default::default(), None, None);
     assert!(result.is_ok());
     //delete results.txt
     let _ = fs::remove_file("results.txt");
@@ -203,6 +203,6 @@ baseCommand:
 pub fn test_run_commandlinetool_array_glob() {
     let dir = tempdir().unwrap();
     let mut tool = CWLDocument::CommandLineTool(load_tool("tests/test_data/array_test.cwl").expect("Tool parsing failed"));
-    let result = run_tool(&mut tool, Default::default(), None, Some(dir.path().to_string_lossy().into_owned()));
+    let result = run_tool(&mut tool, &Default::default(), None, Some(dir.path().to_string_lossy().into_owned()));
     assert!(result.is_ok(), "{result:?}");
 }