diff --git a/changelogs/unreleased/31983-increase-merge-request-diff-file-size-limit-for-default-toggle-opening.yml b/changelogs/unreleased/31983-increase-merge-request-diff-file-size-limit-for-default-toggle-opening.yml
new file mode 100644
index 0000000000000000000000000000000000000000..f61aa0a6b6e905f05be6b661b5327bb6cdb132a4
--- /dev/null
+++ b/changelogs/unreleased/31983-increase-merge-request-diff-file-size-limit-for-default-toggle-opening.yml
@@ -0,0 +1,4 @@
+---
+title: Increase individual diff collapse limit to 100 KB, and render limit to 200 KB
+merge_request:
+author:
diff --git a/lib/feature.rb b/lib/feature.rb
index 2e2b343f82cef27db730adb4182e3e907ba5f2ae..5650a1c13343682b6a9f05506c8550b7c15b1cfb 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -27,6 +27,18 @@ class Feature
all.map(&:name).include?(feature.name)
end
+ def enabled?(key)
+ get(key).enabled?
+ end
+
+ def enable(key)
+ get(key).enable
+ end
+
+ def disable(key)
+ get(key).disable
+ end
+
private
def flipper
diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb
index ccccca965950912380c52c15a26c81745913fc42..6e85b14d5db008b4f73fc4532b54d17443bc8ea6 100644
--- a/lib/gitlab/git/diff.rb
+++ b/lib/gitlab/git/diff.rb
@@ -20,13 +20,25 @@ module Gitlab
# We need this accessor because of `to_hash` and `init_from_hash`
attr_accessor :too_large
- # The maximum size of a diff to display.
- SIZE_LIMIT = 100.kilobytes
+ class << self
+ # The maximum size of a diff to display.
+ def size_limit
+ if Feature.enabled?('gitlab_git_diff_size_limit_increase')
+ 200.kilobytes
+ else
+ 100.kilobytes
+ end
+ end
- # The maximum size before a diff is collapsed.
- COLLAPSE_LIMIT = 10.kilobytes
+ # The maximum size before a diff is collapsed.
+ def collapse_limit
+ if Feature.enabled?('gitlab_git_diff_size_limit_increase')
+ 100.kilobytes
+ else
+ 10.kilobytes
+ end
+ end
- class << self
def between(repo, head, base, options = {}, *paths)
straight = options.delete(:straight) || false
@@ -231,7 +243,7 @@ module Gitlab
def too_large?
if @too_large.nil?
- @too_large = @diff.bytesize >= SIZE_LIMIT
+ @too_large = @diff.bytesize >= self.class.size_limit
else
@too_large
end
@@ -246,7 +258,7 @@ module Gitlab
def collapsed?
return @collapsed if defined?(@collapsed)
- @collapsed = !expanded && @diff.bytesize >= COLLAPSE_LIMIT
+ @collapsed = !expanded && @diff.bytesize >= self.class.collapse_limit
end
def collapse!
@@ -318,14 +330,14 @@ module Gitlab
hunk.each_line do |line|
size += line.content.bytesize
- if size >= SIZE_LIMIT
+ if size >= self.class.size_limit
too_large!
return true
end
end
end
- if !expanded && size >= COLLAPSE_LIMIT
+ if !expanded && size >= self.class.collapse_limit
collapse!
return true
end
diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb
index 0cb75538311e215ea8241f9af6ba2b88d5143cec..c4d5077e5e1d4b3ba6b92ba746ce8cbbc95c5f54 100644
--- a/spec/features/expand_collapse_diffs_spec.rb
+++ b/spec/features/expand_collapse_diffs_spec.rb
@@ -5,6 +5,11 @@ feature 'Expand and collapse diffs', js: true, feature: true do
let(:project) { create(:project, :repository) }
before do
+ # Set the limits to those when these specs were written, to avoid having to
+ # update the test repo every time we change them.
+ allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes)
+ allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(10.kilobytes)
+
login_as :admin
# Ensure that undiffable.md is in .gitattributes
@@ -62,18 +67,6 @@ feature 'Expand and collapse diffs', js: true, feature: true do
expect(small_diff).not_to have_selector('.nothing-here-block')
end
- it 'collapses large diffs by default' do
- expect(large_diff).not_to have_selector('.code')
- expect(large_diff).to have_selector('.nothing-here-block')
- end
-
- it 'collapses large diffs for renamed files by default' do
- expect(large_diff_renamed).not_to have_selector('.code')
- expect(large_diff_renamed).to have_selector('.nothing-here-block')
- expect(large_diff_renamed).to have_selector('.js-file-title .deletion')
- expect(large_diff_renamed).to have_selector('.js-file-title .addition')
- end
-
it 'shows non-renderable diffs as such immediately, regardless of their size' do
expect(undiffable).not_to have_selector('.code')
expect(undiffable).to have_selector('.nothing-here-block')
diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb
index 7f669565085a767fbe6516a435ec1c2da725ed36..27e2d5d16f3712000f7eeab574ed8b9521c31034 100644
--- a/spec/features/merge_requests/conflicts_spec.rb
+++ b/spec/features/merge_requests/conflicts_spec.rb
@@ -100,7 +100,7 @@ feature 'Merge request conflict resolution', js: true, feature: true do
context 'in Parallel view mode' do
before do
- click_link('conflicts', href: /\/conflicts\Z/)
+ click_link('conflicts', href: /\/conflicts\Z/)
click_button 'Side-by-side'
end
diff --git a/spec/features/projects/compare_spec.rb b/spec/features/projects/compare_spec.rb
index 4162f2579d11720bbea7f9de0ad7ce1284cc957f..ee6985ad993dc81552193d5b6c927e113cfbe422 100644
--- a/spec/features/projects/compare_spec.rb
+++ b/spec/features/projects/compare_spec.rb
@@ -24,6 +24,7 @@ describe "Compare", js: true do
expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("binary-encoding")
click_button "Compare"
+
expect(page).to have_content "Commits"
end
diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb
index 3565e719ad3512d7d13c15b3c71e8a5466c38925..a9a7bba2c054701e4f63910ac282616ff3ebde73 100644
--- a/spec/lib/gitlab/git/diff_collection_spec.rb
+++ b/spec/lib/gitlab/git/diff_collection_spec.rb
@@ -341,7 +341,8 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
end
context 'when diff is quite large will collapse by default' do
- let(:iterator) { [{ diff: 'a' * 20480 }] }
+ let(:iterator) { [{ diff: 'a' * (Gitlab::Git::Diff.collapse_limit + 1) }] }
+ let(:max_files) { 100 }
context 'when no collapse is set' do
let(:expanded) { true }
diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb
index 8e24168ad713edba850159950c47310d957fcb39..6cff89b2f04171cd5a08f8bc20d4dcd8bf3a49f2 100644
--- a/spec/lib/gitlab/git/diff_spec.rb
+++ b/spec/lib/gitlab/git/diff_spec.rb
@@ -31,6 +31,36 @@ EOT
[".gitmodules"]).patches.first
end
+ describe 'size limit feature toggles' do
+ context 'when the feature gitlab_git_diff_size_limit_increase is enabled' do
+ before do
+ Feature.enable('gitlab_git_diff_size_limit_increase')
+ end
+
+ it 'returns 200 KB for size_limit' do
+ expect(described_class.size_limit).to eq(200.kilobytes)
+ end
+
+ it 'returns 100 KB for collapse_limit' do
+ expect(described_class.collapse_limit).to eq(100.kilobytes)
+ end
+ end
+
+ context 'when the feature gitlab_git_diff_size_limit_increase is disabled' do
+ before do
+ Feature.disable('gitlab_git_diff_size_limit_increase')
+ end
+
+ it 'returns 100 KB for size_limit' do
+ expect(described_class.size_limit).to eq(100.kilobytes)
+ end
+
+ it 'returns 10 KB for collapse_limit' do
+ expect(described_class.collapse_limit).to eq(10.kilobytes)
+ end
+ end
+ end
+
describe '.new' do
context 'using a Hash' do
context 'with a small diff' do
@@ -47,7 +77,7 @@ EOT
context 'using a diff that is too large' do
it 'prunes the diff' do
- diff = described_class.new(diff: 'a' * 204800)
+ diff = described_class.new(diff: 'a' * (described_class.size_limit + 1))
expect(diff.diff).to be_empty
expect(diff).to be_too_large
@@ -85,8 +115,8 @@ EOT
# The patch total size is 200, with lines between 21 and 54.
# This is a quick-and-dirty way to test this. Ideally, a new patch is
# added to the test repo with a size that falls between the real limits.
- stub_const("#{described_class}::SIZE_LIMIT", 150)
- stub_const("#{described_class}::COLLAPSE_LIMIT", 100)
+ allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(150)
+ allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(100)
end
it 'prunes the diff as a large diff instead of as a collapsed diff' do
@@ -299,7 +329,7 @@ EOT
describe '#collapsed?' do
it 'returns true for a diff that is quite large' do
- diff = described_class.new({ diff: 'a' * 20480 }, expanded: false)
+ diff = described_class.new({ diff: 'a' * (described_class.collapse_limit + 1) }, expanded: false)
expect(diff).to be_collapsed
end