Diffs: Model files newly returned from gitaly

Gitaly's https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5680
allows for returning information about all files in
a diff, not just those that do not exceed file limitations.
Add handling for this in Gitlab to request the metadata for diffs that
exceed set limits and ensure diff collections process
the added metadata as diffs to be stored.

lib/gitlab/git/diff#Diff: Add overflow attr

gitlab/git/diff.rb#Diff: make `collect_patch_overage` a predicate method

gitlab/git/diff.rb#Diff: prune guard on overflow

gitlab/git/diff.rb#Diff: fixup typo

gitlab/git/diff_collection.rb#DiffCollection: force consideration of feature flag

gitlab/git/diff: Review formatting suggestions

gitlab/git/diff_collection: make collect_all_paths a class method

lib/gitlab/git/diff.rb: Reduce one off method useage

spec/lib/gitlab/git/diff_spec.rb: Correct constructor signature

spec/lib/gitlab/git/diff_spec.rb: rubocop

lib/gitlab/git/diff.rb: rm dangling ref

spec/lib/gitlab/git/diff_spec.rb: simulate patch pruned by gitaly

config/feature_flags/development/collect_all_diff_paths.yml

+---
+name: collect_all_diff_paths
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124907
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/421460
+milestone: '16.4'
+type: development
+group: "group::gitaly"
+default_enabled: false

lib/gitlab/git/diff.rb

@@ -81,7 +81,7 @@ def between(repo, head, base, options = {}, *paths)
         #    exceeded
         def filter_diff_options(options, default_options = {})
           allowed_options = [:ignore_whitespace_change, :max_files, :max_lines,
-                             :limits, :expanded]
+                             :limits, :expanded, :collect_all_paths]
 
           if default_options
             actual_defaults = default_options.dup
@@ -126,6 +126,10 @@ def patch_safe_limit_bytes(limit = patch_hard_limit_bytes)
           limit / 10
         end
 
+        def collect_patch_overage?
+          !!Feature.enabled?(:collect_all_diff_paths)
+        end
+
         # Returns the limit for a single diff file (patch).
         #
         # Patches surpassing this limit shouldn't be persisted in the database
@@ -198,9 +202,13 @@ def too_large?
       # This is used by `to_hash` and `init_from_hash`.
       alias_method :too_large, :too_large?
 
-      def too_large!
+      def prune!
         @diff = ''
         @line_count = 0
+      end
+
+      def too_large!
+        prune!
         @too_large = true
       end
 
@@ -211,11 +219,19 @@ def collapsed?
       end
 
       def collapse!
-        @diff = ''
-        @line_count = 0
+        prune!
         @collapsed = true
       end
 
+      def overflow?
+        return @overflow if defined?(@overflow)
+
+        # If overflow is not defined, we're
+        # not recieveing a diff from Gitaly
+        # and overflow has no meaning
+        false
+      end
+
       def json_safe_diff
         return @diff unless detect_binary?(@diff)
 
@@ -248,7 +264,7 @@ def init_from_hash(hash)
       end
 
       def init_from_gitaly(gitaly_diff)
-        @diff = gitaly_diff.respond_to?(:patch) ? encode!(gitaly_diff.patch) : ''
+        @diff = gitaly_diff.try(:patch).present? ? encode!(gitaly_diff.patch) : ''
         @new_path = encode!(gitaly_diff.to_path.dup)
         @old_path = encode!(gitaly_diff.from_path.dup)
         @a_mode = gitaly_diff.old_mode.to_s(8)
@@ -257,11 +273,19 @@ def init_from_gitaly(gitaly_diff)
         @renamed_file = gitaly_diff.from_path != gitaly_diff.to_path
         @deleted_file = gitaly_diff.to_id == BLANK_SHA
         @too_large = gitaly_diff.too_large if gitaly_diff.respond_to?(:too_large)
+        gitaly_overflow = gitaly_diff.try(:overflow_marker)
+        @overflow = Diff.collect_patch_overage? && gitaly_overflow
 
         collapse! if gitaly_diff.respond_to?(:collapsed) && gitaly_diff.collapsed
+        # Diffs exceeding limits returned from gitaly when "collect_all_paths" are enabled
+        # are already pruned, but should be "collapsed" as they have no content
+        @collapsed = true if @overflow
       end
 
       def prune_diff_if_eligible
+        # If we have overflow, diffs are already pruned, retain line counts
+        return if overflow?
+
         if too_large?
           ::Gitlab::Metrics.add_event(:patch_hard_limit_bytes_hit)
 

lib/gitlab/git/diff_collection.rb

@@ -13,6 +13,10 @@ def self.default_limits
         { max_files: ::Commit.diff_safe_max_files, max_lines: ::Commit.diff_safe_max_lines }
       end
 
+      def self.collect_all_paths?(collect_all_paths)
+        Gitlab::Git::Diff.collect_patch_overage? ? collect_all_paths : false
+      end
+
       def self.limits(options = {})
         limits = {}
         defaults = default_limits
@@ -25,7 +29,7 @@ def self.limits(options = {})
         limits[:safe_max_bytes] = limits[:safe_max_files] * 5.kilobytes # Average 5 KB per file
         limits[:max_patch_bytes] = Gitlab::Git::Diff.patch_hard_limit_bytes
         limits[:max_patch_bytes_for_file_extension] = options.fetch(:max_patch_bytes_for_file_extension, {})
-
+        limits[:collect_all_paths] = collect_all_paths?(options.fetch(:collect_all_paths, false))
         limits
       end
 
@@ -164,7 +168,12 @@ def each_gitaly_patch
 
           if raw.overflow_marker
             @overflow = true
-            break
+            # If we're requesting patches with `collect_all_paths` enabled, then
+            # Once we hit the overflow marker, gitlay has still returned diffs, just without
+            # patches, only metadata
+            unless @limits[:collect_all_paths]
+              break
+            end
           end
 
           yield @array[i] = diff

spec/lib/gitlab/git/diff_spec.rb

@@ -131,6 +131,31 @@
           expect(diff.diff).to be_utf8
         end
       end
+
+      context 'using a diff that it too large but collecting all paths' do
+        let(:gitaly_diff) do
+          Gitlab::GitalyClient::Diff.new(
+            from_path: '.gitmodules',
+            to_path: '.gitmodules',
+            old_mode: 0100644,
+            new_mode: 0100644,
+            from_id: '0792c58905eff3432b721f8c4a64363d8e28d9ae',
+            to_id: 'efd587ccb47caf5f31fc954edb21f0a713d9ecc3',
+            overflow_marker: true,
+            collapsed: false,
+            too_large: false,
+            patch: ''
+          )
+        end
+
+        let(:diff) { described_class.new(gitaly_diff) }
+
+        it 'is already pruned and collapsed but not too large' do
+          expect(diff.diff).to be_empty
+          expect(diff).not_to be_too_large
+          expect(diff).to be_collapsed
+        end
+      end
     end
 
     context 'using a Gitaly::CommitDelta' do