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 <l.wag...@proxmox.com>
---
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<String> {
- 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<String> {
- 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<String> {
/// 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<String> {
- 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<String, HandlebarsRenderError> {
+ 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
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
