always use expanded cache when getting mapper descriptions for files (#…

…104)

* always use expanded cache when getting mapper descriptions for files

* add missing loop for mappers

* bump version (1.36.2 => 1.36.3)

code_ownership.gemspec

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

lib/code_ownership/private/glob_cache.rb

@@ -6,8 +6,6 @@ module Private
     class GlobCache
       extend T::Sig
 
-      EXPANDED_CACHE_FILE_COUNT_THRESHOLD = 100
-
       MapperDescription = T.type_alias { String }
 
       CacheShape = T.type_alias do
@@ -36,19 +34,15 @@ def raw_cache_contents
 
       sig { params(files: T::Array[String]).returns(FilesByMapper) }
       def mapper_descriptions_that_map_files(files)
-        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.to_h { |f| [f, Set.new([])] }
+        files_by_mappers = files.to_h { |f| [f, Set.new([])] }
 
-          files_by_mappers_via_expanded_cache.each do |file, mapper|
+        files_by_mappers_via_expanded_cache.each do |file, mappers|
+          mappers.each do |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)
         end
+
+        files_by_mappers
       end
 
       private
@@ -81,31 +75,6 @@ def files_by_mappers_via_expanded_cache
           files_by_mappers
         end
       end
-
-      sig { params(files: T::Array[String]).returns(FilesByMapper) }
-      def files_by_mappers_via_file_fnmatch(files)
-        files_by_mappers = T.let({}, FilesByMapper)
-
-        files.each do |file|
-          files_by_mappers[file] ||= Set.new([])
-          @raw_cache_contents.each do |mapper_description, globs_by_owner|
-            # As much as I'd like to *not* special case the file annotations mapper, using File.fnmatch? on the thousands of files mapped by the
-            # file annotations mapper is a lot of unnecessary extra work.
-            # Therefore we can just check if the file is in the globs directly for file annotations, otherwise use File.fnmatch
-            if mapper_description == OwnershipMappers::FileAnnotations::DESCRIPTION
-              files_by_mappers.fetch(file) << mapper_description if globs_by_owner[file]
-            else
-              globs_by_owner.each_key do |glob|
-                if File.fnmatch?(glob, file, File::FNM_PATHNAME | File::FNM_EXTGLOB)
-                  files_by_mappers.fetch(file) << mapper_description
-                end
-              end
-            end
-          end
-        end
-
-        files_by_mappers
-      end
     end
   end
 end

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

@@ -69,26 +69,6 @@ module CodeOwnership
         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
 
       context 'with mutliple directory ownership files' do

spec/lib/code_ownership_spec.rb

@@ -27,16 +27,10 @@
 
       context 'file ownership with [] characters' do
         before do
-          write_file('app/services/[test]/some_other_file.ts', <<~YML)
+          write_file('app/services/[test]/some_file.ts', <<~TYPESCRIPT)
             // @team Bar
             // Countries
-          YML
-
-          100.times do |i|
-            write_file("app/services/withoutbracket/some_other_file#{i}.ts", <<~YML)
-              // @team Bar
-            YML
-          end
+          TYPESCRIPT
         end
 
         it 'has no validation errors' do