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 079BA1FF183 for ; Wed, 27 Aug 2025 11:12:54 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A83A714B98; Wed, 27 Aug 2025 11:12:54 +0200 (CEST) Mime-Version: 1.0 Date: Wed, 27 Aug 2025 11:12:50 +0200 Message-Id: From: "Lukas Wagner" To: "Proxmox Datacenter Manager development discussion" , "Dominik Csapak" X-Mailer: aerc 0.20.1-0-g2ecb8770224a References: <20250826123336.3970108-1-d.csapak@proxmox.com> <20250826123336.3970108-3-d.csapak@proxmox.com> In-Reply-To: <20250826123336.3970108-3-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1756285964909 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.026 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. [gitlab.com, lib.rs, elastic.co, proxmox.com] Subject: Re: [pdm-devel] [PATCH datacenter-manager v3 2/9] lib: add pdm-search crate 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" Looks good to me, two small suggestions inline. Some of the `pub` fn's and struct members don't have doc comments, would be nice if you could add them as well (can also be in a followup if you prefer). On Tue Aug 26, 2025 at 2:31 PM CEST, Dominik Csapak wrote: > Introduce a new create for search & filter related code. It currently > includes basic parsing & testing of search terms. Intended to be used on > some API calls that allow for more complex filters, such as the > resources API. > > Contains a `SearchTerm` and a `Search` struct. The former represents > a single term to search for, with an optional category and if it's > optional or not. The latter represents a full search with multiple > terms. > > Short syntax summary: > 'sometext' : normal search term > '+sometext' : required search term > 'cat:text' : looks for 'text' in the category 'cat' > > required terms have to exist in the resulting match, while optional > ones are OR'd (so at least one optional match must exist) > > This is loosely inspired by various search syntaxes, e.g. the one from > gitlab[0] or query-dsl from elastic[1] > > [1] https://docs.gitlab.com/user/search/advanced_search/ > [2] https://www.elastic.co/docs/reference/query-languages/query-dsl/query-dsl-simple-query-string-query > > Signed-off-by: Dominik Csapak > --- > Cargo.toml | 2 + > lib/pdm-search/Cargo.toml | 12 ++ > lib/pdm-search/src/lib.rs | 282 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 296 insertions(+) > create mode 100644 lib/pdm-search/Cargo.toml > create mode 100644 lib/pdm-search/src/lib.rs > > diff --git a/Cargo.toml b/Cargo.toml > index 08b9373..236f00b 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -19,6 +19,7 @@ members = [ > "lib/pdm-api-types", > "lib/pdm-client", > "lib/pdm-config", > + "lib/pdm-search", > "lib/pdm-ui-shared", > > "cli/client", > @@ -86,6 +87,7 @@ pdm-api-types = { path = "lib/pdm-api-types" } > pdm-buildcfg = { path = "lib/pdm-buildcfg" } > pdm-config = { path = "lib/pdm-config" } > pdm-client = { version = "0.2", path = "lib/pdm-client" } > +pdm-search = { version = "0.2", path = "lib/pdm-search" } > pdm-ui-shared = { version = "0.2", path = "lib/pdm-ui-shared" } > proxmox-fido2 = { path = "cli/proxmox-fido2" } > > diff --git a/lib/pdm-search/Cargo.toml b/lib/pdm-search/Cargo.toml > new file mode 100644 > index 0000000..5f51e75 > --- /dev/null > +++ b/lib/pdm-search/Cargo.toml > @@ -0,0 +1,12 @@ > +[package] > +name = "pdm-search" > +description = "Proxmox Datacenter Manager shared ui modules" > +homepage = "https://www.proxmox.com" > + > +version.workspace = true > +edition.workspace = true > +license.workspace = true > +repository.workspace = true > + > +[dependencies] > +anyhow.workspace = true > diff --git a/lib/pdm-search/src/lib.rs b/lib/pdm-search/src/lib.rs > new file mode 100644 > index 0000000..85dddda > --- /dev/null > +++ b/lib/pdm-search/src/lib.rs > @@ -0,0 +1,282 @@ > +//! Abstraction over a [`Search`] that contains multiple [`SearchTerm`]s. > +//! > +//! Provides methods to filter an item over a combination of such terms and > +//! construct them from text, and serialize them back to text. > +use std::str::FromStr; > + > +use anyhow::bail; > + > +#[derive(Clone)] > +pub struct Search { > + required_terms: Vec, > + optional_terms: Vec, > +} > + > +impl Default for Search { > + fn default() -> Self { > + Self::new() > + } > +} > + > +impl FromIterator for Search { > + fn from_iter>(iter: T) -> Self { > + let (optional_terms, required_terms) = iter.into_iter().partition(|term| term.optional); > + > + Self { > + required_terms, > + optional_terms, > + } > + } > +} > + > +impl> From for Search { > + fn from(value: S) -> Self { > + value > + .as_ref() > + .split_whitespace() > + .filter_map(|term| term.parse().ok()) > + .collect() > + } > +} > + > +impl Search { > + pub fn new() -> Self { > + Self::with_terms(Vec::new()) > + } > + > + pub fn is_empty(&self) -> bool { > + self.required_terms.is_empty() && self.optional_terms.is_empty() > + } > + > + pub fn with_terms(terms: Vec) -> Self { > + terms.into_iter().collect() > + } > + > + /// Test if the given `Fn(&SearchTerm) -> bool` for all [`SearchTerm`] configured matches > + /// > + /// Returns true if it matches considering the constraints: > + /// if there are no filters, returns true > + pub fn matches bool>(&self, matches: F) -> bool { > + if self.is_empty() { > + return true; > + } > + > + if self.required_terms.iter().map(&matches).any(|f| !f) { > + return false; > + } > + > + if !self.optional_terms.is_empty() && self.optional_terms.iter().map(&matches).all(|f| !f) { > + return false; > + } > + > + true > + } > + > + /// Returns true if the combination of [`SearchTerm`]s require that this category value must be > + /// true. Useful to find out if some condition is required (e.g. type == 'remote') > + pub fn category_value_required(&self, category: &str, value: &str) -> bool { > + for term in &self.required_terms { > + if term.category.as_deref() == Some(category) && value.contains(&term.value) { > + return true; > + } > + } > + > + let mut optional_count = 0; > + > + for term in &self.optional_terms { > + if term.category.as_deref() == Some(category) && term.value == value { > + optional_count += 1; > + } > + } > + > + self.required_terms.is_empty() > + && self.optional_terms.len() == optional_count > + && optional_count > 0 > + } > +} > + > +impl std::fmt::Display for Search { > + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { > + for (count, term) in self.required_terms.iter().enumerate() { > + if count != 0 { > + write!(f, " ")?; > + } > + > + write!(f, "{term}")?; > + } > + > + if !self.required_terms.is_empty() && !self.optional_terms.is_empty() { > + write!(f, " ")?; > + } > + > + for (count, term) in self.optional_terms.iter().enumerate() { > + if count != 0 { > + write!(f, " ")?; > + } > + > + write!(f, "{term}")?; > + } > + > + Ok(()) > + } > +} Would be cool to also add two quick tests for category_value_required and the Display trait implementation; I always find tests valuable as additional documentation about how something is supposed to work. > + > +#[derive(Debug, Clone, PartialEq)] > +pub struct SearchTerm { > + optional: bool, > + pub value: String, > + pub category: Option, > +} > + > +impl SearchTerm { > + /// Creates a new [`SearchTerm`]. > + pub fn new(term: S) -> Self { > + Self { > + value: term.to_string(), > + optional: false, > + category: None, > + } > + } > + > + pub fn category(mut self, category: Option) -> Self { > + self.category = category.map(|s| s.to_string()); > + self > + } > + > + pub fn optional(mut self, optional: bool) -> Self { > + self.optional = optional; > + self > + } > +} > + > +impl FromStr for SearchTerm { > + type Err = anyhow::Error; > + > + fn from_str(s: &str) -> Result { > + let mut optional = true; > + let mut term: String = s.into(); > + if term.starts_with("+") { > + optional = false; > + term.remove(0); > + } > + > + let (term, category) = if let Some(idx) = term.find(":") { > + let mut real_term = term.split_off(idx); > + real_term.remove(0); // remove ':' > + (real_term, Some(term)) > + } else { > + (term, None) > + }; > + > + if term.is_empty() { > + bail!("term cannot be empty"); > + } > + > + if category == Some("".into()) { > + bail!("category cannot be empty"); > + } > + > + Ok(SearchTerm::new(term).optional(optional).category(category)) > + } > +} > + > +impl std::fmt::Display for SearchTerm { > + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { > + if !self.optional { > + f.write_str("+")?; > + } > + > + if let Some(cat) = &self.category { > + f.write_str(cat)?; > + f.write_str(":")?; > + } > + > + f.write_str(&self.value) > + } > +} > + > +#[cfg(test)] > +mod tests { > + use crate::{Search, SearchTerm}; > + > + #[test] > + fn parse_test_simple_filter() { > + assert_eq!( > + "foo".parse::().unwrap(), > + SearchTerm::new("foo").optional(true), > + ); > + } > + > + #[test] > + fn parse_test_requires_filter() { > + assert_eq!( > + "+foo".parse::().unwrap(), > + SearchTerm::new("foo"), > + ); > + } > + > + #[test] > + fn parse_test_category_filter() { > + assert_eq!( > + "foo:bar".parse::().unwrap(), > + SearchTerm::new("bar").optional(true).category(Some("foo")) > + ); > + assert_eq!( > + "+foo:bar".parse::().unwrap(), > + SearchTerm::new("bar").category(Some("foo")) > + ); > + } > + > + #[test] > + fn parse_test_invalid_filter() { > + assert!(":bar".parse::().is_err()); > + assert!("+cat:".parse::().is_err()); > + assert!("+".parse::().is_err()); > + assert!(":".parse::().is_err()); > + } > + > + #[test] > + fn match_tests() { > + let search = Search::from_iter(vec![ > + SearchTerm::new("required1").optional(false), > + SearchTerm::new("required2").optional(false), > + SearchTerm::new("optional1").optional(true), > + SearchTerm::new("optional2").optional(true), > + ]); > + > + // each case contains results for > + // required1, required2, optional1, optional2 > + // and if it should match or not > + let cases = [ > + (true, true, true, false, true), > + (true, true, false, true, true), > + (true, true, true, true, true), > + (true, true, false, false, false), > + (true, false, false, false, false), > + (false, false, false, false, false), > + (false, true, false, false, false), > + (false, false, true, true, false), > + (false, true, true, true, false), > + (true, false, true, true, false), > + (true, false, true, false, false), > + (false, true, true, false, false), > + (false, false, true, false, false), > + (true, false, false, true, false), > + (false, true, false, true, false), > + (false, false, false, true, false), > + ]; Might be a bit more readable if you split the inputs and outputs as a 2-tuple, e.g.: let cases = [ ((true, true, true, false), true)), ... ] > + for case in cases { and then do a `for (input, expected_output)` here. > + assert!( > + search.matches(|term| { > + match term.value.as_str() { > + "required1" => case.0, > + "required2" => case.1, > + "optional1" => case.2, > + "optional2" => case.3, > + _ => unreachable!(), > + } > + }) == case.4 > + ) > + } > + } > +} _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel