From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 8A45A965C for ; Fri, 25 Aug 2023 13:36:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 72C2D7067 for ; Fri, 25 Aug 2023 13:36:09 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 25 Aug 2023 13:36:08 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 8E795448A2 for ; Fri, 25 Aug 2023 13:36:08 +0200 (CEST) From: Lukas Wagner To: pve-devel@lists.proxmox.com Date: Fri, 25 Aug 2023 13:35:57 +0200 Message-Id: <20230825113557.496234-1-l.wagner@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.037 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [html.rs, mod.rs, render.rs, plaintext.rs] Subject: [pve-devel] [PATCH proxmox] notify: make template rendering helpers more robust X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Aug 2023 11:36:09 -0000 This commit has the aim of making template rendering a bit more robust. It does so by a.) Accepting also strings for helpers that expect a number, parsing the number if needed, and b.) Ignoring errors if a template helper fails to render a value and showing an error in the logs, instead of failing to render the whole template (leading to no notification being sent). Signed-off-by: Lukas Wagner --- proxmox-notify/examples/render.rs | 2 +- proxmox-notify/src/renderer/html.rs | 2 +- proxmox-notify/src/renderer/mod.rs | 58 ++++++++++++++++++------ proxmox-notify/src/renderer/plaintext.rs | 4 +- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs index 5c1f2b2..c0a6f27 100644 --- a/proxmox-notify/examples/render.rs +++ b/proxmox-notify/examples/render.rs @@ -43,7 +43,7 @@ fn main() -> Result<(), Error> { "data" : [ { "vmid": 1001, - "size": 1024 * 1024, + "size": "1048576" }, { "vmid": 1002, diff --git a/proxmox-notify/src/renderer/html.rs b/proxmox-notify/src/renderer/html.rs index e9ff2b4..f794902 100644 --- a/proxmox-notify/src/renderer/html.rs +++ b/proxmox-notify/src/renderer/html.rs @@ -42,7 +42,7 @@ fn render_html_table( let entry = row.get(&column.id).unwrap_or(&Value::Null); let text = if let Some(renderer) = &column.renderer { - renderer.render(entry)? + renderer.render(entry) } else { value_to_string(entry) }; diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs index f37e672..24f14aa 100644 --- a/proxmox-notify/src/renderer/mod.rs +++ b/proxmox-notify/src/renderer/mod.rs @@ -29,20 +29,31 @@ fn value_to_string(value: &Value) -> String { } } -/// Render a serde_json::Value as a byte size with proper units (IEC, base 2) +/// Render a `serde_json::Value` as a byte size with proper units (IEC, base 2). +/// Accepts `serde_json::Value::{Number,String}`. /// -/// Will return `None` if `val` does not contain a number. +/// Will return `None` if `val` does not contain a number/parseable string. fn value_to_byte_size(val: &Value) -> Option { - let size = val.as_f64()?; + let size = match val { + Value::Number(n) => n.as_f64(), + Value::String(s) => s.parse().ok(), + _ => None, + }?; + Some(format!("{}", HumanByte::new_binary(size))) } /// Render a serde_json::Value as a duration. /// The value is expected to contain the duration in seconds. +/// Accepts `serde_json::Value::{Number,String}`. /// -/// Will return `None` if `val` does not contain a number. +/// Will return `None` if `val` does not contain a number/parseable string. fn value_to_duration(val: &Value) -> Option { - let duration = val.as_u64()?; + let duration = match val { + Value::Number(n) => n.as_u64(), + Value::String(s) => s.parse().ok(), + _ => None, + }?; let time_span = TimeSpan::from(Duration::from_secs(duration)); Some(format!("{time_span}")) @@ -50,10 +61,15 @@ fn value_to_duration(val: &Value) -> Option { /// Render as serde_json::Value as a timestamp. /// The value is expected to contain the timestamp as a unix epoch. +/// Accepts `serde_json::Value::{Number,String}`. /// -/// Will return `None` if `val` does not contain a number. +/// Will return `None` if `val` does not contain a number/parseable string. fn value_to_timestamp(val: &Value) -> Option { - let timestamp = val.as_i64()?; + let timestamp = match val { + Value::Number(n) => n.as_i64(), + Value::String(s) => s.parse().ok(), + _ => None, + }?; proxmox_time::strftime_local("%F %H:%M:%S", timestamp).ok() } @@ -95,16 +111,15 @@ pub enum ValueRenderFunction { } impl ValueRenderFunction { - fn render(&self, value: &Value) -> Result { + fn render(&self, value: &Value) -> String { match self { ValueRenderFunction::HumanBytes => value_to_byte_size(value), ValueRenderFunction::Duration => value_to_duration(value), ValueRenderFunction::Timestamp => value_to_timestamp(value), } - .ok_or_else(|| { - HandlebarsRenderError::new(format!( - "could not render value {value} with renderer {self:?}" - )) + .unwrap_or_else(|| { + log::error!("could not render value {value} with renderer {self:?}"); + String::from("ERROR") }) } @@ -141,7 +156,7 @@ impl ValueRenderFunction { .ok_or(HandlebarsRenderError::new("parameter not found"))?; let value = param.value(); - out.write(&self.render(value)?)?; + out.write(&self.render(value))?; Ok(()) }, @@ -364,4 +379,21 @@ val3 val4 Ok(()) } + + #[test] + fn test_helpers() { + assert_eq!(value_to_byte_size(&json!(1024)), Some("1 KiB".to_string())); + assert_eq!( + value_to_byte_size(&json!("1024")), + Some("1 KiB".to_string()) + ); + + assert_eq!(value_to_duration(&json!(60)), Some("1min ".to_string())); + assert_eq!(value_to_duration(&json!("60")), Some("1min ".to_string())); + + // The rendered value is in localtime, so we only check if the result is `Some`... + // ... otherwise the test will break in another timezone :S + assert!(value_to_timestamp(&json!(60)).is_some()); + assert!(value_to_timestamp(&json!("60")).is_some()); + } } diff --git a/proxmox-notify/src/renderer/plaintext.rs b/proxmox-notify/src/renderer/plaintext.rs index c8079d7..437e412 100644 --- a/proxmox-notify/src/renderer/plaintext.rs +++ b/proxmox-notify/src/renderer/plaintext.rs @@ -20,7 +20,7 @@ fn optimal_column_widths(table: &Table) -> HashMap<&str, usize> { let entry = row.get(&column.id).unwrap_or(&Value::Null); let text = if let Some(renderer) = &column.renderer { - renderer.render(entry).unwrap_or_default() + renderer.render(entry) } else { value_to_string(entry) }; @@ -63,7 +63,7 @@ fn render_plaintext_table( let width = widths.get(column.label.as_str()).unwrap_or(&0); let text = if let Some(renderer) = &column.renderer { - renderer.render(entry)? + renderer.render(entry) } else { value_to_string(entry) }; -- 2.39.2