Skip to content

Commit

Permalink
Implement package filtering as a repo method
Browse files Browse the repository at this point in the history
- 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 <nico.steinle@eviden.com>
  • Loading branch information
ammernico committed Nov 5, 2024
1 parent 64ed815 commit c8a0851
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 47 deletions.
34 changes: 2 additions & 32 deletions src/commands/source/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<String>("package_name");
let pvers = matches.get_one::<String>("package_version");
let matching_regexp = matches.get_one::<String>("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();
Expand Down
16 changes: 1 addition & 15 deletions src/commands/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
39 changes: 39 additions & 0 deletions src/repository/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -270,6 +271,44 @@ impl Repository {
pub fn packages(&self) -> impl Iterator<Item = &Package> {
self.inner.values()
}

pub fn search_packages<'a>(
&'a self,
pname: &'a Option<PackageName>,
pvers: &'a Option<PackageVersionConstraint>,
matching_regexp: &'a Option<Regex>,
) -> Result<impl Iterator<Item = &Package> + '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)]
Expand Down

0 comments on commit c8a0851

Please sign in to comment.