Merge pull request Nukesor#348 from Nukesor/tests-and-refactorings

Tests and refactorings

CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Added
 
 - Show a hint when calling `pueue log` if the task output has been truncated. [#318](https://github.com/Nukesor/pueue/issues/318)
+- Add `Settings.shared.alias_file`, which allows to set the location of the `pueue_aliases.yml` file.
 
 ### Fixed
 

Cargo.toml

@@ -61,6 +61,7 @@ assert_cmd = "2"
 better-panic = "0.3"
 pretty_assertions = "1"
 rstest = "0.15"
+serde_yaml = "0.8"
 
 # Test specific dev-dependencies
 [target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies]

client/commands/restart.rs

@@ -81,23 +81,28 @@ pub async fn restart(
         new_task.status = new_status.clone();
 
         // Path and command can be edited, if the use specified the -e or -p flag.
-        let mut command = task.original_command.clone();
-        let mut path = task.path.clone();
+        let mut command = None;
+        let mut path = None;
+
+        // Update the command if requested.
         if edit_command {
-            command = edit_line_wrapper(stream, *task_id, &command).await?
+            command = Some(edit_line_wrapper(stream, *task_id, &task.command).await?);
         };
+
+        // Update the path if requested.
         if edit_path {
-            let str_path = path
+            let str_path = task
+                .path
                 .to_str()
                 .context("Failed to convert task path to string")?;
             let changed_path = edit_line_wrapper(stream, *task_id, str_path).await?;
-            path = PathBuf::from(changed_path);
+            path = Some(PathBuf::from(changed_path));
         }
 
         // Add the tasks to the singular message, if we want to restart the tasks in-place.
         // And continue with the next task. The message will then be sent after the for loop.
         if in_place {
-            restart_message.tasks.push(TasksToRestart {
+            restart_message.tasks.push(TaskToRestart {
                 task_id: *task_id,
                 command,
                 path,
@@ -106,10 +111,11 @@ pub async fn restart(
             continue;
         }
 
+        // In case we don't do in-place restarts, we have to add a new task.
         // Create a AddMessage to send the task to the daemon from the updated info and the old task.
         let add_task_message = Message::Add(AddMessage {
-            command,
-            path,
+            command: command.unwrap_or_else(|| task.command.clone()),
+            path: path.unwrap_or_else(|| task.path.clone()),
             envs: task.envs.clone(),
             start_immediately,
             stashed,

daemon/network/message_handler/add.rs

@@ -1,5 +1,6 @@
 use crossbeam_channel::Sender;
 
+use pueue_lib::aliasing::insert_alias;
 use pueue_lib::network::message::*;
 use pueue_lib::state::{GroupStatus, SharedState};
 use pueue_lib::task::{Task, TaskStatus};
@@ -52,6 +53,9 @@ pub fn add_task(
         message.dependencies,
         message.label,
     );
+    // Insert the client alias if we applicable.
+    task.command = insert_alias(settings, task.original_command.clone());
+
     // Sort and deduplicate dependency id.
     task.dependencies.sort_unstable();
     task.dependencies.dedup();

daemon/network/message_handler/edit.rs

@@ -45,7 +45,7 @@ pub fn edit(message: EditMessage, state: &SharedState, settings: &Settings) -> M
 
             task.status = task.prev_status.clone();
             task.original_command = message.command.clone();
-            task.command = insert_alias(message.command.clone());
+            task.command = insert_alias(settings, message.command.clone());
             task.path = message.path.clone();
             ok_or_return_failure_message!(save_state(&state, settings));
 

daemon/network/message_handler/mod.rs

@@ -1,8 +1,9 @@
-use crossbeam_channel::Sender;
-use pueue_lib::settings::Settings;
 use std::fmt::Display;
 
+use crossbeam_channel::Sender;
+
 use pueue_lib::network::message::*;
+use pueue_lib::settings::Settings;
 use pueue_lib::state::SharedState;
 
 use crate::network::response_helper::*;
@@ -45,7 +46,7 @@ pub fn handle_message(
         Message::Pause(message) => pause::pause(message, sender, state),
         Message::Remove(task_ids) => remove::remove(task_ids, state, settings),
         Message::Reset(message) => reset(message, sender),
-        Message::Restart(message) => restart::restart_multiple(message, sender, state),
+        Message::Restart(message) => restart::restart_multiple(message, sender, state, settings),
         Message::Send(message) => send::send(message, sender, state),
         Message::Start(message) => start::start(message, sender, state),
         Message::Stash(task_ids) => stash::stash(task_ids, state),

daemon/network/message_handler/restart.rs

@@ -1,4 +1,5 @@
 use crossbeam_channel::Sender;
+use pueue_lib::settings::Settings;
 use std::sync::MutexGuard;
 
 use pueue_lib::aliasing::insert_alias;
@@ -16,6 +17,7 @@ pub fn restart_multiple(
     message: RestartMessage,
     sender: &Sender<Message>,
     state: &SharedState,
+    settings: &Settings,
 ) -> Message {
     let task_ids: Vec<usize> = message.tasks.iter().map(|task| task.task_id).collect();
     let mut state = state.lock().unwrap();
@@ -31,7 +33,7 @@ pub fn restart_multiple(
 
     // Actually restart all tasks
     for task in message.tasks.iter() {
-        restart(&mut state, task, message.stashed);
+        restart(&mut state, task, message.stashed, settings);
     }
 
     // Tell the task manager to start the task immediately if requested.
@@ -52,7 +54,12 @@ pub fn restart_multiple(
 ///
 /// The "not in-place" restart functionality is actually just a copy the finished task + create a
 /// new task, which is completely handled on the client-side.
-fn restart(state: &mut MutexGuard<State>, to_restart: &TasksToRestart, stashed: bool) {
+fn restart(
+    state: &mut MutexGuard<State>,
+    to_restart: &TaskToRestart,
+    stashed: bool,
+    settings: &Settings,
+) {
     // Check if we actually know this task.
     let task = if let Some(task) = state.tasks.get_mut(&to_restart.task_id) {
         task
@@ -72,10 +79,16 @@ fn restart(state: &mut MutexGuard<State>, to_restart: &TasksToRestart, stashed:
         TaskStatus::Queued
     };
 
-    // Update command and path.
-    task.original_command = to_restart.command.clone();
-    task.command = insert_alias(to_restart.command.clone());
-    task.path = to_restart.path.clone();
+    // Update command if applicable.
+    if let Some(new_command) = &to_restart.command {
+        task.original_command = new_command.clone();
+        task.command = insert_alias(settings, new_command.clone());
+    }
+
+    // Update path if applicable.
+    if let Some(path) = &to_restart.path {
+        task.path = path.clone();
+    }
 
     // Reset all variables of any previous run.
     task.start = None;

daemon/task_handler/children.rs

@@ -17,8 +17,8 @@ impl Children {
     /// A convenience function to check whether there's child with a given task_id.
     /// We have to do a nested linear search, as these datastructure aren't indexed via task_ids.
     pub fn has_child(&self, task_id: usize) -> bool {
-        for (_, pool) in self.0.iter() {
-            for (_, (child_task_id, _)) in pool.iter() {
+        for pool in self.0.values() {
+            for (child_task_id, _) in pool.values() {
                 if child_task_id == &task_id {
                     return true;
                 }
@@ -29,10 +29,11 @@ impl Children {
     }
 
     /// A convenience function to get a child by its respective task_id.
-    /// We have to do a nested linear search, as these datastructure aren't indexed via task_ids.
+    /// We have to do a nested linear search over all children of all pools,
+    /// beceause these datastructure aren't indexed via task_ids.
     pub fn get_child(&self, task_id: usize) -> Option<&Child> {
-        for (_, pool) in self.0.iter() {
-            for (_, (child_task_id, child)) in pool.iter() {
+        for pool in self.0.values() {
+            for (child_task_id, child) in pool.values() {
                 if child_task_id == &task_id {
                     return Some(child);
                 }
@@ -42,11 +43,12 @@ impl Children {
         None
     }
 
-    /// A convenience function to get a child by its respective task_id.
-    /// We have to do a nested linear search, as these datastructure aren't indexed via task_ids.
+    /// A convenience function to get a mutable child by its respective task_id.
+    /// We have to do a nested linear search over all children of all pools,
+    /// beceause these datastructure aren't indexed via task_ids.
     pub fn get_child_mut(&mut self, task_id: usize) -> Option<&mut Child> {
-        for (_, pool) in self.0.iter_mut() {
-            for (_, (child_task_id, child)) in pool.iter_mut() {
+        for pool in self.0.values_mut() {
+            for (child_task_id, child) in pool.values_mut() {
                 if child_task_id == &task_id {
                     return Some(child);
                 }
@@ -59,8 +61,8 @@ impl Children {
     /// A convenience function to get a list with all task_ids of all children.
     pub fn all_task_ids(&self) -> Vec<usize> {
         let mut task_ids = Vec::new();
-        for (_, pool) in self.0.iter() {
-            for (_, (task_id, _)) in pool.iter() {
+        for pool in self.0.values() {
+            for (task_id, _) in pool.values() {
                 task_ids.push(*task_id)
             }
         }

lib/CHANGELOG.md

@@ -8,11 +8,17 @@ The concept of SemVer is applied to the daemon/client API, but not the library A
 
 ## [0.21.0] - unreleased
 
+### Added
+
+- Added `Settings.shared.alias_file`, which can be used to specify the location of the `pueue_aliases.yml`.
+
 ### Changed
 
 - The process handling code has been moved from the daemon to `pueue_lib`. See [#336](https://github.com/Nukesor/pueue/issues/336).
     The reason for this is, that the client will need some of these process handling capabilitites to spawn shell commands when editing tasks.
 - The module structure of the platform specific networking code has been streamlined.
+- Renamed `TasksToRestart` to `TaskToRestart`.
+- Make `TaskToRestart::path` and `TaskToRestart::command` optional.
 
 ## [0.20.0] - 2022-07-21
 

lib/src/aliasing.rs

@@ -2,29 +2,21 @@ use std::collections::HashMap;
 use std::fs::File;
 use std::io::prelude::*;
 
-use log::{info, warn};
+use log::info;
 
-use crate::{error::Error, settings::configuration_directories};
+use crate::error::Error;
+use crate::settings::Settings;
 
 /// Return the contents of the alias file, if it exists and can be parsed. \
 /// The file has to be located in `pueue_directory` and named `pueue_aliases.yml`.
-pub fn get_aliases() -> Result<HashMap<String, String>, Error> {
+pub fn get_aliases(settings: &Settings) -> Result<HashMap<String, String>, Error> {
     // Go through all config directories and check for a alias file.
-    let mut alias_file_path = None;
-    for directory in configuration_directories() {
-        let path = directory.join("pueue_aliases.yml");
-        if path.exists() {
-            alias_file_path = Some(path);
-        }
-    }
+    let path = settings.shared.alias_file();
 
     // Return early if we cannot find the file
-    let path = match alias_file_path {
-        None => {
-            info!("Didn't find pueue alias file.");
-            return Ok(HashMap::new());
-        }
-        Some(alias_file_path) => alias_file_path,
+    if !path.exists() {
+        info!("Didn't find pueue alias file at {path:?}.");
+        return Ok(HashMap::new());
     };
 
     // Read the file content
@@ -42,24 +34,23 @@ pub fn get_aliases() -> Result<HashMap<String, String>, Error> {
 
 /// Check if there exists an alias for a given command.
 /// Only the first word will be replaced.
-pub fn insert_alias(command: String) -> String {
+pub fn insert_alias(settings: &Settings, command: String) -> String {
+    // Get the first word of the command.
     let first = match command.split_whitespace().next() {
         Some(first) => first,
         None => return command,
     };
 
-    let aliases = match get_aliases() {
+    let aliases = match get_aliases(settings) {
         Err(err) => {
-            warn!("Failed to open aliases file: {err}");
+            info!("Couldn't read aliases file: {err}");
             return command;
         }
         Ok(aliases) => aliases,
     };
 
-    for (original, alias) in aliases.iter() {
-        if original == first {
-            return command.replacen(original, alias, 1);
-        }
+    if let Some(alias) = aliases.get(first) {
+        return command.replacen(first, alias, 1);
     }
 
     command

lib/src/network/message.rs

@@ -128,18 +128,22 @@ pub struct StartMessage {
     pub children: bool,
 }
 
+/// The messages used to restart tasks.
+/// It's possible to update the command and paths when restarting tasks.
 #[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
 pub struct RestartMessage {
-    pub tasks: Vec<TasksToRestart>,
+    pub tasks: Vec<TaskToRestart>,
     pub start_immediately: bool,
     pub stashed: bool,
 }
 
 #[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
-pub struct TasksToRestart {
+pub struct TaskToRestart {
     pub task_id: usize,
-    pub command: String,
-    pub path: PathBuf,
+    /// Allow to restart the task with an updated command.
+    pub command: Option<String>,
+    /// Allow to restart the task with an updated path.
+    pub path: Option<PathBuf>,
 }
 
 #[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
@@ -252,7 +256,7 @@ pub struct LogRequestMessage {
 }
 
 /// Helper struct for sending tasks and their log output to the client.
-#[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
+#[derive(PartialEq, Eq, Clone, Deserialize, Serialize)]
 pub struct TaskLogMessage {
     pub task: Task,
     #[serde(default = "bool::default")]
@@ -261,6 +265,18 @@ pub struct TaskLogMessage {
     pub output: Option<Vec<u8>>,
 }
 
+/// We use a custom `Debug` implementation for [TaskLogMessage], as the `output` field
+/// has too much info in it and renders log output unreadable.
+impl std::fmt::Debug for TaskLogMessage {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("TaskLogMessage")
+            .field("task", &self.task)
+            .field("output_complete", &self.output_complete)
+            .field("output", &"hidden")
+            .finish()
+    }
+}
+
 #[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize)]
 pub struct ParallelMessage {
     pub parallel_tasks: usize,