feat: check .codeowner within directory if it is provided to `for_f…

…ile` (#77) * feat: check `.codeowner` within directory if it is provided to `for_file` * bump version * move pathnames to constants * add absolute path test --------- Co-authored-by: Perry Hertler <[email protected]>
rubyatscale · Dec 1, 2023 · bd75d69 · bd75d69
1 parent 5dea641
commit bd75d69
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 20 deletions.
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    code_ownership (1.35.0)
+    code_ownership (1.36.0)
       code_teams (~> 1.0)
       packs-specification
       sorbet-runtime (>= 0.5.10821)

diff --git a/code_ownership.gemspec b/code_ownership.gemspec
@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = 'code_ownership'
-  spec.version       = '1.35.0'
+  spec.version       = '1.36.0'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['[email protected]']
   spec.summary       = 'A gem to help engineering teams declare ownership of code'

diff --git a/lib/code_ownership/private/ownership_mappers/directory_ownership.rb b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb
@@ -10,6 +10,8 @@ class DirectoryOwnership
         include Mapper
 
         CODEOWNERS_DIRECTORY_FILE_NAME = '.codeowner'
+        RELATIVE_ROOT = Pathname('.').freeze
+        ABSOLUTE_ROOT = Pathname('/').freeze
 
         @@directory_cache = T.let({}, T::Hash[String, T.nilable(CodeTeams::Team)]) # rubocop:disable Style/ClassVars
 
@@ -74,36 +76,46 @@ def owner_for_codeowners_file(codeowners_file)
           )
         end
 
-        # takes a file and finds the relevant `.codeowner` file by walking up the directory
+        # Takes a file and finds the relevant `.codeowner` file by walking up the directory
         # structure. Example, given `a/b/c.rb`, this looks for `a/b/.codeowner`, `a/.codeowner`,
         # and `.codeowner` in that order, stopping at the first file to actually exist.
-        # We do additional caching so that we don't have to check for file existence every time
+        # If the parovided file is a directory, it will look for `.codeowner` in that directory and then upwards.
+        # We do additional caching so that we don't have to check for file existence every time.
         sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
         def map_file_to_relevant_owner(file)
           file_path = Pathname.new(file)
-          path_components = file_path.each_filename.to_a.map { |path| Pathname.new(path) }
+          team = T.let(nil, T.nilable(CodeTeams::Team))
 
-          (path_components.length - 1).downto(0).each do |i|
-            potential_relative_path_name = T.must(path_components[0...i]).reduce(Pathname.new('')) { |built_path, path| built_path.join(path) }
-            potential_codeowners_file = potential_relative_path_name.join(CODEOWNERS_DIRECTORY_FILE_NAME)
+          if File.directory?(file)
+            team = get_team_from_codeowners_file_within_directory(file_path)
+          end
+
+          while team.nil? && file_path != RELATIVE_ROOT && file_path != ABSOLUTE_ROOT
+            file_path = file_path.parent
+            team = get_team_from_codeowners_file_within_directory(file_path)
+          end
+
+          team
+        end
 
-            potential_codeowners_file_name = potential_codeowners_file.to_s
+        sig { params(directory: Pathname).returns(T.nilable(CodeTeams::Team)) }
+        def get_team_from_codeowners_file_within_directory(directory)
+          potential_codeowners_file = directory.join(CODEOWNERS_DIRECTORY_FILE_NAME)
 
-            team = nil
-            if @@directory_cache.key?(potential_codeowners_file_name)
-              team = @@directory_cache[potential_codeowners_file_name]
-            elsif potential_codeowners_file.exist?
-              team = owner_for_codeowners_file(potential_codeowners_file)
+          potential_codeowners_file_name = potential_codeowners_file.to_s
 
-              @@directory_cache[potential_codeowners_file_name] = team
-            else
-              @@directory_cache[potential_codeowners_file_name] = nil
-            end
+          team = nil
+          if @@directory_cache.key?(potential_codeowners_file_name)
+            team = @@directory_cache[potential_codeowners_file_name]
+          elsif potential_codeowners_file.exist?
+            team = owner_for_codeowners_file(potential_codeowners_file)
 
-            return team unless team.nil?
+            @@directory_cache[potential_codeowners_file_name] = team
+          else
+            @@directory_cache[potential_codeowners_file_name] = nil
           end
 
-          nil
+          return team
         end
       end
     end

diff --git a/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb
@@ -21,6 +21,11 @@ module CodeOwnership
       it 'can find the owner of files in a sub-directory of a team-owned directory' do
         expect(CodeOwnership.for_file('a/b/c/c_file.jsx').name).to eq 'Bar'
       end
+
+      it 'looks for codeowner file within directory' do
+        expect(CodeOwnership.for_file('a/b').name).to eq 'Bar'
+        expect(CodeOwnership.for_file(Pathname.pwd.join('a/b').to_s).name).to eq 'Bar'
+      end
     end
   end
 end