Skip to content

Commit 0552b2f

Browse files
committed
replacing implementation with fast_code_owners
1 parent 8557b6d commit 0552b2f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+533
-3912
lines changed

Cargo.lock

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

README.md

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,6 @@ This defaults `**/`, which makes it look for `package.json` files across your ap
105105
js_package_paths: []
106106
```
107107

108-
### Custom Ownership
109-
To enable custom ownership, you can inject your own custom classes into `code_ownership`.
110-
To do this, first create a class that adheres to the `CodeOwnership::Mapper` and/or `CodeOwnership::Validator` interface.
111-
Then, in `config/code_ownership.yml`, you can require that file:
112-
```yml
113-
require:
114-
- ./lib/my_extension.rb
115-
```
116-
117-
Now, `bin/codeownership validate` will automatically include your new mapper and/or validator. See [`spec/lib/code_ownership/private/extension_loader_spec.rb](spec/lib/code_ownership/private/extension_loader_spec.rb) for an example of what this looks like.
118-
119108
## Usage: Reading CodeOwnership
120109
### `for_file`
121110
`CodeOwnership.for_file`, given a relative path to a file returns a `CodeTeams::Team` if there is a team that owns the file, `nil` otherwise.

ext/code_ownership/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ magnus = { version = "0.7.1" }
1818
serde = { version = "1.0.219", features = ["derive"] }
1919
serde_magnus = "0.9.0"
2020
# codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.2.4" }
21-
codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", branch = "ph.fast-file-check" }
21+
codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", branch = "ph.blazing-fast-for-file" }
2222

2323
[dev-dependencies]
24-
rb-sys = { version = "0.9.111", features = [
24+
rb-sys = { version = "0.9.117", features = [
2525
"link-ruby",
2626
"bindgen-rbimpls",
2727
"bindgen-deprecated-types",

ext/code_ownership/src/lib.rs

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ pub struct Team {
1111
pub team_config_yml: String,
1212
}
1313

14+
fn for_team(team_name: String) -> Result<Value, Error> {
15+
let run_config = build_run_config();
16+
let team = runner::for_team(&run_config, &team_name);
17+
validate_result(&team)
18+
}
19+
1420
fn for_file(file_path: String) -> Result<Option<Value>, Error> {
1521
let run_config = build_run_config();
1622

@@ -35,12 +41,11 @@ fn validate() -> Result<Value, Error> {
3541
let run_config = build_run_config();
3642
let run_result = runner::validate(&run_config, vec![]);
3743
validate_result(&run_result)
38-
3944
}
4045

41-
fn generate_and_validate() -> Result<Value, Error> {
46+
fn generate_and_validate(skip_stage: bool) -> Result<Value, Error> {
4247
let run_config = build_run_config();
43-
let run_result = runner::generate_and_validate(&run_config, vec![]);
48+
let run_result = runner::generate_and_validate(&run_config, vec![], skip_stage);
4449
validate_result(&run_result)
4550
}
4651

@@ -60,7 +65,6 @@ fn validate_result(run_result: &runner::RunResult) -> Result<Value, Error> {
6065
Ok(serialized)
6166
}
6267
}
63-
6468
fn build_run_config() -> RunConfig {
6569
let project_root = match env::current_dir() {
6670
Ok(path) => path,
@@ -81,8 +85,44 @@ fn build_run_config() -> RunConfig {
8185
fn init(ruby: &Ruby) -> Result<(), Error> {
8286
let module = ruby.define_module("RustCodeOwners")?;
8387
module.define_singleton_method("for_file", function!(for_file, 1))?;
84-
module.define_singleton_method("generate_and_validate", function!(generate_and_validate, 0))?;
88+
module.define_singleton_method("generate_and_validate", function!(generate_and_validate, 1))?;
8589
module.define_singleton_method("validate", function!(validate, 0))?;
90+
module.define_singleton_method("for_team", function!(for_team, 1))?;
8691

8792
Ok(())
8893
}
94+
95+
#[cfg(test)]
96+
mod tests {
97+
use std::env::set_current_dir;
98+
99+
use super::*;
100+
101+
#[test]
102+
#[should_panic]
103+
fn test_for_file_with_invalid_path() {
104+
// panics because not called from a ruby thread
105+
let _result = for_file("invalid/path".to_string());
106+
}
107+
108+
#[test]
109+
fn test_for_file() {
110+
let argv: [*mut c_char; 0] = [];
111+
let argv = argv.as_ptr();
112+
let mut argc = 1;
113+
114+
let manifest_dir = env!("CARGO_MANIFEST_DIR");
115+
let fixture_path = PathBuf::from(manifest_dir).join("tests/fixtures/valid_project");
116+
let _ = set_current_dir(&fixture_path).unwrap();
117+
118+
unsafe {
119+
rb_sys::ruby_sysinit(&mut argc, argv as _);
120+
rb_sys::ruby_init();
121+
122+
init();
123+
let result = generate_and_validate();
124+
assert!(result.is_ok());
125+
rb_sys::ruby_cleanup(0);
126+
}
127+
}
128+
}

lib/code_ownership.rb

Lines changed: 16 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@
88
require 'json'
99
require 'packs-specification'
1010
require 'code_ownership/version'
11-
require 'code_ownership/mapper'
12-
require 'code_ownership/validator'
13-
require 'code_ownership/private'
11+
require 'code_ownership/file_path_finder'
12+
require 'code_ownership/file_path_team_cache'
13+
require 'code_ownership/team_finder'
1414
require 'code_ownership/cli'
15-
require 'code_ownership/configuration'
1615

1716
begin
1817
RUBY_VERSION =~ /(\d+\.\d+)/
@@ -22,7 +21,7 @@
2221
end
2322

2423
if defined?(Packwerk)
25-
require 'code_ownership/private/permit_pack_owner_top_level_key'
24+
require 'code_ownership/permit_pack_owner_top_level_key'
2625
end
2726

2827
module CodeOwnership
@@ -36,50 +35,13 @@ module CodeOwnership
3635

3736
sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
3837
def for_file(file)
39-
@for_file ||= T.let(@for_file, T.nilable(T::Hash[String, T.nilable(CodeTeams::Team)]))
40-
@for_file ||= {}
41-
42-
return nil if file.start_with?('./')
43-
return @for_file[file] if @for_file.key?(file)
44-
45-
Private.load_configuration!
46-
47-
owner = T.let(nil, T.nilable(CodeTeams::Team))
48-
49-
Mapper.all.each do |mapper|
50-
owner = mapper.map_file_to_owner(file)
51-
break if owner # TODO: what if there are multiple owners? Should we respond with an error instead of the first match?
52-
end
53-
54-
@for_file[file] = owner
38+
TeamFinder.for_file(file)
5539
end
5640

5741
sig { params(team: T.any(CodeTeams::Team, String)).returns(String) }
5842
def for_team(team)
5943
team = T.must(CodeTeams.find(team)) if team.is_a?(String)
60-
ownership_information = T.let([], T::Array[String])
61-
62-
ownership_information << "# Code Ownership Report for `#{team.name}` Team"
63-
64-
Private.glob_cache.raw_cache_contents.each do |mapper_description, glob_to_owning_team_map|
65-
ownership_information << "## #{mapper_description}"
66-
ownership_for_mapper = []
67-
glob_to_owning_team_map.each do |glob, owning_team|
68-
next if owning_team != team
69-
70-
ownership_for_mapper << "- #{glob}"
71-
end
72-
73-
if ownership_for_mapper.empty?
74-
ownership_information << 'This team owns nothing in this category.'
75-
else
76-
ownership_information += ownership_for_mapper.sort
77-
end
78-
79-
ownership_information << ''
80-
end
81-
82-
ownership_information.join("\n")
44+
::RustCodeOwners.for_team(team.name)
8345
end
8446

8547
class InvalidCodeOwnershipConfigurationError < StandardError
@@ -102,92 +64,35 @@ def validate!(
10264
stage_changes: true,
10365
files: nil
10466
)
105-
Private.validate!(autocorrect: autocorrect, stage_changes: stage_changes)
67+
if autocorrect
68+
::RustCodeOwners.generate_and_validate(!stage_changes)
69+
else
70+
::RustCodeOwners.validate
71+
end
10672
end
10773

10874
# Given a backtrace from either `Exception#backtrace` or `caller`, find the
10975
# first line that corresponds to a file with assigned ownership
11076
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable(::CodeTeams::Team)) }
11177
def for_backtrace(backtrace, excluded_teams: [])
112-
first_owned_file_for_backtrace(backtrace, excluded_teams: excluded_teams)&.first
78+
TeamFinder.for_backtrace(backtrace, excluded_teams: excluded_teams)
11379
end
11480

11581
# Given a backtrace from either `Exception#backtrace` or `caller`, find the
11682
# first owned file in it, useful for figuring out which file is being blamed.
11783
sig { params(backtrace: T.nilable(T::Array[String]), excluded_teams: T::Array[::CodeTeams::Team]).returns(T.nilable([::CodeTeams::Team, String])) }
11884
def first_owned_file_for_backtrace(backtrace, excluded_teams: [])
119-
backtrace_with_ownership(backtrace).each do |(team, file)|
120-
if team && !excluded_teams.include?(team)
121-
return [team, file]
122-
end
123-
end
124-
125-
nil
126-
end
127-
128-
sig { params(backtrace: T.nilable(T::Array[String])).returns(T::Enumerable[[T.nilable(::CodeTeams::Team), String]]) }
129-
def backtrace_with_ownership(backtrace)
130-
return [] unless backtrace
131-
132-
# The pattern for a backtrace hasn't changed in forever and is considered
133-
# stable: https://github.com/ruby/ruby/blob/trunk/vm_backtrace.c#L303-L317
134-
#
135-
# This pattern matches a line like the following:
136-
#
137-
# ./app/controllers/some_controller.rb:43:in `block (3 levels) in create'
138-
#
139-
backtrace_line = if RUBY_VERSION >= '3.4.0'
140-
%r{\A(#{Pathname.pwd}/|\./)?
141-
(?<file>.+) # Matches 'app/controllers/some_controller.rb'
142-
:
143-
(?<line>\d+) # Matches '43'
144-
:in\s
145-
'(?<function>.*)' # Matches "`block (3 levels) in create'"
146-
\z}x
147-
else
148-
%r{\A(#{Pathname.pwd}/|\./)?
149-
(?<file>.+) # Matches 'app/controllers/some_controller.rb'
150-
:
151-
(?<line>\d+) # Matches '43'
152-
:in\s
153-
`(?<function>.*)' # Matches "`block (3 levels) in create'"
154-
\z}x
155-
end
156-
157-
backtrace.lazy.filter_map do |line|
158-
match = line.match(backtrace_line)
159-
next unless match
160-
161-
file = T.must(match[:file])
162-
163-
[
164-
CodeOwnership.for_file(file),
165-
file
166-
]
167-
end
85+
TeamFinder.first_owned_file_for_backtrace(backtrace, excluded_teams: excluded_teams)
16886
end
169-
private_class_method(:backtrace_with_ownership)
17087

17188
sig { params(klass: T.nilable(T.any(T::Class[T.anything], Module))).returns(T.nilable(::CodeTeams::Team)) }
17289
def for_class(klass)
173-
@memoized_values ||= T.let(@memoized_values, T.nilable(T::Hash[String, T.nilable(::CodeTeams::Team)]))
174-
@memoized_values ||= {}
175-
# We use key because the memoized value could be `nil`
176-
if @memoized_values.key?(klass.to_s)
177-
@memoized_values[klass.to_s]
178-
else
179-
path = Private.path_from_klass(klass)
180-
return nil if path.nil?
181-
182-
value_to_memoize = for_file(path)
183-
@memoized_values[klass.to_s] = value_to_memoize
184-
value_to_memoize
185-
end
90+
TeamFinder.for_class(klass)
18691
end
18792

18893
sig { params(package: Packs::Pack).returns(T.nilable(::CodeTeams::Team)) }
18994
def for_package(package)
190-
Private::OwnershipMappers::PackageOwnership.new.owner_for_package(package)
95+
TeamFinder.for_package(package)
19196
end
19297

19398
# Generally, you should not ever need to do this, because once your ruby process loads, cached content should not change.
@@ -196,14 +101,6 @@ def for_package(package)
196101
# has different ownership and tracked files.
197102
sig { void }
198103
def self.bust_caches!
199-
@for_file = nil
200-
@memoized_values = nil
201-
Private.bust_caches!
202-
Mapper.all.each(&:bust_caches!)
203-
end
204-
205-
sig { returns(Configuration) }
206-
def self.configuration
207-
Private.configuration
104+
FilePathTeamCache.bust_cache!
208105
end
209106
end

0 commit comments

Comments
 (0)