[pdm-devel] [RFC PATCH datacenter-manager 2/2] ui: pbs: snapshot list: change to streaming 'content' api call

[Dominik Csapak <d.csapak@proxmox.com>]

this changes the logic to add elements to the tree a bit, since we now
also show the namespaces in a true tree fashion.

We also have to adapt the sorting logic a bit, since now groups can sit
alongside namespaces.

We also collect all streaming errors we encounter and show a counter on
the bottom.

There are several things to improve here still:
* show the streaming errors in a popup when clicking on the message
* construct tree nodes on demand if the elements come out of order, or a
  snapshot is listed without it's corresponding group, etc.
* add a 'cancel' button for long running requests
* change the namespace selection, either:
  - add a max-depth selection too with a sensible default
  - make the list load only one level at a time and fetch additional
    levels on expanding a namespace
  - simply remove the namespace selector

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ui/src/pbs/snapshot_list.rs | 148 +++++++++++++++++++++++++-----------
 1 file changed, 103 insertions(+), 45 deletions(-)

diff --git a/ui/src/pbs/snapshot_list.rs b/ui/src/pbs/snapshot_list.rs
index b82ffd5c..9f82054e 100644
--- a/ui/src/pbs/snapshot_list.rs
+++ b/ui/src/pbs/snapshot_list.rs
@@ -21,9 +21,11 @@ use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
 use pwt::widget::{error_message, Column, Container, Fa, Progress, Toolbar, Tooltip};
 use pwt::{css, AsyncPool};
 
-use pbs_api_types::{BackupGroup, BackupNamespace, BackupType, SnapshotListItem, VerifyState};
+use pbs_api_types::{
+    BackupGroup, BackupNamespace, BackupType, DatastoreContent, SnapshotListItem, VerifyState,
+};
 
-use proxmox_yew_comp::http_stream::Stream;
+use proxmox_yew_comp::http_stream::{Record, Stream};
 
 use crate::locale_compare;
 use crate::pbs::namespace_selector::NamespaceSelector;
@@ -59,16 +61,20 @@ struct SnapshotVerifyCount {
 #[derive(PartialEq, Clone)]
 enum SnapshotTreeEntry {
     Root(BackupNamespace),
-    Group(BackupGroup, SnapshotVerifyCount),
-    Snapshot(SnapshotListItem),
+    Group(BackupNamespace, BackupGroup, SnapshotVerifyCount),
+    Snapshot(BackupNamespace, SnapshotListItem),
 }
 
 impl ExtractPrimaryKey for SnapshotTreeEntry {
     fn extract_key(&self) -> Key {
         match self {
             SnapshotTreeEntry::Root(namespace) => Key::from(format!("root+{namespace}")),
-            SnapshotTreeEntry::Group(group, _) => Key::from(format!("group+{group}")),
-            SnapshotTreeEntry::Snapshot(entry) => Key::from(entry.backup.to_string()),
+            SnapshotTreeEntry::Group(namespace, group, _) => {
+                Key::from(format!("ns+{namespace}+group+{group}"))
+            }
+            SnapshotTreeEntry::Snapshot(namespace, entry) => {
+                Key::from(format!("ns+{namespace}+snap+{}", entry.backup))
+            }
         }
     }
 }
@@ -77,7 +83,7 @@ impl ExtractPrimaryKey for SnapshotTreeEntry {
 enum Msg {
     SelectionChange,
     ConsumeBuffer,
-    UpdateBuffer(SnapshotListItem),
+    UpdateBuffer(Record<DatastoreContent>),
     UpdateParentNamespace(Key),
     Reload,
     LoadFinished(Result<(), Error>),
@@ -89,7 +95,8 @@ struct SnapshotListComp {
     _async_pool: AsyncPool,
     columns: Rc<Vec<DataTableHeader<SnapshotTreeEntry>>>,
     load_result: Option<Result<(), Error>>,
-    buffer: Vec<SnapshotListItem>,
+    buffer: Vec<DatastoreContent>,
+    errors: Vec<serde_json::Value>,
     current_namespace: BackupNamespace,
     interval: Option<Interval>,
 }
@@ -109,7 +116,7 @@ impl SnapshotListComp {
                                 ("object-group", tr!("Namespace '{0}'", namespace))
                             }
                         }
-                        SnapshotTreeEntry::Group(group, _) => (
+                        SnapshotTreeEntry::Group(_, group, _) => (
                             match group.ty {
                                 BackupType::Vm => "desktop",
                                 BackupType::Ct => "cube",
@@ -117,7 +124,9 @@ impl SnapshotListComp {
                             },
                             group.to_string(),
                         ),
-                        SnapshotTreeEntry::Snapshot(entry) => ("file-o", entry.backup.to_string()),
+                        SnapshotTreeEntry::Snapshot(_, entry) => {
+                            ("file-o", entry.backup.to_string())
+                        }
                     };
                     render_tree_column(Fa::new(icon).fixed_width().into(), res).into()
                 })
@@ -126,10 +135,10 @@ impl SnapshotListComp {
                 .justify("right")
                 .render(|item: &SnapshotTreeEntry| match item {
                     SnapshotTreeEntry::Root(_) => "".into(),
-                    SnapshotTreeEntry::Group(_group, counts) => {
+                    SnapshotTreeEntry::Group(_ns, _group, counts) => {
                         (counts.ok + counts.failed + counts.none).into()
                     }
-                    SnapshotTreeEntry::Snapshot(_entry) => "".into(),
+                    SnapshotTreeEntry::Snapshot(_ns, _entry) => "".into(),
                 })
                 .into(),
             DataTableColumn::new(tr!("Verify State"))
@@ -145,6 +154,8 @@ impl SnapshotListComp {
             .write()
             .set_root(SnapshotTreeEntry::Root(self.current_namespace.clone()));
         self._async_pool = AsyncPool::new();
+        self.buffer.clear();
+        self.errors.clear();
         self.reload(ctx);
     }
 
@@ -193,6 +204,7 @@ impl Component for SnapshotListComp {
             _async_pool: AsyncPool::new(),
             load_result: None,
             buffer: Vec::new(),
+            errors: Vec::new(),
             current_namespace: BackupNamespace::root(),
             interval: None,
         };
@@ -216,50 +228,90 @@ impl Component for SnapshotListComp {
                 }
                 let now = (Date::now() / 1000.0) as i64;
 
+                // TODO: handle out of order items
+                // we have to create the intermediate tree nodes if items are received out of order
+
                 for item in data {
-                    let group = item.backup.group.to_string();
-                    let mut group = if let Some(group) =
-                        root.find_node_by_key_mut(&Key::from(format!("group+{group}")))
-                    {
-                        group
-                    } else {
-                        root.append(SnapshotTreeEntry::Group(
-                            item.backup.group.clone(),
-                            Default::default(),
-                        ))
-                    };
-                    if let SnapshotTreeEntry::Group(_, verify_state) = group.record_mut() {
-                        match item.verification.as_ref() {
-                            Some(state) => {
-                                match state.state {
-                                    VerifyState::Ok => verify_state.ok += 1,
-                                    VerifyState::Failed => verify_state.failed += 1,
+                    let (ns, item) = match item {
+                        DatastoreContent::NameSpace(ns) => {
+                            let ns = ns.ns;
+                            let ns_entry = SnapshotTreeEntry::Root(ns.clone());
+                            if root.find_node_by_key_mut(&ns_entry.extract_key()).is_some() {
+                                // already inserted
+                                continue;
+                            }
+
+                            let parent = SnapshotTreeEntry::Root(ns.parent());
+                            if let Some(mut parent) =
+                                root.find_node_by_key_mut(&parent.extract_key())
+                            {
+                                parent.append(ns_entry);
+                            }
+                            continue;
+                        }
+                        DatastoreContent::Group(group) => {
+                            let ns = SnapshotTreeEntry::Root(group.ns.clone());
+                            if let Some(mut ns) = root.find_node_by_key_mut(&ns.extract_key()) {
+                                let group_entry = SnapshotTreeEntry::Group(
+                                    group.ns,
+                                    group.group.backup,
+                                    Default::default(),
+                                );
+                                if ns.find_node_by_key(&group_entry.extract_key()).is_none() {
+                                    ns.append(group_entry);
                                 }
+                            }
+
+                            continue;
+                        }
+                        DatastoreContent::Snapshot(snapshot) => (snapshot.ns, snapshot.snapshot),
+                    };
 
-                                let age_days = (now - state.upid.starttime) / (30 * 24 * 60 * 60);
-                                if age_days > 30 {
-                                    verify_state.outdated += 1;
+                    let group = SnapshotTreeEntry::Group(
+                        ns.clone(),
+                        item.backup.group.clone(),
+                        Default::default(),
+                    );
+                    if let Some(mut group) = root.find_node_by_key_mut(&group.extract_key()) {
+                        if let SnapshotTreeEntry::Group(_, _, verify_state) = group.record_mut() {
+                            match item.verification.as_ref() {
+                                Some(state) => {
+                                    match state.state {
+                                        VerifyState::Ok => verify_state.ok += 1,
+                                        VerifyState::Failed => verify_state.failed += 1,
+                                    }
+
+                                    let age_days =
+                                        (now - state.upid.starttime) / (30 * 24 * 60 * 60);
+                                    if age_days > 30 {
+                                        verify_state.outdated += 1;
+                                    }
                                 }
+                                None => verify_state.none += 1,
                             }
-                            None => verify_state.none += 1,
                         }
-                    }
-                    group.append(SnapshotTreeEntry::Snapshot(item));
+                        group.append(SnapshotTreeEntry::Snapshot(ns, item));
+                    };
                 }
 
                 store.sort_by(true, |a, b| match (a, b) {
-                    (SnapshotTreeEntry::Group(a, _), SnapshotTreeEntry::Group(b, _)) => {
+                    (SnapshotTreeEntry::Root(a), SnapshotTreeEntry::Root(b)) => a.cmp(b),
+                    (SnapshotTreeEntry::Root(_), _) => std::cmp::Ordering::Less,
+                    (SnapshotTreeEntry::Group(_, a, _), SnapshotTreeEntry::Group(_, b, _)) => {
                         locale_compare(a.to_string(), &b.to_string(), true)
                     }
-                    (SnapshotTreeEntry::Snapshot(a), SnapshotTreeEntry::Snapshot(b)) => {
+                    (SnapshotTreeEntry::Snapshot(_, a), SnapshotTreeEntry::Snapshot(_, b)) => {
                         a.backup.cmp(&b.backup)
                     }
-                    _ => std::cmp::Ordering::Less,
+                    _ => std::cmp::Ordering::Equal,
                 });
                 true
             }
             Msg::UpdateBuffer(item) => {
-                self.buffer.push(item);
+                match item {
+                    Record::Data(data) => self.buffer.push(data),
+                    Record::Error(err) => self.errors.push(err),
+                }
                 false
             }
             Msg::UpdateParentNamespace(ns_key) => {
@@ -294,6 +346,11 @@ impl Component for SnapshotListComp {
             _ => None,
         };
 
+        let err_count = self.errors.len();
+        let streaming_err = (err_count > 0).then_some(error_message(&tr!(
+            "One error during streaming" | "{n} errors during streaming" % err_count
+        )));
+
         let link = ctx.link();
 
         let props = ctx.props();
@@ -333,6 +390,7 @@ impl Component for SnapshotListComp {
                     .selection(self.selection.clone()),
             )
             .with_optional_child(err.map(|err| error_message(&err.to_string())))
+            .with_optional_child(streaming_err)
             .into()
     }
 }
@@ -341,12 +399,12 @@ async fn list_snapshots(
     remote: String,
     datastore: String,
     namespace: BackupNamespace,
-    callback: yew::Callback<SnapshotListItem>,
+    callback: yew::Callback<Record<DatastoreContent>>,
 ) -> Result<(), Error> {
     let path = if namespace.is_root() {
-        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/snapshots")
+        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/content")
     } else {
-        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/snapshots?ns={namespace}")
+        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/content?ns={namespace}")
     };
 
     // TODO: refactor application/json-seq helper for general purpose use
@@ -368,7 +426,7 @@ async fn list_snapshots(
 
     let mut stream = Stream::try_from(raw_reader)?;
 
-    while let Some(entry) = stream.next::<pbs_api_types::SnapshotListItem>().await? {
+    while let Some(entry) = stream.next::<Record<DatastoreContent>>().await? {
         callback.emit(entry);
     }
 
@@ -379,7 +437,7 @@ fn render_verification(entry: &SnapshotTreeEntry) -> Html {
     let now = (Date::now() / 1000.0) as i64;
     match entry {
         SnapshotTreeEntry::Root(_) => "".into(),
-        SnapshotTreeEntry::Group(_, verify_state) => {
+        SnapshotTreeEntry::Group(_, _, verify_state) => {
             let text;
             let icon_class;
             let tip;
@@ -430,7 +488,7 @@ fn render_verification(entry: &SnapshotTreeEntry) -> Html {
                 .tip(tip)
                 .into()
         }
-        SnapshotTreeEntry::Snapshot(entry) => match &entry.verification {
+        SnapshotTreeEntry::Snapshot(_, entry) => match &entry.verification {
             Some(state) => {
                 let age_days = (now - state.upid.starttime) / (30 * 24 * 60 * 60);
                 let (text, icon_class, class) = match state.state {
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel