fix(confluence): say why an image is not in the bundle, as rw serve does

crates/rw-confluence/src/backend.rs

@@ -12,6 +12,14 @@ use rw_renderer::{
     replace_controls_into,
 };
 
+/// The attachment a local image references while its `src` stays as written.
+pub(crate) fn attachment_filename(src: &str) -> Cow<'_, str> {
+    let basename = src.rsplit('/').next().unwrap_or(src);
+    percent_decode_str(basename)
+        .decode_utf8()
+        .unwrap_or(Cow::Borrowed(basename))
+}
+
 /// Writes `<ac:image>` around one `<ri:…/>` resource, optionally sized and
 /// with alt text.
 ///
@@ -148,11 +156,10 @@ impl RenderBackend for ConfluenceBackend {
         let inner = if is_external(src) {
             format!(r#"ri:url ri:value="{}""#, escape_html(src))
         } else {
-            let basename = src.rsplit('/').next().unwrap_or(src);
-            let filename = percent_decode_str(basename)
-                .decode_utf8()
-                .unwrap_or(Cow::Borrowed(basename));
-            format!(r#"ri:attachment ri:filename="{}""#, escape_html(&filename))
+            format!(
+                r#"ri:attachment ri:filename="{}""#,
+                escape_html(&attachment_filename(src))
+            )
         };
         ac_image(None, (!alt.is_empty()).then_some(alt), &inner, out);
     }

crates/rw-confluence/src/images.rs

@@ -6,9 +6,11 @@ use std::sync::Arc;
 
 use parking_lot::Mutex;
 use rw_renderer::ImageSrc;
-use rw_storage::{Storage, image_content_type};
+use rw_storage::{MissingFile, Storage, image_content_type};
 use sha2::{Digest, Sha256};
 
+use crate::backend::attachment_filename;
+
 /// Names each image file after its content, and keeps it until the page shows
 /// which names it references: an image in a title H1 the backend leaves out
 /// is read, but must not be bundled.
@@ -24,6 +26,9 @@ struct State {
     names: HashMap<String, Option<String>>,
     /// The project path and content of each attachment name.
     files: HashMap<String, (String, Vec<u8>)>,
+    /// The project path of each file storage could not find, and why, by the
+    /// attachment name its page references instead.
+    missing: HashMap<String, Vec<(String, MissingFile)>>,
 }
 
 impl ImageAttachments {
@@ -39,6 +44,12 @@ impl ImageAttachments {
     pub(crate) fn take(&self, name: &str) -> Option<(String, Vec<u8>)> {
         self.state.lock().files.remove(name)
     }
+
+    /// The project path of each file storage could not find for the
+    /// attachment `name`, and why.
+    pub(crate) fn take_missing(&self, name: &str) -> Vec<(String, MissingFile)> {
+        self.state.lock().missing.remove(name).unwrap_or_default()
+    }
 }
 
 /// `image_<digest[..12]>.<ext>`: hashing the content keeps two `logo.png`
@@ -76,19 +87,28 @@ pub(crate) fn is_attachment_name(name: &str) -> bool {
 pub(crate) struct ImageNames(pub(crate) Arc<ImageAttachments>);
 
 impl ImageSrc for ImageNames {
-    fn image_src(&self, path: &str, _src: &str) -> Option<String> {
+    fn image_src(&self, path: &str, src: &str) -> Option<String> {
         let mut state = self.0.state.lock();
         if let Some(name) = state.names.get(path) {
             return name.clone();
         }
-        let name = self.0.storage.read_file(path).ok().map(|content| {
+        let name = if let Ok(content) = self.0.storage.read_file(path) {
             let name = attachment_name(path, &content);
             state
                 .files
                 .entry(name.clone())
                 .or_insert_with(|| (path.to_owned(), content));
-            name
-        });
+            Some(name)
+        } else {
+            if let Err(missing) = self.0.storage.find_file(path) {
+                state
+                    .missing
+                    .entry(attachment_filename(src).into_owned())
+                    .or_default()
+                    .push((path.to_owned(), missing));
+            }
+            None
+        };
         state.names.insert(path.to_owned(), name.clone());
         name
     }

crates/rw-confluence/src/renderer.rs

@@ -75,11 +75,22 @@ pub(crate) fn render_page(
         if attachments.contains(&name) {
             continue;
         }
-        let Some((path, content)) = images.as_ref().and_then(|images| images.take(&name)) else {
-            result.warnings.push(format!(
-                "attachment '{name}' is not in the bundle; upload it to the page yourself"
-            ));
-            continue;
+        let taken = images.as_ref().map_or(Err(Vec::new()), |images| {
+            images.take(&name).ok_or_else(|| images.take_missing(&name))
+        });
+        let (path, content) = match taken {
+            Ok(file) => file,
+            Err(missing) => {
+                if missing.is_empty() {
+                    result.warnings.push(format!(
+                        "attachment '{name}' is not in the bundle; upload it to the page yourself"
+                    ));
+                }
+                for (path, missing) in missing {
+                    result.warnings.push(format!("image `{path}` {missing}"));
+                }
+                continue;
+            }
         };
         match std::fs::write(output_dir.join(&name), content) {
             Ok(()) => attachments.push(name),
@@ -248,7 +259,7 @@ mod tests {
     use quick_xml::reader::Reader;
     use rw_diagrams::{DiagramProvider, DiagramSource};
     use rw_renderer::ProjectFiles;
-    use rw_storage::MockStorage;
+    use rw_storage::{Document, MissingFile, MockStorage, PageText, Storage, StorageError};
 
     use super::*;
     use crate::LocalProject;
@@ -585,6 +596,83 @@ mod tests {
         );
     }
 
+    fn render_from(storage: impl Storage + 'static, markdown: &str) -> (RenderResult, Vec<String>) {
+        let dir = tempfile::tempdir().expect("tempdir");
+        let opts = RenderOptions {
+            project: Some(LocalProject {
+                files: ProjectFiles::new(|_| true),
+                source_path: "docs/guide.md".to_owned(),
+                storage: Some(Arc::new(storage)),
+                project_dir: PathBuf::new(),
+                includes: None,
+            }),
+            ..RenderOptions::default()
+        };
+        render_page(markdown, &Providers::empty(), dir.path(), &opts)
+    }
+
+    #[test]
+    fn an_image_storage_cannot_find_warns_like_rw_serve() {
+        let (result, attachments) = render_from(
+            MockStorage::new(),
+            "![a](a/x.png) ![b](b/x.png) ![again](a/x.png)\n",
+        );
+        assert_eq!(attachments, [] as [String; 0]);
+        assert_eq!(
+            result.warnings,
+            [
+                "image `docs/a/x.png` not found",
+                "image `docs/b/x.png` not found"
+            ]
+        );
+    }
+
+    /// A filesystem that ignores case, as on macOS and Windows, holding
+    /// `docs/img/logo.png`.
+    struct CaseInsensitive;
+
+    impl Storage for CaseInsensitive {
+        fn scan(&self) -> Result<(Vec<Document>, Vec<String>), StorageError> {
+            Ok((Vec::new(), Vec::new()))
+        }
+
+        fn read(&self, path: &str) -> Result<PageText, StorageError> {
+            Err(StorageError::not_found(path))
+        }
+
+        fn read_file(&self, path: &str) -> Result<Vec<u8>, StorageError> {
+            Err(StorageError::not_found(path))
+        }
+
+        fn find_file(&self, _: &str) -> Result<(), MissingFile> {
+            Err(MissingFile::Misspelled {
+                on_disk: "docs/img/logo.png".to_owned(),
+            })
+        }
+
+        fn mtime(&self, path: &str) -> Result<f64, StorageError> {
+            Err(StorageError::not_found(path))
+        }
+
+        fn source_dir_in_project(&self) -> String {
+            String::new()
+        }
+    }
+
+    #[test]
+    fn a_misspelled_image_names_the_file_on_disk() {
+        let (result, attachments) = render_from(CaseInsensitive, "![Logo](img/Logo.PNG)\n");
+        assert_eq!(
+            result.html,
+            r#"<p><ac:image ac:alt="Logo"><ri:attachment ri:filename="Logo.PNG" /></ac:image></p>"#
+        );
+        assert_eq!(attachments, [] as [String; 0]);
+        assert_eq!(
+            result.warnings,
+            ["image `docs/img/Logo.PNG` not found; the file on disk is `docs/img/logo.png`"]
+        );
+    }
+
     /// Confluence rejects a storage body that is not XML.
     #[test]
     fn raw_html_void_elements_render_as_well_formed_xml() {

crates/rw-confluence/tests/render_integration.rs

@@ -552,7 +552,7 @@ fn render_leaves_images_it_cannot_bundle_to_the_publisher() {
         [
             "image `../../outside.png` is outside the project",
             "image `img/a.bmp` is not a supported image type",
-            "attachment 'missing.png' is not in the bundle; upload it to the page yourself",
+            "image `docs/img/missing.png` not found",
             "attachment 'outside.png' is not in the bundle; upload it to the page yourself",
             "attachment 'a.bmp' is not in the bundle; upload it to the page yourself",
         ]

crates/rw/tests/cli/confluence_render.rs

@@ -5,7 +5,7 @@ use std::path::Path;
 use std::process::{Command, Output, Stdio};
 
 use rw_kroki::test_support::{KrokiStub, PNG_HEADER, Reply};
-use rw_storage::MAX_FILE_SIZE;
+use rw_storage::{MAX_FILE_SIZE, MissingFile};
 
 use crate::common::rw_cmd;
 
@@ -521,16 +521,70 @@ fn render_does_not_bundle_a_file_storage_refuses() {
     assert!(output.status.success(), "{stderr}");
 
     assert_eq!(files_in(&out_dir), ["page.xhtml"]);
-    for name in ["escape.png", "env.png", "huge.png"] {
+    for (name, missing) in [
+        ("escape.png", MissingFile::NotFound),
+        ("env.png", MissingFile::NotFound),
+        ("huge.png", MissingFile::TooLarge),
+    ] {
         assert!(
-            stderr.contains(&format!(
-                "warning: attachment '{name}' is not in the bundle; upload it to the page yourself"
-            )),
+            stderr.contains(&format!("warning: image `{name}` {missing}\n")),
             "{stderr}"
         );
     }
 }
 
+#[test]
+fn render_strict_fails_on_a_missing_image() {
+    let tmp = tempfile::tempdir().expect("tempdir");
+    let out_dir = tmp.path().join("dist");
+
+    let output = rw_render(tmp.path(), "![Flow](img/flow.png)\n")
+        .arg("--out")
+        .arg(&out_dir)
+        .arg("--strict")
+        .stdin(Stdio::null())
+        .output()
+        .expect("spawn rw");
+    let stderr = String::from_utf8(output.stderr).expect("utf8 stderr");
+    assert_eq!(output.status.code(), Some(1), "{stderr}");
+    assert!(
+        stderr.contains("warning: image `img/flow.png` not found\n"),
+        "{stderr}"
+    );
+    assert!(!stderr.contains("not in the bundle"), "{stderr}");
+}
+
+/// Whether `dir` ignores case. Linux CI does not, macOS and Windows do.
+fn case_insensitive(dir: &Path) -> bool {
+    std::fs::write(dir.join("probe"), "").expect("write probe");
+    dir.join("PROBE").exists()
+}
+
+#[test]
+fn render_strict_fails_on_a_misspelled_image() {
+    let tmp = tempfile::tempdir().expect("tempdir");
+    std::fs::create_dir(tmp.path().join("img")).expect("mkdir");
+    std::fs::write(tmp.path().join("img/logo.png"), "logo").expect("write image");
+    let out_dir = tmp.path().join("dist");
+
+    let output = rw_render(tmp.path(), "![Logo](img/Logo.PNG)\n")
+        .arg("--out")
+        .arg(&out_dir)
+        .arg("--strict")
+        .stdin(Stdio::null())
+        .output()
+        .expect("spawn rw");
+    let stderr = String::from_utf8(output.stderr).expect("utf8 stderr");
+    assert_eq!(output.status.code(), Some(1), "{stderr}");
+    assert_eq!(files_in(&out_dir), ["page.xhtml"]);
+    let warning = if case_insensitive(tmp.path()) {
+        "warning: image `img/Logo.PNG` not found; the file on disk is `img/logo.png`\n"
+    } else {
+        "warning: image `img/Logo.PNG` not found\n"
+    };
+    assert!(stderr.contains(warning), "{stderr}");
+}
+
 /// There is no directory to copy into, but a local image is not a reason to
 /// fail the render the way a diagram is.
 #[test]

docs/confluence.md

@@ -114,11 +114,21 @@ become one attachment, while two different `logo.png` files in different
 folders stay two.
 
 The bundle leaves an image out when the file is missing, outside the project,
-not a [supported type](images.md#supported-types), or larger than 32 MiB, when
-the markdown file itself is outside the project, and with `--out -`, which has
-no directory to copy into. The page then references an attachment named after
-the file (`images/a%20b.png` becomes `a b.png`), the render adds a warning,
-and you upload the file to the page yourself.
+not a [supported type](images.md#supported-types), or larger than 32 MiB, and
+the render warns why, as `rw serve` does. A path in the wrong case
+([Paths](images.md#paths)) names the file on disk:
+
+```text
+warning: image `docs/img/Logo.PNG` not found; the file on disk is `docs/img/logo.png`
+```
+
+The bundle leaves every image out when the markdown file itself is outside the
+project, and with `--out -`, which has no directory to copy into. The render
+warns that the attachment is not in the bundle.
+
+Either way, the page references an attachment named after the file
+(`images/a%20b.png` becomes `a b.png`): fix the path, or upload the file to
+the page yourself.
 
 An image naming a diagram file, such as `flow.puml`, is a diagram, attached as
 `diagram_<hash>.png`; see [Diagrams from files](diagrams.md#diagrams-from-files).