restrict mappers to files that are actually in param (#92)

* restrict mappers to files that are actually in param

* remove need for T.unsafe

* store file count for using expanded cache in constant

---------

Co-authored-by: Todd Sedano <[email protected]>
Co-authored-by: Teal Stannard <[email protected]>
Co-authored-by: Perry Hertler <[email protected]>
Co-authored-by: Ashley Willard <[email protected]>

Author: 5 people

lib/code_ownership/private/glob_cache.rb

@@ -6,6 +6,8 @@ module Private
     class GlobCache
       extend T::Sig
 
+      EXPANDED_CACHE_FILE_COUNT_THRESHOLD = 100
+
       MapperDescription = T.type_alias { String }
 
       CacheShape = T.type_alias do
@@ -34,10 +36,15 @@ def raw_cache_contents
 
       sig { params(files: T::Array[String]).returns(FilesByMapper) }
       def mapper_descriptions_that_map_files(files)
-        if files.count > 100
+        if files.count > EXPANDED_CACHE_FILE_COUNT_THRESHOLD
           # When looking at many files, expanding the cache out using Dir.glob and checking for intersections is faster
           files_by_mappers = files.map{ |f| [f, Set.new([]) ]}.to_h
-          files_by_mappers.merge(files_by_mappers_via_expanded_cache)
+
+          files_by_mappers_via_expanded_cache.each do |file, mapper|
+            T.must(files_by_mappers[file]) << mapper if files_by_mappers[file]
+          end
+
+          files_by_mappers
         else
           # When looking at few files, using File.fnmatch is faster
           files_by_mappers_via_file_fnmatch(files)

spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb

@@ -66,9 +66,27 @@ module CodeOwnership
           end
         end
 
-        context 'the input files do not include the file owned in multiple ways' do
-          it 'ignores the file with multiple ownership' do
-            expect { CodeOwnership.validate!(files: ['app/services/some_other_file.rb']) }.to_not raise_error
+        it "ignores the file with multiple ownership if it's not in the files param" do
+          expect { CodeOwnership.validate!(files: ['app/services/some_other_file.rb']) }.to_not raise_error
+        end
+
+        context 'there are > 100 files in validation check' do
+          let(:files) do
+            files = []
+
+            101.times do |i|
+              filename = "app/services/some_other_file#{i}.rb"
+              files << filename
+              write_file(filename, <<~YML)
+                # @team Bar
+              YML
+            end
+
+            files
+          end
+
+          it "ignores the file with multiple ownership if it's not in the files param" do
+            expect { CodeOwnership.validate!(files: files) }.to_not raise_error
           end
         end
       end