From b92bdec8d81c1f446b542c5acc9f45432b7950ba Mon Sep 17 00:00:00 2001 From: Andi Pieper Date: Mon, 14 Oct 2024 12:05:41 +0200 Subject: [PATCH] feature(cli): content delete (#20) * 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 * rename --------- Co-authored-by: Florian Dieminger --- .cargo/config.toml | 1 + Cargo.lock | 1 + crates/rari-cli/main.rs | 29 +- crates/rari-doc/src/helpers/subpages.rs | 14 +- crates/rari-tools/Cargo.toml | 1 + crates/rari-tools/src/error.rs | 3 + crates/rari-tools/src/git.rs | 43 ++ crates/rari-tools/src/history.rs | 12 +- crates/rari-tools/src/lib.rs | 3 + crates/rari-tools/src/move.rs | 83 +-- crates/rari-tools/src/remove.rs | 581 +++++++++++++++++++ crates/rari-tools/src/tests/fixtures/docs.rs | 1 - crates/rari-tools/src/utils.rs | 59 ++ crates/rari-tools/src/wikihistory.rs | 51 +- crates/rari-types/src/settings.rs | 4 + 15 files changed, 791 insertions(+), 95 deletions(-) create mode 100644 crates/rari-tools/src/git.rs create mode 100644 crates/rari-tools/src/remove.rs create mode 100644 crates/rari-tools/src/utils.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index 0bf84463..c52a0c72 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,3 +1,4 @@ [env] TESTING_CONTENT_ROOT = { value = "tests/data/content/files", relative = true } TESTING_CONTENT_TRANSLATED_ROOT = { value = "tests/data/translated_content/files", relative = true } +TESTING_CACHE_CONTENT = "0" diff --git a/Cargo.lock b/Cargo.lock index 12c76b1e..17e15b2d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2693,6 +2693,7 @@ dependencies = [ "rari-doc", "rari-types", "rari-utils", + "rayon", "reqwest", "serde", "serde_json", diff --git a/crates/rari-cli/main.rs b/crates/rari-cli/main.rs index a507f8cf..9a81d7d6 100644 --- a/crates/rari-cli/main.rs +++ b/crates/rari-cli/main.rs @@ -21,6 +21,7 @@ use rari_doc::utils::TEMPL_RECORDER_SENDER; use rari_tools::history::gather_history; use rari_tools::popularities::update_popularities; use rari_tools::r#move::r#move; +use rari_tools::remove::remove; use rari_types::globals::{build_out_root, content_root, content_translated_root, SETTINGS}; use rari_types::settings::Settings; use self_update::cargo_crate_version; @@ -44,8 +45,6 @@ struct Cli { no_cache: bool, #[arg(long)] skip_updates: bool, - #[arg(short = 'y', long, help = "Assume yes to all prompts")] - assume_yes: bool, #[command(subcommand)] command: Commands, } @@ -66,6 +65,7 @@ enum Commands { enum ContentSubcommand { /// Moves content from one slug to another Move(MoveArgs), + Delete(DeleteArgs), } #[derive(Args)] @@ -73,6 +73,20 @@ struct MoveArgs { old_slug: String, new_slug: String, locale: Option, + #[arg(short = 'y', long, help = "Assume yes to all prompts")] + assume_yes: bool, +} + +#[derive(Args)] +struct DeleteArgs { + slug: String, + locale: Option, + #[arg(short, long, default_value_t = false)] + recursive: bool, + #[arg(long)] + redirect: Option, + #[arg(short = 'y', long, help = "Assume yes to all prompts")] + assume_yes: bool, } #[derive(Args)] @@ -331,7 +345,16 @@ fn main() -> Result<(), Error> { &args.old_slug, &args.new_slug, args.locale.as_deref(), - cli.assume_yes, + args.assume_yes, + )?; + } + ContentSubcommand::Delete(args) => { + remove( + &args.slug, + args.locale.as_deref(), + args.recursive, + args.redirect.as_deref(), + args.assume_yes, )?; } }, diff --git a/crates/rari-doc/src/helpers/subpages.rs b/crates/rari-doc/src/helpers/subpages.rs index 947bfb5a..d1a53a7c 100644 --- a/crates/rari-doc/src/helpers/subpages.rs +++ b/crates/rari-doc/src/helpers/subpages.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use memoize::memoize; use rari_types::fm_types::{FeatureStatus, PageType}; -use rari_types::globals::deny_warnings; +use rari_types::globals::{cache_content, deny_warnings}; use rari_types::locale::Locale; use super::titles::api_page_title; @@ -207,7 +207,7 @@ pub fn get_sub_pages( let doc = Page::from_url(url)?; let full_path = doc.full_path(); if let Some(folder) = full_path.parent() { - let sub_folders = read_sub_folders_cached(folder.to_path_buf(), depth)?; + let sub_folders = read_sub_folders(folder.to_path_buf(), depth)?; let mut sub_pages = sub_folders .iter() @@ -220,9 +220,17 @@ pub fn get_sub_pages( Ok(vec![]) } +fn read_sub_folders(folder: PathBuf, depth: Option) -> Result, ignore::Error> { + if cache_content() { + read_sub_folders_internal(folder, depth) + } else { + memoized_original_read_sub_folders_internal(folder, depth) + } +} + #[memoize(SharedCache)] #[allow(non_snake_case)] -fn read_sub_folders_cached( +fn read_sub_folders_internal( folder: PathBuf, depth: Option, ) -> Result, ignore::Error> { diff --git a/crates/rari-tools/Cargo.toml b/crates/rari-tools/Cargo.toml index eff01521..24d8bd2b 100644 --- a/crates/rari-tools/Cargo.toml +++ b/crates/rari-tools/Cargo.toml @@ -18,6 +18,7 @@ tracing.workspace = true reqwest.workspace = true url.workspace = true indoc.workspace = true +rayon.workspace = true console = "0" dialoguer = "0" diff --git a/crates/rari-tools/src/error.rs b/crates/rari-tools/src/error.rs index ef45dbb9..23d99f75 100644 --- a/crates/rari-tools/src/error.rs +++ b/crates/rari-tools/src/error.rs @@ -41,6 +41,9 @@ pub enum ToolError { #[error(transparent)] RedirectError(#[from] RedirectError), + #[error("Page has subpages: {0}")] + HasSubpagesError(Cow<'static, str>), + #[error("Unknonwn error")] Unknown(&'static str), } diff --git a/crates/rari-tools/src/git.rs b/crates/rari-tools/src/git.rs new file mode 100644 index 00000000..dc62fac2 --- /dev/null +++ b/crates/rari-tools/src/git.rs @@ -0,0 +1,43 @@ +use std::ffi::OsStr; +use std::path::Path; +use std::process::{Command, Output}; + +/// Run `git` with `args` wiht `root` as current directory. +/// For tests this will run the first arg as command, eg.: +/// Instead of `git mv foo bar` -> `mv foo bar`. +pub fn exec_git_with_test_fallback(args: &[impl AsRef], root: impl AsRef) -> Output { + let git = OsStr::new("git"); + let echo = OsStr::new("echo"); + + let (command, args) = if cfg!(test) { + ( + args.first().map(AsRef::as_ref).unwrap_or(echo), + &args[if args.is_empty() { 0 } else { 1 }..], + ) + } else { + (git, args) + }; + exec_git_internal(command, args, root) +} + +pub fn exec_git(args: &[impl AsRef], root: impl AsRef) -> Output { + let output = Command::new("git") + .args(args) + .current_dir(root) + .output() + .expect("failed to execute process"); + output +} + +fn exec_git_internal( + command: impl AsRef, + args: &[impl AsRef], + root: impl AsRef, +) -> Output { + let output = Command::new(command) + .args(args) + .current_dir(root) + .output() + .expect("failed to execute process"); + output +} diff --git a/crates/rari-tools/src/history.rs b/crates/rari-tools/src/history.rs index 8cd4f131..cbcf8f04 100644 --- a/crates/rari-tools/src/history.rs +++ b/crates/rari-tools/src/history.rs @@ -9,6 +9,7 @@ use rari_types::globals::{content_root, content_translated_root}; use rari_types::HistoryEntry; use crate::error::ToolError; +use crate::git::exec_git; pub fn gather_history() -> Result<(), ToolError> { let hanlde = content_translated_root().map(|translated_root| { @@ -33,8 +34,8 @@ fn modification_times(path: &Path) -> Result<(), ToolError> { let repo_root_raw = String::from_utf8_lossy(&output.stdout); let repo_root = repo_root_raw.trim(); - let output = Command::new("git") - .args([ + let output = exec_git( + &[ "log", "--name-only", "--no-decorate", @@ -42,10 +43,9 @@ fn modification_times(path: &Path) -> Result<(), ToolError> { "--date-order", "--reverse", "-z", - ]) - .current_dir(repo_root) - .output() - .expect("failed to execute process"); + ], + repo_root, + ); let output_str = String::from_utf8_lossy(&output.stdout); let mut history = BTreeMap::new(); diff --git a/crates/rari-tools/src/lib.rs b/crates/rari-tools/src/lib.rs index fbbc2be3..41143aed 100644 --- a/crates/rari-tools/src/lib.rs +++ b/crates/rari-tools/src/lib.rs @@ -1,8 +1,11 @@ pub mod error; +pub mod git; pub mod history; pub mod r#move; pub mod popularities; pub mod redirects; +pub mod remove; #[cfg(test)] pub mod tests; +mod utils; pub mod wikihistory; diff --git a/crates/rari-tools/src/move.rs b/crates/rari-tools/src/move.rs index a62c0877..222e9d6c 100644 --- a/crates/rari-tools/src/move.rs +++ b/crates/rari-tools/src/move.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use std::ffi::OsStr; use std::fs::create_dir_all; use std::path::PathBuf; -use std::process::Command; use std::str::FromStr; use std::sync::Arc; @@ -18,7 +17,9 @@ use rari_doc::{ use rari_types::locale::Locale; use crate::error::ToolError; +use crate::git::exec_git_with_test_fallback; use crate::redirects::add_redirects; +use crate::utils::parent_slug; use crate::wikihistory::update_wiki_history; pub fn r#move( @@ -169,25 +170,15 @@ fn do_move( )); } - // Conditional command for testing. In testing, we do not use git, because the test - // fixtures are no under git control. SO instead of `git mv …` we use `mv …`. - let command = if cfg!(test) { "mv" } else { "git" }; - let args = if cfg!(test) { - vec![old_folder_path.as_os_str(), new_folder_path.as_os_str()] - } else { - vec![ + // Execute the git move. + let output = exec_git_with_test_fallback( + &[ OsStr::new("mv"), old_folder_path.as_os_str(), new_folder_path.as_os_str(), - ] - }; - - // Execute the git move. - let output = Command::new(command) - .args(args) - .current_dir(root_for_locale(locale)?) - .output() - .expect("failed to execute process"); + ], + root_for_locale(locale)?, + ); if !output.status.success() { return Err(ToolError::GitError(format!( @@ -214,15 +205,6 @@ fn do_move( Ok(pairs) } -fn parent_slug(slug: &str) -> Result<&str, ToolError> { - let slug = slug.trim_end_matches('/'); - if let Some(i) = slug.rfind('/') { - Ok(&slug[..i]) - } else { - Err(ToolError::InvalidSlug(Cow::Borrowed("slug has no parent"))) - } -} - fn validate_args(old_slug: &str, new_slug: &str) -> Result<(), ToolError> { if old_slug.is_empty() { return Err(ToolError::InvalidSlug(Cow::Borrowed( @@ -247,27 +229,22 @@ fn validate_args(old_slug: &str, new_slug: &str) -> Result<(), ToolError> { Ok(()) } -#[cfg(test)] -use serial_test::file_serial; - // 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 should be able to run all tests -// using the same `key` to be serialized across modules. - +// 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 std::collections::HashMap; - use std::path::Path; - use super::*; - use crate::redirects; use crate::tests::fixtures::docs::DocFixtures; use crate::tests::fixtures::redirects::RedirectFixtures; use crate::tests::fixtures::wikihistory::WikihistoryFixtures; + use crate::utils::test_utils::{check_file_existence, get_redirects_map}; fn s(s: &str) -> String { s.to_string() @@ -413,13 +390,7 @@ mod test { check_file_existence(root_path, &should_exist, &should_not_exist); // check redirects - let mut redirects_path = PathBuf::from(root_path); - redirects_path.push(Locale::EnUs.as_folder_str()); - redirects_path.push("_redirects.txt"); - assert!(&redirects_path.exists()); - let mut redirects = HashMap::new(); - redirects::read_redirects_raw(&redirects_path, &mut redirects).unwrap(); - // println!("{:?}", redirects); + let redirects = get_redirects_map(Locale::EnUs); assert_eq!( redirects.get("/en-US/docs/Web/API/ExampleOne").unwrap(), "/en-US/docs/Web/API/ExampleOneNewLocation" @@ -529,13 +500,7 @@ mod test { check_file_existence(root_path, &should_exist, &should_not_exist); // check redirects - let mut redirects_path = PathBuf::from(root_path); - redirects_path.push(Locale::PtBr.as_folder_str()); - redirects_path.push("_redirects.txt"); - assert!(&redirects_path.exists()); - let mut redirects = HashMap::new(); - redirects::read_redirects_raw(&redirects_path, &mut redirects).unwrap(); - // println!("{:?}", redirects); + let redirects = get_redirects_map(Locale::PtBr); assert_eq!( redirects.get("/pt-BR/docs/Web/API/ExampleOne").unwrap(), "/pt-BR/docs/Web/API/ExampleOneNewLocation" @@ -565,24 +530,4 @@ mod test { "/pt-BR/docs/Web/API/SomethingElse" ); } - - fn check_file_existence(root: &Path, should_exist: &[&str], should_not_exist: &[&str]) { - for relative_path in should_exist { - let parts = relative_path.split('/').collect::>(); - let mut path = PathBuf::from(root); - for part in parts { - path.push(part); - } - assert!(path.exists(), "{} should exist", path.display()); - } - - for relative_path in should_not_exist { - let parts = relative_path.split('/').collect::>(); - let mut path = PathBuf::from(root); - for part in parts { - path.push(part); - } - assert!(!path.exists(), "{} should not exist", path.display()); - } - } } diff --git a/crates/rari-tools/src/remove.rs b/crates/rari-tools/src/remove.rs new file mode 100644 index 00000000..cb2dff29 --- /dev/null +++ b/crates/rari-tools/src/remove.rs @@ -0,0 +1,581 @@ +use std::borrow::Cow; +use std::collections::BTreeSet; +use std::ffi::OsStr; +use std::path::PathBuf; +use std::str::FromStr; + +use console::Style; +use dialoguer::theme::ColorfulTheme; +use dialoguer::Confirm; +use rari_doc::error::DocError; +use rari_doc::helpers::subpages::get_sub_pages; +use rari_doc::pages::page::{self, PageCategory, PageLike}; +use rari_doc::pages::types::doc::Doc; +use rari_doc::reader::read_docs_parallel; +use rari_doc::resolve::build_url; +use rari_doc::utils::root_for_locale; +use rari_types::locale::Locale; +use rayon::iter::{once, IntoParallelIterator, ParallelIterator}; + +use crate::error::ToolError; +use crate::git::exec_git_with_test_fallback; +use crate::redirects::add_redirects; +use crate::wikihistory::delete_from_wiki_history; + +pub fn remove( + slug: &str, + locale: Option<&str>, + recursive: bool, + redirect: Option<&str>, + assume_yes: bool, +) -> Result<(), ToolError> { + validate_args(slug)?; + let locale = if let Some(l) = locale { + Locale::from_str(l)? + } else { + Locale::default() + }; + + let green = Style::new().green(); + let red = Style::new().red(); + let yellow = Style::new().yellow(); + let bold = Style::new().bold(); + let changes = do_remove(slug, locale, recursive, redirect, true)?; + if changes.is_empty() { + println!("{}", green.apply_to("No changes would be made")); + return Ok(()); + } else { + println!( + "{} {} {}", + green.apply_to("This will delete"), + bold.apply_to(changes.len()), + green.apply_to("documents:"), + ); + for slug in changes { + println!("{}", red.apply_to(&slug)); + } + if let Some(redirect) = redirect { + println!( + "{} {} to: {}", + green.apply_to("Redirecting"), + green.apply_to(if recursive { + "each document" + } else { + "document" + }), + green.apply_to(&redirect), + ); + } else { + println!("{}", yellow.apply_to("Deleting without a redirect. Consider using the --redirect option with a related page instead.")); + } + } + + if assume_yes + || Confirm::with_theme(&ColorfulTheme::default()) + .with_prompt("Proceed?") + .default(true) + .interact() + .unwrap_or_default() + { + let removed = do_remove(slug, locale, recursive, redirect, false)?; + let removed_urls = removed + .iter() + .map(|slug| build_url(slug, locale, PageCategory::Doc)) + .collect::, DocError>>()?; + println!( + "{} {} {}", + green.apply_to("Deleted"), + bold.apply_to(removed.len()), + green.apply_to("documents:"), + ); + for url in &removed_urls { + println!("{}", red.apply_to(&url)); + } + + // Find references to deleted documents and + // list them for manual review + println!("Checking references to deleted documents..."); + let mut docs_path = PathBuf::from(root_for_locale(locale)?); + docs_path.push(locale.as_folder_str()); + + let docs = read_docs_parallel::(&[docs_path], None)?; + + let referencing_docs: BTreeSet = docs + .into_par_iter() + .filter_map(|doc| { + for url in &removed { + if doc.content().contains(url) { + return Some(doc.url().to_owned()); + } + } + None + }) + .collect(); + + if referencing_docs.is_empty() { + println!( + "{}", + green.apply_to("No file is referring to the deleted document."), + ); + } else { + println!( + "{} {}", + yellow.apply_to(referencing_docs.len()), + yellow.apply_to("files are referring to the deleted documents. Please update the following files to remove the links:"), + ); + for url in &referencing_docs { + println!("{}", yellow.apply_to(url)); + } + } + } + Ok(()) +} + +fn do_remove( + slug: &str, + locale: Locale, + recursive: bool, + redirect: Option<&str>, + dry_run: bool, +) -> Result, ToolError> { + let doc = Doc::page_from_slug(slug, locale)?; + let real_slug = doc.slug(); + + // If we get a redirect value passed in, it is either a slug or a complete url. + // If it is a slug, check if it actually exists, otherwise bail. + let redirect_target = if let Some(redirect_str) = redirect { + if !redirect_str.starts_with("http") { + let redirect_url = build_url(redirect_str, locale, PageCategory::Doc)?; + if !page::Page::exists(&redirect_url) { + return Err(ToolError::InvalidRedirectToURL(format!( + "redirect slug does not exist: {redirect_url}" + ))); + } + Some(redirect_url) + } else { + Some(redirect_str.to_owned()) + } + } else { + None + }; + + let subpages = get_sub_pages(doc.url(), None, Default::default())?; + if !recursive && !subpages.is_empty() && redirect.is_some() { + return Err(ToolError::HasSubpagesError(Cow::Owned(format!( + "{0}, unable to remove and redirect a document with children", + slug + )))); + } + + let slugs_to_remove = if recursive { + once(&doc) + .chain(&subpages) + .map(|page_ref| page_ref.slug().to_string()) + .collect::>() + } else { + vec![real_slug.to_string()] + }; + + if dry_run { + return Ok(slugs_to_remove); + } + + // Remove the documents. For single documents, we just remove the `index.md` file and + // leave the folder structure in place. For recursive removal, we remove the entire + // folder structure, duplicating the original yari tool behaviour. + + // Conditional command for testing. In testing, we do not use git, because the test + // fixtures are not under git control. Instead of `git rm …` we use `rm …`. + let path = doc.path(); + + if recursive { + let parent = path + .parent() + .ok_or(ToolError::InvalidSlug(Cow::Owned(format!( + "{slug} ({}) has no parent directory", + path.display() + ))))?; + + // Execute the recursive remove command + let output = exec_git_with_test_fallback( + &[OsStr::new("rm"), OsStr::new("-rf"), parent.as_os_str()], + root_for_locale(locale)?, + ); + + if !output.status.success() { + return Err(ToolError::GitError(format!( + "Failed to remove files: {}", + String::from_utf8_lossy(&output.stderr) + ))); + } + } else { + // Execute the single file remove command + let output = exec_git_with_test_fallback( + &[OsStr::new("rm"), path.as_os_str()], + root_for_locale(locale)?, + ); + + if !output.status.success() { + return Err(ToolError::GitError(format!( + "Failed to remove files: {}", + String::from_utf8_lossy(&output.stderr) + ))); + } + } + + // update the wiki history + delete_from_wiki_history(locale, &slugs_to_remove)?; + + // update the redirects map if needed + if let Some(new_target) = redirect_target { + let pairs = slugs_to_remove + .iter() + .map(|slug| { + let old_url = build_url(slug, locale, PageCategory::Doc)?; + // let new_url = build_url(&new_slug, locale, PageCategory::Doc)?; + Ok((old_url, new_target.to_owned())) + }) + .collect::, ToolError>>()?; + add_redirects(locale, &pairs)?; + } + Ok(slugs_to_remove) +} + +fn validate_args(slug: &str) -> Result<(), ToolError> { + if slug.is_empty() { + return Err(ToolError::InvalidSlug(Cow::Borrowed( + "slug cannot be empty", + ))); + } + if slug.contains("#") { + return Err(ToolError::InvalidSlug(Cow::Borrowed( + "slug 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 super::*; + use crate::tests::fixtures::docs::DocFixtures; + use crate::tests::fixtures::redirects::RedirectFixtures; + use crate::tests::fixtures::wikihistory::WikihistoryFixtures; + use crate::utils::test_utils::{check_file_existence, get_redirects_map}; + use crate::wikihistory::test_get_wiki_history; + + #[test] + fn test_validate_args() { + assert!(validate_args("old").is_ok()); + assert!(validate_args("").is_err()); + assert!(validate_args("old#").is_err()); + } + + #[test] + fn test_redirect_option() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/ExampleTwo".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + + // valid redirect + let result = do_remove( + "Web/API/ExampleTwo", + Locale::EnUs, + false, + Some("Web/API/ExampleOne"), + true, + ); + assert!(result.is_ok()); + + // invalid redirect + let result = do_remove( + "Web/API/ExampleTwo", + Locale::EnUs, + false, + Some("Web/API/ExampleNonExisting"), + true, + ); + assert!(matches!(result, Err(ToolError::InvalidRedirectToURL(_)))); + assert!(result.is_err()); + + // external URL + let result = do_remove( + "Web/API/ExampleTwo", + Locale::EnUs, + false, + Some("https://example.com/"), + true, + ); + assert!(result.is_ok()); + + // no redirect + let result = do_remove("Web/API/ExampleTwo", Locale::EnUs, false, None, true); + assert!(result.is_ok()); + } + + #[test] + fn test_existing_subpages() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/ExampleOne/Subpage".to_string(), + "Web/API/RedirectTarget".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + + // no recursive, no redirect, ok even with subpages + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, false, None, true); + assert!(result.is_ok()); + + // no recursive, with redirect, not ok with subpages + let result = do_remove( + "Web/API/ExampleOne", + Locale::EnUs, + false, + Some("Web/API/RedirectTarget"), + true, + ); + assert!(result.is_err()); + assert!(matches!(result, Err(ToolError::HasSubpagesError(_)))); + + // recursive + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, true, None, true); + assert!(result.is_ok()); + } + + #[test] + fn test_no_subpages() { + let slugs = vec!["Web/API/ExampleOne".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + + // no recursive, but no subpages, so it is ok + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, false, None, true); + assert!(result.is_ok()); + + // recursive, all the same + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, true, None, true); + assert!(result.is_ok()); + } + + #[test] + fn test_nonexisting() { + // This does not exist + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, false, None, true); + assert!(result.is_err()); + assert!(matches!(result, Err(ToolError::DocError(_)))); + } + + #[test] + fn test_remove_single() { + let slugs = vec!["Web/API/ExampleOne".to_string()]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _wikihistory = WikihistoryFixtures::new(&slugs, Locale::EnUs); + + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, false, None, false); + assert!(result.is_ok()); + + let should_exist = vec![]; + let should_not_exist = vec!["en-us/web/api/exampleone/index.md"]; + let root_path = root_for_locale(Locale::EnUs).unwrap(); + check_file_existence(root_path, &should_exist, &should_not_exist); + + let wiki_history = test_get_wiki_history(Locale::EnUs); + assert!(!wiki_history.contains_key("Web/API/ExampleOne")); + } + + #[test] + fn test_remove_single_with_redirect() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/RedirectTarget".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _wikihistory = WikihistoryFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + + let result = do_remove( + "Web/API/ExampleOne", + Locale::EnUs, + false, + Some("Web/API/RedirectTarget"), + false, + ); + assert!(result.is_ok()); + + let should_exist = vec!["en-us/web/api/redirecttarget/index.md"]; + let should_not_exist = vec!["en-us/web/api/exampleone/index.md"]; + let root_path = root_for_locale(Locale::EnUs).unwrap(); + check_file_existence(root_path, &should_exist, &should_not_exist); + + let wiki_history = test_get_wiki_history(Locale::EnUs); + assert!(!wiki_history.contains_key("Web/API/ExampleOne")); + assert!(wiki_history.contains_key("Web/API/RedirectTarget")); + + let redirects = get_redirects_map(Locale::EnUs); + assert_eq!(redirects.len(), 1); + assert!(redirects.contains_key("/en-US/docs/Web/API/ExampleOne")); + assert_eq!( + redirects.get("/en-US/docs/Web/API/ExampleOne").unwrap(), + "/en-US/docs/Web/API/RedirectTarget" + ); + } + + #[test] + fn test_remove_single_with_subpages() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/ExampleOne/Subpage".to_string(), + "Web/API/RedirectTarget".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _wikihistory = WikihistoryFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, false, None, false); + assert!(result.is_ok()); + + let should_exist = vec!["en-us/web/api/exampleone/subpage/index.md"]; + let should_not_exist = vec!["en-us/web/api/exampleone/index.md"]; + let root_path = root_for_locale(Locale::EnUs).unwrap(); + check_file_existence(root_path, &should_exist, &should_not_exist); + + let wiki_history = test_get_wiki_history(Locale::EnUs); + assert!(!wiki_history.contains_key("Web/API/ExampleOne")); + assert!(wiki_history.contains_key("Web/API/ExampleOne/Subpage")); + assert!(wiki_history.contains_key("Web/API/RedirectTarget")); + + let redirects = get_redirects_map(Locale::EnUs); + assert_eq!(redirects.len(), 0); + } + + #[test] + fn test_remove_recursive() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/ExampleOne/Subpage".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _wikihistory = WikihistoryFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + + let result = do_remove("Web/API/ExampleOne", Locale::EnUs, true, None, false); + assert!(result.is_ok()); + + let should_exist = vec![]; + let should_not_exist = vec![ + "en-us/web/api/exampleone/index.md", + "en-us/web/api/exampleone/subpage/index.md", + ]; + let root_path = root_for_locale(Locale::EnUs).unwrap(); + check_file_existence(root_path, &should_exist, &should_not_exist); + + let wiki_history = test_get_wiki_history(Locale::EnUs); + assert!(!wiki_history.contains_key("Web/API/ExampleOne")); + assert!(!wiki_history.contains_key("Web/API/ExampleOne/Subpage")); + + let redirects = get_redirects_map(Locale::EnUs); + assert_eq!(redirects.len(), 0); + } + + #[test] + fn test_remove_recursive_with_redirect() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/ExampleOne/Subpage".to_string(), + "Web/API/RedirectTarget".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::EnUs); + let _wikihistory = WikihistoryFixtures::new(&slugs, Locale::EnUs); + let _redirects = RedirectFixtures::new(&vec![], Locale::EnUs); + + let result = do_remove( + "Web/API/ExampleOne", + Locale::EnUs, + true, + Some("Web/API/RedirectTarget"), + false, + ); + assert!(result.is_ok()); + + let should_exist = vec![]; + let should_not_exist = vec![ + "en-us/web/api/exampleone/index.md", + "en-us/web/api/exampleone/subpage/index.md", + ]; + let root_path = root_for_locale(Locale::EnUs).unwrap(); + check_file_existence(root_path, &should_exist, &should_not_exist); + + let wiki_history = test_get_wiki_history(Locale::EnUs); + assert!(!wiki_history.contains_key("Web/API/ExampleOne")); + assert!(!wiki_history.contains_key("Web/API/ExampleOne/Subpage")); + + let redirects = get_redirects_map(Locale::EnUs); + assert_eq!(redirects.len(), 2); + assert_eq!( + redirects.get("/en-US/docs/Web/API/ExampleOne").unwrap(), + "/en-US/docs/Web/API/RedirectTarget" + ); + assert_eq!( + redirects + .get("/en-US/docs/Web/API/ExampleOne/Subpage") + .unwrap(), + "/en-US/docs/Web/API/RedirectTarget" + ); + } + + #[test] + fn test_remove_recursive_with_redirect_translated() { + let slugs = vec![ + "Web/API/ExampleOne".to_string(), + "Web/API/ExampleOne/Subpage".to_string(), + "Web/API/RedirectTarget".to_string(), + ]; + let _docs = DocFixtures::new(&slugs, Locale::PtBr); + let _wikihistory = WikihistoryFixtures::new(&slugs, Locale::PtBr); + let _redirects = RedirectFixtures::new(&vec![], Locale::PtBr); + + let result = do_remove( + "Web/API/ExampleOne", + Locale::PtBr, + true, + Some("Web/API/RedirectTarget"), + false, + ); + assert!(result.is_ok()); + + let should_exist = vec![]; + let should_not_exist = vec![ + "pt-br/web/api/exampleone/index.md", + "pt-br/web/api/exampleone/subpage/index.md", + ]; + let root_path = root_for_locale(Locale::PtBr).unwrap(); + check_file_existence(root_path, &should_exist, &should_not_exist); + + let wiki_history = test_get_wiki_history(Locale::PtBr); + assert!(!wiki_history.contains_key("Web/API/ExampleOne")); + assert!(!wiki_history.contains_key("Web/API/ExampleOne/Subpage")); + + let redirects = get_redirects_map(Locale::PtBr); + assert_eq!(redirects.len(), 2); + assert_eq!( + redirects.get("/pt-BR/docs/Web/API/ExampleOne").unwrap(), + "/pt-BR/docs/Web/API/RedirectTarget" + ); + assert_eq!( + redirects + .get("/pt-BR/docs/Web/API/ExampleOne/Subpage") + .unwrap(), + "/pt-BR/docs/Web/API/RedirectTarget" + ); + } +} diff --git a/crates/rari-tools/src/tests/fixtures/docs.rs b/crates/rari-tools/src/tests/fixtures/docs.rs index d8b928a3..900e0d7c 100644 --- a/crates/rari-tools/src/tests/fixtures/docs.rs +++ b/crates/rari-tools/src/tests/fixtures/docs.rs @@ -27,7 +27,6 @@ impl DocFixtures { fn new_internal(slugs: &[String], locale: Locale, do_not_remove: bool) -> Self { // create doc file for each slug in the vector, in the configured root directory for the locale - // Iterate over each slug and create a file in the root directory let _files: Vec = slugs .iter() diff --git a/crates/rari-tools/src/utils.rs b/crates/rari-tools/src/utils.rs new file mode 100644 index 00000000..71814051 --- /dev/null +++ b/crates/rari-tools/src/utils.rs @@ -0,0 +1,59 @@ +use std::borrow::Cow; + +use crate::error::ToolError; + +pub(crate) fn parent_slug(slug: &str) -> Result<&str, ToolError> { + let slug = slug.trim_end_matches('/'); + if let Some(i) = slug.rfind('/') { + Ok(&slug[..i]) + } else { + Err(ToolError::InvalidSlug(Cow::Borrowed("slug has no parent"))) + } +} + +#[cfg(test)] +pub mod test_utils { + use std::collections::HashMap; + use std::path::{Path, PathBuf}; + + use rari_doc::utils::root_for_locale; + use rari_types::locale::Locale; + + use crate::redirects; + pub(crate) fn check_file_existence( + root: &Path, + should_exist: &[&str], + should_not_exist: &[&str], + ) { + use std::path::PathBuf; + + for relative_path in should_exist { + let parts = relative_path.split('/').collect::>(); + let mut path = PathBuf::from(root); + for part in parts { + path.push(part); + } + assert!(path.exists(), "{} should exist", path.display()); + } + + for relative_path in should_not_exist { + let parts = relative_path.split('/').collect::>(); + let mut path = PathBuf::from(root); + for part in parts { + path.push(part); + } + assert!(!path.exists(), "{} should not exist", path.display()); + } + } + + pub(crate) fn get_redirects_map(locale: Locale) -> HashMap { + let root_path = root_for_locale(locale).unwrap(); + + let mut redirects_path = PathBuf::from(root_path); + redirects_path.push(locale.as_folder_str()); + redirects_path.push("_redirects.txt"); + let mut redirects = HashMap::new(); + redirects::read_redirects_raw(&redirects_path, &mut redirects).unwrap(); + redirects + } +} diff --git a/crates/rari-tools/src/wikihistory.rs b/crates/rari-tools/src/wikihistory.rs index a8fe827c..eec4d36e 100644 --- a/crates/rari-tools/src/wikihistory.rs +++ b/crates/rari-tools/src/wikihistory.rs @@ -10,30 +10,55 @@ use serde_json::Value; use crate::error::ToolError; pub fn update_wiki_history(locale: Locale, pairs: &Vec<(String, String)>) -> Result<(), ToolError> { - // Construct the path to "_wikihistory.json" - let locale_content_root = root_for_locale(locale)?; - let wiki_history_path = Path::new(locale_content_root) - .join(locale.as_folder_str()) - .join("_wikihistory.json"); - - // Read the content of the JSON file - let wiki_history_content = fs::read_to_string(&wiki_history_path)?; - - // Parse the JSON content into a BTreeMap (sorted map) - let mut all: BTreeMap = serde_json::from_str(&wiki_history_content)?; - + let mut all = read_wiki_history(locale)?; for (old_slug, new_slug) in pairs { if let Some(to) = all.remove(old_slug) { all.insert(new_slug.to_string(), to); } } + write_wiki_history(locale, all)?; + Ok(()) +} + +pub fn delete_from_wiki_history(locale: Locale, slugs: &Vec) -> Result<(), ToolError> { + let mut all = read_wiki_history(locale)?; + for slug in slugs { + all.remove(slug); + } + write_wiki_history(locale, all)?; + Ok(()) +} +fn write_wiki_history(locale: Locale, all: BTreeMap) -> Result<(), ToolError> { + let wiki_history_path = wiki_history_path(locale)?; let file = File::create(&wiki_history_path)?; let mut buffer = BufWriter::new(file); // Write the updated pretty JSON back to the file serde_json::to_writer_pretty(&mut buffer, &all)?; // Add a trailing newline buffer.write_all(b"\n")?; - Ok(()) } + +fn read_wiki_history(locale: Locale) -> Result, ToolError> { + let wiki_history_path = wiki_history_path(locale)?; + // Read the content of the JSON file + let wiki_history_content = fs::read_to_string(&wiki_history_path)?; + // Parse the JSON content into a BTreeMap (sorted map) + let all: BTreeMap = serde_json::from_str(&wiki_history_content)?; + Ok(all) +} + +fn wiki_history_path(locale: Locale) -> Result { + let locale_content_root = root_for_locale(locale)?; + Ok(Path::new(locale_content_root) + .join(locale.as_folder_str()) + .join("_wikihistory.json") + .to_string_lossy() + .to_string()) +} + +#[cfg(test)] +pub(crate) fn test_get_wiki_history(locale: Locale) -> BTreeMap { + read_wiki_history(locale).expect("Could not read wiki history") +} diff --git a/crates/rari-types/src/settings.rs b/crates/rari-types/src/settings.rs index 1ef683d2..0dfa687b 100644 --- a/crates/rari-types/src/settings.rs +++ b/crates/rari-types/src/settings.rs @@ -53,6 +53,10 @@ impl Settings { "CONTENT_TRANSLATED_ROOT", std::env::var("TESTING_CONTENT_TRANSLATED_ROOT").unwrap(), ); + std::env::set_var( + "CACHE_CONTENT", + std::env::var("TESTING_CACHE_CONTENT").unwrap(), + ); Self::new_internal() } #[cfg(not(feature = "testing"))]