teams_for_file optimization

Author: perryqh

Cargo.lock

@@ -17,7 +17,8 @@ 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 = { git = "https://github.com/rubyatscale/codeowners-rs.git", tag = "v0.2.14" }
+codeowners = { path = "../../../codeowners-rs" }
 
 [dev-dependencies]
 rb-sys = { version = "0.9.117", features = [

ext/code_ownership/Cargo.toml

@@ -1,4 +1,4 @@
-use std::{env, path::PathBuf};
+use std::{collections::HashMap, env, path::PathBuf};
 
 use codeowners::runner::{self, RunConfig};
 use magnus::{Error, Ruby, Value, function, prelude::*};
@@ -18,6 +18,30 @@ fn for_team(team_name: String) -> Result<Value, Error> {
     validate_result(&team)
 }
 
+fn team_names_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 {
+        Ok(path_teams) => {
+            let mut teams_map: HashMap<String, Option<Team>> = HashMap::new();
+            for (path, team) in path_teams {
+                if let Some(found_team) = team {
+                    teams_map.insert(path, Some(Team {
+                        team_name: found_team.name.to_string(),
+                        team_config_yml: found_team.name.to_string(),
+                        reasons: vec![],
+                    }));
+                } else {
+                    teams_map.insert(path, None);
+                }
+            }
+            let serialized: Value = serialize(&teams_map)?;
+            Ok(serialized)
+        }
+        Err(e) => Err(Error::new(magnus::exception::runtime_error(), e.to_string())),
+    }
+}
+
 fn for_file(file_path: String) -> Result<Option<Value>, Error> {
     let run_config = build_run_config();
 
@@ -102,6 +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))?;
 
     Ok(())
 }

ext/code_ownership/src/lib.rs

@@ -45,6 +45,13 @@ def for_file(file)
     Private::TeamFinder.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
+  end
+
   sig { params(file: String).returns(T.nilable(T::Hash[Symbol, String])) }
   def for_file_verbose(file)
     ::RustCodeOwners.for_file(file)
@@ -56,9 +63,6 @@ def for_team(team)
     ::RustCodeOwners.for_team(team.name)
   end
 
-  class InvalidCodeOwnershipConfigurationError < StandardError
-  end
-
   sig do
     params(
       autocorrect: T::Boolean,

lib/code_ownership.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module CodeOwnership
-  VERSION = '2.0.0-2'
+  VERSION = '2.0.0-3'
 end

lib/code_ownership/code_ownership.bundle

@@ -18,5 +18,8 @@ module RustCodeOwners
 
     def version
     end
+
+    def team_names_for_files(files)
+    end
   end
 end

lib/code_ownership/version.rb

@@ -5,51 +5,101 @@
     expect(CodeOwnership::VERSION).not_to be nil
   end
 
-  describe '.for_file' do
-    subject { CodeOwnership.for_file(file_path) }
-    context 'rust codeowners' do
-      context 'when config is not found' do
-        let(:file_path) { 'app/javascript/[test]/test.js' }
-        it 'raises an error' do
-          expect { subject }.to raise_error(RuntimeError, /Can't open config file:/)
+  describe '.team_names_for_files' do
+    subject { CodeOwnership.team_names_for_files(files) }
+    let(:files) { ['app/services/my_file.rb'] }
+
+    context 'when config is not found' do
+      let(:files) { ['app/javascript/[test]/test.js'] }
+      it 'raises an error' do
+        expect { subject }.to raise_error(RuntimeError, /Can't open config file:/)
+      end
+    end
+
+    context 'with non-empty application' do
+      before do
+        create_non_empty_application
+        # codeowners-rs is matching files against the codeowners file
+        ::RustCodeOwners.generate_and_validate(false)
+      end
+
+      context 'when no ownership is found' do
+        let(:files) { ['app/madeup/file.rb'] }
+        it 'properly assigns ownership' do
+          expect(subject).to eq({ 'app/madeup/file.rb' => nil })
         end
       end
 
-      context 'with non-empty application' do
+      context 'when file path starts with ./' do
+        let(:files) { ['./app/javascript/[test]/test.js'] }
+        it 'properly assigns ownership' do
+          expect(subject).to eq({ './app/javascript/[test]/test.js' => nil })
+        end
+      end
+
+      context 'when ownership is found' do
+        let(:files) { ['packs/my_pack/owned_file.rb'] }
+        it 'returns the correct team' do
+          expect(subject).to eq({ 'packs/my_pack/owned_file.rb' => CodeTeams.find('Bar') })
+        end
+      end
+
+      context 'when ownership is found but team is not found' do
+        let(:file_path) { ['packs/my_pack/owned_file.rb'] }
         before do
-          create_non_empty_application
+          allow(RustCodeOwners).to receive(:team_names_for_files).and_return({ file_path.first => {team_name: 'Made Up Team'} })
+        end
+
+        it 'returns nil' do
+          expect(subject).to eq({ 'packs/my_pack/owned_file.rb' => nil })
         end
+      end
+    end
+  end
+
+  describe '.for_file' do
+    subject { CodeOwnership.for_file(file_path) }
+    context 'when config is not found' do
+      let(:file_path) { 'app/javascript/[test]/test.js' }
+      it 'raises an error' do
+        expect { subject }.to raise_error(RuntimeError, /Can't open config file:/)
+      end
+    end
+
+    context 'with non-empty application' do
+      before do
+        create_non_empty_application
+      end
 
-        context 'when no ownership is found' do
-          let(:file_path) { 'app/madeup/file.rb' }
-          it 'properly assigns ownership' do
-            expect(subject).to be_nil
-          end
+      context 'when no ownership is found' do
+        let(:file_path) { 'app/madeup/file.rb' }
+        it 'properly assigns ownership' do
+          expect(subject).to be_nil
         end
+      end
 
-        context 'when file path starts with ./' do
-          let(:file_path) { './app/javascript/[test]/test.js' }
-          it 'properly assigns ownership' do
-            expect(subject).to be_nil
-          end
+      context 'when file path starts with ./' do
+        let(:file_path) { './app/javascript/[test]/test.js' }
+        it 'properly assigns ownership' do
+          expect(subject).to be_nil
         end
+      end
 
-        context 'when ownership is found' do
-          let(:file_path) { 'packs/my_pack/owned_file.rb' }
-          it 'returns the correct team' do
-            expect(subject).to eq CodeTeams.find('Bar')
-          end
+      context 'when ownership is found' do
+        let(:file_path) { 'packs/my_pack/owned_file.rb' }
+        it 'returns the correct team' do
+          expect(subject).to eq CodeTeams.find('Bar')
         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(:for_file).and_return({ team_name: 'Made Up Team' })
-          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(:for_file).and_return({ team_name: 'Made Up Team' })
+        end
 
-          it 'raises an error' do
-            expect { subject }.to raise_error(StandardError, /Could not find team with name: `Made Up Team`. Make sure the team is one of/)
-          end
+        it 'raises an error' do
+          expect { subject }.to raise_error(StandardError, /Could not find team with name: `Made Up Team`. Make sure the team is one of/)
         end
       end
     end
@@ -215,7 +265,7 @@
 
   describe '.version' do
     it 'returns the version' do
-      expect(described_class.version).to eq ["code_ownership version: #{CodeOwnership::VERSION}", "codeowners-rs version: #{::RustCodeOwners.version}"]
+      expect(described_class.version).to eq ["code_ownership version: #{CodeOwnership::VERSION}", "codeowners-rs version: #{RustCodeOwners.version}"]
     end
   end
 end