Use GlobCache in various validations (#47)

Author: Alex Evanczuk

lib/code_ownership/mapper.rb

@@ -58,5 +58,21 @@ def description
     sig { abstract.void }
     def bust_caches!
     end
+
+    sig { returns(Private::GlobCache) }
+    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.each do |glob, owner|
+          next if owner.nil?
+          glob_to_owner_map_by_mapper_description[mapper.description] ||= {}
+          glob_to_owner_map_by_mapper_description.fetch(mapper.description)[glob] = owner
+        end
+      end
+
+      Private::GlobCache.new(glob_to_owner_map_by_mapper_description)
+    end
   end
 end

lib/code_ownership/private.rb

@@ -7,6 +7,7 @@
 require 'code_ownership/private/team_plugins/github'
 require 'code_ownership/private/codeowners_file'
 require 'code_ownership/private/parse_js_packages'
+require 'code_ownership/private/glob_cache'
 require 'code_ownership/private/validations/files_have_owners'
 require 'code_ownership/private/validations/github_codeowners_up_to_date'
 require 'code_ownership/private/validations/files_have_unique_owners'
@@ -38,7 +39,7 @@ def self.load_configuration!
     def self.bust_caches!
       @configuration = nil
       @tracked_files = nil
-      @files_by_mapper = nil
+      @glob_cache = nil
     end
 
     sig { params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).void }
@@ -88,21 +89,10 @@ def self.find_team!(team_name, location_of_reference)
       end
     end
 
-    sig { params(files: T::Array[String]).returns(T::Hash[String, T::Array[String]]) }
-    def self.files_by_mapper(files)
-      @files_by_mapper ||= T.let(@files_by_mapper, T.nilable(T::Hash[String, T::Array[String]]))
-      @files_by_mapper ||= begin
-        files_by_mapper = files.map { |file| [file, []] }.to_h
-
-        Mapper.all.each do |mapper|
-          mapper.map_files_to_owners(files).each do |file, _team|
-            files_by_mapper[file] ||= []
-            T.must(files_by_mapper[file]) << mapper.description
-          end
-        end
-
-        files_by_mapper
-      end
+    sig { returns(GlobCache) }
+    def self.glob_cache
+      @glob_cache ||= T.let(@glob_cache, T.nilable(GlobCache))
+      @glob_cache ||= Mapper.to_glob_cache
     end
   end
 

lib/code_ownership/private/codeowners_file.rb

@@ -18,17 +18,7 @@ def self.actual_contents_lines
 
       sig { returns(T::Array[T.nilable(String)]) }
       def self.expected_contents_lines
-        # Eventually, we'll get this from the `GlobCache`
-        glob_to_owner_map_by_mapper_description = {}
-
-        Mapper.all.each do |mapper|
-          mapped_files = mapper.codeowners_lines_to_owners
-          mapped_files.each do |glob, owner|
-            next if owner.nil?
-            glob_to_owner_map_by_mapper_description[mapper.description] ||= {}
-            glob_to_owner_map_by_mapper_description.fetch(mapper.description)[glob] = owner
-          end
-        end
+        cache = Private.glob_cache.raw_cache_contents
 
         header = <<~HEADER
           # STOP! - DO NOT EDIT THIS FILE MANUALLY
@@ -52,7 +42,7 @@ def self.expected_contents_lines
 
         codeowners_file_lines = T.let([], T::Array[String])
 
-        glob_to_owner_map_by_mapper_description.each do |mapper_description, ownership_map_cache|
+        cache.each do |mapper_description, ownership_map_cache|
           ownership_entries = []
           ownership_map_cache.each do |path, code_team|
             team_mapping = github_team_map[code_team.name]

lib/code_ownership/private/glob_cache.rb

@@ -0,0 +1,72 @@
+# typed: strict
+# frozen_string_literal: true
+
+module CodeOwnership
+  module Private
+    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
+        ]
+      end
+
+      FilesByMapper = T.type_alias do
+        T::Hash[
+          String,
+          T::Array[MapperDescription]
+        ]
+      end
+
+      sig { params(raw_cache_contents: CacheShape).void }
+      def initialize(raw_cache_contents)
+        @raw_cache_contents = raw_cache_contents
+      end
+
+      sig { returns(CacheShape) }
+      def raw_cache_contents
+        @raw_cache_contents
+      end
+
+      sig { returns(CacheShape) }
+      def expanded_cache
+        @expanded_cache = T.let(@expanded_cache, T.nilable(CacheShape))
+
+        @expanded_cache ||= begin
+          expanded_cache = {}
+          @raw_cache_contents.each do |mapper_description, globs_by_owner|
+            expanded_cache[mapper_description] = {}
+            globs_by_owner.each do |glob, owner|
+              Dir.glob(glob).each do |file, owner|
+                expanded_cache[mapper_description][file] = owner
+              end
+            end
+          end
+          
+          expanded_cache
+        end
+      end
+
+      sig { returns(FilesByMapper) }
+      def files_by_mapper
+        @files_by_mapper ||= T.let(@files_by_mapper, T.nilable(FilesByMapper))
+        @files_by_mapper ||= begin
+          files_by_mapper = {}
+          expanded_cache.each do |mapper_description, file_by_owner|
+            file_by_owner.each do |file, owner|
+              files_by_mapper[file] ||= []
+              files_by_mapper[file] << mapper_description
+            end
+          end
+
+          files_by_mapper
+        end
+      end
+    end
+  end
+end

lib/code_ownership/private/validations/files_have_owners.rb

@@ -11,8 +11,11 @@ class FilesHaveOwners
         sig { override.params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).returns(T::Array[String]) }
         def validation_errors(files:, autocorrect: true, stage_changes: true)
           allow_list = Dir.glob(Private.configuration.unowned_globs)
-          files_by_mapper = Private.files_by_mapper(files)
-          files_not_mapped_at_all = files_by_mapper.select { |_file, mapper_descriptions| mapper_descriptions.count == 0 }.keys
+          files_by_mapper = Private.glob_cache.files_by_mapper
+
+          files_not_mapped_at_all = files.select do |file|
+            files_by_mapper.fetch(file, []).count == 0
+          end
 
           files_without_owners = files_not_mapped_at_all - allow_list
 

lib/code_ownership/private/validations/files_have_unique_owners.rb

@@ -10,9 +10,15 @@ class FilesHaveUniqueOwners
 
         sig { override.params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).returns(T::Array[String]) }
         def validation_errors(files:, autocorrect: true, stage_changes: true)
-          files_by_mapper = Private.files_by_mapper(files)
+          files_by_mapper = Private.glob_cache.files_by_mapper
 
-          files_mapped_by_multiple_mappers = files_by_mapper.select { |_file, mapper_descriptions| mapper_descriptions.count > 1 }.to_h
+          files_mapped_by_multiple_mappers = {}
+          files.each do |file|
+            mappers = files_by_mapper.fetch(file, [])
+            if mappers.count > 1
+              files_mapped_by_multiple_mappers[file] = mappers
+            end
+          end
 
           errors = T.let([], T::Array[String])
 

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

@@ -3,7 +3,11 @@ module CodeOwnership
     describe 'CodeOwnership.validate!' do
       context 'a file in owned_globs has ownership defined in multiple ways' do
         before do
-          create_configuration
+          write_file('config/code_ownership.yml', <<~YML)
+            owned_globs:
+              - '{app,components,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}'
+          YML
+
           write_file('app/services/some_other_file.rb', <<~YML)
             # @team Bar
           YML
@@ -47,17 +51,16 @@ module CodeOwnership
 
         it 'lets the user know that each file can only have ownership defined in one way' do
           expect(CodeOwnership.for_file('app/missing_ownership.rb')).to eq nil
-
           expect { CodeOwnership.validate! }.to raise_error do |e|
             expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
             puts e.message
             expect(e.message).to eq <<~EXPECTED.chomp
               Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways.
 
               - frontend/javascripts/packages/my_package/owned_file.jsx (Annotations at the top of file, Team-specific owned globs, Owner metadata key in package.json)
+              - frontend/javascripts/packages/my_package/package.json (Team-specific owned globs, Owner metadata key in package.json)
               - packs/my_pack/owned_file.rb (Annotations at the top of file, Team-specific owned globs, Owner metadata key in package.yml)
               - packs/my_pack/package.yml (Team-specific owned globs, Owner metadata key in package.yml)
-              - frontend/javascripts/packages/my_package/package.json (Team-specific owned globs, Owner metadata key in package.json)
 
               See https://github.com/rubyatscale/code_ownership#README.md for more details
             EXPECTED