Merge branch 'zj-commit-show-n-1' into 'master'

Batch load blobs for diff generation

Closes #37599

See merge request gitlab-org/gitlab-ce!15370

5 files changed, 34 insertions, 28 deletions

diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb
index 5dc27e2bbba..fd90c0d8bad 100644
--- a/spec/controllers/projects/commit_controller_spec.rb
+++ b/spec/controllers/projects/commit_controller_spec.rb
@@ -1,15 +1,15 @@
 require 'spec_helper'
 
 describe Projects::CommitController do
-  let(:project)  { create(:project, :repository) }
-  let(:user)     { create(:user) }
+  set(:project)  { create(:project, :repository) }
+  set(:user)     { create(:user) }
   let(:commit)   { project.commit("master") }
   let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' }
   let(:master_pickable_commit)  { project.commit(master_pickable_sha) }
 
   before do
     sign_in(user)
-    project.team << [user, :master]
+    project.add_master(user)
   end
 
   describe 'GET show' do
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index c91895cedc3..ff9acfd08b9 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -116,12 +116,8 @@ describe Gitlab::Diff::File do
     end
 
     context 'when renamed' do
-      let(:commit) { project.commit('6907208d755b60ebeacb2e9dfea74c92c3449a1f') }
-      let(:diff_file) { commit.diffs.diff_file_with_new_path('files/js/commit.coffee') }
-
-      before do
-        allow(diff_file.new_blob).to receive(:id).and_return(diff_file.old_blob.id)
-      end
+      let(:commit) { project.commit('94bb47ca1297b7b3731ff2a36923640991e9236f') }
+      let(:diff_file) { commit.diffs.diff_file_with_new_path('CHANGELOG.md') }
 
       it 'returns false' do
         expect(diff_file.content_changed?).to be_falsey
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb
index 47342f98283..81e35e6c931 100644
--- a/spec/models/blob_spec.rb
+++ b/spec/models/blob_spec.rb
@@ -16,6 +16,23 @@ describe Blob do
     end
   end
 
+  describe '.lazy' do
+    let(:project) { create(:project, :repository) }
+    let(:commit) { project.commit_by(oid: 'e63f41fe459e62e1228fcef60d7189127aeba95a') }
+
+    it 'fetches all blobs when the first is accessed' do
+      changelog = described_class.lazy(project, commit.id, 'CHANGELOG')
+      contributing = described_class.lazy(project, commit.id, 'CONTRIBUTING.md')
+
+      expect(Gitlab::Git::Blob).to receive(:batch).once.and_call_original
+      expect(Gitlab::Git::Blob).not_to receive(:find)
+
+      # Access property so the values are loaded
+      changelog.id
+      contributing.id
+    end
+  end
+
   describe '#data' do
     context 'using a binary blob' do
       it 'returns the data as-is' do
diff --git a/spec/models/diff_viewer/base_spec.rb b/spec/models/diff_viewer/base_spec.rb
index b26de3f3b97..c90b32c5d77 100644
--- a/spec/models/diff_viewer/base_spec.rb
+++ b/spec/models/diff_viewer/base_spec.rb
@@ -32,10 +32,8 @@ describe DiffViewer::Base do
       end
 
       context 'when the binaryness does not match' do
-        before do
-          allow(diff_file.old_blob).to receive(:binary?).and_return(false)
-          allow(diff_file.new_blob).to receive(:binary?).and_return(false)
-        end
+        let(:commit) { project.commit_by(oid: 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
+        let(:diff_file) { commit.diffs.diff_file_with_new_path('Gemfile.zip') }
 
         it 'returns false' do
           expect(viewer_class.can_render?(diff_file)).to be_falsey
@@ -60,8 +58,7 @@ describe DiffViewer::Base do
 
       context 'when the binaryness does not match' do
         before do
-          allow(diff_file.old_blob).to receive(:binary?).and_return(true)
-          allow(diff_file.new_blob).to receive(:binary?).and_return(true)
+          allow_any_instance_of(Blob).to receive(:binary?).and_return(true)
         end
 
         it 'returns false' do
@@ -77,12 +74,12 @@ describe DiffViewer::Base do
     end
 
     context 'when the file was renamed and only the old blob is supported' do
-      let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
+      let(:commit) { project.commit_by(oid: '2f63565e7aac07bcdadb654e253078b727143ec4') }
       let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') }
 
       before do
         allow(diff_file).to receive(:renamed_file?).and_return(true)
-        allow(diff_file.new_blob).to receive(:extension).and_return('jpeg')
+        viewer_class.extensions = %w(notjpg)
       end
 
       it 'returns false' do
@@ -94,8 +91,7 @@ describe DiffViewer::Base do
   describe '#collapsed?' do
     context 'when the combined blob size is larger than the collapse limit' do
       before do
-        allow(diff_file.old_blob).to receive(:raw_size).and_return(512.kilobytes)
-        allow(diff_file.new_blob).to receive(:raw_size).and_return(513.kilobytes)
+        allow(diff_file).to receive(:raw_size).and_return(1025.kilobytes)
       end
 
       it 'returns true' do
@@ -113,8 +109,7 @@ describe DiffViewer::Base do
   describe '#too_large?' do
     context 'when the combined blob size is larger than the size limit' do
       before do
-        allow(diff_file.old_blob).to receive(:raw_size).and_return(2.megabytes)
-        allow(diff_file.new_blob).to receive(:raw_size).and_return(4.megabytes)
+        allow(diff_file).to receive(:raw_size).and_return(6.megabytes)
       end
 
       it 'returns true' do
@@ -132,8 +127,7 @@ describe DiffViewer::Base do
   describe '#render_error' do
     context 'when the combined blob size is larger than the size limit' do
       before do
-        allow(diff_file.old_blob).to receive(:raw_size).and_return(2.megabytes)
-        allow(diff_file.new_blob).to receive(:raw_size).and_return(4.megabytes)
+        allow(diff_file).to receive(:raw_size).and_return(6.megabytes)
       end
 
       it 'returns :too_large' do
diff --git a/spec/models/diff_viewer/server_side_spec.rb b/spec/models/diff_viewer/server_side_spec.rb
index 92e613f92de..98a8f6d4cc9 100644
--- a/spec/models/diff_viewer/server_side_spec.rb
+++ b/spec/models/diff_viewer/server_side_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
 describe DiffViewer::ServerSide do
-  let(:project) { create(:project, :repository) }
-  let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') }
-  let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') }
+  set(:project) { create(:project, :repository) }
+  let(:commit) { project.commit_by(oid: '570e7b2abdd848b95f2f578043fc23bd6f6fd24d') }
+  let!(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') }
 
   let(:viewer_class) do
     Class.new(DiffViewer::Base) do
@@ -15,8 +15,7 @@ describe DiffViewer::ServerSide do
 
   describe '#prepare!' do
     it 'loads all diff file data' do
-      expect(diff_file.old_blob).to receive(:load_all_data!)
-      expect(diff_file.new_blob).to receive(:load_all_data!)
+      expect(Blob).to receive(:lazy).at_least(:twice)
 
       subject.prepare!
     end