Merge branch 'sh-optimize-reload-diffs-service' into 'master'

Significantly cut memory and SQL queries when reloading diffs

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

3 files changed, 19 insertions, 8 deletions

diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 90cce826b6c..562ccaf6c0b 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -52,9 +52,9 @@ describe MergeRequestDiff do
 
     context 'when it was not cleaned by the system' do
       it 'returns persisted diffs' do
-        expect(diff).to receive(:load_diffs)
+        expect(diff).to receive(:load_diffs).and_call_original
 
-        diff.diffs
+        diff.diffs.diff_files
       end
     end
 
@@ -76,19 +76,19 @@ describe MergeRequestDiff do
       end
 
       it 'returns persisted diffs if cannot compare with diff refs' do
-        expect(diff).to receive(:load_diffs)
+        expect(diff).to receive(:load_diffs).and_call_original
 
         diff.update!(head_commit_sha: 'invalid-sha')
 
-        diff.diffs
+        diff.diffs.diff_files
       end
 
       it 'returns persisted diffs if diff refs does not exist' do
-        expect(diff).to receive(:load_diffs)
+        expect(diff).to receive(:load_diffs).and_call_original
 
         diff.update!(start_commit_sha: nil, base_commit_sha: nil)
 
-        diff.diffs
+        diff.diffs.diff_files
       end
     end
   end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 85a4ebac66c..7d500f9e579 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -552,9 +552,9 @@ describe MergeRequest do
       it 'delegates to the MR diffs' do
         merge_request.save
 
-        expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options))
+        expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)).and_call_original
 
-        merge_request.diffs(options)
+        merge_request.diffs(options).diff_files
       end
     end
 
diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb
index 21f369a3818..546c9f277c5 100644
--- a/spec/services/merge_requests/reload_diffs_service_spec.rb
+++ b/spec/services/merge_requests/reload_diffs_service_spec.rb
@@ -60,6 +60,17 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin
 
         subject.execute
       end
+
+      it 'avoids N+1 queries', :request_store do
+        current_user
+        merge_request
+
+        control_count = ActiveRecord::QueryRecorder.new do
+          subject.execute
+        end.count
+
+        expect { subject.execute }.not_to exceed_query_limit(control_count)
+      end
     end
   end
 end