Merge branch 'remove_diff_size_feature_flags' into 'master'

Remove diff size related feature flags [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!64228

app/models/commit.rb

@@ -84,43 +84,27 @@ class Commit
       sha[0..MIN_SHA_LENGTH]
     end
 
-    def diff_safe_lines(project: nil)
-      diff_safe_max_lines(project: project)
-    end
-
-    def diff_max_files(project: nil)
-      if Feature.enabled?(:increased_diff_limits, project)
-        3000
-      elsif Feature.enabled?(:configurable_diff_limits, project)
+    def diff_max_files
       Gitlab::CurrentSettings.diff_max_files
-      else
-        1000
-      end
     end
 
-    def diff_max_lines(project: nil)
-      if Feature.enabled?(:increased_diff_limits, project)
-        100000
-      elsif Feature.enabled?(:configurable_diff_limits, project)
+    def diff_max_lines
       Gitlab::CurrentSettings.diff_max_lines
-      else
-        50000
-      end
     end
 
-    def max_diff_options(project: nil)
+    def max_diff_options
       {
-        max_files: diff_max_files(project: project),
-        max_lines: diff_max_lines(project: project)
+        max_files: diff_max_files,
+        max_lines: diff_max_lines
       }
     end
 
-    def diff_safe_max_files(project: nil)
-      diff_max_files(project: project) / DIFF_SAFE_LIMIT_FACTOR
+    def diff_safe_max_files
+      diff_max_files / DIFF_SAFE_LIMIT_FACTOR
     end
 
-    def diff_safe_max_lines(project: nil)
-      diff_max_lines(project: project) / DIFF_SAFE_LIMIT_FACTOR
+    def diff_safe_max_lines
+      diff_max_lines / DIFF_SAFE_LIMIT_FACTOR
     end
 
     def from_hash(hash, container)

app/models/merge_request.rb

@@ -768,7 +768,7 @@ class MergeRequest < ApplicationRecord
   def diff_size
     # Calling `merge_request_diff.diffs.real_size` will also perform
     # highlighting, which we don't need here.
-    merge_request_diff&.real_size || diff_stats&.real_size(project: project) || diffs.real_size
+    merge_request_diff&.real_size || diff_stats&.real_size || diffs.real_size
   end
 
   def modified_paths(past_merge_request_diff: nil, fallback_on_overflow: false)

app/models/merge_request_diff.rb

@@ -719,7 +719,7 @@ class MergeRequestDiff < ApplicationRecord
     if compare.commits.empty?
       new_attributes[:state] = :empty
     else
-      diff_collection = compare.diffs(Commit.max_diff_options(project: merge_request.project))
+      diff_collection = compare.diffs(Commit.max_diff_options)
       new_attributes[:real_size] = diff_collection.real_size
 
       if diff_collection.any?

app/views/projects/diffs/_text_file.html.haml

-- too_big = diff_file.diff_lines.count > Commit.diff_safe_lines(project: @project)
+- too_big = diff_file.diff_lines.count > Commit.diff_safe_max_lines
 - if too_big
   .suppressed-container
     %a.show-suppressed-diff.cursor-pointer.js-show-suppressed-diff= _("Changes suppressed. Click to show.")

config/feature_flags/development/configurable_diff_limits.yml

----
-name: configurable_diff_limits
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56722
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332194
-milestone: '14.0'
-type: development
-group: group::code review
-default_enabled: false

config/feature_flags/development/increased_diff_limits.yml

----
-name: increased_diff_limits
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40357
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/241185
-milestone: '13.5'
-type: development
-group: group::code review
-default_enabled: false

lib/gitlab/diff/file_collection/merge_request_diff_base.rb

@@ -13,7 +13,7 @@ module Gitlab
 
           super(merge_request_diff,
             project: merge_request_diff.project,
-            diff_options: merged_diff_options(diff_options),
+            diff_options: diff_options,
             diff_refs: merge_request_diff.diff_refs,
             fallback_diff_refs: merge_request_diff.fallback_diff_refs)
         end
@@ -68,13 +68,6 @@ module Gitlab
             diff_stats_cache.read || super
           end
         end
-
-        def merged_diff_options(diff_options)
-          project = @merge_request_diff.project
-          max_diff_options = ::Commit.max_diff_options(project: project).merge(project: project)
-
-          diff_options.present? ? diff_options.merge(max_diff_options) : max_diff_options
-        end
       end
     end
   end

lib/gitlab/git/diff_collection.rb

@@ -11,13 +11,13 @@ module Gitlab
 
       delegate :max_files, :max_lines, :max_bytes, :safe_max_files, :safe_max_lines, :safe_max_bytes, to: :limits
 
-      def self.default_limits(project: nil)
-        { max_files: ::Commit.diff_safe_max_files(project: project), max_lines: ::Commit.diff_safe_max_lines(project: project) }
+      def self.default_limits
+        { max_files: ::Commit.diff_safe_max_files, max_lines: ::Commit.diff_safe_max_lines }
       end
 
       def self.limits(options = {})
         limits = {}
-        defaults = default_limits(project: options[:project])
+        defaults = default_limits
         limits[:max_files] = options.fetch(:max_files, defaults[:max_files])
         limits[:max_lines] = options.fetch(:max_lines, defaults[:max_lines])
         limits[:max_bytes] = limits[:max_files] * 5.kilobytes # Average 5 KB per file

lib/gitlab/git/diff_stats_collection.rb

@@ -22,8 +22,8 @@ module Gitlab
         @collection.map(&:path)
       end
 
-      def real_size(project: nil)
-        max_files = ::Commit.max_diff_options(project: project)[:max_files]
+      def real_size
+        max_files = ::Commit.diff_max_files
         if paths.size > max_files
           "#{max_files}+"
         else

spec/features/expand_collapse_diffs_spec.rb

@@ -7,7 +7,6 @@ RSpec.describe 'Expand and collapse diffs', :js do
   let(:project) { create(:project, :repository) }
 
   before do
-    stub_feature_flags(increased_diff_limits: false)
     allow(Gitlab::CurrentSettings).to receive(:diff_max_patch_bytes).and_return(100.kilobytes)
 
     admin = create(:admin)

spec/features/merge_request/user_expands_diff_spec.rb

@@ -7,7 +7,6 @@ RSpec.describe 'User expands diff', :js do
   let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) }
 
   before do
-    stub_feature_flags(increased_diff_limits: false)
     allow(Gitlab::CurrentSettings).to receive(:diff_max_patch_bytes).and_return(100.kilobytes)
 
     visit(diffs_project_merge_request_path(project, merge_request))

spec/lib/gitlab/git/diff_stats_collection_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe Gitlab::Git::DiffStatsCollection do
     end
 
     it 'returns capped number when it is bigger than max_files' do
-      allow(::Commit).to receive(:max_diff_options).and_return(max_files: 1)
+      allow(::Commit).to receive(:diff_max_files).and_return(1)
 
       expect(collection.real_size).to eq('1+')
     end

spec/lib/gitlab/gitaly_client/commit_service_spec.rb

@@ -13,10 +13,6 @@ RSpec.describe Gitlab::GitalyClient::CommitService do
   let(:client) { described_class.new(repository) }
 
   describe '#diff_from_parent' do
-    before do
-      stub_feature_flags(increased_diff_limits: false)
-    end
-
     context 'when a commit has a parent' do
       it 'sends an RPC request with the parent ID as left commit' do
         request = Gitaly::CommitDiffRequest.new(

spec/models/commit_spec.rb

@@ -676,71 +676,21 @@ eos
   describe '.diff_max_files' do
     subject(:diff_max_files) { described_class.diff_max_files }
 
-    let(:increased_diff_limits) { false }
-    let(:configurable_diff_limits) { false }
-
-    before do
-      stub_feature_flags(increased_diff_limits: increased_diff_limits, configurable_diff_limits: configurable_diff_limits)
-    end
-
-    context 'when increased_diff_limits is enabled' do
-      let(:increased_diff_limits) { true }
-
-      it 'returns 3000' do
-        expect(diff_max_files).to eq(3000)
-      end
-    end
-
-    context 'when configurable_diff_limits is enabled' do
-      let(:configurable_diff_limits) { true }
-
     it 'returns the current settings' do
       Gitlab::CurrentSettings.update!(diff_max_files: 1234)
       expect(diff_max_files).to eq(1234)
     end
   end
 
-    context 'when neither feature flag is enabled' do
-      it 'returns 1000' do
-        expect(diff_max_files).to eq(1000)
-      end
-    end
-  end
-
   describe '.diff_max_lines' do
     subject(:diff_max_lines) { described_class.diff_max_lines }
 
-    let(:increased_diff_limits) { false }
-    let(:configurable_diff_limits) { false }
-
-    before do
-      stub_feature_flags(increased_diff_limits: increased_diff_limits, configurable_diff_limits: configurable_diff_limits)
-    end
-
-    context 'when increased_diff_limits is enabled' do
-      let(:increased_diff_limits) { true }
-
-      it 'returns 100000' do
-        expect(diff_max_lines).to eq(100000)
-      end
-    end
-
-    context 'when configurable_diff_limits is enabled' do
-      let(:configurable_diff_limits) { true }
-
     it 'returns the current settings' do
       Gitlab::CurrentSettings.update!(diff_max_lines: 65321)
       expect(diff_max_lines).to eq(65321)
     end
   end
 
-    context 'when neither feature flag is enabled' do
-      it 'returns 50000' do
-        expect(diff_max_lines).to eq(50000)
-      end
-    end
-  end
-
   describe '.diff_safe_max_files' do
     subject(:diff_safe_max_files) { described_class.diff_safe_max_files }
 

spec/models/merge_request_spec.rb

@@ -1132,7 +1132,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
       end
 
       it 'returns the correct overflow count' do
-        allow(Commit).to receive(:max_diff_options).and_return(max_files: 2)
+        allow(Commit).to receive(:diff_max_files).and_return(2)
         set_compare(merge_request)
 
         expect(merge_request.diff_size).to eq('2+')