[pve-devel] [PATCH proxmox v2 03/20] notify: convert Option<Vec<T>> -> Vec<T> in config structs

[Lukas Wagner <l.wagner@proxmox.com>]

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@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@example.com".to_string()]),
+                mailto: vec!["foo@example.com".to_string()],
                 ..Default::default()
             },
         )?;
@@ -228,7 +224,7 @@ mod tests {
             &mut config,
             SendmailConfig {
                 name: "builtin".to_string(),
-                mailto: Some(vec!["foo@example.com".to_string()]),
+                mailto: vec!["foo@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@example.com".into()]),
-                mailto_user: None,
+                mailto: vec!["user1@example.com".into()],
+                mailto_user: vec![],
                 from_address: Some("from@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@example.com".to_string(),
                 "user3@example.com".to_string()
-            ])
+            ]
         );
-        assert_eq!(endpoint.mailto_user, Some(vec!["root@pam".to_string(),]));
+        assert_eq!(endpoint.mailto_user, vec!["root@pam".to_string(),]);
         assert_eq!(endpoint.from_address, Some("root@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@example.com".into()]),
-                mailto_user: None,
+                mailto: vec!["user1@example.com".into()],
+                mailto_user: vec![],
                 from_address: "from@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@example.com".to_string(),
                 "user3@example.com".to_string()
-            ])
+            ]
         );
-        assert_eq!(endpoint.mailto_user, Some(vec!["root@pam".to_string(),]));
+        assert_eq!(endpoint.mailto_user, vec!["root@pam".to_string(),]);
         assert_eq!(endpoint.from_address, "root@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


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel