From 13b1e50f4936f2404fd667a19377fd42c9d8b71e Mon Sep 17 00:00:00 2001
From: Jon Mease <jonmmease@gmail.com>
Date: Thu, 19 Oct 2023 15:19:04 -0400
Subject: [PATCH] Make patching fail if there are changes to any dataset (#406)

---
 vegafusion-core/src/patch.rs | 63 ++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 10 deletions(-)

diff --git a/vegafusion-core/src/patch.rs b/vegafusion-core/src/patch.rs
index 493186e4..7b5df244 100644
--- a/vegafusion-core/src/patch.rs
+++ b/vegafusion-core/src/patch.rs
@@ -4,7 +4,7 @@ use crate::spec::chart::{ChartSpec, ChartVisitor};
 use crate::spec::data::DataSpec;
 use crate::spec::values::StringOrSignalSpec;
 use itertools::Itertools;
-use json_patch::{diff, patch};
+use json_patch::{diff, patch, PatchOperation};
 use serde_json::Value;
 
 /// Attempt to apply the difference between two vega specs to a third pre-transformed spec
@@ -31,6 +31,21 @@ pub fn patch_pre_transformed_spec(
         &serde_json::to_value(planned_spec2)?,
     );
 
+    // Do not apply patch if there are changes to any datasets
+    for patch_op in &spec_patch.0 {
+        let path = match patch_op {
+            PatchOperation::Add(op) => &op.path,
+            PatchOperation::Remove(op) => &op.path,
+            PatchOperation::Replace(op) => &op.path,
+            PatchOperation::Move(op) => &op.path,
+            PatchOperation::Copy(op) => &op.path,
+            PatchOperation::Test(op) => &op.path,
+        };
+        if path.contains("/data/") {
+            return Ok(None);
+        }
+    }
+
     // Attempt to apply patch to pre-transformed spec
     let mut pre_transformed_spec2 = serde_json::to_value(pre_transformed_spec1)?;
     if patch(&mut pre_transformed_spec2, spec_patch.0.as_slice()).is_err() {
@@ -119,8 +134,8 @@ mod tests {
     use crate::spec::chart::ChartSpec;
     use serde_json::{json, Value};
 
-    fn histogram(fill: Value, max_bins: u32) -> ChartSpec {
-        serde_json::from_value(json!(
+    fn histogram(fill: Value, max_bins: u32, add_dataset: bool) -> ChartSpec {
+        let mut chart_spec: ChartSpec = serde_json::from_value(json!(
             {
               "$schema": "https://vega.github.io/schema/vega/v5.json",
               "background": "white",
@@ -245,7 +260,21 @@ mod tests {
                 }
               ]
             }
-        )).unwrap()
+        )).unwrap();
+
+        if add_dataset {
+            chart_spec.data.push(
+                serde_json::from_value(json!(
+                    {
+                        "name": "added_dataset",
+                        "values": [{"a": 1, "b": 2}]
+                    }
+                ))
+                .unwrap(),
+            )
+        }
+
+        chart_spec
     }
 
     fn pre_transformed_histogram(fill: Value) -> ChartSpec {
@@ -455,9 +484,9 @@ mod tests {
 
     #[test]
     fn test_patch_color_succeeds() {
-        let spec1: ChartSpec = histogram(json!({"value": "blue"}), 10);
+        let spec1: ChartSpec = histogram(json!({"value": "blue"}), 10, false);
 
-        let spec2: ChartSpec = histogram(json!({"value": "red"}), 10);
+        let spec2: ChartSpec = histogram(json!({"value": "red"}), 10, false);
 
         let pre_transformed_spec1: ChartSpec = pre_transformed_histogram(json!({"value": "blue"}));
 
@@ -478,8 +507,8 @@ mod tests {
         let fill_value = json!({"value": "blue"});
         let fill_array = json!([{"test": "2 < 3", "value": "red"}, {"value": "green"}]);
 
-        let spec1: ChartSpec = histogram(fill_value.clone(), 10);
-        let spec2: ChartSpec = histogram(fill_array.clone(), 10);
+        let spec1: ChartSpec = histogram(fill_value.clone(), 10, false);
+        let spec2: ChartSpec = histogram(fill_array.clone(), 10, false);
         let pre_transformed_spec1: ChartSpec = pre_transformed_histogram(fill_value);
 
         let pre_transform_spec2 =
@@ -499,9 +528,9 @@ mod tests {
 
     #[test]
     fn test_patch_max_bins_fails() {
-        let spec1: ChartSpec = histogram(json!({"value": "blue"}), 10);
+        let spec1: ChartSpec = histogram(json!({"value": "blue"}), 10, false);
 
-        let spec2: ChartSpec = histogram(json!({"value": "blue"}), 20);
+        let spec2: ChartSpec = histogram(json!({"value": "blue"}), 20, false);
 
         let pre_transformed_spec1: ChartSpec = pre_transformed_histogram(json!({"value": "blue"}));
 
@@ -545,4 +574,18 @@ mod tests {
 
         assert!(pre_transform_spec2.is_none());
     }
+
+    #[test]
+    fn test_patch_add_dataset_fails() {
+        let spec1: ChartSpec = histogram(json!({"value": "blue"}), 10, false);
+
+        let spec2: ChartSpec = histogram(json!({"value": "red"}), 10, true);
+
+        let pre_transformed_spec1: ChartSpec = pre_transformed_histogram(json!({"value": "blue"}));
+
+        let pre_transform_spec2 =
+            patch_pre_transformed_spec(&spec1, &pre_transformed_spec1, &spec2)
+                .expect("Expected patch_pre_transformed_spec to succeed");
+        assert!(pre_transform_spec2.is_none());
+    }
 }