Model: one damaged macro no longer stops a JSON workbook opening

Workbook::macros was read straight into typed macros, so one entry this
build couldn't read (a step type from a later build, an unknown
onConflict value, an entry that isn't an object) failed the whole file
with "not a GridCraft JSON workbook". The field is now read as raw
entries into Workbook::macros_read, and macros::load reads each one
with parse_macro, as for xlsx: short-form steps are accepted, the depth
limit is checked before deserializing, and a bad entry is dropped with
the same warning. Only the first 1,000 entries are read, as in xlsx.

crates/engine/src/macros/mod.rs

@@ -103,6 +103,15 @@ pub fn with_names(wb: &Workbook) -> crate::Result<Option<Workbook>> {
 pub fn load(wb: &mut Workbook) -> Vec<String> {
     let mut warnings = Vec::new();
     let mut bad = 0usize;
+    // Entries of the JSON format's field, each read on its own so a damaged one drops alone.
+    let read = std::mem::take(&mut wb.macros_read);
+    bad = bad.saturating_add(read.len().saturating_sub(gridcraft_model::MAX_MACROS));
+    for v in read.iter().take(gridcraft_model::MAX_MACROS) {
+        match parse_macro(v) {
+            Ok(m) => wb.macros.push(m),
+            Err(_) => bad += 1,
+        }
+    }
     // One pass takes the hidden macro names out of the workbook, grouped by macro in saved order.
     let mut stored: BTreeMap<(u64, String), Vec<DefinedName>> = BTreeMap::new();
     let mut kept = Vec::new();
@@ -126,7 +135,7 @@ pub fn load(wb: &mut Workbook) -> Vec<String> {
             Err(_) => bad += 1,
         }
     }
-    // Macros that came as a field (the JSON format) go through the same checks.
+    // All of them go through the same limits, and a clashing name gets a suffix.
     let mut all = std::mem::take(&mut wb.macros);
     all.extend(found);
     for m in all {

crates/engine/src/macros/tests.rs

@@ -168,9 +168,10 @@ fn damaged_or_oversized_macros_are_dropped_with_a_warning() {
     wb.macros[0].steps = vec![Step::command("x", json!({})); MAX_MACRO_STEPS + 1];
     let bytes = crate::io::save_bytes(&wb, "many.json").unwrap();
     let (back, warnings) = crate::io::open_bytes("many.json", &bytes).unwrap();
-    assert_eq!(back.macros.len(), MAX_MACROS);
+    // Only the first MAX_MACROS entries are read, as in xlsx.
+    assert_eq!(back.macros.len(), MAX_MACROS - 1);
     assert_eq!(back.macros[0].name, "M1");
-    assert!(warnings[0].starts_with("5 macros"), "{warnings:?}");
+    assert!(warnings[0].starts_with("6 macros"), "{warnings:?}");
 
     // A macro too large for the hidden names is refused, so a save never fails on it.
     let huge = "x".repeat(crate::cmd::stash::MAX_ITEM_CHARS);
@@ -254,3 +255,26 @@ fn many_hidden_macro_names_are_read_in_one_pass() {
     assert!(warnings.len() == 1 && warnings[0].starts_with(&format!("{items} macros")), "{warnings:?}");
     assert!(took < std::time::Duration::from_secs(3), "{took:?}");
 }
+
+#[test]
+fn a_damaged_json_macro_drops_alone() {
+    // One macro this build can't read (a newer step type, an unknown value, the wrong shape) is
+    // dropped with the usual warning; the workbook and its other macros open.
+    let mut wb = Workbook::new();
+    wb.sheet_mut(0).unwrap().set_value(gridcraft_core::CellRef::new(0, 0), gridcraft_core::Value::text("kept"));
+    let mut v: serde_json::Value = serde_json::from_slice(&crate::io::save_bytes(&wb, "book.json").unwrap()).unwrap();
+    v["macros"] = json!([
+        {"name": "Good"},
+        {"name": "Newer", "steps": [{"type": "fromALaterBuild"}]},
+        {"name": "Odd", "output": {"onConflict": "explode"}},
+        {"name": "Short", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "x"}}]},
+        "not a macro"
+    ]);
+    let (back, warnings) = crate::io::open_bytes("book.json", v.to_string().as_bytes()).unwrap();
+    let names: Vec<&str> = back.macros.iter().map(|m| m.name.as_str()).collect();
+    assert_eq!(names, ["Good", "Short"]);
+    assert!(matches!(&back.macros[1].steps[0].kind, StepKind::Command { id, .. } if id == "cell.set"));
+    assert!(back.macros_read.is_empty());
+    assert!(warnings.len() == 1 && warnings[0].starts_with("3 macros"), "{warnings:?}");
+    assert_eq!(back.sheet(0).unwrap().value(gridcraft_core::CellRef::new(0, 0)), gridcraft_core::Value::text("kept"));
+}

crates/model/src/macros.rs

@@ -119,6 +119,16 @@ impl Macro {
     }
 }
 
+/// Reads the `macros` field of a JSON workbook as raw entries, whatever they hold, so they can be
+/// checked one at a time.
+pub(crate) fn entries<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Vec<serde_json::Value>, D::Error> {
+    Ok(match serde_json::Value::deserialize(d)? {
+        serde_json::Value::Array(list) => list,
+        serde_json::Value::Null => vec![],
+        other => vec![other],
+    })
+}
+
 /// A range the macro works on, with the shape and content it expects.
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 #[serde(default, rename_all = "camelCase")]

crates/model/src/workbook.rs

@@ -115,8 +115,13 @@ pub struct Workbook {
     pub csv_delimiter: Option<u8>,
     /// GridCraft macros (typed command lists, not VBA), in the order the macro list shows them.
     /// The JSON format keeps them as this field; xlsx keeps them in hidden defined names.
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    #[serde(default, skip_serializing_if = "Vec::is_empty", skip_deserializing)]
     pub macros: Vec<crate::macros::Macro>,
+    /// The `macros` entries of a JSON file as read, before the engine checks each one on its own
+    /// and moves it into `macros`, so one damaged or newer macro doesn't stop the file opening.
+    /// Never written.
+    #[serde(rename = "macros", default, skip_serializing, deserialize_with = "crate::macros::entries")]
+    pub macros_read: Vec<serde_json::Value>,
     /// Unmodelled file content kept for saving (package root and workbook level). Not in JSON.
     #[serde(skip)]
     pub preserved: Option<Arc<crate::preserved::Preserved>>,
@@ -151,6 +156,7 @@ impl Workbook {
             vba_project: None,
             csv_delimiter: None,
             macros: vec![],
+            macros_read: vec![],
             preserved: None,
             source_dir: None,
         }