Merge branch '48246-osw-load-diffs-improvement' into 'master'

Filter diff files by given path on DB instead Ruby

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

3 files changed, 15 insertions, 5 deletions

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index a073bbfad20..dbc072c19a9 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -249,15 +249,13 @@ class MergeRequestDiff < ActiveRecord::Base
   end
 
   def load_diffs(options)
-    raw = merge_request_diff_files.map(&:to_hash)
+    collection = merge_request_diff_files
 
     if paths = options[:paths]
-      raw = raw.select do |diff|
-        paths.include?(diff[:old_path]) || paths.include?(diff[:new_path])
-      end
+      collection = collection.where('old_path IN (?) OR new_path IN (?)', paths, paths)
     end
 
-    Gitlab::Git::DiffCollection.new(raw, options)
+    Gitlab::Git::DiffCollection.new(collection.map(&:to_hash), options)
   end
 
   def load_commits
diff --git a/changelogs/unreleased/48246-osw-load-diffs-improvement.yml b/changelogs/unreleased/48246-osw-load-diffs-improvement.yml
new file mode 100644
index 00000000000..c4292ab0d29
--- /dev/null
+++ b/changelogs/unreleased/48246-osw-load-diffs-improvement.yml
@@ -0,0 +1,5 @@
+---
+title: Improve performance when fetching collapsed diffs and commenting in merge requests
+merge_request: 20940
+author:
+type: performance
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 0aee78ac12d..90cce826b6c 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -127,6 +127,13 @@ describe MergeRequestDiff do
           expect(diffs.map(&:new_path)).to contain_exactly('files/ruby/popen.rb')
         end
 
+        it 'only serializes diff files found by query' do
+          expect(diff_with_commits.merge_request_diff_files.count).to be > 10
+          expect_any_instance_of(MergeRequestDiffFile).to receive(:to_hash).once
+
+          diffs
+        end
+
         it 'uses the diffs from the DB' do
           expect(diff_with_commits).to receive(:load_diffs)