Skip to content

Commit b15333f

Browse files
perryqhperryqh
committed
much faster
1 parent 831de00 commit b15333f

File tree

3 files changed

+64
-88
lines changed

3 files changed

+64
-88
lines changed

‎src/ownership.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ use std::{
99
};
1010
use tracing::{info, instrument};
1111

12+
pub(crate) mod codeowners_file_parser;
1213
mod file_generator;
1314
mod file_owner_finder;
1415
pub mod for_file_fast;
1516
pub(crate) mod mapper;
16-
pub(crate) mod codeowners_file_parser;
1717
mod validator;
1818

1919
use crate::{
@@ -24,9 +24,9 @@ use crate::{
2424
pub use validator::Errors as ValidatorErrors;
2525

2626
use self::{
27+
codeowners_file_parser::parse_for_team,
2728
file_generator::FileGenerator,
2829
mapper::{JavascriptPackageMapper, Mapper, RubyPackageMapper, TeamFileMapper, TeamGemMapper, TeamGlobMapper, TeamYmlMapper},
29-
codeowners_file_parser::parse_for_team,
3030
validator::Validator,
3131
};
3232

‎src/ownership/codeowners_file_parser.rs‎

Lines changed: 33 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use crate::{
44
};
55
use fast_glob::glob_match;
66
use memoize::memoize;
7-
use regex::Regex;
87
use rayon::prelude::*;
8+
use regex::Regex;
99
use std::{
1010
collections::HashMap,
1111
error::Error,
@@ -24,63 +24,50 @@ pub struct Parser {
2424

2525
impl Parser {
2626
pub fn teams_from_files_paths(&self, file_paths: &[PathBuf]) -> Result<HashMap<String, Team>, Box<dyn Error>> {
27-
let mut file_inputs: Vec<(String, String)> = Vec::with_capacity(file_paths.len());
28-
for path in file_paths {
29-
let file_path_str = path
30-
.to_str()
31-
.ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid file path"))?;
32-
let key = file_path_str.to_string();
33-
let slash_prefixed = if file_path_str.starts_with('/') {
34-
file_path_str.to_string()
35-
} else {
36-
format!("/{}", file_path_str)
37-
};
38-
file_inputs.push((key, slash_prefixed));
39-
}
27+
let file_inputs: Vec<(String, String)> = file_paths
28+
.iter()
29+
.map(|path| {
30+
let file_path_str = path
31+
.to_str()
32+
.ok_or(IoError::new(std::io::ErrorKind::InvalidInput, "Invalid file path"))?;
33+
let original = file_path_str.to_string();
34+
let prefixed = if file_path_str.starts_with('/') {
35+
original.clone()
36+
} else {
37+
format!("/{}", file_path_str)
38+
};
39+
Ok((original, prefixed))
40+
})
41+
.collect::<Result<Vec<_>, IoError>>()?;
4042

4143
if file_inputs.is_empty() {
4244
return Ok(HashMap::new());
4345
}
4446

45-
let codeowners_lines_in_priority = build_codeowners_lines_in_priority(
46-
self.codeowners_file_path.to_string_lossy().into_owned(),
47-
);
48-
// Pre-parse lines once to avoid repeated split and to handle malformed lines early
49-
let codeowners_entries: Vec<(String, String)> = codeowners_lines_in_priority
50-
.iter()
51-
.map(|line| {
52-
line
53-
.split_once(' ')
54-
.map(|(glob, team_name)| (glob.to_string(), team_name.to_string()))
55-
.ok_or_else(|| IoError::new(std::io::ErrorKind::InvalidInput, "Invalid line"))
56-
})
57-
.collect::<Result<_, IoError>>()
58-
.map_err(|e| Box::new(e) as Box<dyn Error>)?;
47+
let codeowners_entries: Vec<(String, String)> =
48+
build_codeowners_lines_in_priority(self.codeowners_file_path.to_string_lossy().into_owned())
49+
.iter()
50+
.map(|line| {
51+
line.split_once(' ')
52+
.map(|(glob, team_name)| (glob.to_string(), team_name.to_string()))
53+
.ok_or_else(|| IoError::new(std::io::ErrorKind::InvalidInput, "Invalid line"))
54+
})
55+
.collect::<Result<_, IoError>>()
56+
.map_err(|e| Box::new(e) as Box<dyn Error>)?;
57+
5958
let teams_by_name = teams_by_github_team_name(self.absolute_team_files_globs());
6059

61-
// Parallelize across files: for each file, scan lines in priority order
62-
let result_pairs: Vec<(String, Team)> = file_inputs
60+
let result: HashMap<String, Team> = file_inputs
6361
.par_iter()
6462
.filter_map(|(key, prefixed)| {
65-
for (glob, team_name) in &codeowners_entries {
66-
if glob_match(glob, prefixed) {
67-
// Stop at first match (highest priority). If team missing, treat as unowned.
68-
if let Some(team) = teams_by_name.get(team_name) {
69-
return Some((key.clone(), team.clone()));
70-
} else {
71-
return None;
72-
}
73-
}
74-
}
75-
None
63+
codeowners_entries
64+
.iter()
65+
.find(|(glob, _)| glob_match(glob, prefixed))
66+
.and_then(|(_, team_name)| teams_by_name.get(team_name))
67+
.map(|team| (key.clone(), team.clone()))
7668
})
7769
.collect();
7870

79-
let mut result: HashMap<String, Team> = HashMap::with_capacity(result_pairs.len());
80-
for (k, t) in result_pairs {
81-
result.insert(k, t);
82-
}
83-
8471
Ok(result)
8572
}
8673

‎src/runner.rs‎

Lines changed: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -362,10 +362,8 @@ fn for_file_optimized(run_config: &RunConfig, file_path: &str) -> RunResult {
362362
mod tests {
363363
use tempfile::tempdir;
364364

365-
use crate::{common_test, ownership::mapper::Source};
366-
use ignore::{DirEntry, WalkBuilder, WalkParallel, WalkState};
367-
368365
use super::*;
366+
use crate::{common_test, ownership::mapper::Source};
369367

370368
#[test]
371369
fn test_version() {
@@ -421,42 +419,33 @@ mod tests {
421419

422420
#[test]
423421
fn test_teams_for_files_from_codeowners() {
424-
let project_root = Path::new("/Users/perryhertler/workspace/zenpayroll");
425-
let codeowners_file_path = project_root.join(".github/CODEOWNERS");
426-
let config_path = project_root.join("config/code_ownership.yml");
427-
let run_config = RunConfig {
428-
project_root: project_root.to_path_buf(),
429-
codeowners_file_path: codeowners_file_path.to_path_buf(),
430-
config_path: config_path.to_path_buf(),
431-
no_cache: false,
432-
};
433-
434-
// Collect all files in packs and frontend directories recursively
435-
let mut file_paths = Vec::new();
436-
for dir in ["packs", "frontend"] {
437-
let dir_path = project_root.join(dir);
438-
if dir_path.exists() && dir_path.is_dir() {
439-
for entry in WalkBuilder::new(&dir_path)
440-
.filter_entry(|e| {
441-
let name = e.file_name().to_str().unwrap_or("");
442-
!(name == "node_modules" || name == "dist" || name == ".git")
443-
})
444-
.build()
445-
.filter_map(|e| e.ok())
446-
.filter(|e| e.file_type().map(|t| t.is_file()).unwrap_or(false))
447-
.filter_map(|e| e.path().strip_prefix(project_root).ok().map(|p| p.to_string_lossy().to_string()))
448-
{
449-
file_paths.push(entry);
450-
}
451-
}
452-
}
453-
454-
let start_time = std::time::Instant::now();
455-
let teams = teams_for_files_from_codeowners(&run_config, &file_paths).unwrap();
456-
let end_time = std::time::Instant::now();
457-
println!("Time taken: {:?}", end_time.duration_since(start_time));
458-
println!("Teams: {:?}", teams);
459-
assert_eq!(teams.len(), 1);
460-
422+
let project_root = Path::new("tests/fixtures/valid_project");
423+
let file_paths = [
424+
"javascript/packages/items/item.ts",
425+
"config/teams/payroll.yml",
426+
"ruby/app/models/bank_account.rb",
427+
"made/up/file.rb",
428+
"ruby/ignored_files/git_ignored.rb",
429+
];
430+
let run_config = RunConfig {
431+
project_root: project_root.to_path_buf(),
432+
codeowners_file_path: project_root.join(".github/CODEOWNERS").to_path_buf(),
433+
config_path: project_root.join("config/code_ownership.yml").to_path_buf(),
434+
no_cache: false,
435+
};
436+
let teams =
437+
teams_for_files_from_codeowners(&run_config, &file_paths.iter().map(|s| s.to_string()).collect::<Vec<String>>()).unwrap();
438+
assert_eq!(teams.len(), 3);
439+
assert_eq!(
440+
teams.get("javascript/packages/items/item.ts").map(|t| t.name.as_str()),
441+
Some("Payroll")
442+
);
443+
assert_eq!(teams.get("config/teams/payroll.yml").map(|t| t.name.as_str()), Some("Payroll"));
444+
assert_eq!(
445+
teams.get("ruby/app/models/bank_account.rb").map(|t| t.name.as_str()),
446+
Some("Payments")
447+
);
448+
assert_eq!(teams.get("made/up/file.rb").map(|t| t.name.as_str()), None);
449+
assert_eq!(teams.get("ruby/ignored_files/git_ignored.rb").map(|t| t.name.as_str()), None);
461450
}
462451
}

0 commit comments

Comments
 (0)