Merge branch 'kerrizor/add-diff_stats-fallback-for-overflowed-mrs' into 'master'

Add fallback option for overflowed MRDs

See merge request gitlab-org/gitlab!29533

app/models/merge_request.rb

@@ -577,13 +577,13 @@ class MergeRequest < ApplicationRecord
     merge_request_diff&.real_size || diff_stats&.real_size || diffs.real_size
   end
 
-  def modified_paths(past_merge_request_diff: nil)
+  def modified_paths(past_merge_request_diff: nil, fallback_on_overflow: false)
     if past_merge_request_diff
-      past_merge_request_diff.modified_paths
+      past_merge_request_diff.modified_paths(fallback_on_overflow: fallback_on_overflow)
     elsif compare
       diff_stats&.paths || compare.modified_paths
     else
-      merge_request_diff.modified_paths
+      merge_request_diff.modified_paths(fallback_on_overflow: fallback_on_overflow)
     end
   end
 

app/models/merge_request_diff.rb

@@ -366,9 +366,22 @@ class MergeRequestDiff < ApplicationRecord
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  def modified_paths
-    strong_memoize(:modified_paths) do
-      merge_request_diff_files.pluck(:new_path, :old_path).flatten.uniq
+  def modified_paths(fallback_on_overflow: false)
+    if fallback_on_overflow && overflow?
+      # This is an extremely slow means to find the modified paths for a given
+      #   MergeRequestDiff. This should be avoided, except where the limit of
+      #   1_000 (as of %12.10) entries returned by the default behavior is an
+      #   issue.
+      strong_memoize(:overflowed_modified_paths) do
+        project.repository.diff_stats(
+          base_commit_sha,
+          head_commit_sha
+        ).paths
+      end
+    else
+      strong_memoize(:modified_paths) do
+        merge_request_diff_files.pluck(:new_path, :old_path).flatten.uniq
+      end
     end
   end
 

spec/models/merge_request_diff_spec.rb

@@ -566,6 +566,45 @@ describe MergeRequestDiff do
     it 'returns affected file paths' do
       expect(subject.modified_paths).to eq(%w{foo bar baz})
     end
+
+    context "when fallback_on_overflow is true" do
+      let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
+      let(:diff) { merge_request.merge_request_diff }
+
+      # before do
+      #   # Temporarily unstub diff.modified_paths in favor of original code
+      #   #
+      #   allow(diff).to receive(:modified_paths).and_call_original
+      # end
+
+      context "when the merge_request_diff is overflowed" do
+        before do
+          expect(diff).to receive(:overflow?).and_return(true)
+        end
+
+        it "returns file paths via project.repository#diff_stats" do
+          expect(diff.project.repository).to receive(:diff_stats).and_call_original
+
+          expect(
+            diff.modified_paths(fallback_on_overflow: true)
+          ).to eq(diff.modified_paths)
+        end
+      end
+
+      context "when the merge_request_diff is not overflowed" do
+        before do
+          expect(subject).to receive(:overflow?).and_return(false)
+        end
+
+        it "returns expect file paths withoout called #modified_paths_for_overflowed_mr" do
+          expect(subject.project.repository).not_to receive(:diff_stats)
+
+          expect(
+            subject.modified_paths(fallback_on_overflow: true)
+          ).to eq(subject.modified_paths)
+        end
+      end
+    end
   end
 
   describe '#opening_external_diff' do