Skip to content

Commit 5500ab3

Browse files
author
Alex Evanczuk
authored
Use Codeowners file as incremental cache during validations (#53)
* Allow codeowners to create globcache for incremental updates * Merge map_files_to_owners and codeowners_lines_to_owners as globs_to_owner * unskip test * bump version
1 parent 38760c5 commit 5500ab3

File tree

14 files changed

+143
-87
lines changed

14 files changed

+143
-87
lines changed

‎Gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: .
33
specs:
4-
code_ownership (1.32.7)
4+
code_ownership (1.32.8)
55
code_teams (~> 1.0)
66
packs
77
sorbet-runtime

‎code_ownership.gemspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Gem::Specification.new do |spec|
22
spec.name = "code_ownership"
3-
spec.version = '1.32.7'
3+
spec.version = '1.32.8'
44
spec.authors = ['Gusto Engineers']
55
spec.email = ['[email protected]']
66
spec.summary = 'A gem to help engineering teams declare ownership of code'

‎lib/code_ownership.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ module CodeOwnership
1919
extend T::Helpers
2020

2121
requires_ancestor { Kernel }
22+
GlobsToOwningTeamMap = T.type_alias { T::Hash[String, CodeTeams::Team] }
2223

2324
sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
2425
def for_file(file)
@@ -50,7 +51,7 @@ def for_team(team)
5051
ownership_information << "# Code Ownership Report for `#{team.name}` Team"
5152
Mapper.all.each do |mapper|
5253
ownership_information << "## #{mapper.description}"
53-
codeowners_lines = mapper.codeowners_lines_to_owners
54+
codeowners_lines = mapper.globs_to_owner(Private.tracked_files)
5455
ownership_for_mapper = []
5556
codeowners_lines.each do |line, team_for_line|
5657
next if team_for_line.nil?

‎lib/code_ownership/mapper.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,18 @@ def map_file_to_owner(file)
4040
#
4141
sig do
4242
abstract.params(files: T::Array[String]).
43-
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
43+
returns(T::Hash[String, ::CodeTeams::Team])
4444
end
45-
def map_files_to_owners(files)
45+
def globs_to_owner(files)
4646
end
4747

48+
#
49+
# This should be fast when run with MANY files
50+
#
4851
sig do
49-
abstract.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
52+
abstract.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
5053
end
51-
def codeowners_lines_to_owners
54+
def update_cache(cache, files)
5255
end
5356

5457
sig { abstract.returns(String) }
@@ -64,7 +67,7 @@ def self.to_glob_cache
6467
glob_to_owner_map_by_mapper_description = {}
6568

6669
Mapper.all.each do |mapper|
67-
mapped_files = mapper.codeowners_lines_to_owners
70+
mapped_files = mapper.globs_to_owner(Private.tracked_files)
6871
mapped_files.each do |glob, owner|
6972
next if owner.nil?
7073
glob_to_owner_map_by_mapper_description[mapper.description] ||= {}

‎lib/code_ownership/private.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ def self.bust_caches!
4444

4545
sig { params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).void }
4646
def self.validate!(files:, autocorrect: true, stage_changes: true)
47+
CodeownersFile.update_cache!(files) if CodeownersFile.use_codeowners_cache?
48+
4749
errors = Validator.all.flat_map do |validator|
4850
validator.validation_errors(
4951
files: files,
@@ -92,7 +94,13 @@ def self.find_team!(team_name, location_of_reference)
9294
sig { returns(GlobCache) }
9395
def self.glob_cache
9496
@glob_cache ||= T.let(@glob_cache, T.nilable(GlobCache))
95-
@glob_cache ||= Mapper.to_glob_cache
97+
@glob_cache ||= begin
98+
if CodeownersFile.use_codeowners_cache?
99+
CodeownersFile.to_glob_cache
100+
else
101+
Mapper.to_glob_cache
102+
end
103+
end
96104
end
97105
end
98106

‎lib/code_ownership/private/codeowners_file.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,58 @@ def self.write!
9090
def self.path
9191
Pathname.pwd.join('.github/CODEOWNERS')
9292
end
93+
94+
sig { params(files: T::Array[String]).void }
95+
def self.update_cache!(files)
96+
cache = Private.glob_cache
97+
# Each mapper returns a new copy of the cache subset related to that mapper,
98+
# which is then stored back into the cache.
99+
Mapper.all.each do |mapper|
100+
existing_cache = cache.raw_cache_contents.fetch(mapper.description, {})
101+
updated_cache = mapper.update_cache(existing_cache, files)
102+
cache.raw_cache_contents[mapper.description] = updated_cache
103+
end
104+
end
105+
106+
sig { returns(T::Boolean) }
107+
def self.use_codeowners_cache?
108+
CodeownersFile.path.exist? && !Private.configuration.skip_codeowners_validation
109+
end
110+
111+
sig { returns(GlobCache) }
112+
def self.to_glob_cache
113+
github_team_to_code_team_map = T.let({}, T::Hash[String, CodeTeams::Team])
114+
CodeTeams.all.each do |team|
115+
github_team = TeamPlugins::Github.for(team).github.team
116+
github_team_to_code_team_map[github_team] = team
117+
end
118+
raw_cache_contents = T.let({}, GlobCache::CacheShape)
119+
current_mapper = T.let(nil, T.nilable(String))
120+
mapper_descriptions = Set.new(Mapper.all.map(&:description))
121+
122+
path.readlines.each do |line|
123+
line_with_no_comment = line.chomp.gsub("# ", "")
124+
if mapper_descriptions.include?(line_with_no_comment)
125+
current_mapper = line_with_no_comment
126+
else
127+
next if current_mapper.nil?
128+
next if line.chomp == ""
129+
# The codeowners file stores paths relative to the root of directory
130+
# Since a `/` means root of the file system from the perspective of ruby,
131+
# we remove that beginning slash so we can correctly glob the files out.
132+
normalized_line = line.gsub(/^# /, '').gsub(/^\//, '')
133+
split_line = normalized_line.split
134+
# Most lines will be in the format: /path/to/file my-github-team
135+
# This will skip over lines that are not of the correct form
136+
next if split_line.count > 2
137+
entry, github_team = split_line
138+
raw_cache_contents[current_mapper] ||= {}
139+
raw_cache_contents.fetch(current_mapper)[T.must(entry)] = github_team_to_code_team_map.fetch(T.must(github_team))
140+
end
141+
end
142+
143+
GlobCache.new(raw_cache_contents)
144+
end
93145
end
94146
end
95147
end

‎lib/code_ownership/private/glob_cache.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ class GlobCache
77
extend T::Sig
88

99
MapperDescription = T.type_alias { String }
10-
GlobsByMapper = T.type_alias { T::Hash[String, CodeTeams::Team] }
1110

1211
CacheShape = T.type_alias do
1312
T::Hash[
1413
MapperDescription,
15-
GlobsByMapper
14+
GlobsToOwningTeamMap
1615
]
1716
end
1817

‎lib/code_ownership/private/ownership_mappers/file_annotations.rb

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class FileAnnotations
1818
extend T::Sig
1919
include Mapper
2020

21-
@@map_files_to_owners = T.let({}, T.nilable(T::Hash[String, T.nilable(::CodeTeams::Team)])) # rubocop:disable Style/ClassVars
21+
@@map_files_to_owners = T.let({}, T.nilable(T::Hash[String, ::CodeTeams::Team])) # rubocop:disable Style/ClassVars
2222

2323
TEAM_PATTERN = T.let(/\A(?:#|\/\/) @team (?<team>.*)\Z/.freeze, Regexp)
2424

@@ -33,9 +33,9 @@ def map_file_to_owner(file)
3333
sig do
3434
override.
3535
params(files: T::Array[String]).
36-
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
36+
returns(T::Hash[String, ::CodeTeams::Team])
3737
end
38-
def map_files_to_owners(files)
38+
def globs_to_owner(files)
3939
return @@map_files_to_owners if @@map_files_to_owners&.keys && @@map_files_to_owners.keys.count > 0
4040

4141
@@map_files_to_owners = files.each_with_object({}) do |filename_relative_to_root, mapping| # rubocop:disable Style/ClassVars
@@ -46,6 +46,24 @@ def map_files_to_owners(files)
4646
end
4747
end
4848

49+
sig do
50+
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
51+
end
52+
def update_cache(cache, files)
53+
cache.merge!(globs_to_owner(files))
54+
55+
# TODO: Make `tracked_files` return a set
56+
fileset = Set.new(Private.tracked_files)
57+
invalid_files = cache.keys.select do |file|
58+
!fileset.include?(file)
59+
end
60+
invalid_files.each do |invalid_file|
61+
cache.delete(invalid_file)
62+
end
63+
64+
cache
65+
end
66+
4967
sig { params(filename: String).returns(T.nilable(CodeTeams::Team)) }
5068
def file_annotation_based_owner(filename)
5169
# If for a directory is named with an ownable extension, we need to skip
@@ -96,14 +114,6 @@ def remove_file_annotation!(filename)
96114
end
97115
end
98116

99-
sig do
100-
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
101-
end
102-
def codeowners_lines_to_owners
103-
@@map_files_to_owners = nil # rubocop:disable Style/ClassVars
104-
map_files_to_owners(Private.tracked_files)
105-
end
106-
107117
sig { override.returns(String) }
108118
def description
109119
'Annotations at the top of file'

‎lib/code_ownership/private/ownership_mappers/js_package_ownership.rb

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,10 @@ def map_file_to_owner(file)
2424
end
2525

2626
sig do
27-
override.
28-
params(files: T::Array[String]).
29-
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
27+
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
3028
end
31-
def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
32-
ParseJsPackages.all.each_with_object({}) do |package, res|
33-
owner = owner_for_package(package)
34-
next if owner.nil?
35-
36-
glob = package.directory.join('**/**').to_s
37-
Dir.glob(glob).each do |path|
38-
res[path] = owner
39-
end
40-
end
29+
def update_cache(cache, files)
30+
globs_to_owner(files)
4131
end
4232

4333
#
@@ -49,9 +39,10 @@ def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
4939
# subset of files, but rather we want code ownership for all files.
5040
#
5141
sig do
52-
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
42+
override.params(files: T::Array[String]).
43+
returns(T::Hash[String, ::CodeTeams::Team])
5344
end
54-
def codeowners_lines_to_owners
45+
def globs_to_owner(files)
5546
ParseJsPackages.all.each_with_object({}) do |package, res|
5647
owner = owner_for_package(package)
5748
next if owner.nil?

‎lib/code_ownership/private/ownership_mappers/package_ownership.rb

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,6 @@ def map_file_to_owner(file)
2323
owner_for_package(package)
2424
end
2525

26-
sig do
27-
override.
28-
params(files: T::Array[String]).
29-
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
30-
end
31-
def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
32-
Packs.all.each_with_object({}) do |package, res|
33-
owner = owner_for_package(package)
34-
next if owner.nil?
35-
36-
glob = package.relative_path.join('**/**').to_s
37-
Dir.glob(glob).each do |path|
38-
res[path] = owner
39-
end
40-
end
41-
end
42-
4326
#
4427
# Package ownership ignores the passed in files when generating code owners lines.
4528
# This is because Package ownership knows that the fastest way to find code owners for package based ownership
@@ -49,9 +32,10 @@ def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
4932
# subset of files, but rather we want code ownership for all files.
5033
#
5134
sig do
52-
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
35+
override.params(files: T::Array[String]).
36+
returns(T::Hash[String, ::CodeTeams::Team])
5337
end
54-
def codeowners_lines_to_owners
38+
def globs_to_owner(files)
5539
Packs.all.each_with_object({}) do |package, res|
5640
owner = owner_for_package(package)
5741
next if owner.nil?
@@ -65,6 +49,13 @@ def description
6549
'Owner metadata key in package.yml'
6650
end
6751

52+
sig do
53+
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
54+
end
55+
def update_cache(cache, files)
56+
globs_to_owner(files)
57+
end
58+
6859
sig { params(package: Packs::Pack).returns(T.nilable(CodeTeams::Team)) }
6960
def owner_for_package(package)
7061
raw_owner_value = package.metadata['owner']

0 commit comments

Comments
 (0)