From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 989571FF0E6 for ; Fri, 24 Jul 2026 08:47:07 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 229EE214DB; Fri, 24 Jul 2026 08:47:07 +0200 (CEST) Date: Fri, 24 Jul 2026 08:47:00 +0200 From: Arthur Bied-Charreton To: Lukas Wagner Subject: Re: [PATCH proxmox 10/29] notify: matcher: add InlineCalendarMatcher Message-ID: References: <20260709115716.299836-1-l.wagner@proxmox.com> <20260709115716.299836-11-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260709115716.299836-11-l.wagner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784875591540 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.115 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: BJAUPNVB6KCNBMUX5GOL4RVKMCZMLDPR X-Message-ID-Hash: BJAUPNVB6KCNBMUX5GOL4RVKMCZMLDPR X-MailFrom: a.bied-charreton@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, Jul 09, 2026 at 01:56:57PM +0200, Lukas Wagner wrote: > This move the inline serialization format into a new-type wrapper around > CalendarMatcher. > > This allows us to keep the existing single-line serialization format for > the old configuration keys (match-calendar), while deriving a regular > Serializer for CalendarMatcher that will be used in the new expression > based matcher. > one comment inline > Signed-off-by: Lukas Wagner > --- > proxmox-notify/src/matcher/calendar.rs | 103 +++++++++++++++++++++---- > proxmox-notify/src/matcher/mod.rs | 4 +- > 2 files changed, 88 insertions(+), 19 deletions(-) > > diff --git a/proxmox-notify/src/matcher/calendar.rs b/proxmox-notify/src/matcher/calendar.rs > index 970801c5..c6263eb5 100644 > --- a/proxmox-notify/src/matcher/calendar.rs > +++ b/proxmox-notify/src/matcher/calendar.rs > @@ -1,49 +1,104 @@ > use std::fmt; > use std::str::FromStr; > > +use serde::{Deserialize, Serialize}; > + > use proxmox_time::DailyDuration; > > use crate::{Error, Notification}; > > use super::MatchDirective; > > -/// Match timestamp of the notification. > +/// Convenience wrapper around [`DailyDuration`] that implements [`Serialize`] and > +/// [`Deserialize`]. > #[derive(Clone, Debug)] > +struct DailyDurationWrapper { > + schedule: String, > + daily_duration: DailyDuration, > +} > + > +impl fmt::Display for DailyDurationWrapper { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + f.write_str(self.as_str()) > + } > +} > + > +impl FromStr for DailyDurationWrapper { > + type Err = Error; > + > + fn from_str(s: &str) -> Result { > + let daily_duration = proxmox_time::parse_daily_duration(s) we might wanna trim s here? this fails if s has a leading whitespace, which is a code path reachable via the UI (see [0]), and even if/when that is fixed, i feel like it would be nicer to not fail to parse because of surrounding whitespace (could also be done at another level of the call stack), since that would still be reachable via pvesh etc. [0] https://lore.proxmox.com/pve-devel/20260709115716.299836-1-l.wagner@proxmox.com/T/#mad9286504ecf2264333be5abe8b9e1f4966aeaeb > + .map_err(|e| Error::Generic(format!("could not parse schedule: {e}")))?; > + > + Ok(Self { > + daily_duration, > + schedule: s.to_string(), > + }) > + } > +} > + > +impl DailyDurationWrapper { > + fn as_str(&self) -> &str { > + &self.schedule > + } > +} > + > +proxmox_serde::forward_deserialize_to_from_str!(DailyDurationWrapper); > +proxmox_serde::forward_serialize_to_display!(DailyDurationWrapper); > + > +/// Match timestamp of the notification. > +#[derive(Clone, Debug, Serialize, Deserialize)] > +#[serde(rename_all = "kebab-case")] > pub struct CalendarMatcher { > - schedule: DailyDuration, > - original: String, > + schedule: DailyDurationWrapper, > } > > impl MatchDirective for CalendarMatcher { > fn matches(&self, notification: &Notification) -> Result { > self.schedule > + .daily_duration > .time_match(notification.metadata.timestamp, false) > .map_err(|err| Error::Generic(format!("could not match timestamp: {err}"))) > } > } > > -impl fmt::Display for CalendarMatcher { > - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > - f.write_str(&self.original) > +/// Match the timestamp of a notification. > +/// > +/// This is a wrapper that serializes into an inline format that can be used > +/// in a section config key-value pair. > +#[derive(Clone, Debug)] > +pub struct InlineCalendarMatcher(CalendarMatcher); > + > +impl MatchDirective for InlineCalendarMatcher { > + fn matches(&self, notification: &Notification) -> Result { > + self.0.matches(notification) > } > } > > -impl FromStr for CalendarMatcher { > +impl fmt::Display for InlineCalendarMatcher { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + f.write_str(self.0.schedule.as_str()) > + } > +} > + > +impl FromStr for InlineCalendarMatcher { > type Err = Error; > > fn from_str(s: &str) -> Result { > - let schedule = proxmox_time::parse_daily_duration(s) > - .map_err(|e| Error::Generic(format!("could not parse schedule: {e}")))?; > - > - Ok(Self { > - schedule, > - original: s.to_string(), > - }) > + Ok(Self(CalendarMatcher { > + schedule: s.parse()?, > + })) > } > } > > -proxmox_serde::forward_deserialize_to_from_str!(CalendarMatcher); > -proxmox_serde::forward_serialize_to_display!(CalendarMatcher); > +impl InlineCalendarMatcher { > + pub fn into_inner(self) -> CalendarMatcher { > + self.0 > + } > +} > + > +proxmox_serde::forward_deserialize_to_from_str!(InlineCalendarMatcher); > +proxmox_serde::forward_serialize_to_display!(InlineCalendarMatcher); > > #[cfg(test)] > mod test { > @@ -63,7 +118,21 @@ mod test { > > // Match on a wide rage to avoid issues when running this test case > // in a different time zone. > - let matcher: CalendarMatcher = "thu..sat 0-23".parse().unwrap(); > + let matcher: InlineCalendarMatcher = "thu..sat 0-23".parse().unwrap(); > assert!(matcher.matches(¬ification).unwrap()); > } > + > + #[test] > + fn test_calendar_matcher_de_ser_roundtrip() { > + let calendar_matcher = "{ \"schedule\": \"thu..sat 0-23\" }"; > + > + let calendar_matcher: CalendarMatcher = serde_json::from_str(calendar_matcher).unwrap(); > + > + let s = serde_json::to_string(&calendar_matcher).unwrap(); > + let m: CalendarMatcher = serde_json::from_str(&s).unwrap(); > + > + let a = InlineCalendarMatcher(m); > + > + assert_eq!(a.to_string(), "thu..sat 0-23"); > + } > } > diff --git a/proxmox-notify/src/matcher/mod.rs b/proxmox-notify/src/matcher/mod.rs > index 0267a2b9..a5d53d8a 100644 > --- a/proxmox-notify/src/matcher/mod.rs > +++ b/proxmox-notify/src/matcher/mod.rs > @@ -14,7 +14,7 @@ pub mod calendar; > pub mod field; > pub mod severity; > > -use calendar::CalendarMatcher; > +use calendar::InlineCalendarMatcher; > use field::InlineFieldMatcher; > use severity::InlineSeverityMatcher; > > @@ -112,7 +112,7 @@ pub struct MatcherConfig { > /// List of matched severity levels. > #[serde(default, skip_serializing_if = "Vec::is_empty")] > #[updater(serde(skip_serializing_if = "Option::is_none"))] > - pub match_calendar: Vec, > + pub match_calendar: Vec, > /// Decide if 'all' or 'any' match statements must match. > #[serde(skip_serializing_if = "Option::is_none")] > pub mode: Option, > -- > 2.47.3 > > > > >