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 <[email protected]>

.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"

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,13 +65,28 @@ enum Commands {
 enum ContentSubcommand {
     /// Moves content from one slug to another
     Move(MoveArgs),
+    Delete(DeleteArgs),
 }
 
 #[derive(Args)]
 struct MoveArgs {
     old_slug: String,
     new_slug: String,
     locale: Option<String>,
+    #[arg(short = 'y', long, help = "Assume yes to all prompts")]
+    assume_yes: bool,
+}
+
+#[derive(Args)]
+struct DeleteArgs {
+    slug: String,
+    locale: Option<String>,
+    #[arg(short, long, default_value_t = false)]
+    recursive: bool,
+    #[arg(long)]
+    redirect: Option<String>,
+    #[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,
                 )?;
             }
         },

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<usize>) -> Result<Vec<PathBuf>, 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<usize>,
 ) -> Result<Vec<PathBuf>, ignore::Error> {

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"

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),
 }

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<OsStr>], root: impl AsRef<Path>) -> 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<OsStr>], root: impl AsRef<Path>) -> 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<OsStr>,
+    args: &[impl AsRef<OsStr>],
+    root: impl AsRef<Path>,
+) -> Output {
+    let output = Command::new(command)
+        .args(args)
+        .current_dir(root)
+        .output()
+        .expect("failed to execute process");
+    output
+}

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,19 +34,18 @@ 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",
             "--format=COMMIT:%H_%cI_%P",
             "--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();

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;

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::<Vec<&str>>();
-            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::<Vec<&str>>();
-            let mut path = PathBuf::from(root);
-            for part in parts {
-                path.push(part);
-            }
-            assert!(!path.exists(), "{} should not exist", path.display());
-        }
-    }
 }