Commit 6ae80d0e authored by Nick Thomas's avatar Nick Thomas

Merge branch '219565-increase-limit-for-number-of-files-loaded-for-a-single-diff-be' into 'master'

Resolve "Increase limit for number of files loaded for a single diff"

See merge request gitlab-org/gitlab!40357
parents a6c21f57 0423b6f9
......@@ -29,12 +29,6 @@ class Commit
delegate :repository, to: :container
delegate :project, to: :repository, allow_nil: true
DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines]
# Commits above this size will not be rendered in HTML
DIFF_HARD_LIMIT_FILES = 1000
DIFF_HARD_LIMIT_LINES = 50000
MIN_SHA_LENGTH = Gitlab::Git::Commit::MIN_SHA_LENGTH
COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze
EXACT_COMMIT_SHA_PATTERN = /\A#{COMMIT_SHA_PATTERN}\z/.freeze
......@@ -80,10 +74,30 @@ class Commit
sha[0..MIN_SHA_LENGTH]
end
def max_diff_options
def diff_safe_lines
Gitlab::Git::DiffCollection.default_limits[:max_lines]
end
def diff_hard_limit_files(project: nil)
if Feature.enabled?(:increased_diff_limits, project)
2000
else
1000
end
end
def diff_hard_limit_lines(project: nil)
if Feature.enabled?(:increased_diff_limits, project)
75000
else
50000
end
end
def max_diff_options(project: nil)
{
max_files: DIFF_HARD_LIMIT_FILES,
max_lines: DIFF_HARD_LIMIT_LINES
max_files: diff_hard_limit_files(project: project),
max_lines: diff_hard_limit_lines(project: project)
}
end
......
......@@ -631,7 +631,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 || diffs.real_size
merge_request_diff&.real_size || diff_stats&.real_size(project: project) || diffs.real_size
end
def modified_paths(past_merge_request_diff: nil, fallback_on_overflow: false)
......
......@@ -651,7 +651,7 @@ class MergeRequestDiff < ApplicationRecord
if compare.commits.empty?
new_attributes[:state] = :empty
else
diff_collection = compare.diffs(Commit.max_diff_options)
diff_collection = compare.diffs(Commit.max_diff_options(project: merge_request.project))
new_attributes[:real_size] = diff_collection.real_size
if diff_collection.any?
......
- too_big = diff_file.diff_lines.count > Commit::DIFF_SAFE_LINES
- too_big = diff_file.diff_lines.count > Commit.diff_safe_lines
- if too_big
.suppressed-container
%a.show-suppressed-diff.cursor-pointer.js-show-suppressed-diff= _("Changes suppressed. Click to show.")
......
---
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
group: group::source_code
type: development
default_enabled: false
\ No newline at end of file
......@@ -11,7 +11,7 @@ module Gitlab
super(merge_request_diff,
project: merge_request_diff.project,
diff_options: diff_options,
diff_options: merged_diff_options(diff_options),
diff_refs: merge_request_diff.diff_refs,
fallback_diff_refs: merge_request_diff.fallback_diff_refs)
end
......@@ -64,6 +64,11 @@ module Gitlab
diff_stats_cache.read || super
end
end
def merged_diff_options(diff_options)
max_diff_options = ::Commit.max_diff_options(project: @merge_request_diff.project)
diff_options.present? ? diff_options.merge(max_diff_options) : max_diff_options
end
end
end
end
......
......@@ -7,19 +7,23 @@ module Gitlab
class DiffCollection
include Enumerable
DEFAULT_LIMITS = { max_files: 100, max_lines: 5000 }.freeze
attr_reader :limits
delegate :max_files, :max_lines, :max_bytes, :safe_max_files, :safe_max_lines, :safe_max_bytes, to: :limits
def self.default_limits
{ max_files: 100, max_lines: 5000 }
end
def self.limits(options = {})
limits = {}
limits[:max_files] = options.fetch(:max_files, DEFAULT_LIMITS[:max_files])
limits[:max_lines] = options.fetch(:max_lines, DEFAULT_LIMITS[:max_lines])
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
limits[:safe_max_files] = [limits[:max_files], DEFAULT_LIMITS[:max_files]].min
limits[:safe_max_lines] = [limits[:max_lines], DEFAULT_LIMITS[:max_lines]].min
limits[:safe_max_files] = [limits[:max_files], defaults[:max_files]].min
limits[:safe_max_lines] = [limits[:max_lines], defaults[:max_lines]].min
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
......
......@@ -22,8 +22,8 @@ module Gitlab
@collection.map(&:path)
end
def real_size
max_files = ::Commit.max_diff_options[:max_files]
def real_size(project: nil)
max_files = ::Commit.max_diff_options(project: project)[:max_files]
if paths.size > max_files
"#{max_files}+"
else
......
......@@ -7,6 +7,7 @@ RSpec.describe 'Expand and collapse diffs', :js do
let(:project) { create(:project, :repository) }
before do
stub_feature_flags(increased_diff_limits: false)
sign_in(create(:admin))
# Ensure that undiffable.md is in .gitattributes
......
......@@ -7,6 +7,7 @@ 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)
visit(diffs_project_merge_request_path(project, merge_request))
wait_for_requests
......
......@@ -531,7 +531,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do
let(:iterator) { [fake_diff(1, 1)] * 4 }
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: 2, max_lines: max_lines })
allow(Gitlab::Git::DiffCollection)
.to receive(:default_limits)
.and_return({ max_files: 2, max_lines: max_lines })
end
it 'prunes diffs by default even little ones' do
......@@ -556,7 +558,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do
end
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: max_files, max_lines: 80 })
allow(Gitlab::Git::DiffCollection)
.to receive(:default_limits)
.and_return({ max_files: max_files, max_lines: 80 })
end
it 'prunes diffs by default even little ones' do
......@@ -581,7 +585,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do
end
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: max_files, max_lines: 80 })
allow(Gitlab::Git::DiffCollection)
.to receive(:default_limits)
.and_return({ max_files: max_files, max_lines: 80 })
end
it 'prunes diffs by default even little ones' do
......@@ -665,8 +671,9 @@ RSpec.describe Gitlab::Git::DiffCollection, :seed_helper do
end
before do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS',
{ max_files: max_files, max_lines: 80 })
allow(Gitlab::Git::DiffCollection)
.to receive(:default_limits)
.and_return({ max_files: max_files, max_lines: 80 })
end
it 'considers size of diffs before the offset for prunning' do
......
......@@ -13,6 +13,10 @@ 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(
......
......@@ -1140,7 +1140,7 @@ RSpec.describe API::MergeRequests do
context 'when a merge request has more than the changes limit' do
it "returns a string indicating that more changes were made" do
stub_const('Commit::DIFF_HARD_LIMIT_FILES', 5)
allow(Commit).to receive(:diff_hard_limit_files).and_return(5)
merge_request_overflow = create(:merge_request, :simple,
author: user,
......
......@@ -402,7 +402,9 @@ RSpec.describe API::Repositories do
end
it "returns an empty string when the diff overflows" do
stub_const('Gitlab::Git::DiffCollection::DEFAULT_LIMITS', { max_files: 2, max_lines: 2 })
allow(Gitlab::Git::DiffCollection)
.to receive(:default_limits)
.and_return({ max_files: 2, max_lines: 2 })
get api(route, current_user), params: { from: 'master', to: 'feature' }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment