caching file_path teams on team_names_for_files

Author: perryqh

Cargo.lock

@@ -17,8 +17,7 @@ rb-sys = { version = "0.9.111", features = [
 magnus = { version = "0.7.1" }
 serde = { version = "1.0.219", features = ["derive"] }
 serde_magnus = "0.9.0"
-# codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.2.14" }
-codeowners = { path = "../../../codeowners-rs" }
+codeowners = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.2.15" }
 
 [dev-dependencies]
 rb-sys = { version = "0.9.117", features = [

ext/code_ownership/Cargo.toml

@@ -18,7 +18,7 @@ fn for_team(team_name: String) -> Result<Value, Error> {
     validate_result(&team)
 }
 
-fn team_names_for_files(file_paths: Vec<String>) -> Result<Value, Error> {
+fn teams_for_files(file_paths: Vec<String>) -> Result<Value, Error> {
     let run_config = build_run_config();
     let path_teams = runner::teams_for_files_from_codeowners(&run_config, &file_paths);
     match path_teams {
@@ -126,7 +126,7 @@ fn init(ruby: &Ruby) -> Result<(), Error> {
     module.define_singleton_method("validate", function!(validate, 0))?;
     module.define_singleton_method("for_team", function!(for_team, 1))?;
     module.define_singleton_method("version", function!(version, 0))?;
-    module.define_singleton_method("team_names_for_files", function!(team_names_for_files, 1))?;
+    module.define_singleton_method("teams_for_files", function!(teams_for_files, 1))?;
 
     Ok(())
 }

ext/code_ownership/src/lib.rs

@@ -46,10 +46,8 @@ def for_file(file)
   end
 
   sig { params(files: T::Array[String]).returns(T::Hash[String, T.nilable(CodeTeams::Team)]) }
-  def team_names_for_files(files)
-    ::RustCodeOwners.team_names_for_files(files).transform_values do |team|
-      team ? CodeTeams.find(team[:team_name]) : nil
-    end
+  def teams_for_files(files)
+    Private::TeamFinder.teams_for_files(files)
   end
 
   sig { params(file: String).returns(T.nilable(T::Hash[Symbol, String])) }

lib/code_ownership.rb

@@ -30,6 +30,16 @@ def for_file(file_path)
         FilePathTeamCache.get(file_path)
       end
 
+      sig { params(files: T::Array[String]).returns(T::Hash[String, T.nilable(CodeTeams::Team)]) }
+      def teams_for_files(files)
+        ::RustCodeOwners.teams_for_files(files).each_with_object({}) do |path_team, hash|
+          file_path, team = path_team
+          found_team = team ? CodeTeams.find(team[:team_name]) : nil
+          FilePathTeamCache.set(file_path, found_team)
+          hash[file_path] = found_team
+        end
+      end
+
       sig { params(klass: T.nilable(T.any(T::Class[T.anything], Module))).returns(T.nilable(::CodeTeams::Team)) }
       def for_class(klass)
         file_path = FilePathFinder.path_from_klass(klass)

lib/code_ownership/code_ownership.bundle

@@ -5,8 +5,8 @@
     expect(CodeOwnership::VERSION).not_to be nil
   end
 
-  describe '.team_names_for_files' do
-    subject { CodeOwnership.team_names_for_files(files) }
+  describe '.teams_for_files' do
+    subject { CodeOwnership.teams_for_files(files) }
     let(:files) { ['app/services/my_file.rb'] }
 
     context 'when config is not found' do
@@ -42,12 +42,22 @@
         it 'returns the correct team' do
           expect(subject).to eq({ 'packs/my_pack/owned_file.rb' => CodeTeams.find('Bar') })
         end
+
+        context 'subsequent for_file utilizes cached team' do
+          let(:files) { ['packs/my_pack/owned_file.rb', 'packs/my_pack/owned_file2.rb'] }
+          it 'returns the correct team' do
+            subject # caches paths -> teams
+            allow(RustCodeOwners).to receive(:for_file)
+            expect(described_class.for_file('packs/my_pack/owned_file.rb')).to eq(CodeTeams.find('Bar'))
+            expect(RustCodeOwners).to_not have_received(:for_file)
+          end
+        end
       end
 
       context 'when ownership is found but team is not found' do
         let(:file_path) { ['packs/my_pack/owned_file.rb'] }
         before do
-          allow(RustCodeOwners).to receive(:team_names_for_files).and_return({ file_path.first => {team_name: 'Made Up Team'} })
+          allow(RustCodeOwners).to receive(:teams_for_files).and_return({ file_path.first => {team_name: 'Made Up Team'} })
         end
 
         it 'returns nil' do