[pbs-devel] [PATCH proxmox-backup 1/2] api: add support for notes on backup groups

[pbs-devel] [PATCH proxmox-backup 1/2] api: add support for notes on backup groups

[Stefan Reiter]

Stored in atomically-updated 'notes' file in backup group directory.
Available via dedicated GET/PUT API calls, as well as the first line
being included in list_groups (similar to list_snapshots).

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/api2/admin/datastore.rs | 104 +++++++++++++++++++++++++++++++++++-
 src/api2/types/mod.rs       |   3 ++
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index b65c12ad..184def5a 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -3,6 +3,7 @@
 use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::os::unix::ffi::OsStrExt;
+use std::path::PathBuf;
 
 use anyhow::{bail, format_err, Error};
 use futures::*;
@@ -17,7 +18,9 @@ use proxmox::api::{
 };
 use proxmox::api::router::{ReturnType, SubdirMap};
 use proxmox::api::schema::*;
-use proxmox::tools::fs::{replace_file, CreateOptions};
+use proxmox::tools::fs::{
+    file_read_firstline, file_read_optional_string, replace_file, CreateOptions,
+};
 use proxmox::{http_err, identity, list_subdirs_api_method, sortable};
 
 use pxar::accessor::aio::Accessor;
@@ -46,6 +49,15 @@ use crate::config::acl::{
     PRIV_DATASTORE_VERIFY,
 };
 
+const GROUP_NOTES_FILE_NAME: &str = "notes";
+
+fn get_group_note_path(store: &DataStore, group: &BackupGroup) -> PathBuf {
+    let mut note_path = store.base_path();
+    note_path.push(group.group_path());
+    note_path.push(GROUP_NOTES_FILE_NAME);
+    note_path
+}
+
 fn check_priv_or_backup_owner(
     store: &DataStore,
     group: &BackupGroup,
@@ -204,6 +216,9 @@ pub fn list_groups(
                 })
                 .to_owned();
 
+            let note_path = get_group_note_path(&datastore, &group);
+            let comment = file_read_firstline(&note_path).ok();
+
             group_info.push(GroupListItem {
                 backup_type: group.backup_type().to_string(),
                 backup_id: group.backup_id().to_string(),
@@ -211,6 +226,7 @@ pub fn list_groups(
                 owner: Some(owner),
                 backup_count,
                 files: last_backup.files,
+                comment,
             });
 
             group_info
@@ -1558,6 +1574,86 @@ pub fn get_rrd_stats(
     )
 }
 
+#[api(
+    input: {
+        properties: {
+            store: {
+                schema: DATASTORE_SCHEMA,
+            },
+            "backup-type": {
+                schema: BACKUP_TYPE_SCHEMA,
+            },
+            "backup-id": {
+                schema: BACKUP_ID_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_BACKUP, true),
+    },
+)]
+/// Get "notes" for a backup group
+pub fn get_group_notes(
+    store: String,
+    backup_type: String,
+    backup_id: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<String, Error> {
+    let datastore = DataStore::lookup_datastore(&store)?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let backup_group = BackupGroup::new(backup_type, backup_id);
+
+    check_priv_or_backup_owner(&datastore, &backup_group, &auth_id, PRIV_DATASTORE_AUDIT)?;
+
+    let note_path = get_group_note_path(&datastore, &backup_group);
+    Ok(file_read_optional_string(note_path)?.unwrap_or_else(|| "".to_owned()))
+}
+
+#[api(
+    input: {
+        properties: {
+            store: {
+                schema: DATASTORE_SCHEMA,
+            },
+            "backup-type": {
+                schema: BACKUP_TYPE_SCHEMA,
+            },
+            "backup-id": {
+                schema: BACKUP_ID_SCHEMA,
+            },
+            notes: {
+                description: "A multiline text.",
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["datastore", "{store}"],
+                                           PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_BACKUP,
+                                           true),
+    },
+)]
+/// Set "notes" for a backup group
+pub fn set_group_notes(
+    store: String,
+    backup_type: String,
+    backup_id: String,
+    notes: String,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let datastore = DataStore::lookup_datastore(&store)?;
+
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let backup_group = BackupGroup::new(backup_type, backup_id);
+
+    check_priv_or_backup_owner(&datastore, &backup_group, &auth_id, PRIV_DATASTORE_MODIFY)?;
+
+    let note_path = get_group_note_path(&datastore, &backup_group);
+    replace_file(note_path, notes.as_bytes(), CreateOptions::new())?;
+
+    Ok(())
+}
+
 #[api(
     input: {
         properties: {
@@ -1782,6 +1878,12 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
             .get(&API_METHOD_GARBAGE_COLLECTION_STATUS)
             .post(&API_METHOD_START_GARBAGE_COLLECTION)
     ),
+    (
+        "group-notes",
+        &Router::new()
+            .get(&API_METHOD_GET_GROUP_NOTES)
+            .put(&API_METHOD_SET_GROUP_NOTES)
+    ),
     (
         "groups",
         &Router::new()
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 6698f4b7..c9ac56e4 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -513,6 +513,9 @@ pub struct GroupListItem {
     /// The owner of group
     #[serde(skip_serializing_if="Option::is_none")]
     pub owner: Option<Authid>,
+    /// The first line from group "notes"
+    #[serde(skip_serializing_if="Option::is_none")]
+    pub comment: Option<String>,
 }
 
 #[api()]
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/2] ui: add support for notes on backup groups

[Stefan Reiter]

Currently done a little bit hacky in a seperate API call following the
initial list_snapshots, as we previously didn't call list_groups at all
and instead calculated the groups from the snapshots.

This calls it async and updates the view with group comments when data
arrives. The editor is simply reused with the 'group-notes' API call,
since the semantics are the same.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 www/datastore/Content.js | 59 ++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 101763aa..29d58fc3 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -125,6 +125,29 @@ Ext.define('PBS.DataStoreContent', {
 	    return groups;
 	},
 
+	updateGroupNotes: function(view) {
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/admin/datastore/${view.datastore}/groups`,
+		method: 'GET',
+		success: function(response) {
+		    let groups = response.result.data;
+		    let map = {};
+		    for (const group of groups) {
+			map[`${group["backup-type"]}/${group["backup-id"]}`] = group["comment"];
+		    }
+		    view.getRootNode().cascade(node => {
+			if (node.parentNode && node.parentNode.id === 'root') {
+			    node.set(
+				'comment',
+				map[`${node.data.backup_type}/${node.data.backup_id}`],
+				{ dirty: false },
+			    );
+			}
+		    });
+		},
+	    });
+	},
+
 	onLoad: function(store, records, success, operation) {
 	    let me = this;
 	    let view = this.getView();
@@ -242,6 +265,8 @@ Ext.define('PBS.DataStoreContent', {
 		children: children,
 	    });
 
+	    this.updateGroupNotes(view);
+
 	    if (selected !== undefined) {
 		let selection = view.getRootNode().findChildBy(function(item) {
 		    let id = item.data.text;
@@ -358,19 +383,31 @@ Ext.define('PBS.DataStoreContent', {
 	    });
 	},
 
-	onNotesEdit: function(view, data) {
+	onNotesEdit: function(view, data, isGroup) {
 	    let me = this;
 
-	    let url = `/admin/datastore/${view.datastore}/notes`;
+	    let url = `/admin/datastore/${view.datastore}/`;
+	    url += isGroup ? 'group-notes' : 'notes';
+
+	    let params;
+	    if (isGroup) {
+		params = {
+		    "backup-type": data.backup_type,
+		    "backup-id": data.backup_id,
+		};
+	    } else {
+		params = {
+		    "backup-type": data["backup-type"],
+		    "backup-id": data["backup-id"],
+		    "backup-time": (data['backup-time'].getTime()/1000).toFixed(0),
+		};
+	    }
+
 	    Ext.create('PBS.window.NotesEdit', {
 		url: url,
 		autoShow: true,
 		apiCallDone: () => me.reload(), // FIXME: do something more efficient?
-		extraRequestParams: {
-		    "backup-type": data["backup-type"],
-		    "backup-id": data["backup-id"],
-		    "backup-time": (data['backup-time'].getTime()/1000).toFixed(0),
-		},
+		extraRequestParams: params,
 	    });
 	},
 
@@ -585,7 +622,7 @@ Ext.define('PBS.DataStoreContent', {
 	    flex: 1,
 	    renderer: (v, meta, record) => {
 		let data = record.data;
-		if (!data || data.leaf || record.parentNode.id === 'root') {
+		if (!data || data.leaf) {
 		    return '';
 		}
 		if (v === undefined || v === null) {
@@ -608,17 +645,17 @@ Ext.define('PBS.DataStoreContent', {
 			}
 			let view = tree.up();
 			let controller = view.controller;
-			controller.onNotesEdit(view, rec.data);
+			controller.onNotesEdit(view, rec.data, rec.parentNode.id === 'root');
 		    });
 		},
 		dblclick: function(tree, el, row, col, ev, rec) {
 		    let data = rec.data || {};
-		    if (data.leaf || rec.parentNode.id === 'root') {
+		    if (data.leaf) {
 			return;
 		    }
 		    let view = tree.up();
 		    let controller = view.controller;
-		    controller.onNotesEdit(view, rec.data);
+		    controller.onNotesEdit(view, rec.data, rec.parentNode.id === 'root');
 		},
 	    },
 	},
-- 
2.30.2

[pbs-devel] applied: [PATCH proxmox-backup 1/2] api: add support for notes on backup groups

[Thomas Lamprecht]

On 08.07.21 16:45, Stefan Reiter wrote:
> Stored in atomically-updated 'notes' file in backup group directory.
> Available via dedicated GET/PUT API calls, as well as the first line
> being included in list_groups (similar to list_snapshots).
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  src/api2/admin/datastore.rs | 104 +++++++++++++++++++++++++++++++++++-
>  src/api2/types/mod.rs       |   3 ++
>  2 files changed, 106 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

FYI: resolved a merge conflict due to the types moving to their own workspace-crate.

Re: [pbs-devel] [PATCH proxmox-backup 2/2] ui: add support for notes on backup groups

[Thomas Lamprecht]

On 08.07.21 16:45, Stefan Reiter wrote:
> Currently done a little bit hacky in a seperate API call following the
> initial list_snapshots, as we previously didn't call list_groups at all
> and instead calculated the groups from the snapshots.
> 
> This calls it async and updates the view with group comments when data
> arrives. The editor is simply reused with the 'group-notes' API call,
> since the semantics are the same.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  www/datastore/Content.js | 59 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/www/datastore/Content.js b/www/datastore/Content.js
> index 101763aa..29d58fc3 100644
> --- a/www/datastore/Content.js
> +++ b/www/datastore/Content.js
> @@ -125,6 +125,29 @@ Ext.define('PBS.DataStoreContent', {
>  	    return groups;
>  	},
>  
> +	updateGroupNotes: function(view) {
> +	    Proxmox.Utils.API2Request({
> +		url: `/api2/extjs/admin/datastore/${view.datastore}/groups`,
> +		method: 'GET',
> +		success: function(response) {
> +		    let groups = response.result.data;
> +		    let map = {};
> +		    for (const group of groups) {
> +			map[`${group["backup-type"]}/${group["backup-id"]}`] = group["comment"];

eslint throws an error on this, as the rules enforce that it'd be written in dot notation:

group.comment

Please actually use the build system before sending such things, a simple `make install`
in the www/ folder would have shown that this cannot work...

Fixed in a followup, rewrote that whole function to async while at it.