[pbs-devel] [PATCH proxmox-backup 1/2] api: datastore: make group notes path helper a DataStore method

[Christian Ebner]

Move and make the helper function to get a backup groups notes file
path a `DataStore` method instead. This allows it to be reused when
access to the notes path is required from the datastore itself.

Further, use the plural `notes` wording also in the helper to be
consistent with the rest of the codebase.

In preparation for correctly removing the notes file from the backup
group on destruction.

No functional changes intended.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
 pbs-datastore/src/datastore.rs | 11 +++++++++++
 src/api2/admin/datastore.rs    | 26 +++++++-------------------
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index cbf78ecb6..91c7e76be 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -40,6 +40,8 @@ use crate::DataBlob;
 static DATASTORE_MAP: LazyLock<Mutex<HashMap<String, Arc<DataStoreImpl>>>> =
     LazyLock::new(|| Mutex::new(HashMap::new()));
 
+const GROUP_NOTES_FILE_NAME: &str = "notes";
+
 /// checks if auth_id is owner, or, if owner is a token, if
 /// auth_id is the user of the token
 pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error> {
@@ -524,6 +526,15 @@ impl DataStore {
         full_path
     }
 
+    /// Returns the absolute path of a backup groups notes file
+    pub fn group_notes_path(
+        &self,
+        ns: &BackupNamespace,
+        group: &pbs_api_types::BackupGroup,
+    ) -> PathBuf {
+        self.group_path(ns, group).join(GROUP_NOTES_FILE_NAME)
+    }
+
     /// Returns the absolute path for backup_dir
     pub fn snapshot_path(
         &self,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 392494488..cc7e17a29 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -4,7 +4,7 @@ use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::ops::Deref;
 use std::os::unix::ffi::OsStrExt;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::sync::Arc;
 
 use anyhow::{bail, format_err, Context, Error};
@@ -77,18 +77,6 @@ use crate::backup::{
 
 use crate::server::jobstate::{compute_schedule_status, Job, JobState};
 
-const GROUP_NOTES_FILE_NAME: &str = "notes";
-
-fn get_group_note_path(
-    store: &DataStore,
-    ns: &BackupNamespace,
-    group: &pbs_api_types::BackupGroup,
-) -> PathBuf {
-    let mut note_path = store.group_path(ns, group);
-    note_path.push(GROUP_NOTES_FILE_NAME);
-    note_path
-}
-
 // helper to unify common sequence of checks:
 // 1. check privs on NS (full or limited access)
 // 2. load datastore
@@ -244,8 +232,8 @@ pub fn list_groups(
                 })
                 .to_owned();
 
-            let note_path = get_group_note_path(&datastore, &ns, group.as_ref());
-            let comment = file_read_firstline(note_path).ok();
+            let notes_path = datastore.group_notes_path(&ns, group.as_ref());
+            let comment = file_read_firstline(notes_path).ok();
 
             group_info.push(GroupListItem {
                 backup: group.into(),
@@ -2053,8 +2041,8 @@ pub fn get_group_notes(
         &backup_group,
     )?;
 
-    let note_path = get_group_note_path(&datastore, &ns, &backup_group);
-    Ok(file_read_optional_string(note_path)?.unwrap_or_else(|| "".to_owned()))
+    let notes_path = datastore.group_notes_path(&ns, &backup_group);
+    Ok(file_read_optional_string(notes_path)?.unwrap_or_else(|| "".to_owned()))
 }
 
 #[api(
@@ -2101,8 +2089,8 @@ pub fn set_group_notes(
         &backup_group,
     )?;
 
-    let note_path = get_group_note_path(&datastore, &ns, &backup_group);
-    replace_file(note_path, notes.as_bytes(), CreateOptions::new(), false)?;
+    let notes_path = datastore.group_notes_path(&ns, &backup_group);
+    replace_file(notes_path, notes.as_bytes(), CreateOptions::new(), false)?;
 
     Ok(())
 }
-- 
2.39.5