From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 7C83B1FF183 for ; Wed, 10 Sep 2025 12:59:20 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 50C3C1E963; Wed, 10 Sep 2025 12:59:23 +0200 (CEST) Date: Wed, 10 Sep 2025 12:59:18 +0200 From: Wolfgang Bumiller To: Dominik Csapak Message-ID: References: <20250910082649.1007491-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250910082649.1007491-1-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1757501933278 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.075 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [main.rs, lib.rs] Subject: Re: [pdm-devel] [PATCH datacenter-manager] ui: main/main menu: fix handling of remote list cache X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: pdm-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" 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 > --- > 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(link: &yew::html::Scope) -> Option { > 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; > > @@ -45,6 +47,7 @@ struct DatacenterManagerApp { > running_tasks: Loader>, > running_tasks_timeout: Option, > remote_list: RemoteList, > + remote_list_cache: PersistentState>, > remote_list_error: Option, > remote_list_timeout: Option, > search_provider: SearchProvider, > @@ -97,18 +100,37 @@ impl DatacenterManagerApp { > > fn update_remotes(&mut self, ctx: &Context, 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 = 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, first: bool) -> Option { > @@ -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, > } > > 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>, > - _remote_list_observer: ContextHandle, > } > > 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 = 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 { > - 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 { > 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, 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