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

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

applied, thanks

On Wed, Sep 10, 2025 at 10:26:44AM +0200, Dominik Csapak wrote:
> 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