fix: enable pair serialization to inputs.json and outputs.json during execution (#538)

Author: adthrasher

wdl-engine/CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 #### Added
 
 * Added `cpu_limit_behavior` and `memory_limit_behavior` options to task execution configuration ([#543](https://github.com/stjude-rust-labs/wdl/pull/543))
+* Serialize `Pair` as `Object` for execution-level `inputs.json` and `outputs.json` ([#538](https://github.com/stjude-rust-labs/wdl/pull/538)).
 
 #### Changed
 

wdl-engine/src/inputs.rs

@@ -11,6 +11,7 @@ use anyhow::Result;
 use anyhow::bail;
 use indexmap::IndexMap;
 use serde::Serialize;
+use serde::ser::SerializeMap;
 use serde_json::Value as JsonValue;
 use serde_yaml_ng::Value as YamlValue;
 use wdl_analysis::Document;
@@ -326,7 +327,12 @@ impl Serialize for TaskInputs {
         S: serde::Serializer,
     {
         // Only serialize the input values
-        self.inputs.serialize(serializer)
+        let mut map = serializer.serialize_map(Some(self.inputs.len()))?;
+        for (k, v) in &self.inputs {
+            let serialized_value = crate::ValueSerializer::new(v, true);
+            map.serialize_entry(k, &serialized_value)?;
+        }
+        map.end()
     }
 }
 
@@ -659,7 +665,12 @@ impl Serialize for WorkflowInputs {
     {
         // Note: for serializing, only serialize the direct inputs, not the nested
         // inputs
-        self.inputs.serialize(serializer)
+        let mut map = serializer.serialize_map(Some(self.inputs.len()))?;
+        for (k, v) in &self.inputs {
+            let serialized_value = crate::ValueSerializer::new(v, true);
+            map.serialize_entry(k, &serialized_value)?;
+        }
+        map.end()
     }
 }
 

wdl-engine/src/outputs.rs

@@ -65,28 +65,12 @@ impl Serialize for Outputs {
     {
         use serde::ser::SerializeMap;
 
-        /// Helper `Serialize` implementation for serializing element values.
-        struct Serialize<'a> {
-            /// The value being serialized.
-            value: &'a Value,
-        }
-
-        impl serde::Serialize for Serialize<'_> {
-            fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
-            where
-                S: serde::Serializer,
-            {
-                self.value.serialize(serializer)
-            }
-        }
-
         let mut s = serializer.serialize_map(Some(self.values.len()))?;
         for (k, v) in &self.values {
+            let v = crate::ValueSerializer::new(v, true);
             match &self.name {
-                Some(prefix) => {
-                    s.serialize_entry(&format!("{prefix}.{k}"), &Serialize { value: v })?
-                }
-                None => s.serialize_entry(k, &Serialize { value: v })?,
+                Some(prefix) => s.serialize_entry(&format!("{prefix}.{k}"), &v)?,
+                None => s.serialize_entry(k, &v)?,
             }
         }
 

wdl-engine/src/stdlib/write_json.rs

@@ -47,8 +47,7 @@ fn write_json(context: CallContext<'_>) -> Result<Value, Diagnostic> {
     // Serialize the value
     let mut writer = BufWriter::new(file.as_file_mut());
     let mut serializer = serde_json::Serializer::pretty(&mut writer);
-    context.arguments[0]
-        .value
+    crate::ValueSerializer::new(&context.arguments[0].value, false)
         .serialize(&mut serializer)
         .map_err(|e| {
             function_call_failed(

wdl-engine/src/value.rs

@@ -697,24 +697,6 @@ impl From<CallValue> for Value {
     }
 }
 
-impl serde::Serialize for Value {
-    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        use serde::ser::Error;
-
-        match self {
-            Self::None => serializer.serialize_none(),
-            Self::Primitive(v) => v.serialize(serializer),
-            Self::Compound(v) => v.serialize(serializer),
-            Self::Task(_) | Self::Hints(_) | Self::Input(_) | Self::Output(_) | Self::Call(_) => {
-                Err(S::Error::custom("value cannot be serialized"))
-            }
-        }
-    }
-}
-
 impl<'de> serde::Deserialize<'de> for Value {
     fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
     where
@@ -2612,63 +2594,6 @@ impl From<Struct> for CompoundValue {
     }
 }
 
-impl serde::Serialize for CompoundValue {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        use serde::ser::Error;
-
-        match self {
-            Self::Pair(_) => Err(S::Error::custom("a pair cannot be serialized")),
-            Self::Array(v) => {
-                let mut s = serializer.serialize_seq(Some(v.len()))?;
-                for v in v.as_slice() {
-                    s.serialize_element(v)?;
-                }
-
-                s.end()
-            }
-            Self::Map(v) => {
-                if !v
-                    .ty()
-                    .as_map()
-                    .expect("type should be a map")
-                    .key_type()
-                    .is_coercible_to(&PrimitiveType::String.into())
-                {
-                    return Err(S::Error::custom(
-                        "only maps with `String` key types may be serialized",
-                    ));
-                }
-
-                let mut s = serializer.serialize_map(Some(v.len()))?;
-                for (k, v) in v.iter() {
-                    s.serialize_entry(k, v)?;
-                }
-
-                s.end()
-            }
-            Self::Object(object) => {
-                let mut s = serializer.serialize_map(Some(object.len()))?;
-                for (k, v) in object.iter() {
-                    s.serialize_entry(k, v)?;
-                }
-
-                s.end()
-            }
-            Self::Struct(Struct { members, .. }) => {
-                let mut s = serializer.serialize_map(Some(members.len()))?;
-                for (k, v) in members.iter() {
-                    s.serialize_entry(k, v)?;
-                }
-
-                s.end()
-            }
-        }
-    }
-}
-
 /// Immutable data for task values.
 #[derive(Debug)]
 struct TaskData {
@@ -3069,6 +2994,125 @@ impl fmt::Display for CallValue {
     }
 }
 
+/// Serializes a value with optional serialization of pairs.
+pub struct ValueSerializer<'a> {
+    /// The value to serialize.
+    value: &'a Value,
+    /// Whether pairs should be serialized as a map with `left` and `right`
+    /// keys.
+    allow_pairs: bool,
+}
+
+impl<'a> ValueSerializer<'a> {
+    /// Constructs a new `ValueSerializer`.
+    pub fn new(value: &'a Value, allow_pairs: bool) -> Self {
+        Self { value, allow_pairs }
+    }
+}
+
+impl serde::Serialize for ValueSerializer<'_> {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::Error;
+
+        match &self.value {
+            Value::None => serializer.serialize_none(),
+            Value::Primitive(v) => v.serialize(serializer),
+            Value::Compound(v) => {
+                CompoundValueSerializer::new(v, self.allow_pairs).serialize(serializer)
+            }
+            Value::Task(_)
+            | Value::Hints(_)
+            | Value::Input(_)
+            | Value::Output(_)
+            | Value::Call(_) => Err(S::Error::custom("value cannot be serialized")),
+        }
+    }
+}
+
+/// Serializes a `CompoundValue` with optional serialization of pairs.
+pub struct CompoundValueSerializer<'a> {
+    /// The compound value to serialize.
+    value: &'a CompoundValue,
+    /// Whether pairs should be serialized as a map with `left` and `right`
+    /// keys.
+    allow_pairs: bool,
+}
+
+impl<'a> CompoundValueSerializer<'a> {
+    /// Constructs a new `CompoundValueSerializer`.
+    pub fn new(value: &'a CompoundValue, allow_pairs: bool) -> Self {
+        Self { value, allow_pairs }
+    }
+}
+
+impl serde::Serialize for CompoundValueSerializer<'_> {
+    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::Error;
+
+        match &self.value {
+            CompoundValue::Pair(pair) if self.allow_pairs => {
+                let mut state = serializer.serialize_map(Some(2))?;
+                let left = ValueSerializer::new(pair.left(), self.allow_pairs);
+                let right = ValueSerializer::new(pair.right(), self.allow_pairs);
+                state.serialize_entry("left", &left)?;
+                state.serialize_entry("right", &right)?;
+                state.end()
+            }
+            CompoundValue::Pair(_) => Err(S::Error::custom("a pair cannot be serialized")),
+            CompoundValue::Array(v) => {
+                let mut s = serializer.serialize_seq(Some(v.len()))?;
+                for v in v.as_slice() {
+                    s.serialize_element(&ValueSerializer::new(v, self.allow_pairs))?;
+                }
+
+                s.end()
+            }
+            CompoundValue::Map(v) => {
+                if !v
+                    .ty()
+                    .as_map()
+                    .expect("type should be a map")
+                    .key_type()
+                    .is_coercible_to(&PrimitiveType::String.into())
+                {
+                    return Err(S::Error::custom(
+                        "only maps with `String` key types may be serialized",
+                    ));
+                }
+
+                let mut s = serializer.serialize_map(Some(v.len()))?;
+                for (k, v) in v.iter() {
+                    s.serialize_entry(k, &ValueSerializer::new(v, self.allow_pairs))?;
+                }
+
+                s.end()
+            }
+            CompoundValue::Object(object) => {
+                let mut s = serializer.serialize_map(Some(object.len()))?;
+                for (k, v) in object.iter() {
+                    s.serialize_entry(k, &ValueSerializer::new(v, self.allow_pairs))?;
+                }
+
+                s.end()
+            }
+            CompoundValue::Struct(Struct { members, .. }) => {
+                let mut s = serializer.serialize_map(Some(members.len()))?;
+                for (k, v) in members.iter() {
+                    s.serialize_entry(k, &ValueSerializer::new(v, self.allow_pairs))?;
+                }
+
+                s.end()
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use approx::assert_relative_eq;
@@ -3450,7 +3494,7 @@ Caused by:
 
         let ty = PairType::new(PrimitiveType::File, PrimitiveType::String);
         let value: Value = Pair::new(ty, left, right)
-            .expect("should create map value")
+            .expect("should create pair value")
             .into();
 
         // Pair[File, String] -> Pair[String, File]
@@ -3549,4 +3593,34 @@ Caused by:
             r#"Foo {foo: 1.101000, bar: "foo", baz: 1234}"#
         );
     }
+
+    #[test]
+    fn pair_serialization() {
+        let pair_ty = PairType::new(PrimitiveType::File, PrimitiveType::String);
+        let pair: Value = Pair::new(
+            pair_ty,
+            PrimitiveValue::new_file("foo"),
+            PrimitiveValue::new_string("bar"),
+        )
+        .expect("should create pair value")
+        .into();
+        // Serialize pair with `left` and `right` keys
+        let value_serializer = ValueSerializer::new(&pair, true);
+        let serialized = serde_json::to_string(&value_serializer).expect("should serialize");
+        assert_eq!(serialized, r#"{"left":"foo","right":"bar"}"#);
+
+        // Serialize pair without `left` and `right` keys (should fail)
+        let value_serializer = ValueSerializer::new(&pair, false);
+        assert!(serde_json::to_string(&value_serializer).is_err());
+
+        let array_ty = ArrayType::new(PairType::new(PrimitiveType::File, PrimitiveType::String));
+        let array: Value = Array::new(array_ty, [pair])
+            .expect("should create array value")
+            .into();
+
+        // Serialize array of pairs with `left` and `right` keys
+        let value_serializer = ValueSerializer::new(&array, true);
+        let serialized = serde_json::to_string(&value_serializer).expect("should serialize");
+        assert_eq!(serialized, r#"[{"left":"foo","right":"bar"}]"#);
+    }
 }