diff --git a/crates/rari-cli/main.rs b/crates/rari-cli/main.rs index 9a81d7d6..8b345c4e 100644 --- a/crates/rari-cli/main.rs +++ b/crates/rari-cli/main.rs @@ -18,6 +18,7 @@ use rari_doc::pages::types::doc::Doc; use rari_doc::reader::read_docs_parallel; use rari_doc::search_index::build_search_index; use rari_doc::utils::TEMPL_RECORDER_SENDER; +use rari_tools::add_redirect::add_redirect; use rari_tools::history::gather_history; use rari_tools::popularities::update_popularities; use rari_tools::r#move::r#move; @@ -66,6 +67,7 @@ enum ContentSubcommand { /// Moves content from one slug to another Move(MoveArgs), Delete(DeleteArgs), + AddRedirect(AddRedirectArgs), } #[derive(Args)] @@ -89,6 +91,12 @@ struct DeleteArgs { assume_yes: bool, } +#[derive(Args)] +struct AddRedirectArgs { + from_url: String, + to_url: String, +} + #[derive(Args)] struct UpdateArgs { #[arg(long)] @@ -357,6 +365,9 @@ fn main() -> Result<(), Error> { args.assume_yes, )?; } + ContentSubcommand::AddRedirect(args) => { + add_redirect(&args.from_url, &args.to_url)?; + } }, Commands::Update(args) => update(args.version)?, } diff --git a/crates/rari-tools/src/add_redirect.rs b/crates/rari-tools/src/add_redirect.rs new file mode 100644 index 00000000..c21c074a --- /dev/null +++ b/crates/rari-tools/src/add_redirect.rs @@ -0,0 +1,181 @@ +use std::borrow::Cow; + +use console::Style; +use rari_doc::resolve::{url_meta_from, UrlMeta}; +use rari_types::locale::Locale; + +use crate::error::ToolError; +use crate::redirects::add_redirects; + +pub fn add_redirect(from_url: &str, to_url: &str) -> Result<(), ToolError> { + do_add_redirect(from_url, to_url)?; + + let green = Style::new().green(); + let bold = Style::new().bold(); + + println!( + "{} {} {} {}", + green.apply_to("Saved"), + bold.apply_to(from_url), + green.apply_to("→"), + bold.apply_to(to_url), + ); + + Ok(()) +} + +fn do_add_redirect(from_url: &str, to_url: &str) -> Result<(), ToolError> { + validate_args(from_url, to_url)?; + let UrlMeta { + locale: from_locale, + .. + } = url_meta_from(from_url)?; + if !to_url.starts_with("http") { + if from_url == to_url { + return Err(ToolError::InvalidRedirectToURL(format!( + "redirect url is the same as the from url: {to_url}" + ))); + } + + let UrlMeta { + locale: to_locale, .. + } = url_meta_from(to_url)?; + + // Enforce that the locales match or the target is in the default Locale + if from_locale != to_locale && to_locale != Locale::EnUs { + return Err(ToolError::InvalidRedirectToURL(format!( + "redirect url locales do not match: {from_locale} != {to_locale}" + ))); + } + } + add_redirects(from_locale, &[(from_url.to_owned(), to_url.to_owned())])?; + Ok(()) +} + +fn validate_args(from_url: &str, to_url: &str) -> Result<(), ToolError> { + if from_url.is_empty() { + return Err(ToolError::InvalidUrl(Cow::Borrowed( + "from_url cannot be empty", + ))); + } + if to_url.is_empty() { + return Err(ToolError::InvalidUrl(Cow::Borrowed( + "to_url cannot be empty", + ))); + } + if from_url.contains("#") { + return Err(ToolError::InvalidUrl(Cow::Borrowed( + "from_url cannot contain '#'", + ))); + } + Ok(()) +} + +// These tests use file system fixtures to simulate content and translated content. +// The file system is a shared resource, so we force tests to be run serially, +// to avoid concurrent fixture management issues. +// Using `file_serial` as a synchonization lock, we run all tests using +// the same `key` (here: file_fixtures) to be serialized across modules. +#[cfg(test)] +use serial_test::file_serial; +#[cfg(test)] +#[file_serial(file_fixtures)] +mod test { + use rari_types::locale::Locale; + + use super::*; + use crate::tests::fixtures::docs::DocFixtures; + use crate::tests::fixtures::redirects::RedirectFixtures; + use crate::utils::test_utils::get_redirects_map; + + #[test] + fn test_add_redirect() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/SomethingElse".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new( + &vec![( + "docs/Web/API/Something".to_string(), + "docs/Web/API/SomethingElse".to_string(), + )], + Locale::EnUs, + ); + + let result = do_add_redirect( + "/en-US/docs/Web/API/ExampleGone", + "/en-US/docs/Web/API/ExampleOne", + ); + assert!(result.is_ok()); + + let redirects = get_redirects_map(Locale::EnUs); + assert_eq!(redirects.len(), 2); + assert!(redirects.contains_key("/en-US/docs/Web/API/ExampleGone")); + assert_eq!( + redirects.get("/en-US/docs/Web/API/ExampleGone").unwrap(), + "/en-US/docs/Web/API/ExampleOne" + ); + } + + #[test] + fn test_add_redirect_missing_target() { + let slugs = vec!["Web/API/ExampleOne".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + + let result = do_add_redirect( + "/en-US/docs/Web/API/ExampleGone", + "/en-US/docs/Web/API/ExampleMissing", + ); + assert!(result.is_err()); + } + + #[test] + fn test_add_redirect_differing_locales() { + let slugs = vec!["Web/API/ExampleOne".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _docs_pt = DocFixtures::new(&slugs, Locale::PtBr); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + let _redirects_pt = RedirectFixtures::new(&vec![], Locale::PtBr); + + // Locales do not match + let result = do_add_redirect( + "/en-US/docs/Web/API/ExampleGone", + "/pt-BR/docs/Web/API/ExampleOne", + ); + assert!(result.is_err()); + assert!(matches!(result, Err(ToolError::InvalidRedirectToURL(_)))); + + // Target is en-US, even if locales differ + let result = do_add_redirect( + "/pt-BR/docs/Web/API/ExampleGone", + "/en-US/docs/Web/API/ExampleOne", + ); + assert!(result.is_ok()); + + let redirects = get_redirects_map(Locale::PtBr); + assert!(redirects.contains_key("/pt-BR/docs/Web/API/ExampleGone")); + assert_eq!( + redirects.get("/pt-BR/docs/Web/API/ExampleGone").unwrap(), + "/en-US/docs/Web/API/ExampleOne" + ); + } + + #[test] + fn test_add_redirect_external() { + let slugs = vec!["Web/API/ExampleOne".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + + let result = do_add_redirect("/en-US/docs/Web/API/ExampleGone", "https://example.com/"); + assert!(result.is_ok()); + + let redirects = get_redirects_map(Locale::EnUs); + assert!(redirects.contains_key("/en-US/docs/Web/API/ExampleGone")); + assert_eq!( + redirects.get("/en-US/docs/Web/API/ExampleGone").unwrap(), + "https://example.com/" + ); + } +} diff --git a/crates/rari-tools/src/error.rs b/crates/rari-tools/src/error.rs index 23d99f75..66e0125d 100644 --- a/crates/rari-tools/src/error.rs +++ b/crates/rari-tools/src/error.rs @@ -10,6 +10,8 @@ use thiserror::Error; pub enum ToolError { #[error("Invalid slug: {0}")] InvalidSlug(Cow<'static, str>), + #[error("Invalid url: {0}")] + InvalidUrl(Cow<'static, str>), #[error("Git error: {0}")] GitError(String), diff --git a/crates/rari-tools/src/lib.rs b/crates/rari-tools/src/lib.rs index 41143aed..68d182da 100644 --- a/crates/rari-tools/src/lib.rs +++ b/crates/rari-tools/src/lib.rs @@ -1,3 +1,4 @@ +pub mod add_redirect; pub mod error; pub mod git; pub mod history; diff --git a/crates/rari-tools/src/move.rs b/crates/rari-tools/src/move.rs index 222e9d6c..d1c66a6b 100644 --- a/crates/rari-tools/src/move.rs +++ b/crates/rari-tools/src/move.rs @@ -318,6 +318,7 @@ mod test { "Web/API/ExampleOne".to_string(), "Web/API/ExampleOne/SubExampleOne".to_string(), "Web/API/ExampleOne/SubExampleTwo".to_string(), + "Web/API/SomethingElse".to_string(), ]; let redirects = vec![ ( @@ -353,7 +354,7 @@ mod test { Locale::EnUs, false, ); - + println!("result: {:?}", result); assert!(result.is_ok()); let result = result.unwrap(); assert!(result.len() == 3); @@ -428,6 +429,7 @@ mod test { "Web/API/ExampleOne".to_string(), "Web/API/ExampleOne/SubExampleOne".to_string(), "Web/API/ExampleOne/SubExampleTwo".to_string(), + "Web/API/SomethingElse".to_string(), ]; let redirects = vec![ ( @@ -463,7 +465,6 @@ mod test { Locale::PtBr, false, ); - assert!(result.is_ok()); let result = result.unwrap(); assert!(result.len() == 3); diff --git a/crates/rari-tools/src/redirects.rs b/crates/rari-tools/src/redirects.rs index 91bab089..dcdbf995 100644 --- a/crates/rari-tools/src/redirects.rs +++ b/crates/rari-tools/src/redirects.rs @@ -237,7 +237,7 @@ pub fn add_redirects(locale: Locale, update_pairs: &[(String, String)]) -> Resul let clean_pairs: HashMap = short_cuts(&clean_pairs)?.into_iter().collect(); - validate_pairs(&clean_pairs, &locale)?; + validate_pairs(&clean_pairs, locale)?; // Write the updated map back to the redirects file write_redirects(&path, &clean_pairs)?; @@ -258,7 +258,7 @@ pub fn add_redirects(locale: Locale, update_pairs: &[(String, String)]) -> Resul /// /// * `Ok(())` if all pairs are valid. /// * `Err(ToolError)` if any pair is invalid. -fn validate_pairs(pairs: &HashMap, locale: &Locale) -> Result<(), ToolError> { +fn validate_pairs(pairs: &HashMap, locale: Locale) -> Result<(), ToolError> { for (from, to) in pairs { validate_from_url(from, locale)?; validate_to_url(to, locale)?; @@ -284,7 +284,7 @@ fn validate_pairs(pairs: &HashMap, locale: &Locale) -> Result<() /// /// * `Ok(())` if the URL is valid. /// * `Err(ToolError)` if the URL is invalid. -fn validate_from_url(url: &str, locale: &Locale) -> Result<(), ToolError> { +fn validate_from_url(url: &str, locale: Locale) -> Result<(), ToolError> { let url = url.to_lowercase(); if !url.starts_with('/') { return Err(ToolError::InvalidRedirectFromURL(format!( @@ -347,7 +347,7 @@ fn validate_from_url(url: &str, locale: &Locale) -> Result<(), ToolError> { /// /// * `Ok(())` if the URL is valid. /// * `Err(ToolError)` if the URL is invalid. -fn validate_to_url(url: &str, locale: &Locale) -> Result<(), ToolError> { +fn validate_to_url(url: &str, locale: Locale) -> Result<(), ToolError> { if is_vanity_redirect_url(url) { return Ok(()); } @@ -364,32 +364,19 @@ fn validate_to_url(url: &str, locale: &Locale) -> Result<(), ToolError> { } } else if url.starts_with('/') { // Internal URL, perform validations - check_url_invalid_symbols(url)?; - validate_url_locale(url)?; // Split by '#', take the bare URL let bare_url = url.split('#').next().unwrap_or(""); - let parts: Vec<&str> = bare_url.split('/').collect(); - if parts.len() < 2 { - return Err(ToolError::InvalidRedirectToURL(format!( - "To-URL '{}' does not have enough parts for locale validation.", - url - ))); - } - - let to_locale = parts[1]; - if to_locale != locale.as_url_str().to_lowercase() { - // Different locale, no need to check path - return Ok(()); - } - let UrlMeta { - folder_path: path, .. - } = url_meta_from(url)?; - let path = root_for_locale(*locale)? - .join(locale.as_folder_str()) + folder_path: path, + locale: to_locale, + .. + } = url_meta_from(bare_url)?; + + let path = root_for_locale(to_locale)? + .join(to_locale.as_folder_str()) .join(path); if !path.exists() { return Err(ToolError::InvalidRedirectToURL(format!( @@ -408,26 +395,6 @@ fn validate_to_url(url: &str, locale: &Locale) -> Result<(), ToolError> { Ok(()) } -fn validate_url_locale(url: &str) -> Result<(), ToolError> { - let parts: Vec<&str> = url.split('/').collect(); - if parts.len() < 3 { - return Err(ToolError::InvalidRedirectToURL(format!( - "To-URL '{}' does not have enough parts for locale validation.", - url - ))); - } - - let to_locale = parts[1]; - if Locale::from_str(to_locale).is_err() { - return Err(ToolError::InvalidRedirectToURL(format!( - "Locale prefix '{}' in To-URL '{}' is not valid.", - to_locale, url - ))); - } - - Ok(()) -} - fn is_vanity_redirect_url(url: &str) -> bool { url.strip_prefix('/') .and_then(|url| url.strip_suffix('/')) @@ -649,8 +616,12 @@ where } #[cfg(test)] +use serial_test::file_serial; +#[cfg(test)] +#[file_serial(file_fixtures)] mod tests { use super::*; + use crate::tests::fixtures::docs::DocFixtures; fn s(s: &str) -> String { s.to_string() @@ -849,11 +820,65 @@ mod tests { ); } + #[test] + fn test_validate_to_url_happy_path() { + let slugs = vec!["A".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let url = "/en-US/docs/A"; + let locale = Locale::EnUs; + let result = validate_to_url(url, locale); + assert!(result.is_ok()); + } + + #[test] + fn test_validate_to_url_non_existing_doc() { + let slugs = vec!["A".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let url = "/en-US/docs/B"; + let locale = Locale::EnUs; + let result = validate_to_url(url, locale); + assert!(result.is_err()); + } + + #[test] + fn test_validate_to_url_no_enough_parts() { + let slugs = vec!["A".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let url = "/"; + let locale = Locale::EnUs; + let result = validate_to_url(url, locale); + assert!(result.is_err()); + + let url = "/en-US/A"; + let locale = Locale::EnUs; + let result = validate_to_url(url, locale); + assert!(result.is_err()); + } + + #[test] + fn test_validate_to_url_invalid_symbols() { + let url = "/en-US/docs/\nA"; + let locale = Locale::EnUs; + let result = validate_to_url(url, locale); + assert!(result.is_err()); + } + + #[test] + fn test_validate_to_url_diff_locale() { + let slugs = vec!["A".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let url = "/en-US/docs/A"; + let locale = Locale::PtBr; + let result = validate_to_url(url, locale); + println!("{:?}", result); + assert!(result.is_ok()); + } + #[test] fn test_validate_from_url_happy_path() { let url = "/en-US/docs/A"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_ok()); } @@ -861,7 +886,7 @@ mod tests { fn test_validate_from_url_url_does_not_start_with_slash() { let url = "en-US/docs/A"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_err()); } @@ -869,7 +894,7 @@ mod tests { fn test_validate_from_url_url_has_insufficient_parts() { let url = "/en-US/docs"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_err()); } @@ -877,7 +902,7 @@ mod tests { fn test_validate_from_url_locale_mismatch() { let url = "/pt-BR/docs/A"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_err()); } @@ -885,7 +910,7 @@ mod tests { fn test_validate_from_url_missing_docs_segment() { let url = "/en-US/A"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_err()); } @@ -893,7 +918,7 @@ mod tests { fn test_validate_from_url_invalid_locale_prefix() { let url = "/hu/docs/A"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_err()); } @@ -901,7 +926,7 @@ mod tests { fn test_validate_from_url_forbidden_symbol() { let url = "/en-US/docs/\nA"; let locale = Locale::EnUs; - let result = validate_from_url(url, &locale); + let result = validate_from_url(url, locale); assert!(result.is_err()); }