diff --git a/crates/xlsx/src/passthrough/write.rs b/crates/xlsx/src/passthrough/write.rs index 8b8d24591..5b294a500 100644 --- a/crates/xlsx/src/passthrough/write.rs +++ b/crates/xlsx/src/passthrough/write.rs @@ -254,27 +254,37 @@ pub(crate) fn sheet_names_in(formula: &str) -> Vec { /// Whether a kept chart sheet can be written back: its chart's series must still refer to sheets /// that exist under the same names (the chart is kept as it was, so a series on a deleted or /// renamed sheet would be left pointing at nothing), and no worksheet may have taken its name. -fn chart_sheet_usable(wb: &Workbook, p: &Preserved, s: &PreservedSheet) -> bool { +/// +/// `refs` caches, by relationship id, the sheet names a chart sheet's graph refers to (`None` +/// when the relationship is missing): entries sharing one graph are analysed once. +fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet, refs: &mut HashMap<&'p str, Option>>) -> bool { let same = |a: &str, b: &str| a.chars().take(31).collect::().eq_ignore_ascii_case(&b.chars().take(31).collect::()); if wb.sheets.iter().any(|w| same(&w.name, &s.name)) { return false; } - let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false }; - let parts = reachable(p, root); - let mut formulas = vec![]; - for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) { - if let Ok(x) = xml::parse(&q.data) { - chart_formulas(&x, &mut formulas); + let names = refs.entry(s.rel_id.as_str()).or_insert_with(|| { + let root = p.rels.iter().find(|r| r.id == s.rel_id)?; + let parts = reachable(p, root); + let mut formulas = vec![]; + for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) { + if let Ok(x) = xml::parse(&q.data) { + chart_formulas(&x, &mut formulas); + } } - } - formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n))) + Some(formulas.iter().flat_map(|f| sheet_names_in(f)).collect()) + }); + let Some(names) = names else { return false }; + names.iter().all(|n| wb.sheets.iter().any(|w| same(&w.name, n))) } /// Writes the workbook's kept parts and relationships. Chart sheets that can't be written back /// (see [`chart_sheet_usable`]) are left out with the parts only they reach. pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> WorkbookKept { let Some(p) = wb.preserved.as_deref() else { return WorkbookKept::default() }; - let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s)); + let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = { + let mut refs = HashMap::new(); + p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s, &mut refs)) + }; let root_of = |s: &PreservedSheet| p.rels.iter().find(|r| r.id == s.rel_id); let dropped_ids: HashSet<&str> = dropped.iter().map(|s| s.rel_id.as_str()).collect(); // Parts reached only by dropped chart sheets go with them. diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index 28113be0d..7ff597f2e 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -696,6 +696,32 @@ fn chartsheet_package() -> Vec { ]) } +/// Thousands of chart-sheet entries naming one chart sheet's relationship: its graph is walked +/// and its charts parsed once when saving, not once per entry. +#[test] +fn chart_sheet_entries_sharing_a_graph_are_checked_once() { + let bytes = chartsheet_package(); + let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut parts: Vec<(String, Vec)> = Vec::new(); + for i in 0..z.len() { + let mut f = z.by_index(i).unwrap(); + let mut data = Vec::new(); + f.read_to_end(&mut data).unwrap(); + parts.push((f.name().to_string(), data)); + } + let extra: String = (0..3_000).map(|i| format!(r#""#, 100 + i)).collect(); + for (name, data) in &mut parts { + if name == "xl/workbook.xml" { + *data = String::from_utf8(data.clone()).unwrap().replace("", &format!("{extra}")).into_bytes(); + } + } + let refs: Vec<(&str, &[u8])> = parts.iter().map(|(n, d)| (n.as_str(), d.as_slice())).collect(); + let (wb, _) = read_xlsx(&make_zip(&refs)).unwrap(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + u.assert_valid(); + assert!(written_sheets(&u).iter().any(|(n, _)| n == "Chart1")); +} + /// The sheets of a written workbook: (name, state) in file order. fn written_sheets(u: &Unpacked) -> Vec<(String, String)> { let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap();