notify: make template rendering helpers more robust

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.wagner@proxmox.com>
This commit is contained in:
Lukas Wagner 2023-08-25 13:35:57 +02:00 committed by Thomas Lamprecht
parent c81068097b
commit d49fc1aa2f
4 changed files with 49 additions and 17 deletions

View File

@ -43,7 +43,7 @@ fn main() -> Result<(), Error> {
"data" : [ "data" : [
{ {
"vmid": 1001, "vmid": 1001,
"size": 1024 * 1024, "size": "1048576"
}, },
{ {
"vmid": 1002, "vmid": 1002,

View File

@ -42,7 +42,7 @@ fn render_html_table(
let entry = row.get(&column.id).unwrap_or(&Value::Null); let entry = row.get(&column.id).unwrap_or(&Value::Null);
let text = if let Some(renderer) = &column.renderer { let text = if let Some(renderer) = &column.renderer {
renderer.render(entry)? renderer.render(entry)
} else { } else {
value_to_string(entry) value_to_string(entry)
}; };

View File

@ -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> { 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))) Some(format!("{}", HumanByte::new_binary(size)))
} }
/// Render a serde_json::Value as a duration. /// Render a serde_json::Value as a duration.
/// The value is expected to contain the duration in seconds. /// 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> { 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)); let time_span = TimeSpan::from(Duration::from_secs(duration));
Some(format!("{time_span}")) Some(format!("{time_span}"))
@ -50,10 +61,15 @@ fn value_to_duration(val: &Value) -> Option<String> {
/// Render as serde_json::Value as a timestamp. /// Render as serde_json::Value as a timestamp.
/// The value is expected to contain the timestamp as a unix epoch. /// 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> { 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() proxmox_time::strftime_local("%F %H:%M:%S", timestamp).ok()
} }
@ -95,16 +111,15 @@ pub enum ValueRenderFunction {
} }
impl ValueRenderFunction { impl ValueRenderFunction {
fn render(&self, value: &Value) -> Result<String, HandlebarsRenderError> { fn render(&self, value: &Value) -> String {
match self { match self {
ValueRenderFunction::HumanBytes => value_to_byte_size(value), ValueRenderFunction::HumanBytes => value_to_byte_size(value),
ValueRenderFunction::Duration => value_to_duration(value), ValueRenderFunction::Duration => value_to_duration(value),
ValueRenderFunction::Timestamp => value_to_timestamp(value), ValueRenderFunction::Timestamp => value_to_timestamp(value),
} }
.ok_or_else(|| { .unwrap_or_else(|| {
HandlebarsRenderError::new(format!( log::error!("could not render value {value} with renderer {self:?}");
"could not render value {value} with renderer {self:?}" String::from("ERROR")
))
}) })
} }
@ -141,7 +156,7 @@ impl ValueRenderFunction {
.ok_or(HandlebarsRenderError::new("parameter not found"))?; .ok_or(HandlebarsRenderError::new("parameter not found"))?;
let value = param.value(); let value = param.value();
out.write(&self.render(value)?)?; out.write(&self.render(value))?;
Ok(()) Ok(())
}, },
@ -364,4 +379,21 @@ val3 val4
Ok(()) 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());
}
} }

View File

@ -20,7 +20,7 @@ fn optimal_column_widths(table: &Table) -> HashMap<&str, usize> {
let entry = row.get(&column.id).unwrap_or(&Value::Null); let entry = row.get(&column.id).unwrap_or(&Value::Null);
let text = if let Some(renderer) = &column.renderer { let text = if let Some(renderer) = &column.renderer {
renderer.render(entry).unwrap_or_default() renderer.render(entry)
} else { } else {
value_to_string(entry) value_to_string(entry)
}; };
@ -63,7 +63,7 @@ fn render_plaintext_table(
let width = widths.get(column.label.as_str()).unwrap_or(&0); let width = widths.get(column.label.as_str()).unwrap_or(&0);
let text = if let Some(renderer) = &column.renderer { let text = if let Some(renderer) = &column.renderer {
renderer.render(entry)? renderer.render(entry)
} else { } else {
value_to_string(entry) value_to_string(entry)
}; };