[pve-devel] [PATCH proxmox v2 02/20] notify: make api methods take config struct ownership

Lukas Wagner l.wagner at proxmox.com
Fri Apr 19 16:17:05 CEST 2024


This saves us from some of the awkward cloning steps when updating.

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/gotify.rs   | 46 +++++++++---------
 proxmox-notify/src/api/matcher.rs  | 38 +++++++--------
 proxmox-notify/src/api/mod.rs      | 14 +++---
 proxmox-notify/src/api/sendmail.rs | 40 +++++++--------
 proxmox-notify/src/api/smtp.rs     | 78 +++++++++++++++---------------
 5 files changed, 108 insertions(+), 108 deletions(-)

diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index a93a024..15d94cb 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -41,8 +41,8 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<GotifyConfig, HttpErr
 /// Panics if the names of the private config and the public config do not match.
 pub fn add_endpoint(
     config: &mut Config,
-    endpoint_config: &GotifyConfig,
-    private_endpoint_config: &GotifyPrivateConfig,
+    endpoint_config: GotifyConfig,
+    private_endpoint_config: GotifyPrivateConfig,
 ) -> Result<(), HttpError> {
     if endpoint_config.name != private_endpoint_config.name {
         // Programming error by the user of the crate, thus we panic
@@ -51,11 +51,11 @@ pub fn add_endpoint(
 
     super::ensure_unique(config, &endpoint_config.name)?;
 
-    set_private_config_entry(config, private_endpoint_config)?;
+    set_private_config_entry(config, &private_endpoint_config)?;
 
     config
         .config
-        .set_data(&endpoint_config.name, GOTIFY_TYPENAME, endpoint_config)
+        .set_data(&endpoint_config.name, GOTIFY_TYPENAME, &endpoint_config)
         .map_err(|e| {
             http_err!(
                 INTERNAL_SERVER_ERROR,
@@ -75,8 +75,8 @@ pub fn add_endpoint(
 pub fn update_endpoint(
     config: &mut Config,
     name: &str,
-    endpoint_config_updater: &GotifyConfigUpdater,
-    private_endpoint_config_updater: &GotifyPrivateConfigUpdater,
+    endpoint_config_updater: GotifyConfigUpdater,
+    private_endpoint_config_updater: GotifyPrivateConfigUpdater,
     delete: Option<&[DeleteableGotifyProperty]>,
     digest: Option<&[u8]>,
 ) -> Result<(), HttpError> {
@@ -93,11 +93,11 @@ pub fn update_endpoint(
         }
     }
 
-    if let Some(server) = &endpoint_config_updater.server {
-        endpoint.server = server.into();
+    if let Some(server) = endpoint_config_updater.server {
+        endpoint.server = server;
     }
 
-    if let Some(token) = &private_endpoint_config_updater.token {
+    if let Some(token) = private_endpoint_config_updater.token {
         set_private_config_entry(
             config,
             &GotifyPrivateConfig {
@@ -107,12 +107,12 @@ pub fn update_endpoint(
         )?;
     }
 
-    if let Some(comment) = &endpoint_config_updater.comment {
-        endpoint.comment = Some(comment.into());
+    if let Some(comment) = endpoint_config_updater.comment {
+        endpoint.comment = Some(comment)
     }
 
-    if let Some(disable) = &endpoint_config_updater.disable {
-        endpoint.disable = Some(*disable);
+    if let Some(disable) = endpoint_config_updater.disable {
+        endpoint.disable = Some(disable);
     }
 
     config
@@ -173,13 +173,13 @@ mod tests {
     pub fn add_default_gotify_endpoint(config: &mut Config) -> Result<(), HttpError> {
         add_endpoint(
             config,
-            &GotifyConfig {
+            GotifyConfig {
                 name: "gotify-endpoint".into(),
                 server: "localhost".into(),
                 comment: Some("comment".into()),
                 ..Default::default()
             },
-            &GotifyPrivateConfig {
+            GotifyPrivateConfig {
                 name: "gotify-endpoint".into(),
                 token: "supersecrettoken".into(),
             },
@@ -196,8 +196,8 @@ mod tests {
         assert!(update_endpoint(
             &mut config,
             "test",
-            &Default::default(),
-            &Default::default(),
+            Default::default(),
+            Default::default(),
             None,
             None
         )
@@ -214,8 +214,8 @@ mod tests {
         assert!(update_endpoint(
             &mut config,
             "gotify-endpoint",
-            &Default::default(),
-            &Default::default(),
+            Default::default(),
+            Default::default(),
             None,
             Some(&[0; 32])
         )
@@ -234,12 +234,12 @@ mod tests {
         update_endpoint(
             &mut config,
             "gotify-endpoint",
-            &GotifyConfigUpdater {
+            GotifyConfigUpdater {
                 server: Some("newhost".into()),
                 comment: Some("newcomment".into()),
                 ..Default::default()
             },
-            &GotifyPrivateConfigUpdater {
+            GotifyPrivateConfigUpdater {
                 token: Some("changedtoken".into()),
             },
             None,
@@ -263,8 +263,8 @@ mod tests {
         update_endpoint(
             &mut config,
             "gotify-endpoint",
-            &Default::default(),
-            &Default::default(),
+            Default::default(),
+            Default::default(),
             Some(&[DeleteableGotifyProperty::Comment]),
             None,
         )?;
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
index ca01bc9..f0eabd9 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -36,7 +36,7 @@ pub fn get_matcher(config: &Config, name: &str) -> Result<MatcherConfig, HttpErr
 /// Returns a `HttpError` if:
 ///   - an entity with the same name already exists (`400 Bad request`)
 ///   - the configuration could not be saved (`500 Internal server error`)
-pub fn add_matcher(config: &mut Config, matcher_config: &MatcherConfig) -> Result<(), HttpError> {
+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() {
@@ -45,7 +45,7 @@ pub fn add_matcher(config: &mut Config, matcher_config: &MatcherConfig) -> Resul
 
     config
         .config
-        .set_data(&matcher_config.name, MATCHER_TYPENAME, matcher_config)
+        .set_data(&matcher_config.name, MATCHER_TYPENAME, &matcher_config)
         .map_err(|e| {
             http_err!(
                 INTERNAL_SERVER_ERROR,
@@ -67,7 +67,7 @@ pub fn add_matcher(config: &mut Config, matcher_config: &MatcherConfig) -> Resul
 pub fn update_matcher(
     config: &mut Config,
     name: &str,
-    matcher_updater: &MatcherConfigUpdater,
+    matcher_updater: MatcherConfigUpdater,
     delete: Option<&[DeleteableMatcherProperty]>,
     digest: Option<&[u8]>,
 ) -> Result<(), HttpError> {
@@ -90,16 +90,16 @@ pub fn update_matcher(
         }
     }
 
-    if let Some(match_severity) = &matcher_updater.match_severity {
-        matcher.match_severity = Some(match_severity.clone());
+    if let Some(match_severity) = matcher_updater.match_severity {
+        matcher.match_severity = Some(match_severity);
     }
 
-    if let Some(match_field) = &matcher_updater.match_field {
-        matcher.match_field = Some(match_field.clone());
+    if let Some(match_field) = matcher_updater.match_field {
+        matcher.match_field = Some(match_field);
     }
 
-    if let Some(match_calendar) = &matcher_updater.match_calendar {
-        matcher.match_calendar = Some(match_calendar.clone());
+    if let Some(match_calendar) = matcher_updater.match_calendar {
+        matcher.match_calendar = Some(match_calendar);
     }
 
     if let Some(mode) = matcher_updater.mode {
@@ -110,17 +110,17 @@ pub fn update_matcher(
         matcher.invert_match = Some(invert_match);
     }
 
-    if let Some(comment) = &matcher_updater.comment {
-        matcher.comment = Some(comment.into());
+    if let Some(comment) = matcher_updater.comment {
+        matcher.comment = Some(comment);
     }
 
-    if let Some(disable) = &matcher_updater.disable {
-        matcher.disable = Some(*disable);
+    if let Some(disable) = matcher_updater.disable {
+        matcher.disable = Some(disable);
     }
 
-    if let Some(target) = &matcher_updater.target {
+    if let Some(target) = matcher_updater.target {
         super::ensure_endpoints_exist(config, target.as_slice())?;
-        matcher.target = Some(target.clone());
+        matcher.target = Some(target);
     }
 
     config
@@ -178,7 +178,7 @@ matcher: matcher2
     #[test]
     fn test_update_not_existing_returns_error() -> Result<(), HttpError> {
         let mut config = empty_config();
-        assert!(update_matcher(&mut config, "test", &Default::default(), None, None).is_err());
+        assert!(update_matcher(&mut config, "test", Default::default(), None, None).is_err());
         Ok(())
     }
 
@@ -188,7 +188,7 @@ matcher: matcher2
         assert!(update_matcher(
             &mut config,
             "matcher1",
-            &Default::default(),
+            Default::default(),
             None,
             Some(&[0u8; 32])
         )
@@ -206,7 +206,7 @@ matcher: matcher2
         update_matcher(
             &mut config,
             "matcher1",
-            &MatcherConfigUpdater {
+            MatcherConfigUpdater {
                 mode: Some(MatchModeOperator::Any),
                 match_field: None,
                 match_severity: None,
@@ -230,7 +230,7 @@ matcher: matcher2
         update_matcher(
             &mut config,
             "matcher1",
-            &Default::default(),
+            Default::default(),
             Some(&[
                 DeleteableMatcherProperty::InvertMatch,
                 DeleteableMatcherProperty::Mode,
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index 7cc2593..bb0371d 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -173,13 +173,13 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet<String> {
 #[allow(unused)]
 fn set_private_config_entry<T: Serialize>(
     config: &mut Config,
-    private_config: &T,
+    private_config: T,
     typename: &str,
     name: &str,
 ) -> Result<(), HttpError> {
     config
         .private_config
-        .set_data(name, typename, private_config)
+        .set_data(name, typename, &private_config)
         .map_err(|e| {
             http_err!(
                 INTERNAL_SERVER_ERROR,
@@ -217,7 +217,7 @@ mod tests {
 
         sendmail::add_endpoint(
             &mut config,
-            &SendmailConfig {
+            SendmailConfig {
                 name: "sendmail".to_string(),
                 mailto: Some(vec!["foo at example.com".to_string()]),
                 ..Default::default()
@@ -226,7 +226,7 @@ mod tests {
 
         sendmail::add_endpoint(
             &mut config,
-            &SendmailConfig {
+            SendmailConfig {
                 name: "builtin".to_string(),
                 mailto: Some(vec!["foo at example.com".to_string()]),
                 origin: Some(Origin::Builtin),
@@ -236,12 +236,12 @@ mod tests {
 
         gotify::add_endpoint(
             &mut config,
-            &GotifyConfig {
+            GotifyConfig {
                 name: "gotify".to_string(),
                 server: "localhost".to_string(),
                 ..Default::default()
             },
-            &GotifyPrivateConfig {
+            GotifyPrivateConfig {
                 name: "gotify".to_string(),
                 token: "foo".to_string(),
             },
@@ -249,7 +249,7 @@ mod tests {
 
         matcher::add_matcher(
             &mut config,
-            &MatcherConfig {
+            MatcherConfig {
                 name: "matcher".to_string(),
                 target: Some(vec![
                     "sendmail".to_string(),
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index e911505..3285ac7 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -37,7 +37,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SendmailConfig, HttpE
 ///   - an entity with the same name already exists (`400 Bad request`)
 ///   - the configuration could not be saved (`500 Internal server error`)
 ///   - mailto *and* mailto_user are both set to `None`
-pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<(), HttpError> {
+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() {
@@ -49,7 +49,7 @@ pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<()
 
     config
         .config
-        .set_data(&endpoint.name, SENDMAIL_TYPENAME, endpoint)
+        .set_data(&endpoint.name, SENDMAIL_TYPENAME, &endpoint)
         .map_err(|e| {
             http_err!(
                 INTERNAL_SERVER_ERROR,
@@ -69,7 +69,7 @@ pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<()
 pub fn update_endpoint(
     config: &mut Config,
     name: &str,
-    updater: &SendmailConfigUpdater,
+    updater: SendmailConfigUpdater,
     delete: Option<&[DeleteableSendmailProperty]>,
     digest: Option<&[u8]>,
 ) -> Result<(), HttpError> {
@@ -90,28 +90,28 @@ pub fn update_endpoint(
         }
     }
 
-    if let Some(mailto) = &updater.mailto {
-        endpoint.mailto = Some(mailto.iter().map(String::from).collect());
+    if let Some(mailto) = updater.mailto {
+        endpoint.mailto = Some(mailto);
     }
 
-    if let Some(mailto_user) = &updater.mailto_user {
-        endpoint.mailto_user = Some(mailto_user.iter().map(String::from).collect());
+    if let Some(mailto_user) = updater.mailto_user {
+        endpoint.mailto_user = Some(mailto_user);
     }
 
-    if let Some(from_address) = &updater.from_address {
-        endpoint.from_address = Some(from_address.into());
+    if let Some(from_address) = updater.from_address {
+        endpoint.from_address = Some(from_address);
     }
 
-    if let Some(author) = &updater.author {
-        endpoint.author = Some(author.into());
+    if let Some(author) = updater.author {
+        endpoint.author = Some(author);
     }
 
-    if let Some(comment) = &updater.comment {
-        endpoint.comment = Some(comment.into());
+    if let Some(comment) = updater.comment {
+        endpoint.comment = Some(comment);
     }
 
-    if let Some(disable) = &updater.disable {
-        endpoint.disable = Some(*disable);
+    if let Some(disable) = updater.disable {
+        endpoint.disable = Some(disable);
     }
 
     if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
@@ -162,7 +162,7 @@ pub mod tests {
     ) -> Result<(), HttpError> {
         add_endpoint(
             config,
-            &SendmailConfig {
+            SendmailConfig {
                 name: name.into(),
                 mailto: Some(vec!["user1 at example.com".into()]),
                 mailto_user: None,
@@ -193,7 +193,7 @@ pub mod tests {
     fn test_update_not_existing_returns_error() -> Result<(), HttpError> {
         let mut config = empty_config();
 
-        assert!(update_endpoint(&mut config, "test", &Default::default(), None, None,).is_err());
+        assert!(update_endpoint(&mut config, "test", Default::default(), None, None,).is_err());
 
         Ok(())
     }
@@ -206,7 +206,7 @@ pub mod tests {
         assert!(update_endpoint(
             &mut config,
             "sendmail-endpoint",
-            &SendmailConfigUpdater {
+            SendmailConfigUpdater {
                 mailto: Some(vec!["user2 at example.com".into(), "user3 at example.com".into()]),
                 mailto_user: None,
                 from_address: Some("root at example.com".into()),
@@ -232,7 +232,7 @@ pub mod tests {
         update_endpoint(
             &mut config,
             "sendmail-endpoint",
-            &SendmailConfigUpdater {
+            SendmailConfigUpdater {
                 mailto: Some(vec!["user2 at example.com".into(), "user3 at example.com".into()]),
                 mailto_user: Some(vec!["root at pam".into()]),
                 from_address: Some("root at example.com".into()),
@@ -262,7 +262,7 @@ pub mod tests {
         update_endpoint(
             &mut config,
             "sendmail-endpoint",
-            &Default::default(),
+            Default::default(),
             Some(&[
                 DeleteableSendmailProperty::FromAddress,
                 DeleteableSendmailProperty::Author,
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 6bd0c4b..16d103c 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -40,8 +40,8 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SmtpConfig, HttpError
 ///   - mailto *and* mailto_user are both set to `None`
 pub fn add_endpoint(
     config: &mut Config,
-    endpoint_config: &SmtpConfig,
-    private_endpoint_config: &SmtpPrivateConfig,
+    endpoint_config: SmtpConfig,
+    private_endpoint_config: SmtpPrivateConfig,
 ) -> Result<(), HttpError> {
     if endpoint_config.name != private_endpoint_config.name {
         // Programming error by the user of the crate, thus we panic
@@ -66,7 +66,7 @@ pub fn add_endpoint(
 
     config
         .config
-        .set_data(&endpoint_config.name, SMTP_TYPENAME, endpoint_config)
+        .set_data(&endpoint_config.name, SMTP_TYPENAME, &endpoint_config)
         .map_err(|e| {
             http_err!(
                 INTERNAL_SERVER_ERROR,
@@ -86,8 +86,8 @@ pub fn add_endpoint(
 pub fn update_endpoint(
     config: &mut Config,
     name: &str,
-    updater: &SmtpConfigUpdater,
-    private_endpoint_config_updater: &SmtpPrivateConfigUpdater,
+    updater: SmtpConfigUpdater,
+    private_endpoint_config_updater: SmtpPrivateConfigUpdater,
     delete: Option<&[DeleteableSmtpProperty]>,
     digest: Option<&[u8]>,
 ) -> Result<(), HttpError> {
@@ -105,7 +105,7 @@ pub fn update_endpoint(
                 DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user = None,
                 DeleteableSmtpProperty::Password => super::set_private_config_entry(
                     config,
-                    &SmtpPrivateConfig {
+                    SmtpPrivateConfig {
                         name: name.to_string(),
                         password: None,
                     },
@@ -118,49 +118,49 @@ pub fn update_endpoint(
         }
     }
 
-    if let Some(mailto) = &updater.mailto {
-        endpoint.mailto = Some(mailto.iter().map(String::from).collect());
+    if let Some(mailto) = updater.mailto {
+        endpoint.mailto = Some(mailto);
     }
-    if let Some(mailto_user) = &updater.mailto_user {
-        endpoint.mailto_user = Some(mailto_user.iter().map(String::from).collect());
+    if let Some(mailto_user) = updater.mailto_user {
+        endpoint.mailto_user = Some(mailto_user);
     }
-    if let Some(from_address) = &updater.from_address {
-        endpoint.from_address = from_address.into();
+    if let Some(from_address) = updater.from_address {
+        endpoint.from_address = from_address;
     }
-    if let Some(server) = &updater.server {
-        endpoint.server = server.into();
+    if let Some(server) = updater.server {
+        endpoint.server = server;
     }
-    if let Some(port) = &updater.port {
-        endpoint.port = Some(*port);
+    if let Some(port) = updater.port {
+        endpoint.port = Some(port);
     }
-    if let Some(username) = &updater.username {
-        endpoint.username = Some(username.into());
+    if let Some(username) = updater.username {
+        endpoint.username = Some(username);
     }
-    if let Some(mode) = &updater.mode {
-        endpoint.mode = Some(*mode);
+    if let Some(mode) = updater.mode {
+        endpoint.mode = Some(mode);
     }
-    if let Some(password) = &private_endpoint_config_updater.password {
+    if let Some(password) = private_endpoint_config_updater.password {
         super::set_private_config_entry(
             config,
-            &SmtpPrivateConfig {
+            SmtpPrivateConfig {
                 name: name.into(),
-                password: Some(password.into()),
+                password: Some(password),
             },
             SMTP_TYPENAME,
             name,
         )?;
     }
 
-    if let Some(author) = &updater.author {
-        endpoint.author = Some(author.into());
+    if let Some(author) = updater.author {
+        endpoint.author = Some(author);
     }
 
-    if let Some(comment) = &updater.comment {
-        endpoint.comment = Some(comment.into());
+    if let Some(comment) = updater.comment {
+        endpoint.comment = Some(comment);
     }
 
-    if let Some(disable) = &updater.disable {
-        endpoint.disable = Some(*disable);
+    if let Some(disable) = updater.disable {
+        endpoint.disable = Some(disable);
     }
 
     if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
@@ -209,7 +209,7 @@ pub mod tests {
     pub fn add_smtp_endpoint_for_test(config: &mut Config, name: &str) -> Result<(), HttpError> {
         add_endpoint(
             config,
-            &SmtpConfig {
+            SmtpConfig {
                 name: name.into(),
                 mailto: Some(vec!["user1 at example.com".into()]),
                 mailto_user: None,
@@ -222,7 +222,7 @@ pub mod tests {
                 username: Some("username".into()),
                 ..Default::default()
             },
-            &SmtpPrivateConfig {
+            SmtpPrivateConfig {
                 name: name.into(),
                 password: Some("password".into()),
             },
@@ -252,8 +252,8 @@ pub mod tests {
         assert!(update_endpoint(
             &mut config,
             "test",
-            &Default::default(),
-            &Default::default(),
+            Default::default(),
+            Default::default(),
             None,
             None,
         )
@@ -270,8 +270,8 @@ pub mod tests {
         assert!(update_endpoint(
             &mut config,
             "sendmail-endpoint",
-            &Default::default(),
-            &Default::default(),
+            Default::default(),
+            Default::default(),
             None,
             Some(&[0; 32]),
         )
@@ -290,7 +290,7 @@ pub mod tests {
         update_endpoint(
             &mut config,
             "smtp-endpoint",
-            &SmtpConfigUpdater {
+            SmtpConfigUpdater {
                 mailto: Some(vec!["user2 at example.com".into(), "user3 at example.com".into()]),
                 mailto_user: Some(vec!["root at pam".into()]),
                 from_address: Some("root at example.com".into()),
@@ -302,7 +302,7 @@ pub mod tests {
                 username: Some("newusername".into()),
                 ..Default::default()
             },
-            &Default::default(),
+            Default::default(),
             None,
             Some(&digest),
         )?;
@@ -325,8 +325,8 @@ pub mod tests {
         update_endpoint(
             &mut config,
             "smtp-endpoint",
-            &Default::default(),
-            &Default::default(),
+            Default::default(),
+            Default::default(),
             Some(&[
                 DeleteableSmtpProperty::Author,
                 DeleteableSmtpProperty::MailtoUser,
-- 
2.39.2




More information about the pve-devel mailing list