[pdm-devel] [PATCH proxmox-datacenter-manager 03/15] task cache: add basic test for TaskCache

[Lukas Wagner]

This one helps us to verify that we did not break the core functionality
in the upcoming changes.

Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
---
 server/src/task_cache.rs | 100 +++++++++++++++++++++++++++++++--------
 1 file changed, 79 insertions(+), 21 deletions(-)

diff --git a/server/src/task_cache.rs b/server/src/task_cache.rs
index 7ded540..211beb4 100644
--- a/server/src/task_cache.rs
+++ b/server/src/task_cache.rs
@@ -25,8 +25,13 @@ pub async fn get_tasks(max_age: i64, filters: TaskFilters) -> Result<Vec<TaskLis
 
     let mut all_tasks = Vec::new();
 
+    let api_uid = pdm_config::api_user()?.uid;
+    let api_gid = pdm_config::api_group()?.gid;
+
+    let file_options = CreateOptions::new().owner(api_uid).group(api_gid);
+
     let cache_path = Path::new(pdm_buildcfg::PDM_CACHE_DIR).join("taskcache.json");
-    let mut cache = TaskCache::new(cache_path)?;
+    let mut cache = TaskCache::new(cache_path, file_options)?;
 
     // Force a refresh for all tasks of a remote if a task is finished.
     // Not super nice, but saves us from persisting finished tasks. Also,
@@ -246,27 +251,30 @@ struct TaskCache {
 
     /// File-location at which the cached tasks are stored.
     cachefile_path: PathBuf,
+
+    /// File mode/owner/group for the cache file.
+    cachefile_options: CreateOptions,
 }
 
 impl TaskCache {
     /// Create a new tasks cache instance by loading
     /// the cache from disk.
-    fn new(cachefile_path: PathBuf) -> Result<Self, Error> {
+    fn new(cachefile_path: PathBuf, cachefile_options: CreateOptions) -> Result<Self, Error> {
         Ok(Self {
-            content: Self::load_content()?,
+            content: Self::load_content(&cachefile_path)?,
             new_or_updated: Default::default(),
             dirty: false,
             cachefile_path,
+            cachefile_options,
         })
     }
 
     /// Load the task cache contents from disk.
-    fn load_content() -> Result<TaskCacheContent, Error> {
-        let taskcache_path = Path::new(pdm_buildcfg::PDM_CACHE_DIR).join("taskcache.json");
-        let content = proxmox_sys::fs::file_read_optional_string(taskcache_path)?;
+    fn load_content(path: &Path) -> Result<TaskCacheContent, Error> {
+        let content = proxmox_sys::fs::file_read_optional_string(path)?;
 
         let content = if let Some(content) = content {
-            serde_json::from_str(&content)?
+            serde_json::from_str(&content).unwrap_or_default()
         } else {
             Default::default()
         };
@@ -293,7 +301,7 @@ impl TaskCache {
         let _guard = self.lock(Duration::from_secs(5))?;
 
         // Read content again, in case somebody has changed it in the meanwhile
-        let mut content = Self::load_content()?;
+        let mut content = Self::load_content(&self.cachefile_path)?;
 
         for (remote_name, entry) in self.new_or_updated.remote_tasks.drain() {
             if let Some(existing_entry) = content.remote_tasks.get_mut(&remote_name) {
@@ -308,12 +316,12 @@ impl TaskCache {
 
         let bytes = serde_json::to_vec_pretty(&content)?;
 
-        let api_uid = pdm_config::api_user()?.uid;
-        let api_gid = pdm_config::api_group()?.gid;
-
-        let file_options = CreateOptions::new().owner(api_uid).group(api_gid);
-
-        proxmox_sys::fs::replace_file(&self.cachefile_path, &bytes, file_options, true)?;
+        proxmox_sys::fs::replace_file(
+            &self.cachefile_path,
+            &bytes,
+            self.cachefile_options.clone(),
+            true,
+        )?;
 
         self.dirty = false;
 
@@ -358,22 +366,23 @@ impl TaskCache {
     // without a lock, since the cache file is replaced atomically
     // when updating.
     fn lock(&self, duration: Duration) -> Result<File, Error> {
-        let api_uid = pdm_config::api_user()?.uid;
-        let api_gid = pdm_config::api_group()?.gid;
-
-        let file_options = CreateOptions::new().owner(api_uid).group(api_gid);
-        proxmox_sys::fs::open_file_locked(self.lockfile_path(), duration, true, file_options)
+        proxmox_sys::fs::open_file_locked(
+            self.lockfile_path(),
+            duration,
+            true,
+            self.cachefile_options.clone(),
+        )
     }
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Debug, Serialize, Deserialize)]
 /// Per-remote entry in the task cache.
 struct TaskCacheEntry {
     timestamp: i64,
     tasks: Vec<TaskListItem>,
 }
 
-#[derive(Default, Serialize, Deserialize)]
+#[derive(Debug, Default, Serialize, Deserialize)]
 /// Content of the task cache file.
 struct TaskCacheContent {
     remote_tasks: HashMap<String, TaskCacheEntry>,
@@ -522,3 +531,52 @@ pub fn tasktype(status: &str) -> TaskStateType {
         TaskStateType::Error
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::test_support::temp::NamedTempFile;
+
+    #[test]
+    fn basic_task_cache() -> Result<(), Error> {
+        let options = CreateOptions::new()
+            .owner(nix::unistd::Uid::effective())
+            .group(nix::unistd::Gid::effective())
+            .perm(nix::sys::stat::Mode::from_bits_truncate(0o600));
+
+        let temp_file = NamedTempFile::new(options.clone())?;
+
+        let mut cache = TaskCache::new(temp_file.path().into(), options.clone())?;
+
+        let mut tasks = Vec::new();
+
+        let now = proxmox_time::epoch_i64();
+        for i in (0..20).rev() {
+            let upid: PveUpid =
+                "UPID:pve-node:0000C530:001C9BEC:677E934A:stopall::root at pam:".parse()?;
+
+            tasks.push(TaskListItem {
+                upid: upid.to_string(),
+                node: upid.node,
+                pid: upid.pid as i64,
+                pstart: upid.pstart,
+                starttime: now - 10 * i,
+                worker_type: upid.worker_type,
+                worker_id: upid.worker_id,
+                user: upid.auth_id,
+                endtime: None,
+                status: None,
+            });
+        }
+
+        cache.set_tasks("some-remote", tasks.clone(), now);
+        cache.save()?;
+
+        let cache = TaskCache::new(temp_file.path().into(), options)?;
+
+        let res = cache.get_tasks("some-remote", now, 10000).unwrap();
+        assert_eq!(tasks, res);
+
+        Ok(())
+    }
+}
-- 
2.39.5