Merge w/objects (a6afa053) into w/form-controls

crates/model/src/workbook.rs: w/objects' next_object_id over free_object_ids replaces this branch's loop; the form controls' ids join the used ids it collects, as the branch's loop had them; the branch's test that a control's id is in the sequence stays beside w/objects' tests.
crates/engine/src/cmd/sheet/tabs.rs (auto-merged): the count of free ids a copied sheet asks for includes its form controls, which the copy renumbers from the same iterator; without it the controls would all take u32::MAX.

crates/engine/src/cmd/sheet/tabs.rs

@@ -164,13 +164,15 @@ pub(super) fn move_sheet(s: &mut Session, p: &Json) -> Result<Json> {
             sh.tables.clear(); // table names must be unique
             sh.preserved = None; // kept file content (form controls, OLE objects…) stays with the original
             // Object ids are unique across the workbook (selection and the `{kind, id}` commands
-            // rely on it), so the copied objects get fresh ones.
-            let mut next = cx.wb.next_object_id();
-            let mut fresh = || {
-                let id = next;
-                next = next.saturating_add(1);
-                id
-            };
+            // rely on it), so the copied objects get fresh ones (free ones, even when a file used
+            // the largest id).
+            let count = sh.charts.len() + sh.images.len() + sh.shapes.len() + sh.slicers.len() + sh.controls.len();
+            let ids = cx.wb.free_object_ids(count);
+            if ids.len() < count {
+                return Err(bad("sheet.move", "no free object ids are left for the copied objects"));
+            }
+            let mut ids = ids.into_iter();
+            let mut fresh = || ids.next().unwrap_or(u32::MAX);
             sh.charts.iter_mut().for_each(|o| o.id = fresh());
             sh.images.iter_mut().for_each(|o| o.id = fresh());
             sh.shapes.iter_mut().for_each(|o| o.id = fresh());

crates/engine/src/tests/objects.rs

@@ -291,3 +291,23 @@ fn bubble_sizes_follow_edits_like_the_other_series_references() {
     s.execute("sheet.rename", json!({"sheet": 0, "name": "Data"})).unwrap();
     assert_eq!(sizes(&s), "Data!$F$2:$F$4", "a renamed sheet is renamed in the bubble sizes");
 }
+
+/// A file can carry an object with the largest id. New objects and a copied sheet's objects then
+/// take free ids instead of the largest one again, so `{kind, id}` still names one object.
+#[test]
+fn object_ids_stay_unique_when_a_file_used_the_largest_id() {
+    let mut s = s();
+    let first = id_of(&s.execute("insert.shape", json!({"kind": "rectangle", "at": "B2"})).unwrap(), "shape");
+    // The file's shape carries the largest id (set on the model, as a loaded file would).
+    std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap().shapes.iter_mut().find(|o| o.id == first).unwrap().id = u32::MAX;
+    let second = id_of(&s.execute("insert.shape", json!({"kind": "ellipse", "at": "D2"})).unwrap(), "shape");
+    let third = id_of(&s.execute("insert.shape", json!({"kind": "triangle", "at": "F2"})).unwrap(), "shape");
+    assert!(second != u32::MAX && third != u32::MAX && second != third, "new shapes take free ids, not the largest again: {second}, {third}");
+    s.execute("sheet.move", json!({"sheet": 0, "to": 1, "copy": true})).unwrap();
+    let wb = &s.doc().unwrap().wb;
+    let mut ids: Vec<u32> = wb.sheets.iter().flat_map(|sh| sh.shapes.iter().map(|o| o.id)).collect();
+    assert_eq!(ids.len(), 6);
+    ids.sort_unstable();
+    ids.dedup();
+    assert_eq!(ids.len(), 6, "every shape id is distinct across both sheets");
+}

crates/model/src/workbook.rs

@@ -463,29 +463,55 @@ impl Workbook {
         }
         None
     }
+    /// A free id for a new chart, picture, shape, table, slicer or form control: one more than the largest in
+    /// use, or, when that is `u32::MAX` (a file can carry any id), the lowest free one.
     pub fn next_object_id(&self) -> u32 {
-        let mut m = 0;
-        for s in &self.sheets {
-            for c in &s.charts {
-                m = m.max(c.id);
-            }
-            for c in &s.images {
-                m = m.max(c.id);
-            }
-            for c in &s.shapes {
-                m = m.max(c.id);
-            }
-            for c in &s.tables {
-                m = m.max(c.id);
-            }
-            for c in &s.slicers {
-                m = m.max(c.id);
-            }
-            for c in &s.controls {
-                m = m.max(c.id);
-            }
+        self.free_object_ids(1).first().copied().unwrap_or(u32::MAX)
+    }
+
+    /// `count` distinct ids no object in the workbook uses, sorted: the ones above the largest id
+    /// in use while they last, then the lowest free ones. Fewer come back only when the id space
+    /// is exhausted.
+    pub fn free_object_ids(&self, count: usize) -> Vec<u32> {
+        let mut used: Vec<u32> = self
+            .sheets
+            .iter()
+            .flat_map(|s| {
+                s.charts
+                    .iter()
+                    .map(|o| o.id)
+                    .chain(s.images.iter().map(|o| o.id))
+                    .chain(s.shapes.iter().map(|o| o.id))
+                    .chain(s.tables.iter().map(|o| o.id))
+                    .chain(s.slicers.iter().map(|o| o.id))
+                    .chain(s.controls.iter().map(|o| o.id))
+            })
+            .collect();
+        used.sort_unstable();
+        used.dedup();
+        let max = used.last().copied().unwrap_or(0);
+        let mut out = Vec::with_capacity(count.min(1 << 16));
+        let mut next = max.checked_add(1);
+        while out.len() < count
+            && let Some(id) = next
+        {
+            out.push(id);
+            next = id.checked_add(1);
         }
-        m.saturating_add(1)
+        // Past the top: the gaps below the largest id, from 1 (0 is never an object's id).
+        let mut taken = used.iter().copied().peekable();
+        let mut id = 1u32;
+        while out.len() < count && id < max {
+            while taken.next_if(|&t| t < id).is_some() {}
+            if taken.peek() == Some(&id) {
+                taken.next();
+            } else {
+                out.push(id);
+            }
+            id += 1;
+        }
+        out.sort_unstable();
+        out
     }
 }
 
@@ -537,7 +563,7 @@ mod tests {
     }
 
     #[test]
-    fn object_ids_saturate_instead_of_overflowing() {
+    fn object_ids_never_overflow_or_repeat() {
         let mut wb = Workbook::new();
         let image = |id| crate::Image { id, anchor: Default::default(), data: Default::default(), mime: String::new(), alt: String::new() };
         assert_eq!(wb.next_object_id(), 1);
@@ -552,6 +578,40 @@ mod tests {
         if let Some(sh) = wb.sheet_mut(0) {
             sh.images.push(image(u32::MAX));
         }
-        assert_eq!(wb.next_object_id(), u32::MAX, "an id read from a file at the maximum saturates");
+        // An id read from a file at the maximum: the lowest free id, not the maximum again.
+        assert_eq!(wb.next_object_id(), 1);
+        if let Some(sh) = wb.sheet_mut(0) {
+            sh.images.push(image(1));
+            sh.images.push(image(3));
+        }
+        assert_eq!(wb.next_object_id(), 2);
+        assert_eq!(wb.free_object_ids(4), vec![2, 4, 5, 6], "the gaps, skipping 1, 3 and 7");
+        let mut wb = Workbook::new();
+        if let Some(sh) = wb.sheet_mut(0) {
+            sh.images.push(image(u32::MAX - 1));
+        }
+        assert_eq!(wb.free_object_ids(3), vec![1, 2, u32::MAX], "the top first, then from 1");
+    }
+
+    /// A file can use the largest id and the one below it: nothing is free above them, so the free
+    /// ids are the lowest unused ones from 1 (0 is never an object's id), and a large request is
+    /// served in time linear in the count and the ids in use.
+    #[test]
+    fn free_object_ids_come_from_below_when_nothing_is_free_above() {
+        let mut wb = Workbook::new();
+        let image = |id| crate::Image { id, anchor: Default::default(), data: Default::default(), mime: String::new(), alt: String::new() };
+        if let Some(sh) = wb.sheet_mut(0) {
+            sh.images.push(image(u32::MAX));
+            sh.images.push(image(u32::MAX - 1));
+            sh.images.push(image(0));
+        }
+        assert_eq!(wb.free_object_ids(3), vec![1, 2, 3]);
+        assert_eq!(wb.next_object_id(), 1);
+        // The 100,000 lowest free ids: distinct, ascending, none of them in use.
+        let many = wb.free_object_ids(100_000);
+        assert_eq!(many.len(), 100_000);
+        assert!(many.windows(2).all(|w| w[0] < w[1]), "ascending without repeats");
+        assert!(!many.contains(&0) && !many.contains(&(u32::MAX - 1)) && !many.contains(&u32::MAX));
+        assert_eq!(wb.free_object_ids(0), Vec::<u32>::new());
     }
 }