Merge pull request #7 from arminhammer/fix-set-and-wait

cdavernas · web-flow · commit 11c1c5c25574 · 2025-11-29T15:29:37.000+01:00
fix: update wait and set task syntax
diff --git a/builders/src/lib.rs b/builders/src/lib.rs
@@ -434,7 +434,12 @@ mod unit_tests {
                 .iter()
                 .any(|entry| entry.get(&set_task_name.to_string()).map_or(false, |task|{
                     if let TaskDefinition::Set(set_task) = task {
-                        set_task.set == set_task_variables.clone()
+                        match &set_task.set {
+                            serverless_workflow_core::models::task::SetValue::Map(map) => {
+                                map == &set_task_variables
+                            }
+                            _ => false
+                        }
                     }
                     else{
                         false
@@ -482,12 +487,12 @@ mod unit_tests {
                 .iter()
                 .any(|entry| entry.get(&wait_task_name.to_string()).map_or(false, |task| {
                     if let TaskDefinition::Wait(wait_task) = task {
-                        wait_task.duration == wait_duration
+                        wait_task.wait == wait_duration
                     } else {
                         false
                     }
                 })),
-            "Expected a task with key '{}' and a WaitTaskDefinition with 'duration'={}",
+            "Expected a task with key '{}' and a WaitTaskDefinition with 'wait'={}",
             wait_task_name,
             wait_duration);
     }
diff --git a/builders/src/services/task.rs b/builders/src/services/task.rs
@@ -1042,13 +1042,29 @@ impl SetTaskDefinitionBuilder{
 
     /// Sets the specified variable
     pub fn variable(&mut self, name: &str, value: Value) -> &mut Self{
-        self.task.set.insert(name.to_string(), value);
+        match &mut self.task.set {
+            serverless_workflow_core::models::task::SetValue::Map(map) => {
+                map.insert(name.to_string(), value);
+            }
+            serverless_workflow_core::models::task::SetValue::Expression(_) => {
+                // If it was an expression, convert to map
+                let mut map = HashMap::new();
+                map.insert(name.to_string(), value);
+                self.task.set = serverless_workflow_core::models::task::SetValue::Map(map);
+            }
+        }
+        self
+    }
+
+    /// Configures the variable as an expression
+    pub fn variable_expression(&mut self, expression: String) -> &mut Self{
+        self.task.set = serverless_workflow_core::models::task::SetValue::Expression(expression);
         self
     }
 
     /// Configures the task to set the specified variables
     pub fn variables(&mut self, variables: HashMap<String, Value>) -> &mut Self{
-        self.task.set = variables;
+        self.task.set = serverless_workflow_core::models::task::SetValue::Map(variables);
         self
     }
 
@@ -1820,18 +1836,18 @@ impl ScriptProcessDefinitionBuilder{
     }
 
     /// Adds a new argument to execute the script with
-    pub fn with_argument(&mut self, key: &str, value: &str) -> &mut Self{
+    pub fn with_argument(&mut self, value: &str) -> &mut Self{
         if self.process.arguments.is_none(){
-            self.process.arguments = Some(HashMap::new());
+            self.process.arguments = Some(Vec::new());
         }
         if let Some(arguments) = &mut self.process.arguments {
-            arguments.insert(key.to_string(), value.to_string());
+            arguments.push(value.to_string());
         }
         self
     }
 
     /// Sets the arguments of the script to execute
-    pub fn with_arguments(&mut self, arguments: HashMap<String, String>) -> &mut Self{
+    pub fn with_arguments(&mut self, arguments: Vec<String>) -> &mut Self{
         self.process.arguments = Some(arguments);
         self
     }
@@ -1853,6 +1869,11 @@ impl ScriptProcessDefinitionBuilder{
         self
     }
 
+    pub fn with_stdin(&mut self, stdin: &str) -> &mut Self{
+        self.process.stdin = Some(stdin.to_string());
+        self
+    }
+
     /// Builds the configured RunTaskDefinition
     pub fn build(self) -> RunTaskDefinition{
         let mut run_task = RunTaskDefinition::default();
diff --git a/core/src/lib.rs b/core/src/lib.rs
@@ -237,4 +237,140 @@ mod unit_tests {
             }
         }
     }
+
+    #[test]
+    fn test_set_value_map_deserialization() {
+        // Test SetValue with a map (object) of key-value pairs
+        let set_value_map = serde_json::json!({
+            "foo": "bar",
+            "count": 42
+        });
+
+        let result: Result<SetTaskDefinition, _> = serde_json::from_value(serde_json::json!({
+            "set": set_value_map
+        }));
+        assert!(result.is_ok(), "Failed to deserialize set task with map: {:?}", result.err());
+
+        let set_task = result.unwrap();
+        match set_task.set {
+            SetValue::Map(map) => {
+                assert_eq!(map.len(), 2);
+                assert_eq!(map.get("foo").and_then(|v| v.as_str()), Some("bar"));
+                assert_eq!(map.get("count").and_then(|v| v.as_u64()), Some(42));
+            }
+            SetValue::Expression(_) => {
+                panic!("Expected SetValue::Map but got SetValue::Expression");
+            }
+        }
+    }
+
+    #[test]
+    fn test_set_value_expression_deserialization() {
+        // Test SetValue with a runtime expression string
+        let set_value_expr_json = serde_json::json!("${ $workflow.input[0] }");
+
+        let result: Result<SetTaskDefinition, _> = serde_json::from_value(serde_json::json!({
+            "set": set_value_expr_json
+        }));
+        assert!(result.is_ok(), "Failed to deserialize set task with expression: {:?}", result.err());
+
+        let set_task = result.unwrap();
+        match set_task.set {
+            SetValue::Expression(expr) => {
+                assert_eq!(expr, "${ $workflow.input[0] }");
+            }
+            SetValue::Map(_) => {
+                panic!("Expected SetValue::Expression but got SetValue::Map");
+            }
+        }
+    }
+
+    #[test]
+    fn test_wait_task_iso8601_deserialization() {
+        // Test WaitTask with ISO 8601 duration string
+        let wait_task_json = serde_json::json!({
+            "wait": "PT30S"
+        });
+        let result: Result<WaitTaskDefinition, _> = serde_json::from_value(wait_task_json);
+        assert!(result.is_ok(), "Failed to deserialize wait task with ISO 8601: {:?}", result.err());
+
+        let wait_task = result.unwrap();
+        match wait_task.wait {
+            OneOfDurationOrIso8601Expression::Iso8601Expression(expr) => {
+                assert_eq!(expr, "PT30S");
+            }
+            OneOfDurationOrIso8601Expression::Duration(_) => {
+                panic!("Expected Iso8601Expression but got Duration");
+            }
+        }
+    }
+
+    #[test]
+    fn test_wait_task_inline_duration_deserialization() {
+        // Test WaitTask with inline duration properties
+        let wait_task_json = serde_json::json!({
+            "wait": {
+                "seconds": 30
+            }
+        });
+        let result: Result<WaitTaskDefinition, _> = serde_json::from_value(wait_task_json);
+        assert!(result.is_ok(), "Failed to deserialize wait task with inline duration: {:?}", result.err());
+
+        let wait_task = result.unwrap();
+        match wait_task.wait {
+            OneOfDurationOrIso8601Expression::Duration(duration) => {
+                assert_eq!(duration.seconds, Some(30));
+            }
+            OneOfDurationOrIso8601Expression::Iso8601Expression(_) => {
+                panic!("Expected Duration but got Iso8601Expression");
+            }
+        }
+    }
+
+    #[test]
+    fn test_script_process_arguments_array_deserialization() {
+        use crate::models::task::ScriptProcessDefinition;
+
+        // Test ScriptProcessDefinition with arguments as an array (spec-compliant)
+        let script_process_json = serde_json::json!({
+            "language": "javascript",
+            "code": "console.log('test')",
+            "arguments": ["hello", "world"]
+        });
+        let result: Result<ScriptProcessDefinition, _> = serde_json::from_value(script_process_json);
+        assert!(result.is_ok(), "Failed to deserialize script with array arguments: {:?}", result.err());
+
+        let script = result.unwrap();
+        assert_eq!(script.language, "javascript");
+        assert!(script.arguments.is_some());
+
+        let args = script.arguments.unwrap();
+        assert_eq!(args.len(), 2);
+        assert_eq!(args[0], "hello");
+        assert_eq!(args[1], "world");
+    }
+
+    #[test]
+    fn test_script_process_with_stdin_deserialization() {
+        use crate::models::task::ScriptProcessDefinition;
+
+        // Test ScriptProcessDefinition with stdin property
+        let script_process_json = serde_json::json!({
+            "language": "python",
+            "code": "print('test')",
+            "stdin": "Hello Workflow",
+            "arguments": ["arg1"],
+            "environment": {"FOO": "bar"}
+        });
+        let result: Result<ScriptProcessDefinition, _> = serde_json::from_value(script_process_json);
+        assert!(result.is_ok(), "Failed to deserialize script with stdin: {:?}", result.err());
+
+        let script = result.unwrap();
+        assert_eq!(script.language, "python");
+        assert_eq!(script.stdin, Some("Hello Workflow".to_string()));
+        assert!(script.arguments.is_some());
+        assert_eq!(script.arguments.as_ref().unwrap().len(), 1);
+        assert!(script.environment.is_some());
+        assert_eq!(script.environment.as_ref().unwrap().get("FOO"), Some(&"bar".to_string()));
+    }
 }
diff --git a/core/src/models/task.rs b/core/src/models/task.rs
@@ -732,7 +732,7 @@ impl ContainerProcessDefinition {
 /// Represents the definition of a script evaluation process
 #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
 pub struct ScriptProcessDefinition{
-    
+
     /// Gets/sets the language of the script to run
     #[serde(rename = "language")]
     pub language: String,
@@ -745,9 +745,13 @@ pub struct ScriptProcessDefinition{
     #[serde(rename = "source", skip_serializing_if = "Option::is_none")]
     pub source: Option<ExternalResourceDefinition>,
 
-    /// Gets/sets a key/value mapping of the arguments, if any, to pass to the script to run
+    /// Gets/sets the data to pass to the process via stdin
+    #[serde(rename = "stdin", skip_serializing_if = "Option::is_none")]
+    pub stdin: Option<String>,
+
+    /// Gets/sets a list of arguments, if any, to pass to the script (argv)
     #[serde(rename = "arguments", skip_serializing_if = "Option::is_none")]
-    pub arguments: Option<HashMap<String, String>>,
+    pub arguments: Option<Vec<String>>,
 
     /// Gets/sets a key/value mapping of the environment variables, if any, to use when running the configured process
     #[serde(rename = "environment", skip_serializing_if = "Option::is_none")]
@@ -757,23 +761,25 @@ pub struct ScriptProcessDefinition{
 impl ScriptProcessDefinition {
 
     /// Initializes a new script from code
-    pub fn from_code(language: &str, code: String, arguments: Option<HashMap<String, String>>, environment: Option<HashMap<String, String>>) -> Self{
-        Self { 
-            language: language.to_string(), 
-            code: Some(code), 
-            source: None, 
-            arguments, 
+    pub fn from_code(language: &str, code: String, stdin: Option<String>, arguments: Option<Vec<String>>, environment: Option<HashMap<String, String>>) -> Self{
+        Self {
+            language: language.to_string(),
+            code: Some(code),
+            source: None,
+            stdin,
+            arguments,
             environment
          }
     }
 
     /// Initializes a new script from an external resource
-    pub fn from_source(language: &str, source: ExternalResourceDefinition, arguments: Option<HashMap<String, String>>, environment: Option<HashMap<String, String>>) -> Self{
-        Self { 
-            language: language.to_string(), 
-            code: None, 
-            source: Some(source), 
-            arguments, 
+    pub fn from_source(language: &str, source: ExternalResourceDefinition, stdin: Option<String>, arguments: Option<Vec<String>>, environment: Option<HashMap<String, String>>) -> Self{
+        Self {
+            language: language.to_string(),
+            code: None,
+            source: Some(source),
+            stdin,
+            arguments,
             environment
          }
     }
@@ -838,13 +844,29 @@ impl WorkflowProcessDefinition {
     }
 }
 
+/// Represents the value that can be set in a Set task - either a map or a runtime expression string
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(untagged)]
+pub enum SetValue {
+    /// A map of key-value pairs to set
+    Map(HashMap<String, Value>),
+    /// A runtime expression string that evaluates to the data to set
+    Expression(String),
+}
+
+impl Default for SetValue {
+    fn default() -> Self {
+        SetValue::Map(HashMap::new())
+    }
+}
+
 /// Represents the definition of a task used to set data
 #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
 pub struct SetTaskDefinition{
 
     /// Gets/sets the data to set
     #[serde(rename = "set")]
-    pub set: HashMap<String, Value>,
+    pub set: SetValue,
 
     /// Gets/sets the task's common fields
     #[serde(flatten)]
@@ -859,8 +881,8 @@ impl TaskDefinitionBase for SetTaskDefinition {
 impl SetTaskDefinition {
     /// Initializes a new SetTaskDefinition
     pub fn new() -> Self{
-        Self { 
-            set: HashMap::new(),
+        Self {
+            set: SetValue::Map(HashMap::new()),
             common: TaskDefinitionFields::new()
         }
     }
@@ -988,8 +1010,8 @@ pub struct ErrorFilterDefinition{
 pub struct WaitTaskDefinition{
 
     /// Gets/sets the amount of time to wait before resuming workflow
-    #[serde(rename = "duration")]
-    pub duration: OneOfDurationOrIso8601Expression,
+    #[serde(rename = "wait")]
+    pub wait: OneOfDurationOrIso8601Expression,
 
     /// Gets/sets the task's common fields
     #[serde(flatten)]
@@ -1002,11 +1024,11 @@ impl TaskDefinitionBase for WaitTaskDefinition {
     }
 }
 impl WaitTaskDefinition {
-    
+
     /// Initializes a new WaitTaskDefinition
-    pub fn new(duration: OneOfDurationOrIso8601Expression) -> Self{
-        Self { 
-            duration,
+    pub fn new(wait: OneOfDurationOrIso8601Expression) -> Self{
+        Self {
+            wait,
             common: TaskDefinitionFields::new()
         }
     }
