Optimize MergeRequestDiff#last_commit_shash-optimize-last-commit-sha

The previous implementation would load all the MergeRequestDiffCommit
entries into memory and retrieve the SHA from the latest entry. We can
save a bit of query time and memory by retrieving the SHA directly from
the first item from the database. This is particularly helpful when
there are many merge requests that need this information.

Seen while investigating Todo API performance in
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25711.

2 files changed, 15 insertions, 2 deletions

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index e286a4e57f2..3556448e547 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -118,7 +118,7 @@ class MergeRequestDiff < ActiveRecord::Base
   end
 
   def last_commit_sha
-    commit_shas.first
+    merge_request_diff_commits.first&.sha
   end
 
   def first_commit
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 1849d3bac12..fff3e810977 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe MergeRequestDiff do
-  let(:diff_with_commits) { create(:merge_request).merge_request_diff }
+  let(:merge_request) { create(:merge_request) }
+  let(:diff_with_commits) { merge_request.merge_request_diff }
 
   describe 'create new record' do
     subject { diff_with_commits }
@@ -224,6 +225,18 @@ describe MergeRequestDiff do
     end
   end
 
+  describe '#last_commit_sha' do
+    let(:commit_shas) { diff_with_commits.commit_shas }
+    let(:sorted_commits) { merge_request.merge_request_diff.merge_request_diff_commits.sort_by { |x| x.committed_date } }
+    let(:last_commit_sha) { diff_with_commits.last_commit_sha }
+
+    it 'returns the most recent commit SHA' do
+      expect(last_commit_sha).to eq(commit_shas.first)
+      # Sanity check the sort order
+      expect(last_commit_sha).to eq(sorted_commits.last.sha)
+    end
+  end
+
   describe '#commits_by_shas' do
     let(:commit_shas) { diff_with_commits.commit_shas }