[pve-devel] [PATCH proxmox 02/27] notify: factor out notification content into its own type

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

This will be useful later for system mail forwarding, where
the content of the mail should be forwarded unchanged.

This moves notification properties into this new type and calls them
'data'. They will exclusively used for template rendering.
`Notification` will receive a separate field for metadata, which
will be useful for notification filtering. This decouples
template rendering and filtering, which enables us to be very precise
about which metadata fields we allow to be used in filters.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-notify/examples/render.rs        |  4 +-
 proxmox-notify/src/endpoints/gotify.rs   | 26 +++++---
 proxmox-notify/src/endpoints/sendmail.rs | 62 +++++++++---------
 proxmox-notify/src/filter.rs             | 10 +--
 proxmox-notify/src/lib.rs                | 81 ++++++++++++++----------
 proxmox-notify/src/renderer/mod.rs       | 15 ++---
 6 files changed, 109 insertions(+), 89 deletions(-)

diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs
index c0a6f27..d705fd0 100644
--- a/proxmox-notify/examples/render.rs
+++ b/proxmox-notify/examples/render.rs
@@ -53,10 +53,10 @@ fn main() -> Result<(), Error> {
         }
     });
 
-    let output = render_template(TemplateRenderer::Html, TEMPLATE, Some(&properties))?;
+    let output = render_template(TemplateRenderer::Html, TEMPLATE, &properties)?;
     println!("{output}");
 
-    let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, Some(&properties))?;
+    let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, &properties)?;
     println!("{output}");
 
     Ok(())
diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs
index 83df41f..af86f9c 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater};
 use crate::context::context;
 use crate::renderer::TemplateRenderer;
 use crate::schema::ENTITY_NAME_SCHEMA;
-use crate::{renderer, Endpoint, Error, Notification, Severity};
+use crate::{renderer, Content, Endpoint, Error, Notification, Severity};
 
 fn severity_to_priority(level: Severity) -> u32 {
     match level {
@@ -85,15 +85,21 @@ pub enum DeleteableGotifyProperty {
 
 impl Endpoint for GotifyEndpoint {
     fn send(&self, notification: &Notification) -> Result<(), Error> {
-        let properties = notification.properties.as_ref();
-
-        let title = renderer::render_template(
-            TemplateRenderer::Plaintext,
-            &notification.title,
-            properties,
-        )?;
-        let message =
-            renderer::render_template(TemplateRenderer::Plaintext, &notification.body, properties)?;
+
+        let (title, message) = match &notification.content {
+            Content::Template {
+                title_template,
+                body_template,
+                data
+            } => {
+                let rendered_title =
+                    renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+                let rendered_message =
+                    renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+
+                (rendered_title, rendered_message)
+            }
+        };
 
         // We don't have a TemplateRenderer::Markdown yet, so simply put everything
         // in code tags. Otherwise tables etc. are not formatted properly
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index 26e2a17..c540925 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -8,7 +8,7 @@ use proxmox_schema::{api, Updater};
 use crate::context::context;
 use crate::renderer::TemplateRenderer;
 use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA};
-use crate::{renderer, Endpoint, Error, Notification};
+use crate::{renderer, Content, Endpoint, Error, Notification};
 
 pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail";
 
@@ -102,41 +102,43 @@ impl Endpoint for SendmailEndpoint {
             }
         }
 
-        let properties = notification.properties.as_ref();
-
-        let subject = renderer::render_template(
-            TemplateRenderer::Plaintext,
-            &notification.title,
-            properties,
-        )?;
-        let html_part =
-            renderer::render_template(TemplateRenderer::Html, &notification.body, properties)?;
-        let text_part =
-            renderer::render_template(TemplateRenderer::Plaintext, &notification.body, properties)?;
-
-        let author = self
-            .config
-            .author
-            .clone()
-            .unwrap_or_else(|| context().default_sendmail_author());
-
+        let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
         let mailfrom = self
             .config
             .from_address
             .clone()
             .unwrap_or_else(|| context().default_sendmail_from());
 
-        let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
-
-        proxmox_sys::email::sendmail(
-            &recipients_str,
-            &subject,
-            Some(&text_part),
-            Some(&html_part),
-            Some(&mailfrom),
-            Some(&author),
-        )
-        .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+        match &notification.content {
+            Content::Template {
+                title_template,
+                body_template,
+                data,
+            } => {
+                let subject =
+                    renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+                let html_part =
+                    renderer::render_template(TemplateRenderer::Html, body_template, data)?;
+                let text_part =
+                    renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+
+                let author = self
+                    .config
+                    .author
+                    .clone()
+                    .unwrap_or_else(|| context().default_sendmail_author());
+
+                proxmox_sys::email::sendmail(
+                    &recipients_str,
+                    &subject,
+                    Some(&text_part),
+                    Some(&html_part),
+                    Some(&mailfrom),
+                    Some(&author),
+                )
+                .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+            }
+        }
     }
 
     fn name(&self) -> &str {
diff --git a/proxmox-notify/src/filter.rs b/proxmox-notify/src/filter.rs
index 748ec4e..e014a59 100644
--- a/proxmox-notify/src/filter.rs
+++ b/proxmox-notify/src/filter.rs
@@ -160,7 +160,7 @@ impl<'a> FilterMatcher<'a> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::config;
+    use crate::{config, Content};
 
     fn parse_filters(config: &str) -> Result<Vec<FilterConfig>, Error> {
         let (config, _) = config::config(config)?;
@@ -169,10 +169,12 @@ mod tests {
 
     fn empty_notification_with_severity(severity: Severity) -> Notification {
         Notification {
-            title: String::new(),
-            body: String::new(),
+            content: Content::Template {
+                title_template: String::new(),
+                body_template: String::new(),
+                data: Default::default(),
+            },
             severity,
-            properties: Default::default(),
         }
     }
 
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index f7d480c..d40d017 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -116,17 +116,44 @@ pub trait Endpoint {
     fn filter(&self) -> Option<&str>;
 }
 
+#[derive(Debug, Clone)]
+pub enum Content {
+    /// Title and body will be rendered as a template
+    Template {
+        /// Template for the notification title.
+        title_template: String,
+        /// Template for the notification body.
+        body_template: String,
+        /// Data that can be used for template rendering.
+        data: Value,
+    },
+}
+
 #[derive(Debug, Clone)]
 /// Notification which can be sent
 pub struct Notification {
     /// Notification severity
-    pub severity: Severity,
-    /// The title of the notification
-    pub title: String,
-    /// Notification text
-    pub body: String,
-    /// Additional metadata for the notification
-    pub properties: Option<Value>,
+    severity: Severity,
+    /// Notification content
+    content: Content,
+}
+
+impl Notification {
+    pub fn new_templated<S: AsRef<str>>(
+        severity: Severity,
+        title: S,
+        body: S,
+        properties: Value,
+    ) -> Self {
+        Self {
+            severity,
+            content: Content::Template {
+                title_template: title.as_ref().to_string(),
+                body_template: body.as_ref().to_string(),
+                data: properties,
+            },
+        }
+    }
 }
 
 /// Notification configuration
@@ -384,9 +411,11 @@ impl Bus {
     pub fn test_target(&self, target: &str) -> Result<(), Error> {
         let notification = Notification {
             severity: Severity::Info,
-            title: "Test notification".into(),
-            body: "This is a test of the notification target '{{ target }}'".into(),
-            properties: Some(json!({ "target": target })),
+            content: Content::Template {
+                title_template: "Test notification".into(),
+                body_template: "This is a test of the notification target '{{ target }}'".into(),
+                data: json!({ "target": target }),
+            },
         };
 
         let mut errors: Vec<Box<dyn StdError + Send + Sync>> = Vec::new();
@@ -473,12 +502,7 @@ mod tests {
         // Send directly to endpoint
         bus.send(
             "endpoint",
-            &Notification {
-                title: "Title".into(),
-                body: "Body".into(),
-                severity: Severity::Info,
-                properties: Default::default(),
-            },
+            &Notification::new_templated(Severity::Info, "Title", "Body", Default::default()),
         );
         let messages = mock.messages();
         assert_eq!(messages.len(), 1);
@@ -511,15 +535,9 @@ mod tests {
         bus.add_endpoint(Box::new(endpoint2.clone()));
 
         let send_to_group = |channel| {
-            bus.send(
-                channel,
-                &Notification {
-                    title: "Title".into(),
-                    body: "Body".into(),
-                    severity: Severity::Info,
-                    properties: Default::default(),
-                },
-            )
+            let notification =
+                Notification::new_templated(Severity::Info, "Title", "Body", Default::default());
+            bus.send(channel, &notification)
         };
 
         send_to_group("group1");
@@ -579,15 +597,10 @@ mod tests {
         });
 
         let send_with_severity = |severity| {
-            bus.send(
-                "channel1",
-                &Notification {
-                    title: "Title".into(),
-                    body: "Body".into(),
-                    severity,
-                    properties: Default::default(),
-                },
-            );
+            let notification =
+                Notification::new_templated(severity, "Title", "Body", Default::default());
+
+            bus.send("channel1", &notification);
         };
 
         send_with_severity(Severity::Info);
diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs
index 24f14aa..e9f36e6 100644
--- a/proxmox-notify/src/renderer/mod.rs
+++ b/proxmox-notify/src/renderer/mod.rs
@@ -228,11 +228,9 @@ impl BlockRenderFunctions {
 
 fn render_template_impl(
     template: &str,
-    properties: Option<&Value>,
+    data: &Value,
     renderer: TemplateRenderer,
 ) -> Result<String, Error> {
-    let properties = properties.unwrap_or(&Value::Null);
-
     let mut handlebars = Handlebars::new();
     handlebars.register_escape_fn(renderer.escape_fn());
 
@@ -242,7 +240,7 @@ fn render_template_impl(
     ValueRenderFunction::register_helpers(&mut handlebars);
 
     let rendered_template = handlebars
-        .render_template(template, properties)
+        .render_template(template, data)
         .map_err(|err| Error::RenderError(err.into()))?;
 
     Ok(rendered_template)
@@ -255,11 +253,11 @@ fn render_template_impl(
 pub fn render_template(
     renderer: TemplateRenderer,
     template: &str,
-    properties: Option<&Value>,
+    data: &Value,
 ) -> Result<String, Error> {
     let mut rendered_template = String::from(renderer.prefix());
 
-    rendered_template.push_str(&render_template_impl(template, properties, renderer)?);
+    rendered_template.push_str(&render_template_impl(template, data, renderer)?);
     rendered_template.push_str(renderer.postfix());
 
     Ok(rendered_template)
@@ -314,7 +312,7 @@ mod tests {
 
     #[test]
     fn test_render_template() -> Result<(), Error> {
-        let properties = json!({
+        let data = json!({
             "dur": 12345,
             "size": 1024 * 15,
 
@@ -370,8 +368,7 @@ val1        val2
 val3        val4        
 "#;
 
-        let rendered_plaintext =
-            render_template(TemplateRenderer::Plaintext, template, Some(&properties))?;
+        let rendered_plaintext = render_template(TemplateRenderer::Plaintext, template, &data)?;
 
         // Let's not bother about testing the HTML output, too fragile.
 
-- 
2.39.2