From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 14F621FF187 for ; Mon, 25 Aug 2025 15:14:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6E52812FBA; Mon, 25 Aug 2025 15:14:27 +0200 (CEST) Message-ID: <1cc7cd7a-a9d1-4892-85d0-26f95271d497@proxmox.com> Date: Mon, 25 Aug 2025 15:14:23 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox Datacenter Manager development discussion , Dominik Csapak References: <20250825090014.1138485-1-d.csapak@proxmox.com> <20250825090014.1138485-4-d.csapak@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20250825090014.1138485-4-d.csapak@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.708 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH datacenter-manager v2 3/9] server: api: resources: add more complex filter syntax 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" comments inline On 8/25/25 11:00 AM, Dominik Csapak wrote: > by using the new pdm-search crate for the resources api call. > > We have to do 3 filter passes: > * one fast pass for remotes if the filter are constructed in a way that > must filter to 'remote' (in this case we don't have to look at/return > the resources at all, and can skip remotes that don't match) > * a pass for the resources > * a second pass for the remotes that check if they match for > remote/non-remote mixed results > > Signed-off-by: Dominik Csapak > --- > server/Cargo.toml | 1 + > server/src/api/resources.rs | 84 +++++++++++++++++++++++++++++++------ > 2 files changed, 73 insertions(+), 12 deletions(-) > > diff --git a/server/Cargo.toml b/server/Cargo.toml > index 24a2e40..ada7c80 100644 > --- a/server/Cargo.toml > +++ b/server/Cargo.toml > @@ -74,6 +74,7 @@ proxmox-acme-api = { workspace = true, features = [ "impl" ] } > pdm-api-types.workspace = true > pdm-buildcfg.workspace = true > pdm-config.workspace = true > +pdm-search.workspace = true > > pve-api-types = { workspace = true, features = [ "client" ] } > pbs-api-types.workspace = true > diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs > index 6a8c8ef..027298a 100644 > --- a/server/src/api/resources.rs > +++ b/server/src/api/resources.rs > @@ -15,12 +15,13 @@ use pdm_api_types::subscription::{ > NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel, > }; > use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT}; > +use pdm_search::{Search, SearchTerm}; > use proxmox_access_control::CachedUserInfo; > use proxmox_router::{ > http_bail, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap, > }; > use proxmox_rrd_api_types::RrdTimeframe; > -use proxmox_schema::api; > +use proxmox_schema::{api, parse_boolean}; > use proxmox_sortable_macro::sortable; > use proxmox_subscription::SubscriptionStatus; > use pve_api_types::{ClusterResource, ClusterResourceType}; > @@ -45,6 +46,44 @@ const SUBDIRS: SubdirMap = &sorted!([ > ), > ]); > > +fn resource_matches_search_term(resource: &Resource, term: &SearchTerm) -> bool { > + match &term.category { > + Some(category) => match category.as_str() { > + "type" => resource.resource_type().contains(&term.value), > + "name" => resource.name().contains(&term.value), > + "id" => resource.id().contains(&term.value), > + "status" => resource.status().contains(&term.value), > + "template" => match resource { > + Resource::PveQemu(PveQemuResource { template, .. }, ..) > + | Resource::PveLxc(PveLxcResource { template, .. }) => { > + match parse_boolean(&term.value) { > + Ok(boolean) => boolean == *template, > + Err(_) => false, > + } > + } > + _ => false, > + }, > + "remote" => true, // this has to be checked beforehand > + _ => false, > + }, > + None => resource.name().contains(&term.value) || resource.id().contains(&term.value), > + } > +} > + > +fn remote_matches_search_term(remote_name: &str, online: Option, term: &SearchTerm) -> bool { > + match term.category.as_deref() { > + Some("remote" | "name" | "id") => remote_name.contains(&term.value), > + Some("type") => "remote".contains(&term.value), > + Some("status") => match online { > + None => true, > + Some(true) => "online".contains(&term.value), > + Some(false) => "offline".contains(&term.value), > + }, > + None => remote_name.contains(&term.value) || "remote".contains(&term.value), > + Some(_) => false, > + } > +} > + I wonder, if in the future it would make sense to make SearchTerm generic over SearchTerm with String as default for both. But currently that's probably a YAGNI. > #[api( > // FIXME:: see list-like API calls in resource routers, we probably want more fine-grained > // checks.. > @@ -104,6 +143,14 @@ pub(crate) async fn get_resources_impl( > let (remotes_config, _) = pdm_config::remotes::config()?; > let mut join_handles = Vec::new(); > > + let mut filters = Search::new(); > + > + if let Some(search) = &search { > + filters = Search::from(search); > + } nit: if we had a Default implementation we could do let mut filters = search.map(Search::from).unwrap_or_default(); > + > + let remotes_only = filters.category_value_required("type", "remote"); > + > for (remote_name, remote) in remotes_config { > if let Some(ref auth_id) = opt_auth_id { > let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]); > @@ -111,12 +158,27 @@ pub(crate) async fn get_resources_impl( > continue; > } > } > + > + if remotes_only > + && !filters.matches(|term| remote_matches_search_term(&remote_name, None, term)) > + { > + continue; > + } > + let filter = filters.clone(); > let handle = tokio::spawn(async move { > - let (resources, error) = match get_resources_for_remote(remote, max_age).await { > + let (mut resources, error) = match get_resources_for_remote(remote, max_age).await { > Ok(resources) => (resources, None), > Err(error) => (Vec::new(), Some(error.to_string())), > }; > > + if remotes_only { > + resources.clear(); > + } else if !filter.is_empty() { > + resources.retain(|resource| { > + filter.matches(|filter| resource_matches_search_term(resource, filter)) > + }); > + } > + > RemoteResources { > remote: remote_name, > resources, > @@ -132,17 +194,15 @@ pub(crate) async fn get_resources_impl( > remote_resources.push(handle.await?); > } > > - if let Some(search) = search { > - // FIXME implement more complex filter syntax > - remote_resources.retain_mut(|res| { > - if res.remote.contains(&search) { > - true > - } else { > - res.resources > - .retain(|res| res.id().contains(&search) || res.name().contains(&search)); > - !res.resources.is_empty() > + if !filters.is_empty() { > + remote_resources.retain(|res| { > + if !res.resources.is_empty() { > + return true; > } > - }); > + filters.matches(|filter| { > + remote_matches_search_term(&res.remote, Some(res.error.is_none()), filter) > + }) > + }) > } > > Ok(remote_resources) _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel