diff --git a/crates/rw-confluence/src/backend.rs b/crates/rw-confluence/src/backend.rs index a312f00a..2482da19 100644 --- a/crates/rw-confluence/src/backend.rs +++ b/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 `` around one `` 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); } diff --git a/crates/rw-confluence/src/images.rs b/crates/rw-confluence/src/images.rs index 0167dacf..dd3e108b 100644 --- a/crates/rw-confluence/src/images.rs +++ b/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>, /// The project path and content of each attachment name. files: HashMap)>, + /// The project path of each file storage could not find, and why, by the + /// attachment name its page references instead. + missing: HashMap>, } impl ImageAttachments { @@ -39,6 +44,12 @@ impl ImageAttachments { pub(crate) fn take(&self, name: &str) -> Option<(String, Vec)> { 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_.`: 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); impl ImageSrc for ImageNames { - fn image_src(&self, path: &str, _src: &str) -> Option { + fn image_src(&self, path: &str, src: &str) -> Option { 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 } diff --git a/crates/rw-confluence/src/renderer.rs b/crates/rw-confluence/src/renderer.rs index 930100c9..2640502e 100644 --- a/crates/rw-confluence/src/renderer.rs +++ b/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) { + 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, Vec), StorageError> { + Ok((Vec::new(), Vec::new())) + } + + fn read(&self, path: &str) -> Result { + Err(StorageError::not_found(path)) + } + + fn read_file(&self, path: &str) -> Result, 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 { + 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#"

"# + ); + 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() { diff --git a/crates/rw-confluence/tests/render_integration.rs b/crates/rw-confluence/tests/render_integration.rs index 497fa5bc..b25ed5bd 100644 --- a/crates/rw-confluence/tests/render_integration.rs +++ b/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", ] diff --git a/crates/rw/tests/cli/confluence_render.rs b/crates/rw/tests/cli/confluence_render.rs index e0c850b4..907d03b1 100644 --- a/crates/rw/tests/cli/confluence_render.rs +++ b/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] diff --git a/docs/confluence.md b/docs/confluence.md index fb705cef..50c8b958 100644 --- a/docs/confluence.md +++ b/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_.png`; see [Diagrams from files](diagrams.md#diagrams-from-files).