diff --git a/vegafusion-core/src/planning/projection_pushdown.rs b/vegafusion-core/src/planning/projection_pushdown.rs index e4cb3ec7..decd6c0e 100644 --- a/vegafusion-core/src/planning/projection_pushdown.rs +++ b/vegafusion-core/src/planning/projection_pushdown.rs @@ -753,7 +753,7 @@ impl ChartVisitor for CollectVlSelectionTestFieldsVisitor { if let Ok(resolved) = self.task_scope.resolve_scope(&store_var, scope) { let scoped_brush_var: ScopedVariable = (resolved.var, resolved.scope); - if let Some(value) = &signal.value { + if let Some(value) = &signal.value.as_option() { if let Ok(table) = VegaFusionTable::from_json(value) { // Check that we have "field", "channel", and "type" columns let schema = &table.schema; diff --git a/vegafusion-core/src/planning/stitch.rs b/vegafusion-core/src/planning/stitch.rs index fa34ec7a..bf4a18b1 100644 --- a/vegafusion-core/src/planning/stitch.rs +++ b/vegafusion-core/src/planning/stitch.rs @@ -3,6 +3,7 @@ use crate::proto::gen::tasks::VariableNamespace; use crate::spec::chart::ChartSpec; use crate::spec::data::DataSpec; use crate::spec::signal::SignalSpec; +use crate::spec::values::MissingNullOrValue; use crate::task_graph::graph::ScopedVariable; use crate::task_graph::scope::TaskScope; use serde_json::Value; @@ -93,13 +94,13 @@ fn make_stub( let stub_value = from_spec .get_nested_signal(&stub_path, &stub_name) .ok() - .and_then(|s| s.value.clone()); + .and_then(|s| s.value.as_option()); let new_signal_spec = SignalSpec { name: stub_name, init: None, update: None, - value: stub_value, + value: MissingNullOrValue::from(stub_value), on: vec![], extra: Default::default(), }; diff --git a/vegafusion-core/src/spec/signal.rs b/vegafusion-core/src/spec/signal.rs index 61c3b194..688be16e 100644 --- a/vegafusion-core/src/spec/signal.rs +++ b/vegafusion-core/src/spec/signal.rs @@ -1,7 +1,7 @@ use crate::expression::parser::parse; use crate::planning::plan::PlannerConfig; use crate::spec::data::DependencyNodeSupported; -use crate::spec::values::StringOrStringList; +use crate::spec::values::{MissingNullOrValue, StringOrStringList}; use crate::task_graph::scope::TaskScope; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -17,8 +17,8 @@ pub struct SignalSpec { #[serde(skip_serializing_if = "Option::is_none")] pub update: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub value: Option, + #[serde(default, skip_serializing_if = "MissingNullOrValue::is_missing")] + pub value: MissingNullOrValue, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub on: Vec, @@ -34,7 +34,7 @@ impl SignalSpec { task_scope: &TaskScope, scope: &[u32], ) -> DependencyNodeSupported { - if self.value.is_some() { + if !self.value.is_missing() { return DependencyNodeSupported::Supported; } else if let Some(expr) = &self.update { if self.on.is_empty() { @@ -126,3 +126,23 @@ pub struct SignalOnSourceEvent { #[serde(flatten)] pub extra: HashMap, } + +#[cfg(test)] +mod tests { + use crate::spec::signal::SignalSpec; + + #[test] + fn test_signal_null_value_not_dropped() { + // No value is valid + let s = r#"{"name":"foo"}"#; + let sig: SignalSpec = serde_json::from_str(s).unwrap(); + let res = serde_json::to_string(&sig).unwrap(); + assert_eq!(res, s); + + // Null value should not be dropped + let s = r#"{"name":"foo","value":null}"#; + let sig: SignalSpec = serde_json::from_str(s).unwrap(); + let res = serde_json::to_string(&sig).unwrap(); + assert_eq!(res, s); + } +} diff --git a/vegafusion-core/src/spec/values.rs b/vegafusion-core/src/spec/values.rs index 113ed5c3..d818496a 100644 --- a/vegafusion-core/src/spec/values.rs +++ b/vegafusion-core/src/spec/values.rs @@ -1,7 +1,7 @@ use crate::error::Result; use crate::expression::parser::parse; use crate::task_graph::task::InputVariable; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(untagged)] @@ -139,3 +139,62 @@ impl SortOrderOrList { } } } + +/// Helper struct that will not drop null values on round trip (de)serialization +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MissingNullOrValue { + Missing, + Null, + Value(serde_json::Value), +} + +impl MissingNullOrValue { + pub fn is_missing(&self) -> bool { + matches!(self, MissingNullOrValue::Missing) + } + + pub fn as_option(&self) -> Option { + match self { + MissingNullOrValue::Missing => None, + MissingNullOrValue::Null => Some(serde_json::Value::Null), + MissingNullOrValue::Value(v) => Some(v.clone()), + } + } +} + +impl Default for MissingNullOrValue { + fn default() -> Self { + MissingNullOrValue::Missing + } +} + +impl From> for MissingNullOrValue { + fn from(opt: Option) -> MissingNullOrValue { + match opt { + Some(v) => MissingNullOrValue::Value(v), + None => MissingNullOrValue::Null, + } + } +} + +impl<'de> Deserialize<'de> for MissingNullOrValue { + fn deserialize(deserializer: D) -> std::result::Result + where + D: Deserializer<'de>, + { + Option::deserialize(deserializer).map(Into::into) + } +} + +impl Serialize for MissingNullOrValue { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: Serializer, + { + match self { + MissingNullOrValue::Missing => None::>.serialize(serializer), + MissingNullOrValue::Null => serde_json::Value::Null.serialize(serializer), + MissingNullOrValue::Value(v) => v.serialize(serializer), + } + } +} diff --git a/vegafusion-core/src/spec/visitors.rs b/vegafusion-core/src/spec/visitors.rs index 60f8292f..34fa6ca7 100644 --- a/vegafusion-core/src/spec/visitors.rs +++ b/vegafusion-core/src/spec/visitors.rs @@ -234,7 +234,7 @@ impl<'a> ChartVisitor for MakeTasksVisitor<'a> { fn visit_signal(&mut self, signal: &SignalSpec, scope: &[u32]) -> Result<()> { let signal_var = Variable::new_signal(&signal.name); - let task = if let Some(value) = &signal.value { + let task = if let Some(value) = &signal.value.as_option() { let value = TaskValue::Scalar(ScalarValue::from_json(value)?); Task::new_value(signal_var, scope, value) } else if let Some(update) = &signal.update { @@ -334,7 +334,7 @@ impl<'a> ChartVisitor for UpdateVarsChartVisitor<'a> { fn visit_signal(&mut self, signal: &SignalSpec, scope: &[u32]) -> Result<()> { // Signal is an update variable if it's not an empty stub - if signal.value.is_some() + if signal.value.as_option().is_some() || signal.init.is_some() || signal.update.is_some() || !signal.on.is_empty()