Skip to content

Commit 1bfafb7

Browse files
committed
refactor: use mockable trait
1 parent 792e127 commit 1bfafb7

2 files changed

Lines changed: 37 additions & 36 deletions

File tree

src/app.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{
1616
use clap::Parser;
1717
use hdd::any_path_is_in_hdd;
1818
use pipe_trait::Pipe;
19-
use std::{fs::canonicalize, io::stdin, time::Duration};
19+
use std::{io::stdin, time::Duration};
2020
use sub::JsonOutputParam;
2121
use sysinfo::Disks;
2222

@@ -140,11 +140,8 @@ impl App {
140140
// The current implementation has a quirk in which symbolic link to the real path of another
141141
// argument would disappear. However, considering that nobody would use pdu to measure mere
142142
// symbolic links, we trade this bug for a simpler codebase.
143-
deduplicate_arguments::deduplicate_arguments(
144-
&mut self.args.files,
145-
|path| canonicalize(path),
146-
|a, b| a.starts_with(b),
147-
);
143+
use deduplicate_arguments::{deduplicate_arguments, RealApi};
144+
deduplicate_arguments::<RealApi>(&mut self.args.files);
148145
}
149146

150147
let report_error = if self.args.silent_errors {

src/app/deduplicate_arguments.rs

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,36 @@
11
use pipe_trait::Pipe;
2-
use std::{collections::HashSet, mem::take};
2+
use std::{collections::HashSet, fs::canonicalize, io, mem::take, path::PathBuf};
3+
4+
/// Mockable APIs to interact with the system.
5+
pub trait Api {
6+
type Argument;
7+
type RealPath: Eq;
8+
type RealPathError;
9+
fn canonicalize(path: &Self::Argument) -> Result<Self::RealPath, Self::RealPathError>;
10+
fn starts_with(a: &Self::RealPath, b: &Self::RealPath) -> bool;
11+
}
12+
13+
/// Implementation of [`Api`] that interacts with the real system.
14+
pub struct RealApi;
15+
impl Api for RealApi {
16+
type Argument = PathBuf;
17+
type RealPath = PathBuf;
18+
type RealPathError = io::Error;
19+
20+
fn canonicalize(path: &Self::Argument) -> Result<Self::RealPath, Self::RealPathError> {
21+
canonicalize(path)
22+
}
23+
24+
fn starts_with(a: &Self::RealPath, b: &Self::RealPath) -> bool {
25+
a.starts_with(b)
26+
}
27+
}
328

429
/// Hardlinks deduplication doesn't work properly if there are more than 1 paths pointing to
530
/// the same tree or if a path points to a subtree of another path. Therefore, we must find
631
/// and remove such duplications before they cause problem.
7-
pub fn deduplicate_arguments<'a, Argument, Canonicalize, StartsWith, RealPath, CanonicalizeError>(
8-
arguments: &'a mut Vec<Argument>,
9-
canonicalize: Canonicalize,
10-
starts_with: StartsWith,
11-
) where
12-
Canonicalize: for<'r> FnMut(&Argument) -> Result<RealPath, CanonicalizeError>,
13-
StartsWith: for<'r> FnMut(&'r RealPath, &'r RealPath) -> bool,
14-
RealPath: Eq,
15-
{
16-
let to_remove = find_argument_duplications_to_remove(arguments, canonicalize, starts_with);
32+
pub fn deduplicate_arguments<Api: self::Api>(arguments: &mut Vec<Api::Argument>) {
33+
let to_remove = find_argument_duplications_to_remove::<Api>(arguments);
1734
remove_items_from_vec_by_indices(arguments, &to_remove);
1835
}
1936

@@ -23,23 +40,10 @@ pub fn deduplicate_arguments<'a, Argument, Canonicalize, StartsWith, RealPath, C
2340
///
2441
/// Prefer keeping the first instance of the path over the later instances (returning the indices of
2542
/// the later instances).
26-
pub fn find_argument_duplications_to_remove<
27-
Argument,
28-
Canonicalize,
29-
StartsWith,
30-
RealPath,
31-
CanonicalizeError,
32-
>(
33-
arguments: &[Argument],
34-
canonicalize: Canonicalize,
35-
mut starts_with: StartsWith,
36-
) -> HashSet<usize>
37-
where
38-
Canonicalize: for<'r> FnMut(&Argument) -> Result<RealPath, CanonicalizeError>,
39-
StartsWith: for<'r> FnMut(&'r RealPath, &'r RealPath) -> bool,
40-
RealPath: Eq,
41-
{
42-
let real_paths: Vec<_> = arguments.iter().map(canonicalize).collect();
43+
pub fn find_argument_duplications_to_remove<Api: self::Api>(
44+
arguments: &[Api::Argument],
45+
) -> HashSet<usize> {
46+
let real_paths: Vec<_> = arguments.iter().map(Api::canonicalize).collect();
4347
assert_eq!(arguments.len(), real_paths.len());
4448

4549
let mut to_remove = HashSet::new();
@@ -53,13 +57,13 @@ where
5357
}
5458

5559
// `left` starts with `right` means `left` is subtree of `right`, remove `left`
56-
if starts_with(left, right) {
60+
if Api::starts_with(left, right) {
5761
to_remove.insert(left_index);
5862
continue;
5963
}
6064

6165
// `right` starts with `left` means `right` is subtree of `left`, remove `right`
62-
if starts_with(right, left) {
66+
if Api::starts_with(right, left) {
6367
to_remove.insert(right_index);
6468
continue;
6569
}

0 commit comments

Comments
 (0)