On 9/15/21 15:41, Fabian Grünbichler wrote:
at the API level, this is a simple (wrapped) Vec of Strings with a
verifier function. all users should use the provided helper to get the
actual GroupFilter enum values, which can't be directly used in the API
schema because of restrictions of the api macro.
validation of the schema + parsing into the proper type uses the same fn
intentionally to avoid running out of sync, even if it means compiling
the REs twice.
Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
---
this is working around some api limitations
- updated doesn't support a Vec<String> type
- api-macro doesn't support enum with values
the validation fn and the parser/converted use the same code to avoid
getting out of sync, at the expense of parsing potentially expensive REs
twice..
pbs-api-types/src/jobs.rs | 90 +++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 1526dbc4..94638fe5 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -1,3 +1,7 @@
+use anyhow::format_err;
+use std::str::FromStr;
+
+use regex::Regex;
use serde::{Deserialize, Serialize};
use proxmox::const_regex;
@@ -7,6 +11,7 @@ use proxmox::api::{api, schema::*};
use crate::{
Userid, Authid, REMOTE_ID_SCHEMA, DRIVE_NAME_SCHEMA,
MEDIA_POOL_NAME_SCHEMA,
SINGLE_LINE_COMMENT_SCHEMA, PROXMOX_SAFE_ID_FORMAT, DATASTORE_SCHEMA,
+ BACKUP_GROUP_SCHEMA, BACKUP_TYPE_SCHEMA,
};
const_regex!{
@@ -319,6 +324,91 @@ pub struct TapeBackupJobStatus {
pub next_media_label: Option<String>,
}
+#[derive(Clone, Debug)]
+/// Filter for matching `BackupGroup`s, for use with `BackupGroup::filter`.
+pub enum GroupFilter {
+ /// BackupGroup type - either `vm`, `ct`, or `host`.
+ BackupType(String),
+ /// Full identifier of BackupGroup, including type
+ Group(String),
+ /// A regular expression matched against the full identifier of the
BackupGroup
+ Regex(Regex),
+}
Would it not make sense to have an already parsed "BackupGroup" in
the Group Variant instead of a string? we would have
to move the pub struct BackupGroup here, but i think the impl block can
stay in pbs-datastore
+
+impl std::str::FromStr for GroupFilter {
+ type Err = anyhow::Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ match s.split_once(":") {
+ Some(("group", value)) => parse_simple_value(value,
&BACKUP_GROUP_SCHEMA).map(|_| GroupFilter::Group(value.to_string())),
here you could parse it directly to a 'BackupGroup' then we would not
need the BACKUP_GROUP_SCHEMA
+ Some(("type", value)) => parse_simple_value(value,
&BACKUP_TYPE_SCHEMA).map(|_| GroupFilter::BackupType(value.to_string())),
nit: would it not be enough to match the regex instead of the schema?
(not that i think it'd make a difference)
+ Some(("regex", value)) =>
Ok(GroupFilter::Regex(Regex::new(value)?)),
+ Some((ty, _value)) => Err(format_err!("expected 'group', 'type' or
'regex' prefix, got '{}'", ty)),
+ None => Err(format_err!("input doesn't match expected format
'<group:GROUP||type:<vm|ct|host>|regex:REGEX>'")),
+ }.map_err(|err| format_err!("'{}' - {}", s, err))
+ }
+}
+
+// used for serializing below, caution!
+impl std::fmt::Display for GroupFilter {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ GroupFilter::BackupType(backup_type) => write!(f, "type:{}",
backup_type),
+ GroupFilter::Group(backup_group) => write!(f, "group:{}",
backup_group),
+ GroupFilter::Regex(regex) => write!(f, "regex:{}", regex.as_str()),
+ }
+ }
+}
+
+impl Serialize for GroupFilter {
+ fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+ where
+ S: serde::Serializer,
+ {
+ serializer.serialize_str(&self.to_string())
+ }
+}
+
+impl<'de> Deserialize<'de> for GroupFilter {
+ fn deserialize<D>(deserializer: D) -> Result<GroupFilter, D::Error>
+ where
+ D: serde::Deserializer<'de>,
+ {
+ String::deserialize(deserializer).and_then(|string| {
+ GroupFilter::from_str(&string).map_err(serde::de::Error::custom)
+ })
+ }
+}
+
+fn verify_group_filter(input: &str) -> Result<(), anyhow::Error> {
+ GroupFilter::from_str(input).map(|_| ())
+}
+
+pub const GROUP_FILTER_SCHEMA: Schema = StringSchema::new(
+ "Group filter based on group identifier ('group:GROUP'), group type
('type:<vm|ct|host>'), or regex ('regex:RE').")
+ .format(&ApiStringFormat::VerifyFn(verify_group_filter))
+ .type_text("<type:<vm|ct|host>|group:GROUP|regex:RE>")
+ .schema();
+
+#[api(
+ type: Array,
+ description: "List of group filters.",
+ items: {
+ schema: GROUP_FILTER_SCHEMA,
+ },
+)]
+#[derive(Serialize, Deserialize, Clone, UpdaterType)]
+pub struct GroupFilterList(Vec<String>);
+
+impl GroupFilterList {
+ /// converts schema-validated Vec<String> into Vec<GroupFilter>
+ pub fn filters(self) -> Vec<GroupFilter> {
+ self.0.iter()
+ .map(|group_filter| GroupFilter::from_str(group_filter).unwrap())
+ .collect()
+ }
+}
+
IMHO that has to be easier:
instead of the custom type, i'd do:
const GROUP_FILTER_LIST_SCHEMA: Schema = ArraySchema::new("",
&GROUP_FILTER_SCHEMA).schema()
the only thing you'd have to do is to convice the updater that
there is an updaterType for it: (in proxmox/src/api/schema.rs of the
proxmox crate)
impl<T> UpdaterType for Vec<T> {
type Updater = Option<Self>;
}
that implies though that a user always has to give the complete list
(so no single removal from the list), though i think that's what
we want anyway..
i tested it here, and AFAICS, this works (i can update/list the sync job
properly)
with that you can also remove the 'filters' fn and can use
Vec<GroupFilter> as the api call parameters.
makes the code a little easier to read
#[api(
properties: {
id: {
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
