Merge branch...

Merge branch '254823-automatically-expand-file-on-merge-request-with-changes-to-single-file' into 'master'

Automatically expand file on merge request with changes to single file

See merge request gitlab-org/gitlab!44629

changelogs/unreleased/254823-automatically-expand-file-on-merge-request-with-changes-to-single-.yml

+---
+title: Automatically expand diffs for merge requests with changes to a single file
+merge_request: 44629
+author:
+type: changed

lib/gitlab/diff/highlight_cache.rb

@@ -24,6 +24,10 @@ module Gitlab
 
         return [] unless content
 
+        # TODO: We could add some kind of flag to #initialize that would allow
+        #   us to force re-caching
+        #   https://gitlab.com/gitlab-org/gitlab/-/issues/263508
+        #
         if content.empty? && recache_due_to_size?(diff_file)
           # If the file is missing from the cache and there's reason to believe
           #   it is uncached due to a size issue around changing the values for

lib/gitlab/git/diff_collection.rb

@@ -118,11 +118,17 @@ module Gitlab
         files >= safe_max_files || @line_count > safe_max_lines || @byte_count >= safe_max_bytes
       end
 
+      def expand_diff?
+        # Force single-entry diff collections to always present as expanded
+        #
+        @iterator.size == 1 || !@enforce_limits || @expanded
+      end
+
       def each_gitaly_patch
         i = @array.length
 
         @iterator.each do |raw|
-          diff = Gitlab::Git::Diff.new(raw, expanded: !@enforce_limits || @expanded)
+          diff = Gitlab::Git::Diff.new(raw, expanded: expand_diff?)
 
           if raw.overflow_marker
             @overflow = true
@@ -145,11 +151,9 @@ module Gitlab
             break
           end
 
-          expanded = !@enforce_limits || @expanded
-
-          diff = Gitlab::Git::Diff.new(raw, expanded: expanded)
+          diff = Gitlab::Git::Diff.new(raw, expanded: expand_diff?)
 
-          if !expanded && over_safe_limits?(i) && diff.line_count > 0
+          if !expand_diff? && over_safe_limits?(i) && diff.line_count > 0
             diff.collapse!
           end
 

lib/gitlab/gitaly_client/diff_stitcher.rb

@@ -5,8 +5,10 @@ module Gitlab
     class DiffStitcher
       include Enumerable
 
-      def initialize(rpc_response)
-        @rpc_response = rpc_response
+      delegate :size, to: :rpc_response
+
+      def initialize(rpc_response_param)
+        @rpc_response = rpc_response_param
       end
 
       def each
@@ -31,6 +33,10 @@ module Gitlab
           end
         end
       end
+
+      private
+
+      attr_reader :rpc_response
     end
   end
 end

spec/lib/gitlab/git/diff_collection_spec.rb

@@ -9,8 +9,11 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do
     MutatingConstantIterator.class_eval do
       include Enumerable
 
+      attr_reader :size
+
       def initialize(count, value)
         @count = count
+        @size  = count
         @value = value
       end
 
@@ -517,14 +520,30 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do
             .to yield_with_args(an_instance_of(Gitlab::Git::Diff))
         end
 
-        it 'prunes diffs that are quite big' do
-          diff = nil
+        context 'single-file collections' do
+          it 'does not prune diffs' do
+            diff = nil
 
-          subject.each do |d|
-            diff = d
+            subject.each do |d|
+              diff = d
+            end
+
+            expect(diff.diff).not_to eq('')
           end
+        end
+
+        context 'multi-file collections' do
+          let(:iterator) { [{ diff: 'b' }, { diff: 'a' * 20480 }]}
+
+          it 'prunes diffs that are quite big' do
+            diff = nil
 
-          expect(diff.diff).to eq('')
+            subject.each do |d|
+              diff = d
+            end
+
+            expect(diff.diff).to eq('')
+          end
         end
 
         context 'when go over safe limits on files' do