engine: apply the photo album limits when reopening an album

crates/engine/src/cmd/insert/album.rs

@@ -592,9 +592,16 @@ fn infer_album(slides: &[&deckcraft_model::Slide], theme_now: &str) -> (String,
     (layout, frame.into(), captions, any && gray, theme_now.to_string())
 }
 
+/// The album's items for the dialog, within the limits `insert.photoAlbum` itself takes (items,
+/// bytes per picture and in all), checked before a picture is encoded: an album slide set can
+/// name one large picture many times.
 fn items_from_slides(doc: &deckcraft_model::Presentation, slides: &[&deckcraft_model::Slide]) -> Vec<Value> {
     let mut items = Vec::new();
-    for slide in slides {
+    let mut total = 0usize;
+    'slides: for slide in slides {
+        if items.len() >= ALBUM_MAX_ITEMS {
+            break;
+        }
         let pics: Vec<&deckcraft_model::Shape> = slide.shapes.iter().filter(|sh| matches!(sh.kind, ShapeKind::Picture { .. })).collect();
         if pics.is_empty() {
             let text = slide
@@ -612,6 +619,11 @@ fn items_from_slides(doc: &deckcraft_model::Presentation, slides: &[&deckcraft_m
         for sh in pics {
             let ShapeKind::Picture { fill } = &sh.kind else { continue };
             let media = doc.media(fill.media);
+            let size = media.map_or(0, |m| m.data.len());
+            if items.len() >= ALBUM_MAX_ITEMS || size > ALBUM_MAX_BYTES || total.saturating_add(size) > ALBUM_MAX_TOTAL {
+                break 'slides;
+            }
+            total += size;
             let name = if !sh.descr.is_empty() {
                 sh.descr.clone()
             } else {
@@ -722,3 +734,28 @@ fn insert_album(s: &mut Session, items: Vec<AlbumItem>, opts: &AlbumOpts, at: Op
     s.execute("slide.go", &json!({"index": first}))?;
     Ok(json!({"slides": slides, "first": first, "pictures": pictures, "album": album_id}))
 }
+
+#[cfg(test)]
+mod seed_tests {
+    use std::sync::Arc;
+
+    use deckcraft_model::style::PictureFill;
+    use deckcraft_model::{Shape, ShapeId, defaults};
+
+    use super::*;
+
+    #[test]
+    fn reopening_an_album_seeds_at_most_the_items_an_album_takes() {
+        // One picture named by 500 shapes: the dialog gets 48 items, not 500 encoded copies.
+        let mut p = defaults::new_presentation(None);
+        let img = p.add_media("a.png", "image/png", vec![1; 64 * 1024]).unwrap();
+        let pic = |k: u32| Shape {
+            id: ShapeId(10_000 + k),
+            kind: ShapeKind::Picture { fill: PictureFill { media: img, ..Default::default() } },
+            ..Default::default()
+        };
+        Arc::make_mut(&mut p.slides[0]).shapes = (0..500).map(pic).collect();
+        let slides: Vec<&deckcraft_model::Slide> = p.slides.iter().map(|s| s.as_ref()).collect();
+        assert_eq!(items_from_slides(&p, &slides).len(), ALBUM_MAX_ITEMS);
+    }
+}

crates/pptx/src/opc.rs

@@ -171,7 +171,6 @@ fn rels_of(doc: &xml::Doc, part: &str) -> Rels {
         let external = r.attr("TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("external"));
         let target_raw = target.to_string();
         let target = if external { target_raw.clone() } else { resolve(&base, target) };
-        out.by_id.entry(id.to_string()).or_insert(out.items.len());
         out.items.push(Rel {
             id: id.to_string(),
             kind: ty.rsplit('/').next().unwrap_or(ty).to_string(),
@@ -212,13 +211,11 @@ pub struct Rel {
 #[derive(Clone, Debug, Default)]
 pub struct Rels {
     pub items: Vec<Rel>,
-    /// Id → index of its first relationship in `items`.
-    by_id: HashMap<String, usize>,
 }
 
 impl Rels {
     pub fn get(&self, id: &str) -> Option<&Rel> {
-        self.by_id.get(id).and_then(|&i| self.items.get(i))
+        self.items.iter().find(|r| r.id == id)
     }
     pub fn target(&self, id: &str) -> Option<&str> {
         self.get(id).map(|r| r.target.as_str())
@@ -342,29 +339,23 @@ pub fn relative(from: &str, to: &str) -> String {
 #[derive(Default)]
 pub struct RelsOut {
     pub items: Vec<(String, String, String, bool)>,
-    /// (type, target, external) → id of the relationship [`RelsOut::add`] made for it.
-    added: HashMap<(String, String, bool), String>,
 }
 
 impl RelsOut {
     /// Add (or reuse) a relationship; `kind` is a full type URI or a suffix of the standard base.
     pub fn add(&mut self, kind: &str, target: &str, external: bool) -> String {
         let ty = if kind.starts_with("http") { kind.to_string() } else { format!("{RT_BASE}{kind}") };
-        let key = (ty, target.to_string(), external);
-        if let Some(id) = self.added.get(&key) {
+        if let Some((id, ..)) = self.items.iter().find(|(_, t, tg, e)| *t == ty && tg == target && *e == external) {
             return id.clone();
         }
         let id = format!("rId{}", self.items.len() + 1);
-        self.items.push((id.clone(), key.0.clone(), key.1.clone(), external));
-        self.added.insert(key, id.clone());
+        self.items.push((id.clone(), ty, target.to_string(), external));
         id
     }
     /// Always add a new relationship even if an identical one exists.
     pub fn add_new(&mut self, kind: &str, target: &str, external: bool) -> String {
         let ty = if kind.starts_with("http") { kind.to_string() } else { format!("{RT_BASE}{kind}") };
         let id = format!("rId{}", self.items.len() + 1);
-        // `add` reuses the first relationship for a target, whichever call made it.
-        self.added.entry((ty.clone(), target.to_string(), external)).or_insert_with(|| id.clone());
         self.items.push((id.clone(), ty, target.to_string(), external));
         id
     }
@@ -509,27 +500,6 @@ mod tests {
         assert!(p.get("e").is_some());
     }
 
-    #[test]
-    fn many_relationships_are_found_by_index_and_first_id_wins() {
-        let n = 50_000;
-        let mut x = String::from(r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships">"#);
-        for i in 0..n {
-            x.push_str(&format!(r#"<Relationship Id="rId{i}" Type="{RT_BASE}image" Target="../media/image{i}.png"/>"#));
-        }
-        x.push_str(&format!(r#"<Relationship Id="rId0" Type="{RT_BASE}image" Target="../media/dup.png"/></Relationships>"#));
-        let rels = rels_from_bytes(x.as_bytes(), "ppt/slides/slide1.xml");
-        assert_eq!(rels.target("rId0"), Some("ppt/media/image0.png"), "the first of a repeated id wins");
-        assert!((0..n).all(|i| rels.target(&format!("rId{i}")) == Some(&*format!("ppt/media/image{i}.png"))));
-
-        let mut out = RelsOut::default();
-        let ids: Vec<String> = (0..n).map(|i| out.add("hyperlink", &format!("https://example.com/{i}"), true)).collect();
-        assert_eq!(out.add("hyperlink", "https://example.com/7", true), ids[7], "an identical target reuses its id");
-        assert_eq!(out.add("hyperlink", "https://example.com/7", false), format!("rId{}", n + 1), "internal is distinct");
-        let fresh = out.add_new("image", "../media/a.png", false);
-        assert_ne!(out.add_new("image", "../media/a.png", false), fresh);
-        assert_eq!(out.add("image", "../media/a.png", false), fresh, "add reuses the first add_new made");
-    }
-
     #[test]
     fn backslash_zip_entry_names_are_found() {
         let z = zip_of(&[("PPT\\Slides\\Slide1.xml", 3)]);