Skip to content

Commit

Permalink
Preserve null signal values on roundtrip (de)serialization
Browse files Browse the repository at this point in the history
  • Loading branch information
jonmmease committed Nov 17, 2023
1 parent bfcb79b commit 1125d4d
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 10 deletions.
2 changes: 1 addition & 1 deletion vegafusion-core/src/planning/projection_pushdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions vegafusion-core/src/planning/stitch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
};
Expand Down
28 changes: 24 additions & 4 deletions vegafusion-core/src/spec/signal.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,8 +17,8 @@ pub struct SignalSpec {
#[serde(skip_serializing_if = "Option::is_none")]
pub update: Option<String>,

#[serde(skip_serializing_if = "Option::is_none")]
pub value: Option<Value>,
#[serde(default, skip_serializing_if = "MissingNullOrValue::is_missing")]
pub value: MissingNullOrValue,

#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub on: Vec<SignalOnSpec>,
Expand All @@ -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() {
Expand Down Expand Up @@ -126,3 +126,23 @@ pub struct SignalOnSourceEvent {
#[serde(flatten)]
pub extra: HashMap<String, Value>,
}

#[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);
}
}
61 changes: 60 additions & 1 deletion vegafusion-core/src/spec/values.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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<serde_json::Value> {
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<Option<serde_json::Value>> for MissingNullOrValue {
fn from(opt: Option<serde_json::Value>) -> MissingNullOrValue {
match opt {
Some(v) => MissingNullOrValue::Value(v),
None => MissingNullOrValue::Null,
}
}
}

impl<'de> Deserialize<'de> for MissingNullOrValue {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: Deserializer<'de>,
{
Option::deserialize(deserializer).map(Into::into)
}
}

impl Serialize for MissingNullOrValue {
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
where
S: Serializer,
{
match self {
MissingNullOrValue::Missing => None::<Option<serde_json::Value>>.serialize(serializer),
MissingNullOrValue::Null => serde_json::Value::Null.serialize(serializer),
MissingNullOrValue::Value(v) => v.serialize(serializer),
}
}
}
4 changes: 2 additions & 2 deletions vegafusion-core/src/spec/visitors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 1125d4d

Please sign in to comment.