Skip to content

Commit

Permalink
Use Codeowners file as incremental cache during validations (#53)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Alex Evanczuk authored Apr 12, 2023
1 parent 38760c5 commit 5500ab3
Show file tree
Hide file tree
Showing 14 changed files with 143 additions and 87 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
code_ownership (1.32.7)
code_ownership (1.32.8)
code_teams (~> 1.0)
packs
sorbet-runtime
Expand Down
2 changes: 1 addition & 1 deletion code_ownership.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |spec|
spec.name = "code_ownership"
spec.version = '1.32.7'
spec.version = '1.32.8'
spec.authors = ['Gusto Engineers']
spec.email = ['[email protected]']
spec.summary = 'A gem to help engineering teams declare ownership of code'
Expand Down
3 changes: 2 additions & 1 deletion lib/code_ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module CodeOwnership
extend T::Helpers

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

sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
def for_file(file)
Expand Down Expand Up @@ -50,7 +51,7 @@ def for_team(team)
ownership_information << "# Code Ownership Report for `#{team.name}` Team"
Mapper.all.each do |mapper|
ownership_information << "## #{mapper.description}"
codeowners_lines = mapper.codeowners_lines_to_owners
codeowners_lines = mapper.globs_to_owner(Private.tracked_files)
ownership_for_mapper = []
codeowners_lines.each do |line, team_for_line|
next if team_for_line.nil?
Expand Down
13 changes: 8 additions & 5 deletions lib/code_ownership/mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,18 @@ def map_file_to_owner(file)
#
sig do
abstract.params(files: T::Array[String]).
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
returns(T::Hash[String, ::CodeTeams::Team])
end
def map_files_to_owners(files)
def globs_to_owner(files)
end

#
# This should be fast when run with MANY files
#
sig do
abstract.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
abstract.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
end
def codeowners_lines_to_owners
def update_cache(cache, files)
end

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

Mapper.all.each do |mapper|
mapped_files = mapper.codeowners_lines_to_owners
mapped_files = mapper.globs_to_owner(Private.tracked_files)
mapped_files.each do |glob, owner|
next if owner.nil?
glob_to_owner_map_by_mapper_description[mapper.description] ||= {}
Expand Down
10 changes: 9 additions & 1 deletion lib/code_ownership/private.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ def self.bust_caches!

sig { params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).void }
def self.validate!(files:, autocorrect: true, stage_changes: true)
CodeownersFile.update_cache!(files) if CodeownersFile.use_codeowners_cache?

errors = Validator.all.flat_map do |validator|
validator.validation_errors(
files: files,
Expand Down Expand Up @@ -92,7 +94,13 @@ def self.find_team!(team_name, location_of_reference)
sig { returns(GlobCache) }
def self.glob_cache
@glob_cache ||= T.let(@glob_cache, T.nilable(GlobCache))
@glob_cache ||= Mapper.to_glob_cache
@glob_cache ||= begin
if CodeownersFile.use_codeowners_cache?
CodeownersFile.to_glob_cache
else
Mapper.to_glob_cache
end
end
end
end

Expand Down
52 changes: 52 additions & 0 deletions lib/code_ownership/private/codeowners_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,58 @@ def self.write!
def self.path
Pathname.pwd.join('.github/CODEOWNERS')
end

sig { params(files: T::Array[String]).void }
def self.update_cache!(files)
cache = Private.glob_cache
# Each mapper returns a new copy of the cache subset related to that mapper,
# which is then stored back into the cache.
Mapper.all.each do |mapper|
existing_cache = cache.raw_cache_contents.fetch(mapper.description, {})
updated_cache = mapper.update_cache(existing_cache, files)
cache.raw_cache_contents[mapper.description] = updated_cache
end
end

sig { returns(T::Boolean) }
def self.use_codeowners_cache?
CodeownersFile.path.exist? && !Private.configuration.skip_codeowners_validation
end

sig { returns(GlobCache) }
def self.to_glob_cache
github_team_to_code_team_map = T.let({}, T::Hash[String, CodeTeams::Team])
CodeTeams.all.each do |team|
github_team = TeamPlugins::Github.for(team).github.team
github_team_to_code_team_map[github_team] = team
end
raw_cache_contents = T.let({}, GlobCache::CacheShape)
current_mapper = T.let(nil, T.nilable(String))
mapper_descriptions = Set.new(Mapper.all.map(&:description))

path.readlines.each do |line|
line_with_no_comment = line.chomp.gsub("# ", "")
if mapper_descriptions.include?(line_with_no_comment)
current_mapper = line_with_no_comment
else
next if current_mapper.nil?
next if line.chomp == ""
# The codeowners file stores paths relative to the root of directory
# Since a `/` means root of the file system from the perspective of ruby,
# we remove that beginning slash so we can correctly glob the files out.
normalized_line = line.gsub(/^# /, '').gsub(/^\//, '')
split_line = normalized_line.split
# Most lines will be in the format: /path/to/file my-github-team
# This will skip over lines that are not of the correct form
next if split_line.count > 2
entry, github_team = split_line
raw_cache_contents[current_mapper] ||= {}
raw_cache_contents.fetch(current_mapper)[T.must(entry)] = github_team_to_code_team_map.fetch(T.must(github_team))
end
end

GlobCache.new(raw_cache_contents)
end
end
end
end
3 changes: 1 addition & 2 deletions lib/code_ownership/private/glob_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ class GlobCache
extend T::Sig

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

CacheShape = T.type_alias do
T::Hash[
MapperDescription,
GlobsByMapper
GlobsToOwningTeamMap
]
end

Expand Down
32 changes: 21 additions & 11 deletions lib/code_ownership/private/ownership_mappers/file_annotations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class FileAnnotations
extend T::Sig
include Mapper

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

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

Expand All @@ -33,9 +33,9 @@ def map_file_to_owner(file)
sig do
override.
params(files: T::Array[String]).
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
returns(T::Hash[String, ::CodeTeams::Team])
end
def map_files_to_owners(files)
def globs_to_owner(files)
return @@map_files_to_owners if @@map_files_to_owners&.keys && @@map_files_to_owners.keys.count > 0

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

sig do
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
end
def update_cache(cache, files)
cache.merge!(globs_to_owner(files))

# TODO: Make `tracked_files` return a set
fileset = Set.new(Private.tracked_files)
invalid_files = cache.keys.select do |file|
!fileset.include?(file)
end
invalid_files.each do |invalid_file|
cache.delete(invalid_file)
end

cache
end

sig { params(filename: String).returns(T.nilable(CodeTeams::Team)) }
def file_annotation_based_owner(filename)
# If for a directory is named with an ownable extension, we need to skip
Expand Down Expand Up @@ -96,14 +114,6 @@ def remove_file_annotation!(filename)
end
end

sig do
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
end
def codeowners_lines_to_owners
@@map_files_to_owners = nil # rubocop:disable Style/ClassVars
map_files_to_owners(Private.tracked_files)
end

sig { override.returns(String) }
def description
'Annotations at the top of file'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,10 @@ def map_file_to_owner(file)
end

sig do
override.
params(files: T::Array[String]).
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
end
def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
ParseJsPackages.all.each_with_object({}) do |package, res|
owner = owner_for_package(package)
next if owner.nil?

glob = package.directory.join('**/**').to_s
Dir.glob(glob).each do |path|
res[path] = owner
end
end
def update_cache(cache, files)
globs_to_owner(files)
end

#
Expand All @@ -49,9 +39,10 @@ def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
# subset of files, but rather we want code ownership for all files.
#
sig do
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
override.params(files: T::Array[String]).
returns(T::Hash[String, ::CodeTeams::Team])
end
def codeowners_lines_to_owners
def globs_to_owner(files)
ParseJsPackages.all.each_with_object({}) do |package, res|
owner = owner_for_package(package)
next if owner.nil?
Expand Down
29 changes: 10 additions & 19 deletions lib/code_ownership/private/ownership_mappers/package_ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,6 @@ def map_file_to_owner(file)
owner_for_package(package)
end

sig do
override.
params(files: T::Array[String]).
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
end
def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
Packs.all.each_with_object({}) do |package, res|
owner = owner_for_package(package)
next if owner.nil?

glob = package.relative_path.join('**/**').to_s
Dir.glob(glob).each do |path|
res[path] = owner
end
end
end

#
# Package ownership ignores the passed in files when generating code owners lines.
# This is because Package ownership knows that the fastest way to find code owners for package based ownership
Expand All @@ -49,9 +32,10 @@ def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
# subset of files, but rather we want code ownership for all files.
#
sig do
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
override.params(files: T::Array[String]).
returns(T::Hash[String, ::CodeTeams::Team])
end
def codeowners_lines_to_owners
def globs_to_owner(files)
Packs.all.each_with_object({}) do |package, res|
owner = owner_for_package(package)
next if owner.nil?
Expand All @@ -65,6 +49,13 @@ def description
'Owner metadata key in package.yml'
end

sig do
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
end
def update_cache(cache, files)
globs_to_owner(files)
end

sig { params(package: Packs::Pack).returns(T.nilable(CodeTeams::Team)) }
def owner_for_package(package)
raw_owner_value = package.metadata['owner']
Expand Down
21 changes: 14 additions & 7 deletions lib/code_ownership/private/ownership_mappers/team_globs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ class TeamGlobs
include Mapper
include Validator

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

sig do
override.
params(files: T::Array[String]).
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
params(files: T::Array[String]).
returns(T::Hash[String, ::CodeTeams::Team])
end
def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
return @@map_files_to_owners if @@map_files_to_owners&.keys && @@map_files_to_owners.keys.count > 0
Expand Down Expand Up @@ -93,9 +92,17 @@ def map_file_to_owner(file)
end

sig do
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
end
def codeowners_lines_to_owners
def update_cache(cache, files)
globs_to_owner(files)
end

sig do
override.params(files: T::Array[String]).
returns(T::Hash[String, ::CodeTeams::Team])
end
def globs_to_owner(files)
return @@codeowners_lines_to_owners if @@codeowners_lines_to_owners&.keys && @@codeowners_lines_to_owners.keys.count > 0

@@codeowners_lines_to_owners = CodeTeams.all.each_with_object({}) do |team, map| # rubocop:disable Style/ClassVars
Expand Down
21 changes: 14 additions & 7 deletions lib/code_ownership/private/ownership_mappers/team_yml_ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@ class TeamYmlOwnership
extend T::Sig
include Mapper

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

sig do
override.
params(files: T::Array[String]).
returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
params(files: T::Array[String]).
returns(T::Hash[String, ::CodeTeams::Team])
end
def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
return @@map_files_to_owners if @@map_files_to_owners&.keys && @@map_files_to_owners.keys.count > 0
Expand All @@ -36,9 +35,10 @@ def map_file_to_owner(file)
end

sig do
override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
override.params(files: T::Array[String]).
returns(T::Hash[String, ::CodeTeams::Team])
end
def codeowners_lines_to_owners
def globs_to_owner(files)
return @@codeowners_lines_to_owners if @@codeowners_lines_to_owners&.keys && @@codeowners_lines_to_owners.keys.count > 0

@@codeowners_lines_to_owners = CodeTeams.all.each_with_object({}) do |team, map| # rubocop:disable Style/ClassVars
Expand All @@ -52,6 +52,13 @@ def bust_caches!
@@map_files_to_owners = {} # rubocop:disable Style/ClassVars
end

sig do
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
end
def update_cache(cache, files)
globs_to_owner(files)
end

sig { override.returns(String) }
def description
'Team YML ownership'
Expand Down
Loading

0 comments on commit 5500ab3

Please sign in to comment.