From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 592CD1FF166 for ; Fri, 25 Oct 2024 14:15:28 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 17D351F1F1; Fri, 25 Oct 2024 14:15:28 +0200 (CEST) Date: Fri, 25 Oct 2024 14:15:21 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20241024080150.30200-1-c.ebner@proxmox.com> <20241024080150.30200-3-c.ebner@proxmox.com> In-Reply-To: <20241024080150.30200-3-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1729857821.37okdbzjnk.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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. [self.name, datastore.rs, proxmox.com] Subject: Re: [pbs-devel] [PATCH v3 proxmox-backup 2/5] api types: introduce `BackupArchiveName` type X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On October 24, 2024 10:01 am, Christian Ebner wrote: > Introduces a dedicated wrapper type to be used for backup archive > names instead of plain strings and associated helper methods for > archive type checks and archive name mappings. > > Signed-off-by: Christian Ebner > --- > changes since version 2: > - reworded commit message > > pbs-api-types/src/datastore.rs | 107 ++++++++++++++++++++++++++++++++- > 1 file changed, 106 insertions(+), 1 deletion(-) > > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs > index dfa6bb259..62e4f7345 100644 > --- a/pbs-api-types/src/datastore.rs > +++ b/pbs-api-types/src/datastore.rs > @@ -1,5 +1,7 @@ > +use std::convert::{AsRef, TryFrom}; > use std::fmt; > use std::path::{Path, PathBuf}; > +use std::str::FromStr; > > use anyhow::{bail, format_err, Error}; > use const_format::concatcp; > @@ -1570,7 +1572,7 @@ pub fn print_store_and_ns(store: &str, ns: &BackupNamespace) -> String { > } > } > > -#[derive(PartialEq, Eq)] > +#[derive(Clone, PartialEq, Eq)] > /// Allowed variants of backup archives to be contained in a snapshot's manifest > pub enum ArchiveType { > FixedIndex, > @@ -1590,3 +1592,106 @@ impl ArchiveType { > Ok(archive_type) > } > } > + > +#[derive(Clone, PartialEq, Eq)] > +/// Name of archive files contained in snapshot's manifest > +pub struct BackupArchiveName { > + // archive name including the `.fidx`, `.didx` or `.blob` extension > + name: String, > + // type parsed based on given extension > + ty: ArchiveType, > +} > + > +impl fmt::Display for BackupArchiveName { > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > + write!(f, "{name}", name = self.name) > + } > +} > + > +serde_plain::derive_deserialize_from_fromstr!(BackupArchiveName, "archive name"); > + > +impl FromStr for BackupArchiveName { > + type Err = Error; > + > + fn from_str(name: &str) -> Result { > + Self::try_from(name) > + } > +} > + > +serde_plain::derive_serialize_from_display!(BackupArchiveName); > + > +impl TryFrom<&str> for BackupArchiveName { > + type Error = anyhow::Error; > + > + fn try_from(value: &str) -> Result { > + let (name, ty) = Self::parse_archive_type(value)?; > + Ok(Self { name, ty }) > + } > +} > + > +impl AsRef for BackupArchiveName { > + fn as_ref(&self) -> &str { > + &self.name > + } > +} > + > +impl BackupArchiveName { > + pub fn from_path(path: impl AsRef) -> Result { > + let path = path.as_ref(); > + if path.as_os_str().as_encoded_bytes().last() == Some(&b'/') { > + bail!("invalid archive name, got directory"); > + } > + let file_name = path > + .file_name() > + .ok_or_else(|| format_err!("invalid archive name"))?; > + let name = file_name > + .to_str() > + .ok_or_else(|| format_err!("archive name not valid UTF-8"))?; > + > + Self::try_from(name) > + } > + > + pub fn archive_type(&self) -> ArchiveType { > + self.ty.clone() > + } > + > + pub fn ends_with(&self, postfix: &str) -> bool { > + self.name.ends_with(postfix) > + } > + > + pub fn has_pxar_filename_extension(&self) -> bool { > + self.name.ends_with(".pxar.didx") > + || self.name.ends_with(".mpxar.didx") > + || self.name.ends_with(".ppxar.didx") > + } > + > + pub fn without_type_extension(&self) -> String { > + match self.ty { > + ArchiveType::DynamicIndex => self.name.strip_suffix(".didx").unwrap().into(), > + ArchiveType::FixedIndex => self.name.strip_suffix(".fidx").unwrap().into(), > + ArchiveType::Blob => self.name.strip_suffix(".blob").unwrap().into(), if ArchiveType would have a getter for the corresponding extension, then this could just become self.name.strip_suffix(self.ty.extension()).unwrap().into() > + } > + } > + > + fn parse_archive_type(archive_name: &str) -> Result<(String, ArchiveType), Error> { > + if archive_name.ends_with(".didx") > + || archive_name.ends_with(".fidx") > + || archive_name.ends_with(".blob") > + { > + Ok((archive_name.into(), ArchiveType::from_path(archive_name)?)) and this here could maybe also be turned around -> get the extension from archive_name: if let Ok(ty) = ArchiveType::from_path(..) { Ok((archive_name.into(), ty)) } else if .. > + } else if archive_name.ends_with(".pxar") > + || archive_name.ends_with(".mpxar") > + || archive_name.ends_with(".ppxar") > + { > + Ok((format!("{archive_name}.didx"), ArchiveType::DynamicIndex)) > + } else if archive_name.ends_with(".img") { > + Ok((format!("{archive_name}.fidx"), ArchiveType::FixedIndex)) not sure whether we want these associations (between ArchiveType and contained files) to live somewhere more declarative? > + } else { > + Ok((format!("{archive_name}.blob"), ArchiveType::Blob)) this last catchall here might be a bit dangerous? it basically makes the introduction of a new archive type collide with any existing blobs that happen to have a file name that ends with that new archive type.. e.g., qemu-server.conf.blob with this patch, qemu-server.conf will automatically expand to qemu-server.conf.blob if we then at some point introduce an archive type `conf`, then it wouldn't anymore, but instead be interpreted as archive of that type.. similarly, all of the above "inner" extensions are burned for archive type usage already.. this was basically the only reason I didn't apply this right now leaving the rest above as follow-up material ;) > + } > + } > +} > + > +impl ApiType for BackupArchiveName { > + const API_SCHEMA: Schema = BACKUP_ARCHIVE_NAME_SCHEMA; > +} > -- > 2.39.5 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel