From c8a085103ee1e4451fda9e9de9f9ccb0fc12cd0e Mon Sep 17 00:00:00 2001 From: Nico Steinle Date: Mon, 14 Oct 2024 14:55:16 +0200 Subject: [PATCH] Implement package filtering as a repo method - Moved the package filtering logic from `download` and `verify` functions into a new `search_packages` method in the `Repository` struct. - Updated `download` and `verify` functions to use the new `search_packages` method. This refactor improves code reuse and readability by centralizing the package filtering logic. Fixes #428 We already fixed this for the download function in 76a3cde. The package filtering for the verifying function was implemented in the same way but didn't fail yet. Signed-off-by: Nico Steinle --- src/commands/source/download.rs | 34 ++-------------------------- src/commands/source/mod.rs | 16 +------------- src/repository/repository.rs | 39 +++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/commands/source/download.rs b/src/commands/source/download.rs index 3a84fca7..c0520c1b 100644 --- a/src/commands/source/download.rs +++ b/src/commands/source/download.rs @@ -233,38 +233,8 @@ pub async fn download( NUMBER_OF_MAX_CONCURRENT_DOWNLOADS, )); - let mut r = repo.packages() - .filter(|p| { - match (pname.as_ref(), pvers.as_ref(), matching_regexp.as_ref()) { - (None, None, None) => true, - (Some(pname), None, None) => p.name() == pname, - (Some(pname), Some(vers), None) => p.name() == pname && vers.matches(p.version()), - (None, None, Some(regex)) => regex.is_match(p.name()), - - (_, _, _) => { - panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.") - }, - } - }).peekable(); - - // check if the iterator is empty - if r.peek().is_none() { - let pname = matches.get_one::("package_name"); - let pvers = matches.get_one::("package_version"); - let matching_regexp = matches.get_one::("matching"); - - match (pname, pvers, matching_regexp) { - (Some(pname), None, None) => return Err(anyhow!("{} not found", pname)), - (Some(pname), Some(vers), None) => return Err(anyhow!("{} {} not found", pname, vers)), - (None, None, Some(regex)) => return Err(anyhow!("{} regex not found", regex)), - - (_, _, _) => { - panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.") - } - } - } - - let r = r + let r = repo + .search_packages(&pname, &pvers, &matching_regexp)? .flat_map(|p| { sc.sources_for(p).into_iter().map(|source| { let download_sema = download_sema.clone(); diff --git a/src/commands/source/mod.rs b/src/commands/source/mod.rs index 6dba295d..ae4a47a7 100644 --- a/src/commands/source/mod.rs +++ b/src/commands/source/mod.rs @@ -74,21 +74,7 @@ pub async fn verify( .map(|s| crate::commands::util::mk_package_name_regex(s.as_ref())) .transpose()?; - let packages = repo - .packages() - .filter(|p| { - match (pname.as_ref(), pvers.as_ref(), matching_regexp.as_ref()) { - (None, None, None) => true, - (Some(pname), None, None) => p.name() == pname, - (Some(pname), Some(vers), None) => p.name() == pname && vers.matches(p.version()), - (None, None, Some(regex)) => regex.is_match(p.name()), - - (_, _, _) => { - panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.") - }, - } - }) - .inspect(|p| trace!("Found for verification: {} {}", p.name(), p.version())); + let packages = repo.search_packages(&pname, &pvers, &matching_regexp)?; verify_impl(packages, &sc, &progressbars).await } diff --git a/src/repository/repository.rs b/src/repository/repository.rs index eb3fc356..1f549504 100644 --- a/src/repository/repository.rs +++ b/src/repository/repository.rs @@ -17,6 +17,7 @@ use anyhow::anyhow; use anyhow::Context; use anyhow::Error; use anyhow::Result; +use regex::Regex; use resiter::AndThen; use resiter::FilterMap; use resiter::Map; @@ -270,6 +271,44 @@ impl Repository { pub fn packages(&self) -> impl Iterator { self.inner.values() } + + pub fn search_packages<'a>( + &'a self, + pname: &'a Option, + pvers: &'a Option, + matching_regexp: &'a Option, + ) -> Result + 'a> { + let mut r = self.inner.values() + .filter(move |p| { + match (pname, pvers, matching_regexp) { + (None, None, None) => true, + (Some(pname), None, None) => p.name() == pname, + (Some(pname), Some(vers), None) => p.name() == pname && vers.matches(p.version()), + (None, None, Some(regex)) => regex.is_match(p.name()), + + (_, _, _) => { + panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.") + }, + } + }).peekable(); + + // check if the iterator is empty + if r.peek().is_none() { + match (pname, pvers, matching_regexp) { + (Some(pname), None, None) => return Err(anyhow!("{} not found", pname)), + (Some(pname), Some(vers), None) => { + return Err(anyhow!("{} {} not found", pname, vers)) + } + (None, None, Some(regex)) => return Err(anyhow!("{} regex not found", regex)), + + (_, _, _) => { + panic!("This should not be possible, either we select packages by name and (optionally) version, or by regex.") + } + } + } + + Ok(r) + } } #[cfg(test)]