feature(cli): add content add-redirect (#21)

* move 'assume yes' cli option under individual content commands, added cli delete/remove option, scaffolded functionality for this * make cache settings of get_subfolders configurable * CLI info + logic * refactored wiki history management, more tests * fix history * utils refactored, redirects handling, tests * added dangling references check to delete * cleanup * move git code and nits * fix history * nit * added add-redirect command * depend on redirects module for some validation, fixes, tests * added console output, cli integration * fixed a bug in redirect logic, added some tests, make locale always pass by value in a few cases * removed early return for differing locales on target url validation, removed commented code --------- Co-authored-by: Florian Dieminger <[email protected]>
mdn · Oct 15, 2024 · df26184 · df26184
1 parent 309f6bf
commit df26184
Show file tree

Hide file tree

Showing 6 changed files with 274 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);