Merge branch 'cache-commit-author-lookup' into 'master'

Cache the commit author in RequestStore to avoid extra lookups in PostReceive

See merge request !5537

3 files changed, 37 insertions, 1 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 89aea9f5c50..d555d23860d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 8.11.0 (unreleased)
   - Fix the title of the toggle dropdown button. !5515 (herminiotorres)
   - Remove magic comments (`# encoding: UTF-8`) from Ruby files. !5456 (winniehell)
   - Fix CI status icon link underline (ClemMakesApps)
+  - Cache the commit author in RequestStore to avoid extra lookups in PostReceive
   - Fix of 'Commits being passed to custom hooks are already reachable when using the UI'
   - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable
   - Optimize maximum user access level lookup in loading of notes
diff --git a/app/models/commit.rb b/app/models/commit.rb
index f80f1063406..486ad6714d9 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -178,7 +178,18 @@ class Commit
   end
 
   def author
-    @author ||= User.find_by_any_email(author_email.downcase)
+    if RequestStore.active?
+      key = "commit_author:#{author_email.downcase}"
+      # nil is a valid value since no author may exist in the system
+      if RequestStore.store.has_key?(key)
+        @author = RequestStore.store[key]
+      else
+        @author = find_author_by_any_email
+        RequestStore.store[key] = @author
+      end
+    else
+      @author ||= find_author_by_any_email
+    end
   end
 
   def committer
@@ -306,6 +317,10 @@ class Commit
 
   private
 
+  def find_author_by_any_email
+    User.find_by_any_email(author_email.downcase)
+  end
+
   def repo_changes
     changes = { added: [], modified: [], removed: [] }
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index ec1544bf815..c3392ee7440 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -13,6 +13,26 @@ describe Commit, models: true do
     it { is_expected.to include_module(StaticModel) }
   end
 
+  describe '#author' do
+    it 'looks up the author in a case-insensitive way' do
+      user = create(:user, email: commit.author_email.upcase)
+      expect(commit.author).to eq(user)
+    end
+
+    it 'caches the author' do
+      user = create(:user, email: commit.author_email)
+      expect(RequestStore).to receive(:active?).twice.and_return(true)
+      expect_any_instance_of(Commit).to receive(:find_author_by_any_email).and_call_original
+
+      expect(commit.author).to eq(user)
+      key = "commit_author:#{commit.author_email}"
+      expect(RequestStore.store[key]).to eq(user)
+
+      expect(commit.author).to eq(user)
+      RequestStore.store.clear
+    end
+  end
+
   describe '#to_reference' do
     it 'returns a String reference to the object' do
       expect(commit.to_reference).to eq commit.id