fix: go back to old way of creating paths in directory ownership (#84)

* fix: go back to old way of creating paths in directory ownership

* add some more tests for `..` in filename

Author: att14

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    code_ownership (1.36.0)
+    code_ownership (1.36.1)
       code_teams (~> 1.0)
       packs-specification
       sorbet-runtime (>= 0.5.10821)

code_ownership.gemspec

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

lib/code_ownership/private/ownership_mappers/directory_ownership.rb

@@ -10,8 +10,6 @@ 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
 
@@ -88,11 +86,19 @@ def map_file_to_relevant_owner(file)
 
           if File.directory?(file)
             team = get_team_from_codeowners_file_within_directory(file_path)
+            return team unless team.nil?
           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)
+          path_components = file_path.each_filename.to_a
+          if file_path.absolute?
+            path_components = ['/', *path_components]
+          end
+
+          (path_components.length - 1).downto(0).each do |i|
+            team = get_team_from_codeowners_file_within_directory(
+               Pathname.new(File.join(*T.unsafe(path_components[0...i])))
+            )
+            return team unless team.nil?
           end
 
           team

spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb

@@ -1,5 +1,5 @@
 module CodeOwnership
-  RSpec.describe Private::OwnershipMappers::JsPackageOwnership do
+  RSpec.describe Private::OwnershipMappers::DirectoryOwnership do
     describe 'CodeOwnershp.for_file' do
       before do
         write_configuration
@@ -14,17 +14,30 @@ module CodeOwnership
         CONTENTS
       end
 
+      subject { described_class.new }
+
+      before do
+        subject.bust_caches!
+      end
+
       it 'can find the owner of files in team-owned directory' do
-        expect(CodeOwnership.for_file('a/b/b_file.jsx').name).to eq 'Bar'
+        expect(subject.map_file_to_owner('a/b/b_file.jsx').name).to eq 'Bar'
       end
 
       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'
+        expect(subject.map_file_to_owner('a/b/c/c_file.jsx').name).to eq 'Bar'
+      end
+
+      it 'returns null when no team is found' do
+        expect(subject.map_file_to_owner('tmp/tmp/foo.txt')).to be_nil
+        expect(subject.map_file_to_owner('../tmp/tmp/foo.txt')).to be_nil
+        expect(subject.map_file_to_owner(Pathname.pwd.join('tmp/tmp/foo.txt').to_s)).to be_nil
       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'
+        expect(subject.map_file_to_owner('a/b').name).to eq 'Bar'
+        expect(subject.map_file_to_owner('a/../a/b').name).to eq 'Bar'
+        expect(subject.map_file_to_owner(Pathname.pwd.join('a/b').to_s).name).to eq 'Bar'
       end
     end
   end