xlsx: repair repeated sheet names through a set of assigned names

Importing a workbook whose sheets share a name rescanned every sheet for
every suffix tried, so a file of repeated names opened in cubic time.
SheetNames keeps the assigned names and the next suffix per stem.

crates/xlsx/src/ods.rs

@@ -317,7 +317,7 @@ impl Import {
                     return Err(IoError::TooLarge("ODS contains more than 1024 sheets".into()));
                 }
                 let original = attr(reader, e, TABLE, b"name")?.unwrap_or_default();
-                let mut name = crate::read::unique_name(&self.wb, original.trim_matches('\''), self.wb.sheets.len());
+                let mut name = crate::read::SheetNames::of(&self.wb).unique(original.trim_matches('\''), self.wb.sheets.len());
                 if self.wb.check_sheet_name(&name, None).is_err() {
                     name = self.wb.next_sheet_name();
                 }

crates/xlsx/src/read.rs

@@ -1,5 +1,6 @@
 //! Reading the workbook part and everything hanging off it.
 
+use std::collections::{HashMap, HashSet};
 use std::sync::Arc;
 
 use gridcraft_core::{DateSystem, RangeRef};
@@ -304,6 +305,7 @@ pub fn read_xlsx<'a>(bytes: &'a [u8], known_function: Option<&'a dyn Fn(&str) ->
     let mut file_to_model: Vec<Option<usize>> = vec![];
     let mut pivot_caches = crate::pivot::CachesIn::new(&wb_xml, &wb_rels);
     let sheet_els: Vec<El> = wb_xml.child("sheets").map(|s| s.kids("sheet").cloned().collect()).unwrap_or_default();
+    let mut names = SheetNames::default();
     for (i, se) in sheet_els.iter().enumerate() {
         let name = se.attr("name").unwrap_or("").to_string();
         let rel = se.attr("id").and_then(|id| wb_rels.iter().find(|r| r.id == id)).cloned();
@@ -322,7 +324,7 @@ pub fn read_xlsx<'a>(bytes: &'a [u8], known_function: Option<&'a dyn Fn(&str) ->
             file_to_model.push(None);
             continue;
         }
-        let name = unique_name(&wb, &name, i);
+        let name = names.unique(&name, i);
         let mut sheet = match crate::sheet_read::read_sheet(&mut cx, &rel.target, &name) {
             Ok(s) => s,
             Err(IoError::TooLarge(m)) => return Err(IoError::TooLarge(m)),
@@ -455,15 +457,44 @@ pub fn read_xlsx<'a>(bytes: &'a [u8], known_function: Option<&'a dyn Fn(&str) ->
     Ok((wb, cx.report))
 }
 
-pub(crate) fn unique_name(wb: &Workbook, name: &str, i: usize) -> String {
-    let mut base: String = name.chars().filter(|c| !matches!(c, ':' | '\\' | '/' | '?' | '*' | '[' | ']')).take(31).collect();
-    if base.trim().is_empty() {
-        base = format!("Sheet{}", i + 1);
+/// The sheet names an import has assigned so far, for repairing repeated ones.
+#[derive(Default)]
+pub(crate) struct SheetNames {
+    /// Assigned names, ASCII-lowercased.
+    taken: HashSet<String>,
+    /// For each lowercased stem, the next suffix to try: a file full of repeated names would
+    /// otherwise retry every occupied suffix for every sheet.
+    next: HashMap<String, u64>,
+}
+
+impl SheetNames {
+    pub(crate) fn of(wb: &Workbook) -> Self {
+        SheetNames { taken: wb.sheets.iter().map(|s| s.name.to_ascii_lowercase()).collect(), next: HashMap::new() }
     }
-    if wb.sheet_index(&base).is_none() {
-        return base;
+
+    /// A valid name for the i-th sheet, unique among the names assigned so far, which it joins.
+    pub(crate) fn unique(&mut self, name: &str, i: usize) -> String {
+        let mut base: String = name.chars().filter(|c| !matches!(c, ':' | '\\' | '/' | '?' | '*' | '[' | ']')).take(31).collect();
+        if base.trim().is_empty() {
+            base = format!("Sheet{}", i + 1);
+        }
+        let name = if self.taken.contains(&base.to_ascii_lowercase()) {
+            let stem: String = base.chars().take(25).collect();
+            let next = self.next.entry(stem.to_ascii_lowercase()).or_insert(2);
+            let found = (*next..).find(|n| !self.taken.contains(&format!("{stem} ({n})").to_ascii_lowercase()));
+            match found {
+                Some(n) => {
+                    *next = n + 1;
+                    format!("{stem} ({n})")
+                }
+                None => base,
+            }
+        } else {
+            base
+        };
+        self.taken.insert(name.to_ascii_lowercase());
+        name
     }
-    (2..).map(|n| format!("{} ({n})", base.chars().take(25).collect::<String>())).find(|n| wb.sheet_index(n).is_none()).unwrap_or(base)
 }
 
 /// Strips a sheet qualifier (`'My Sheet'!$A$1` → `$A$1`).

crates/xlsx/src/tests/sheet_names.rs

@@ -35,3 +35,17 @@ fn sheet_ids_survive_a_save() {
     let (again, _) = crate::read_xlsx(&write_xlsx(&read).unwrap()).unwrap();
     assert_eq!(again.sheets.iter().map(|s| s.file_id).collect::<Vec<_>>(), ids);
 }
+
+/// Repairing repeated names takes one set lookup per candidate, so thousands of sheets sharing
+/// a name are renamed without rescanning the workbook for every suffix tried.
+#[test]
+fn repeated_sheet_names_are_repaired_without_rescans() {
+    let mut names = crate::read::SheetNames::default();
+    assert_eq!(names.unique("Data", 0), "Data");
+    assert_eq!(names.unique("data", 1), "data (2)");
+    assert_eq!(names.unique("", 2), "Sheet3");
+    assert_eq!(names.unique("Data (3)", 3), "Data (3)");
+    for i in 4..100_000 {
+        assert_eq!(names.unique("Data", i), format!("Data ({i})"));
+    }
+}