Re: [PATCH proxmox v2 13/34] notify: api: support new expression parameter

["Michael Köppl" <m.koeppl@proxmox.com>]

On Thu Oct 1, 2026 at 10:36 AM CEST, Lukas Wagner wrote:

[snip]

> +    fn ensure_no_ignored_properties(&self) -> Result<(), Error> {
> +        if self.expression.is_some() {
> +            let conflicting_properties = self.properties_conflicting_with_expression();
> +
> +            if !conflicting_properties.is_empty() {
> +                let joined = conflicting_properties.join(", ");
> +                return Err(Error::Generic(format!(
> +                    "'expression' property is mutually exclusive with: {joined}"
> +                )));
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +
> +    fn ensure_valid_expression(&self) -> Result<(), Error> {
> +        if let Some(expression) = &self.expression {
> +            match serde_json::from_str::<Expression<NotificationMatcher>>(expression) {
> +                Ok(expr) => {

When the expression passed in is pretty-printed JSON (e.g. if read from
a JSON file as part of a script or something similar), this check here
still work, but the original unescaped string is what gets stored,
which fails with

	could not serialize configuration: writing "notifications.cfg"
	failed: detected unexpected control character in section
	'json-pretty' key 'expression'

Since parsing happens here already anyway, perhaps the parsed expression
could be used in add_matcher() and update_matcher() instead of the
original string?

> +                    Self::verify_match_expression_constraints(&expr)?;
> +                }
> +                Err(err) => {
> +                    return Err(Error::Generic(format!(
> +                        "'expression' is not valid: {err:#}"
> +                    )));
> +                }
> +            }
> +        }
> +
> +        Ok(())
> +    }
> +

[snip]