From f6a37f40f69a423c8f067383ab611995cd2a311e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com>
Date: Wed, 25 May 2022 15:14:56 +0200
Subject: [PATCH] tree-wide: remove DatastoreWithNamespace
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

instead move the acl_path helper to BackupNamespace, and introduce a new
helper for printing a store+ns when logging/generating error messages.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 49 ++++++++++++++--------------------
 pbs-api-types/src/jobs.rs      | 22 +++++++--------
 2 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index e2bf70aa..88724c3e 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -694,6 +694,17 @@ impl BackupNamespace {
         }
         Ok(())
     }
+
+    pub fn acl_path<'a>(&'a self, store: &'a str) -> Vec<&'a str> {
+        let mut path: Vec<&str> = vec!["datastore", store];
+
+        if self.is_root() {
+            path
+        } else {
+            path.extend(self.inner.iter().map(|comp| comp.as_str()));
+            path
+        }
+    }
 }
 
 impl fmt::Display for BackupNamespace {
@@ -1026,35 +1037,6 @@ impl fmt::Display for BackupDir {
     }
 }
 
-/// Helper struct for places where sensible formatting of store+NS combo is required
-pub struct DatastoreWithNamespace {
-    pub store: String,
-    pub ns: BackupNamespace,
-}
-
-impl fmt::Display for DatastoreWithNamespace {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if self.ns.is_root() {
-            write!(f, "datastore {}, root namespace", self.store)
-        } else {
-            write!(f, "datastore '{}', namespace '{}'", self.store, self.ns)
-        }
-    }
-}
-
-impl DatastoreWithNamespace {
-    pub fn acl_path(&self) -> Vec<&str> {
-        let mut path: Vec<&str> = vec!["datastore", &self.store];
-
-        if self.ns.is_root() {
-            path
-        } else {
-            path.extend(self.ns.inner.iter().map(|comp| comp.as_str()));
-            path
-        }
-    }
-}
-
 /// Used when both a backup group or a directory can be valid.
 pub enum BackupPart {
     Group(BackupGroup),
@@ -1479,3 +1461,12 @@ pub fn print_ns_and_snapshot(ns: &BackupNamespace, dir: &BackupDir) -> String {
         format!("{}/{}", ns.display_as_path(), dir)
     }
 }
+
+/// Prints a Datastore name and [`BackupNamespace`] for logs/errors.
+pub fn print_store_and_ns(store: &str, ns: &BackupNamespace) -> String {
+    if ns.is_root() {
+        format!("datastore '{}', root namespace", store)
+    } else {
+        format!("datastore '{}', namespace '{}'", store, ns)
+    }
+}
diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index d3739315..c65a6085 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -7,9 +7,9 @@ use serde::{Deserialize, Serialize};
 use proxmox_schema::*;
 
 use crate::{
-    Authid, BackupNamespace, BackupType, DatastoreWithNamespace, RateLimitConfig, Userid,
-    BACKUP_GROUP_SCHEMA, BACKUP_NAMESPACE_SCHEMA, DATASTORE_SCHEMA, DRIVE_NAME_SCHEMA,
-    MEDIA_POOL_NAME_SCHEMA, NS_MAX_DEPTH_REDUCED_SCHEMA, PROXMOX_SAFE_ID_FORMAT, REMOTE_ID_SCHEMA,
+    Authid, BackupNamespace, BackupType, RateLimitConfig, Userid, BACKUP_GROUP_SCHEMA,
+    BACKUP_NAMESPACE_SCHEMA, DATASTORE_SCHEMA, DRIVE_NAME_SCHEMA, MEDIA_POOL_NAME_SCHEMA,
+    NS_MAX_DEPTH_REDUCED_SCHEMA, PROXMOX_SAFE_ID_FORMAT, REMOTE_ID_SCHEMA,
     SINGLE_LINE_COMMENT_SCHEMA,
 };
 
@@ -224,10 +224,10 @@ pub struct VerificationJobConfig {
 }
 
 impl VerificationJobConfig {
-    pub fn store_with_ns(&self) -> DatastoreWithNamespace {
-        DatastoreWithNamespace {
-            store: self.store.clone(),
-            ns: self.ns.clone().unwrap_or_default(),
+    pub fn acl_path(&self) -> Vec<&str> {
+        match self.ns.as_ref() {
+            Some(ns) => ns.acl_path(&self.store),
+            None => vec!["datastore", &self.store],
         }
     }
 }
@@ -508,10 +508,10 @@ pub struct SyncJobConfig {
 }
 
 impl SyncJobConfig {
-    pub fn store_with_ns(&self) -> DatastoreWithNamespace {
-        DatastoreWithNamespace {
-            store: self.store.clone(),
-            ns: self.ns.clone().unwrap_or_default(),
+    pub fn acl_path(&self) -> Vec<&str> {
+        match self.ns.as_ref() {
+            Some(ns) => ns.acl_path(&self.store),
+            None => vec!["datastore", &self.store],
         }
     }
 }