[pdm-devel] [PATCH datacenter-manager] ui: main/main menu: fix handling of remote list cache

* [pdm-devel] [PATCH datacenter-manager] ui: main/main menu: fix handling of remote list cache
@ 2025-09-10  8:26 Dominik Csapak
  2025-09-10 10:59 ` Wolfgang Bumiller
  2025-09-10 11:01 ` Wolfgang Bumiller
  0 siblings, 2 replies; 3+ messages in thread
From: Dominik Csapak @ 2025-09-10  8:26 UTC (permalink / raw)
  To: pdm-devel

Commit
0f84394 (ui: main menu: use initial remote list value from context)
tried to handle the initial value of the RemoteList context.

While this fixed one race (loading of the remote list finished before
the main menu was created) it introduced another race: if the load now
takes longer than the creation, we'll overwrite the PersistentState
(browser local storage) once with an empty list and once with the
correct one. This leads to a refresh not keeping the route since at that
time the route does not exists (since the remotelist is empty)

To properly fix this, we have to couple the remote list update with the
cache update, as that is the only point in time where we want to do
that.

For this we have to move the cache update logic to the main entrypoint
(where we update the RemoteList context too) and give the main menu the
list as a a property (from the cache). This way the main menu always has
the view from the cache and this get's updated on every successful api
call to update the remote list.

We still keep the remotelist context around, since other panels deeper
might need it to query information (e.g. the RemoteSelector)

Fixes: 0f84394 (ui: main menu: use initial remote list value from context)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ui/src/lib.rs       | 11 ++++++++-
 ui/src/main.rs      | 38 +++++++++++++++++++++++------
 ui/src/main_menu.rs | 58 ++++++++++-----------------------------------
 3 files changed, 53 insertions(+), 54 deletions(-)

diff --git a/ui/src/lib.rs b/ui/src/lib.rs
index 37e6458..3628f41 100644
--- a/ui/src/lib.rs
+++ b/ui/src/lib.rs
@@ -1,4 +1,7 @@
-use pdm_api_types::resource::{PveLxcResource, PveQemuResource};
+use pdm_api_types::{
+    remotes::RemoteType,
+    resource::{PveLxcResource, PveQemuResource},
+};
 use pdm_client::types::Resource;
 use serde::{Deserialize, Serialize};
 
@@ -61,6 +64,12 @@ impl std::ops::Deref for RemoteList {
     }
 }
 
+#[derive(Clone, Serialize, Deserialize, PartialEq)]
+pub struct RemoteListCacheEntry {
+    pub ty: RemoteType,
+    pub id: String,
+}
+
 /// Get the global remote list if loaded
 pub(crate) fn get_remote_list<C: yew::Component>(link: &yew::html::Scope<C>) -> Option<RemoteList> {
     let (list, _) = link.context(yew::Callback::from(|_: RemoteList| {}))?;
diff --git a/ui/src/main.rs b/ui/src/main.rs
index 2640428..3e4c5df 100644
--- a/ui/src/main.rs
+++ b/ui/src/main.rs
@@ -10,7 +10,7 @@ use yew::prelude::*;
 
 use pwt::prelude::*;
 use pwt::props::TextRenderFn;
-use pwt::state::Loader;
+use pwt::state::{Loader, PersistentState};
 use pwt::widget::{Column, DesktopApp, Dialog, Mask};
 
 use proxmox_login::Authentication;
@@ -23,7 +23,9 @@ use proxmox_yew_comp::{
 
 //use pbs::MainMenu;
 use pdm_api_types::subscription::{RemoteSubscriptionState, RemoteSubscriptions};
-use pdm_ui::{register_pve_tasks, MainMenu, RemoteList, SearchProvider, TopNavBar};
+use pdm_ui::{
+    register_pve_tasks, MainMenu, RemoteList, RemoteListCacheEntry, SearchProvider, TopNavBar,
+};
 
 type MsgRemoteList = Result<RemoteList, Error>;
 
@@ -45,6 +47,7 @@ struct DatacenterManagerApp {
     running_tasks: Loader<Vec<TaskListItem>>,
     running_tasks_timeout: Option<Timeout>,
     remote_list: RemoteList,
+    remote_list_cache: PersistentState<Vec<RemoteListCacheEntry>>,
     remote_list_error: Option<String>,
     remote_list_timeout: Option<Timeout>,
     search_provider: SearchProvider,
@@ -97,18 +100,37 @@ impl DatacenterManagerApp {
 
     fn update_remotes(&mut self, ctx: &Context<Self>, result: MsgRemoteList) -> bool {
         self.remote_list_timeout = Self::poll_remote_list(ctx, false);
+        let mut changed = false;
         match result {
             Err(err) => {
-                self.remote_list_error = Some(err.to_string());
+                if self.remote_list_error.is_none() {
+                    self.remote_list_error = Some(err.to_string());
+                    changed = true;
+                }
                 // do not touch remote_list data
-                true
             }
             Ok(list) => {
-                self.remote_list_error = None;
-                self.remote_list = list;
-                true
+                if self.remote_list_error.is_some() {
+                    self.remote_list_error = None;
+                    changed = true;
+                }
+                self.remote_list = list.clone();
+
+                let remote_list_cache: Vec<RemoteListCacheEntry> = list
+                    .into_iter()
+                    .map(|item| RemoteListCacheEntry {
+                        id: item.id.clone(),
+                        ty: item.ty,
+                    })
+                    .collect();
+
+                if *self.remote_list_cache != remote_list_cache {
+                    self.remote_list_cache.update(remote_list_cache);
+                    changed = true;
+                }
             }
         }
+        changed
     }
 
     fn poll_remote_list(ctx: &Context<Self>, first: bool) -> Option<Timeout> {
@@ -166,6 +188,7 @@ impl Component for DatacenterManagerApp {
             running_tasks,
             running_tasks_timeout: None,
             remote_list: Vec::new().into(),
+            remote_list_cache: PersistentState::new("PdmRemoteListCache"),
             remote_list_error: None,
             remote_list_timeout: None,
             search_provider: SearchProvider::new(),
@@ -247,6 +270,7 @@ impl Component for DatacenterManagerApp {
                 let main_view: Html = if self.login_info.is_some() && !loading {
                     MainMenu::new()
                         .username(username.clone())
+                        .remote_list(self.remote_list_cache.clone())
                         .remote_list_loading(self.remote_list_error.is_some())
                         .into()
                 } else {
diff --git a/ui/src/main_menu.rs b/ui/src/main_menu.rs
index d9e8f7c..475b17b 100644
--- a/ui/src/main_menu.rs
+++ b/ui/src/main_menu.rs
@@ -1,14 +1,11 @@
 use std::rc::Rc;
 
-use serde::{Deserialize, Serialize};
-
 use html::IntoPropValue;
-use wasm_bindgen::UnwrapThrowExt;
 use yew::virtual_dom::{Key, VComp, VNode};
 
 use pwt::css::{self, Display, FlexFit};
 use pwt::prelude::*;
-use pwt::state::{PersistentState, Selection};
+use pwt::state::Selection;
 use pwt::widget::nav::{Menu, MenuItem, NavigationDrawer};
 use pwt::widget::{Container, Row, SelectionView, SelectionViewRenderInfo};
 
@@ -19,7 +16,7 @@ use pdm_api_types::remotes::RemoteType;
 use crate::remotes::RemotesPanel;
 use crate::sdn::evpn::EvpnPanel;
 use crate::{
-    AccessControl, CertificatesPanel, Dashboard, RemoteList, ServerAdministration,
+    AccessControl, CertificatesPanel, Dashboard, RemoteListCacheEntry, ServerAdministration,
     SystemConfiguration,
 };
 
@@ -49,6 +46,11 @@ pub struct MainMenu {
     #[builder(IntoPropValue, into_prop_value)]
     #[prop_or_default]
     pub remote_list_loading: bool,
+
+    /// Set the list of remotes
+    #[builder]
+    #[prop_or_default]
+    pub remote_list: Vec<RemoteListCacheEntry>,
 }
 
 impl MainMenu {
@@ -59,20 +61,11 @@ impl MainMenu {
 
 pub enum Msg {
     Select(Key),
-    RemoteListChanged(RemoteList),
-}
-
-#[derive(Clone, Serialize, Deserialize, PartialEq)]
-struct RemoteListCacheEntry {
-    ty: RemoteType,
-    id: String,
 }
 
 pub struct PdmMainMenu {
     active: Key,
     menu_selection: Selection,
-    remote_list_cache: PersistentState<Vec<RemoteListCacheEntry>>,
-    _remote_list_observer: ContextHandle<RemoteList>,
 }
 
 fn register_view(
@@ -109,43 +102,17 @@ fn register_submenu(
     );
 }
 
-impl PdmMainMenu {
-    fn update_remote_list(&mut self, remote_list: RemoteList) -> bool {
-        let remote_list_cache: Vec<RemoteListCacheEntry> = remote_list
-            .into_iter()
-            .map(|item| RemoteListCacheEntry {
-                id: item.id.clone(),
-                ty: item.ty,
-            })
-            .collect();
-
-        if *self.remote_list_cache != remote_list_cache {
-            self.remote_list_cache.update(remote_list_cache);
-            true
-        } else {
-            false
-        }
-    }
-}
+impl PdmMainMenu {}
 
 impl Component for PdmMainMenu {
     type Message = Msg;
     type Properties = MainMenu;
 
-    fn create(ctx: &Context<Self>) -> Self {
-        let (remote_list, _remote_list_observer) = ctx
-            .link()
-            .context(ctx.link().callback(Msg::RemoteListChanged))
-            .unwrap_throw();
-        let mut this = Self {
+    fn create(_ctx: &Context<Self>) -> Self {
+        Self {
             active: Key::from("dashboard"),
             menu_selection: Selection::new(),
-            remote_list_cache: PersistentState::new("PdmRemoteListCache"),
-            _remote_list_observer,
-        };
-
-        this.update_remote_list(remote_list);
-        this
+        }
     }
 
     fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
@@ -154,7 +121,6 @@ impl Component for PdmMainMenu {
                 self.active = key;
                 true
             }
-            Msg::RemoteListChanged(remote_list) => self.update_remote_list(remote_list),
         }
     }
 
@@ -264,7 +230,7 @@ impl Component for PdmMainMenu {
 
         let mut remote_submenu = Menu::new();
 
-        for remote in self.remote_list_cache.iter() {
+        for remote in props.remote_list.iter() {
             register_view(
                 &mut remote_submenu,
                 &mut content,
-- 
2.47.3



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


^ permalink raw reply	[flat|nested] 3+ messages in thread