[pve-devel] [PATCH proxmox v2 03/20] notify: convert Option<Vec<T>> -> Vec<T> in config structs
Lukas Wagner
l.wagner at proxmox.com
Fri Apr 19 16:17:06 CEST 2024
Suggested-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
Tested-by: Folke Gleumes <f.gleumes at proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner at proxmox.com>
---
proxmox-notify/src/api/matcher.rs | 27 +++++++--------
proxmox-notify/src/api/mod.rs | 22 +++++-------
proxmox-notify/src/api/sendmail.rs | 22 ++++++------
proxmox-notify/src/api/smtp.rs | 24 ++++++-------
proxmox-notify/src/endpoints/common/mail.rs | 20 ++++-------
proxmox-notify/src/endpoints/sendmail.rs | 14 ++++----
proxmox-notify/src/endpoints/smtp.rs | 18 +++++-----
proxmox-notify/src/lib.rs | 10 +++---
proxmox-notify/src/matcher.rs | 38 ++++++++++++---------
9 files changed, 96 insertions(+), 99 deletions(-)
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
index f0eabd9..63ec73d 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -38,10 +38,7 @@ pub fn get_matcher(config: &Config, name: &str) -> Result<MatcherConfig, HttpErr
/// - the configuration could not be saved (`500 Internal server error`)
pub fn add_matcher(config: &mut Config, matcher_config: MatcherConfig) -> Result<(), HttpError> {
super::ensure_unique(config, &matcher_config.name)?;
-
- if let Some(targets) = matcher_config.target.as_deref() {
- super::ensure_endpoints_exist(config, targets)?;
- }
+ super::ensure_endpoints_exist(config, &matcher_config.target)?;
config
.config
@@ -78,10 +75,10 @@ pub fn update_matcher(
if let Some(delete) = delete {
for deleteable_property in delete {
match deleteable_property {
- DeleteableMatcherProperty::MatchSeverity => matcher.match_severity = None,
- DeleteableMatcherProperty::MatchField => matcher.match_field = None,
- DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar = None,
- DeleteableMatcherProperty::Target => matcher.target = None,
+ DeleteableMatcherProperty::MatchSeverity => matcher.match_severity.clear(),
+ DeleteableMatcherProperty::MatchField => matcher.match_field.clear(),
+ DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar.clear(),
+ DeleteableMatcherProperty::Target => matcher.target.clear(),
DeleteableMatcherProperty::Mode => matcher.mode = None,
DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None,
DeleteableMatcherProperty::Comment => matcher.comment = None,
@@ -91,15 +88,15 @@ pub fn update_matcher(
}
if let Some(match_severity) = matcher_updater.match_severity {
- matcher.match_severity = Some(match_severity);
+ matcher.match_severity = match_severity;
}
if let Some(match_field) = matcher_updater.match_field {
- matcher.match_field = Some(match_field);
+ matcher.match_field = match_field;
}
if let Some(match_calendar) = matcher_updater.match_calendar {
- matcher.match_calendar = Some(match_calendar);
+ matcher.match_calendar = match_calendar;
}
if let Some(mode) = matcher_updater.mode {
@@ -120,7 +117,7 @@ pub fn update_matcher(
if let Some(target) = matcher_updater.target {
super::ensure_endpoints_exist(config, target.as_slice())?;
- matcher.target = Some(target);
+ matcher.target = target;
}
config
@@ -244,9 +241,9 @@ matcher: matcher2
let matcher = get_matcher(&config, "matcher1")?;
assert_eq!(matcher.invert_match, None);
- assert!(matcher.match_severity.is_none());
- assert!(matches!(matcher.match_field, None));
- assert_eq!(matcher.target, None);
+ assert!(matcher.match_severity.is_empty());
+ assert!(matcher.match_field.is_empty());
+ assert!(matcher.target.is_empty());
assert!(matcher.mode.is_none());
assert_eq!(matcher.comment, None);
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index bb0371d..a2cf29e 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -102,10 +102,8 @@ fn get_referrers(config: &Config, entity: &str) -> Result<HashSet<String>, HttpE
let mut referrers = HashSet::new();
for matcher in matcher::get_matchers(config)? {
- if let Some(targets) = matcher.target {
- if targets.iter().any(|target| target == entity) {
- referrers.insert(matcher.name.clone());
- }
+ if matcher.target.iter().any(|target| target == entity) {
+ referrers.insert(matcher.name.clone());
}
}
@@ -149,11 +147,9 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet<String> {
let mut new = HashSet::new();
for entity in entities {
- if let Ok(group) = matcher::get_matcher(config, entity) {
- if let Some(targets) = group.target {
- for target in targets {
- new.insert(target.clone());
- }
+ if let Ok(matcher) = matcher::get_matcher(config, entity) {
+ for target in matcher.target {
+ new.insert(target.clone());
}
}
}
@@ -219,7 +215,7 @@ mod tests {
&mut config,
SendmailConfig {
name: "sendmail".to_string(),
- mailto: Some(vec!["foo at example.com".to_string()]),
+ mailto: vec!["foo at example.com".to_string()],
..Default::default()
},
)?;
@@ -228,7 +224,7 @@ mod tests {
&mut config,
SendmailConfig {
name: "builtin".to_string(),
- mailto: Some(vec!["foo at example.com".to_string()]),
+ mailto: vec!["foo at example.com".to_string()],
origin: Some(Origin::Builtin),
..Default::default()
},
@@ -251,11 +247,11 @@ mod tests {
&mut config,
MatcherConfig {
name: "matcher".to_string(),
- target: Some(vec![
+ target: vec![
"sendmail".to_string(),
"gotify".to_string(),
"builtin".to_string(),
- ]),
+ ],
..Default::default()
},
)?;
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index 3285ac7..c20a3e5 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -40,7 +40,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SendmailConfig, HttpE
pub fn add_endpoint(config: &mut Config, endpoint: SendmailConfig) -> Result<(), HttpError> {
super::ensure_unique(config, &endpoint.name)?;
- if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
+ if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -83,19 +83,19 @@ pub fn update_endpoint(
DeleteableSendmailProperty::FromAddress => endpoint.from_address = None,
DeleteableSendmailProperty::Author => endpoint.author = None,
DeleteableSendmailProperty::Comment => endpoint.comment = None,
- DeleteableSendmailProperty::Mailto => endpoint.mailto = None,
- DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user = None,
+ DeleteableSendmailProperty::Mailto => endpoint.mailto.clear(),
+ DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user.clear(),
DeleteableSendmailProperty::Disable => endpoint.disable = None,
}
}
}
if let Some(mailto) = updater.mailto {
- endpoint.mailto = Some(mailto);
+ endpoint.mailto = mailto;
}
if let Some(mailto_user) = updater.mailto_user {
- endpoint.mailto_user = Some(mailto_user);
+ endpoint.mailto_user = mailto_user;
}
if let Some(from_address) = updater.from_address {
@@ -114,7 +114,7 @@ pub fn update_endpoint(
endpoint.disable = Some(disable);
}
- if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
+ if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -164,8 +164,8 @@ pub mod tests {
config,
SendmailConfig {
name: name.into(),
- mailto: Some(vec!["user1 at example.com".into()]),
- mailto_user: None,
+ mailto: vec!["user1 at example.com".into()],
+ mailto_user: vec![],
from_address: Some("from at example.com".into()),
author: Some("root".into()),
comment: Some("Comment".into()),
@@ -248,12 +248,12 @@ pub mod tests {
assert_eq!(
endpoint.mailto,
- Some(vec![
+ vec![
"user2 at example.com".to_string(),
"user3 at example.com".to_string()
- ])
+ ]
);
- assert_eq!(endpoint.mailto_user, Some(vec!["root at pam".to_string(),]));
+ assert_eq!(endpoint.mailto_user, vec!["root at pam".to_string(),]);
assert_eq!(endpoint.from_address, Some("root at example.com".to_string()));
assert_eq!(endpoint.author, Some("newauthor".to_string()));
assert_eq!(endpoint.comment, Some("new comment".to_string()));
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 16d103c..7a58677 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -50,7 +50,7 @@ pub fn add_endpoint(
super::ensure_unique(config, &endpoint_config.name)?;
- if endpoint_config.mailto.is_none() && endpoint_config.mailto_user.is_none() {
+ if endpoint_config.mailto.is_empty() && endpoint_config.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -101,8 +101,8 @@ pub fn update_endpoint(
DeleteableSmtpProperty::Author => endpoint.author = None,
DeleteableSmtpProperty::Comment => endpoint.comment = None,
DeleteableSmtpProperty::Disable => endpoint.disable = None,
- DeleteableSmtpProperty::Mailto => endpoint.mailto = None,
- DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user = None,
+ DeleteableSmtpProperty::Mailto => endpoint.mailto.clear(),
+ DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user.clear(),
DeleteableSmtpProperty::Password => super::set_private_config_entry(
config,
SmtpPrivateConfig {
@@ -119,10 +119,10 @@ pub fn update_endpoint(
}
if let Some(mailto) = updater.mailto {
- endpoint.mailto = Some(mailto);
+ endpoint.mailto = mailto;
}
if let Some(mailto_user) = updater.mailto_user {
- endpoint.mailto_user = Some(mailto_user);
+ endpoint.mailto_user = mailto_user;
}
if let Some(from_address) = updater.from_address {
endpoint.from_address = from_address;
@@ -163,7 +163,7 @@ pub fn update_endpoint(
endpoint.disable = Some(disable);
}
- if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
+ if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -211,8 +211,8 @@ pub mod tests {
config,
SmtpConfig {
name: name.into(),
- mailto: Some(vec!["user1 at example.com".into()]),
- mailto_user: None,
+ mailto: vec!["user1 at example.com".into()],
+ mailto_user: vec![],
from_address: "from at example.com".into(),
author: Some("root".into()),
comment: Some("Comment".into()),
@@ -311,12 +311,12 @@ pub mod tests {
assert_eq!(
endpoint.mailto,
- Some(vec![
+ vec![
"user2 at example.com".to_string(),
"user3 at example.com".to_string()
- ])
+ ]
);
- assert_eq!(endpoint.mailto_user, Some(vec!["root at pam".to_string(),]));
+ assert_eq!(endpoint.mailto_user, vec!["root at pam".to_string(),]);
assert_eq!(endpoint.from_address, "root at example.com".to_string());
assert_eq!(endpoint.author, Some("newauthor".to_string()));
assert_eq!(endpoint.comment, Some("new comment".to_string()));
@@ -343,7 +343,7 @@ pub mod tests {
assert_eq!(endpoint.comment, None);
assert_eq!(endpoint.port, None);
assert_eq!(endpoint.username, None);
- assert_eq!(endpoint.mailto_user, None);
+ assert!(endpoint.mailto_user.is_empty());
Ok(())
}
diff --git a/proxmox-notify/src/endpoints/common/mail.rs b/proxmox-notify/src/endpoints/common/mail.rs
index 0929d7c..b1c4823 100644
--- a/proxmox-notify/src/endpoints/common/mail.rs
+++ b/proxmox-notify/src/endpoints/common/mail.rs
@@ -2,22 +2,16 @@ use std::collections::HashSet;
use crate::context;
-pub(crate) fn get_recipients(
- email_addrs: Option<&[String]>,
- users: Option<&[String]>,
-) -> HashSet<String> {
+pub(crate) fn get_recipients(email_addrs: &[String], users: &[String]) -> HashSet<String> {
let mut recipients = HashSet::new();
- if let Some(mailto_addrs) = email_addrs {
- for addr in mailto_addrs {
- recipients.insert(addr.clone());
- }
+ for addr in email_addrs {
+ recipients.insert(addr.clone());
}
- if let Some(users) = users {
- for user in users {
- if let Some(addr) = context::context().lookup_email_for_user(user) {
- recipients.insert(addr);
- }
+
+ for user in users {
+ if let Some(addr) = context::context().lookup_email_for_user(user) {
+ recipients.insert(addr);
}
}
recipients
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index 6178b5e..fa04002 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -44,11 +44,13 @@ pub struct SendmailConfig {
#[updater(skip)]
pub name: String,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto: Vec<String>,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto_user: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto_user: Vec<String>,
/// `From` address for the mail
#[serde(skip_serializing_if = "Option::is_none")]
pub from_address: Option<String>,
@@ -90,8 +92,8 @@ pub struct SendmailEndpoint {
impl Endpoint for SendmailEndpoint {
fn send(&self, notification: &Notification) -> Result<(), Error> {
let recipients = mail::get_recipients(
- self.config.mailto.as_deref(),
- self.config.mailto_user.as_deref(),
+ self.config.mailto.as_slice(),
+ self.config.mailto_user.as_slice(),
);
let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index f0c836a..ded5baf 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -44,8 +44,8 @@ pub enum SmtpMode {
},
mailto: {
type: Array,
- items: {
- schema: EMAIL_SCHEMA,
+ items: {
+ schema: EMAIL_SCHEMA,
},
optional: true,
},
@@ -80,11 +80,13 @@ pub struct SmtpConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub username: Option<String>,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto: Vec<String>,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto_user: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto_user: Vec<String>,
/// `From` address for the mail
pub from_address: String,
/// Author of the mail
@@ -177,8 +179,8 @@ impl Endpoint for SmtpEndpoint {
let transport = transport_builder.build();
let recipients = mail::get_recipients(
- self.config.mailto.as_deref(),
- self.config.mailto_user.as_deref(),
+ self.config.mailto.as_slice(),
+ self.config.mailto_user.as_slice(),
);
let mail_from = self.config.from_address.clone();
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 9f8683f..35dcb17 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -611,7 +611,7 @@ mod tests {
bus.add_endpoint(Box::new(mock.clone()));
let matcher = MatcherConfig {
- target: Some(vec!["endpoint".into()]),
+ target: vec!["endpoint".into()],
..Default::default()
};
@@ -642,15 +642,15 @@ mod tests {
bus.add_matcher(MatcherConfig {
name: "matcher1".into(),
- match_severity: Some(vec!["warning,error".parse()?]),
- target: Some(vec!["mock1".into()]),
+ match_severity: vec!["warning,error".parse()?],
+ target: vec!["mock1".into()],
..Default::default()
});
bus.add_matcher(MatcherConfig {
name: "matcher2".into(),
- match_severity: Some(vec!["error".parse()?]),
- target: Some(vec!["mock2".into()]),
+ match_severity: vec!["error".parse()?],
+ target: vec!["mock2".into()],
..Default::default()
});
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index d6051ac..b387fef 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -113,17 +113,19 @@ pub struct MatcherConfig {
pub name: String,
/// List of matched metadata fields
- #[serde(skip_serializing_if = "Option::is_none")]
- pub match_field: Option<Vec<FieldMatcher>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub match_field: Vec<FieldMatcher>,
/// List of matched severity levels
- #[serde(skip_serializing_if = "Option::is_none")]
- pub match_severity: Option<Vec<SeverityMatcher>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub match_severity: Vec<SeverityMatcher>,
/// List of matched severity levels
- #[serde(skip_serializing_if = "Option::is_none")]
- pub match_calendar: Option<Vec<CalendarMatcher>>,
-
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub match_calendar: Vec<CalendarMatcher>,
/// Decide if 'all' or 'any' match statements must match
#[serde(skip_serializing_if = "Option::is_none")]
pub mode: Option<MatchModeOperator>,
@@ -133,8 +135,9 @@ pub struct MatcherConfig {
pub invert_match: Option<bool>,
/// Targets to notify
- #[serde(skip_serializing_if = "Option::is_none")]
- pub target: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub target: Vec<String>,
/// Comment
#[serde(skip_serializing_if = "Option::is_none")]
@@ -284,29 +287,32 @@ impl MatcherConfig {
// If there are no matching directives, the matcher will always match
let mut no_matchers = true;
- if let Some(severity_matchers) = self.match_severity.as_deref() {
+ if !self.match_severity.is_empty() {
no_matchers = false;
is_match = mode.apply(
is_match,
- self.check_matches(notification, severity_matchers)?,
+ self.check_matches(notification, &self.match_severity)?,
);
}
- if let Some(field_matchers) = self.match_field.as_deref() {
+ if !self.match_field.is_empty() {
no_matchers = false;
- is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?);
+ is_match = mode.apply(
+ is_match,
+ self.check_matches(notification, &self.match_field)?,
+ );
}
- if let Some(calendar_matchers) = self.match_calendar.as_deref() {
+ if !self.match_calendar.is_empty() {
no_matchers = false;
is_match = mode.apply(
is_match,
- self.check_matches(notification, calendar_matchers)?,
+ self.check_matches(notification, &self.match_calendar)?,
);
}
let invert_match = self.invert_match.unwrap_or_default();
Ok(if is_match != invert_match || no_matchers {
- Some(self.target.as_deref().unwrap_or_default())
+ Some(&self.target)
} else {
None
})
--
2.39.2
More information about the pve-devel
mailing list