diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-05-26 18:27:30 -0500 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-05-29 17:02:02 -0500 |
commit | aed0387f97ec62b184da90bdca0ce40f9dc58b45 (patch) | |
tree | 54223237527ddbcaa93bfea552f2fda2b6c1c61f | |
parent | 1ac12698c613774bdace72475573916c142a07e4 (diff) | |
download | gitlab-ce-aed0387f97ec62b184da90bdca0ce40f9dc58b45.tar.gz |
Consistent diff and blob size limit names
32 files changed, 153 insertions, 166 deletions
diff --git a/app/controllers/concerns/renders_blob.rb b/app/controllers/concerns/renders_blob.rb index 1d37e4cb3bd..54dcd7c61ce 100644 --- a/app/controllers/concerns/renders_blob.rb +++ b/app/controllers/concerns/renders_blob.rb @@ -18,7 +18,7 @@ module RendersBlob } end - def override_max_blob_size(blob) - blob.override_max_size! if params[:override_max_size] == 'true' + def conditionally_expand_blob(blob) + blob.expand! if params[:expanded] == 'true' end end diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 1224e9503c9..a146526d088 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -27,7 +27,7 @@ class Projects::ArtifactsController < Projects::ApplicationController def file blob = @entry.blob - override_max_blob_size(blob) + conditionally_expand_blob(blob) respond_to do |format| format.html do diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 87721fbe2f5..7025c7a1de6 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -35,7 +35,7 @@ class Projects::BlobController < Projects::ApplicationController end def show - override_max_blob_size(@blob) + conditionally_expand_blob(@blob) respond_to do |format| format.html do diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index 3b2b0d9e502..3a97c1e98af 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -56,7 +56,7 @@ class Projects::SnippetsController < Projects::ApplicationController def show blob = @snippet.blob - override_max_blob_size(blob) + conditionally_expand_blob(blob) respond_to do |format| format.html do diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 7445f61195d..5b2d143ee79 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -58,7 +58,7 @@ class SnippetsController < ApplicationController def show blob = @snippet.blob - override_max_blob_size(blob) + conditionally_expand_blob(blob) @note = Note.new(noteable: @snippet) @noteable = @snippet diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 622e14e21ff..63a4d215113 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -240,14 +240,10 @@ module BlobHelper def blob_render_error_reason(viewer) case viewer.render_error + when :collapsed + "it is larger than #{number_to_human_size(viewer.collapse_limit)}" when :too_large - max_size = - if viewer.can_override_max_size? - viewer.overridable_max_size - else - viewer.max_size - end - "it is larger than #{number_to_human_size(max_size)}" + "it is larger than #{number_to_human_size(viewer.size_limit)}" when :server_side_but_stored_externally case viewer.blob.external_storage when :lfs @@ -264,8 +260,8 @@ module BlobHelper error = viewer.render_error options = [] - if error == :too_large && viewer.can_override_max_size? - options << link_to('load it anyway', url_for(params.merge(viewer: viewer.type, override_max_size: true, format: nil))) + if error == :collapsed + options << link_to('load it anyway', url_for(params.merge(viewer: viewer.type, expanded: true, format: nil))) end # If the error is `:server_side_but_stored_externally`, the simple viewer will show the same error, diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 4c4fbdd4d39..0726dc6eafd 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -8,8 +8,8 @@ module DiffHelper [marked_old_line, marked_new_line] end - def expand_all_diffs? - params[:expand_all_diffs].present? + def diffs_expanded? + params[:expanded].present? end def diff_view @@ -22,10 +22,10 @@ module DiffHelper end def diff_options - options = { ignore_whitespace_change: hide_whitespace?, no_collapse: expand_all_diffs? } + options = { ignore_whitespace_change: hide_whitespace?, expanded: diffs_expanded? } if action_name == 'diff_for_path' - options[:no_collapse] = true + options[:expanded] = true options[:paths] = params.values_at(:old_path, :new_path) end diff --git a/app/models/blob.rb b/app/models/blob.rb index e75926241ba..6a42a12891c 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -102,10 +102,6 @@ class Blob < SimpleDelegator raw_size == 0 end - def too_large? - size && truncated? - end - def external_storage_error? if external_storage == :lfs !project&.lfs_enabled? @@ -160,7 +156,7 @@ class Blob < SimpleDelegator end def readable_text? - text? && !stored_externally? && !too_large? + text? && !stored_externally? && !truncated? end def simple_viewer @@ -187,9 +183,9 @@ class Blob < SimpleDelegator rendered_as_text? && rich_viewer end - def override_max_size! - simple_viewer&.override_max_size = true - rich_viewer&.override_max_size = true + def expand! + simple_viewer&.expanded = true + rich_viewer&.expanded = true end private diff --git a/app/models/blob_viewer/auxiliary.rb b/app/models/blob_viewer/auxiliary.rb index 07a207730cf..1bea225f17c 100644 --- a/app/models/blob_viewer/auxiliary.rb +++ b/app/models/blob_viewer/auxiliary.rb @@ -7,8 +7,8 @@ module BlobViewer included do self.loading_partial_name = 'loading_auxiliary' self.type = :auxiliary - self.overridable_max_size = 100.kilobytes - self.max_size = 100.kilobytes + self.collapse_limit = 100.kilobytes + self.size_limit = 100.kilobytes end def visible_to?(current_user) diff --git a/app/models/blob_viewer/base.rb b/app/models/blob_viewer/base.rb index 26a3778c2a3..e6119d25fab 100644 --- a/app/models/blob_viewer/base.rb +++ b/app/models/blob_viewer/base.rb @@ -2,14 +2,14 @@ module BlobViewer class Base PARTIAL_PATH_PREFIX = 'projects/blob/viewers'.freeze - class_attribute :partial_name, :loading_partial_name, :type, :extensions, :file_types, :load_async, :binary, :switcher_icon, :switcher_title, :overridable_max_size, :max_size + class_attribute :partial_name, :loading_partial_name, :type, :extensions, :file_types, :load_async, :binary, :switcher_icon, :switcher_title, :collapse_limit, :size_limit self.loading_partial_name = 'loading' delegate :partial_path, :loading_partial_path, :rich?, :simple?, :text?, :binary?, to: :class attr_reader :blob - attr_accessor :override_max_size + attr_accessor :expanded delegate :project, to: :blob @@ -61,24 +61,16 @@ module BlobViewer self.class.load_async? && render_error.nil? end - def exceeds_overridable_max_size? - overridable_max_size && blob.raw_size > overridable_max_size - end - - def exceeds_max_size? - max_size && blob.raw_size > max_size - end + def collapsed? + return @collapsed if defined?(@collapsed) - def can_override_max_size? - exceeds_overridable_max_size? && !exceeds_max_size? + @collapsed = !expanded && collapse_limit && blob.raw_size > collapse_limit end def too_large? - if override_max_size - exceeds_max_size? - else - exceeds_overridable_max_size? - end + return @too_large if defined?(@too_large) + + @too_large = size_limit && blob.raw_size > size_limit end # This method is used on the server side to check whether we can attempt to @@ -95,6 +87,8 @@ module BlobViewer def render_error if too_large? :too_large + elsif collapsed? + :collapsed end end diff --git a/app/models/blob_viewer/client_side.rb b/app/models/blob_viewer/client_side.rb index cc68236f92b..079cfbe3616 100644 --- a/app/models/blob_viewer/client_side.rb +++ b/app/models/blob_viewer/client_side.rb @@ -4,8 +4,8 @@ module BlobViewer included do self.load_async = false - self.overridable_max_size = 10.megabytes - self.max_size = 50.megabytes + self.collapse_limit = 10.megabytes + self.size_limit = 50.megabytes end end end diff --git a/app/models/blob_viewer/server_side.rb b/app/models/blob_viewer/server_side.rb index 87884dcd6bf..05a3dd7d913 100644 --- a/app/models/blob_viewer/server_side.rb +++ b/app/models/blob_viewer/server_side.rb @@ -4,8 +4,8 @@ module BlobViewer included do self.load_async = true - self.overridable_max_size = 2.megabytes - self.max_size = 5.megabytes + self.collapse_limit = 2.megabytes + self.size_limit = 5.megabytes end def prepare! diff --git a/app/models/blob_viewer/text.rb b/app/models/blob_viewer/text.rb index eddca50b4d4..f68cbb7e212 100644 --- a/app/models/blob_viewer/text.rb +++ b/app/models/blob_viewer/text.rb @@ -5,7 +5,7 @@ module BlobViewer self.partial_name = 'text' self.binary = false - self.overridable_max_size = 1.megabyte - self.max_size = 10.megabytes + self.collapse_limit = 1.megabyte + self.size_limit = 10.megabytes end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 356af776b8d..56fec3ca39c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -220,10 +220,10 @@ class MergeRequest < ActiveRecord::Base def diffs(diff_options = {}) if compare - # When saving MR diffs, `no_collapse` is implicitly added (because we need + # When saving MR diffs, `expanded` is implicitly added (because we need # to save the entire contents to the DB), so add that here for # consistency. - compare.diffs(diff_options.merge(no_collapse: true)) + compare.diffs(diff_options.merge(expanded: true)) else merge_request_diff.diffs(diff_options) end diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml index c7e22a0b4ec..59844bc00cd 100644 --- a/app/views/projects/diffs/_content.html.haml +++ b/app/views/projects/diffs/_content.html.haml @@ -3,7 +3,7 @@ .diff-content - if diff_file.too_large? .nothing-here-block This diff could not be displayed because it is too large. - - elsif blob.too_large? + - elsif blob.truncated? .nothing-here-block The file could not be displayed because it is too large. - elsif blob.readable_text? - if !diff_file.repository.diffable?(blob) diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 4768438c29e..d538c4c86c8 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -5,8 +5,8 @@ .content-block.oneline-block.files-changed .inline-parallel-buttons - - if !expand_all_diffs? && diff_files.any? { |diff_file| diff_file.collapsed? } - = link_to 'Expand all', url_for(params.merge(expand_all_diffs: 1, format: nil)), class: 'btn btn-default' + - if !diffs_expanded? && diff_files.any? { |diff_file| diff_file.collapsed? } + = link_to 'Expand all', url_for(params.merge(expanded: 1, format: nil)), class: 'btn btn-default' - if show_whitespace_toggle - if current_controller?(:commit) = commit_diff_whitespace_link(diffs.project, @commit, class: 'hidden-xs') diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 621b9dcecd9..904993d1606 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -176,7 +176,7 @@ module API } if params[:path] - commit.raw_diffs(all_diffs: true).each do |diff| + commit.raw_diffs(no_limits: true).each do |diff| next unless diff.new_path == params[:path] lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 8c5e5c91769..9799fe68a76 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -331,7 +331,7 @@ module API class MergeRequestChanges < MergeRequest expose :diffs, as: :changes, using: Entities::RepoDiff do |compare, _| - compare.raw_diffs(all_diffs: true).to_a + compare.raw_diffs(no_limits: true).to_a end end @@ -344,7 +344,7 @@ module API expose :commits, using: Entities::RepoCommit expose :diffs, using: Entities::RepoDiff do |compare, _| - compare.raw_diffs(all_diffs: true).to_a + compare.raw_diffs(no_limits: true).to_a end end @@ -548,7 +548,7 @@ module API end expose :diffs, using: Entities::RepoDiff do |compare, options| - compare.diffs(all_diffs: true).to_a + compare.diffs(no_limits: true).to_a end expose :compare_timeout do |compare, options| diff --git a/lib/api/v3/commits.rb b/lib/api/v3/commits.rb index 674de592f0a..ec6c6c7b298 100644 --- a/lib/api/v3/commits.rb +++ b/lib/api/v3/commits.rb @@ -167,7 +167,7 @@ module API } if params[:path] - commit.raw_diffs(all_diffs: true).each do |diff| + commit.raw_diffs(no_limits: true).each do |diff| next unless diff.new_path == params[:path] lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 2e1b243c2db..7fbd4949850 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -226,7 +226,7 @@ module API class MergeRequestChanges < MergeRequest expose :diffs, as: :changes, using: ::API::Entities::RepoDiff do |compare, _| - compare.raw_diffs(all_diffs: true).to_a + compare.raw_diffs(no_limits: true).to_a end end diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index 79836a2fbab..a6007ebf531 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -7,7 +7,7 @@ module Gitlab delegate :count, :size, :real_size, to: :diff_files def self.default_options - ::Commit.max_diff_options.merge(ignore_whitespace_change: false, no_collapse: false) + ::Commit.max_diff_options.merge(ignore_whitespace_change: false, expanded: false) end def initialize(diffable, project:, diff_options: nil, diff_refs: nil, fallback_diff_refs: nil) diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 6c69cd9e6a9..ea035e33eff 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -42,7 +42,7 @@ module Gitlab return unless compare # This diff is more moderated in number of files and lines - @diffs ||= compare.diffs(max_files: 30, max_lines: 5000, no_collapse: true).diff_files + @diffs ||= compare.diffs(max_files: 30, max_lines: 5000, expanded: true).diff_files end def diffs_count diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index c1b31618e0d..6b0a66365a7 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -88,6 +88,7 @@ module Gitlab new( id: blob_entry[:oid], name: blob_entry[:name], + size: 0, data: '', path: path, commit_id: sha diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index deade337354..201a115503b 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -15,13 +15,13 @@ module Gitlab alias_method :deleted_file?, :deleted_file alias_method :renamed_file?, :renamed_file - attr_accessor :too_large + attr_accessor :expanded # The maximum size of a diff to display. - DIFF_SIZE_LIMIT = 102400 # 100 KB + SIZE_LIMIT = 100.kilobytes # The maximum size before a diff is collapsed. - DIFF_COLLAPSE_LIMIT = 10240 # 10 KB + COLLAPSE_LIMIT = 10.kilobytes class << self def between(repo, head, base, options = {}, *paths) @@ -152,7 +152,7 @@ module Gitlab :include_untracked_content, :skip_binary_check, :include_typechange, :include_typechange_trees, :ignore_filemode, :recurse_ignored_dirs, :paths, - :max_files, :max_lines, :all_diffs, :no_collapse] + :max_files, :max_lines, :no_limits, :expanded] if default_options actual_defaults = default_options.dup @@ -177,16 +177,18 @@ module Gitlab end end - def initialize(raw_diff, collapse: false) + def initialize(raw_diff, expanded: true) + @expanded = expanded + case raw_diff when Hash init_from_hash(raw_diff) - prune_diff_if_eligible(collapse) + prune_diff_if_eligible when Rugged::Patch, Rugged::Diff::Delta - init_from_rugged(raw_diff, collapse: collapse) + init_from_rugged(raw_diff) when Gitaly::CommitDiffResponse init_from_gitaly(raw_diff) - prune_diff_if_eligible(collapse) + prune_diff_if_eligible when Gitaly::CommitDelta init_from_gitaly(raw_diff) when nil @@ -225,18 +227,12 @@ module Gitlab end def too_large? - if @too_large.nil? - @too_large = @diff.bytesize >= DIFF_SIZE_LIMIT - else - @too_large - end - end + return @too_large if defined?(@too_large) - def collapsible? - @diff.bytesize >= DIFF_COLLAPSE_LIMIT + @too_large = @diff.bytesize >= SIZE_LIMIT end - def prune_large_diff! + def too_large! @diff = '' @line_count = 0 @too_large = true @@ -244,10 +240,11 @@ module Gitlab def collapsed? return @collapsed if defined?(@collapsed) - false + + @collapsed = !expanded && @diff.bytesize >= COLLAPSE_LIMIT end - def prune_collapsed_diff! + def collapse! @diff = '' @line_count = 0 @collapsed = true @@ -255,9 +252,9 @@ module Gitlab private - def init_from_rugged(rugged, collapse: false) + def init_from_rugged(rugged) if rugged.is_a?(Rugged::Patch) - init_from_rugged_patch(rugged, collapse: collapse) + init_from_rugged_patch(rugged) d = rugged.delta else d = rugged @@ -272,10 +269,10 @@ module Gitlab @deleted_file = d.deleted? end - def init_from_rugged_patch(patch, collapse: false) + def init_from_rugged_patch(patch) # Don't bother initializing diffs that are too large. If a diff is # binary we're not going to display anything so we skip the size check. - return if !patch.delta.binary? && prune_large_patch(patch, collapse) + return if !patch.delta.binary? && prune_large_patch(patch) @diff = encode!(strip_diff_headers(patch.to_s)) end @@ -299,29 +296,32 @@ module Gitlab @deleted_file = msg.to_id == BLANK_SHA end - def prune_diff_if_eligible(collapse = false) - prune_large_diff! if too_large? - prune_collapsed_diff! if collapse && collapsible? + def prune_diff_if_eligible + if too_large? + too_large! + elsif collapsed? + collapse! + end end # If the patch surpasses any of the diff limits it calls the appropiate # prune method and returns true. Otherwise returns false. - def prune_large_patch(patch, collapse) + def prune_large_patch(patch) size = 0 patch.each_hunk do |hunk| hunk.each_line do |line| size += line.content.bytesize - if size >= DIFF_SIZE_LIMIT - prune_large_diff! + if size >= SIZE_LIMIT + too_large! return true end end end - if collapse && size >= DIFF_COLLAPSE_LIMIT - prune_collapsed_diff! + if !expanded && size >= COLLAPSE_LIMIT + collapse! return true end diff --git a/lib/gitlab/git/diff_collection.rb b/lib/gitlab/git/diff_collection.rb index 898a5ae15f2..9b217b9080d 100644 --- a/lib/gitlab/git/diff_collection.rb +++ b/lib/gitlab/git/diff_collection.rb @@ -9,12 +9,12 @@ module Gitlab @iterator = iterator @max_files = options.fetch(:max_files, DEFAULT_LIMITS[:max_files]) @max_lines = options.fetch(:max_lines, DEFAULT_LIMITS[:max_lines]) - @max_bytes = @max_files * 5120 # Average 5 KB per file + @max_bytes = @max_files * 5.kilobytes # Average 5 KB per file @safe_max_files = [@max_files, DEFAULT_LIMITS[:max_files]].min @safe_max_lines = [@max_lines, DEFAULT_LIMITS[:max_lines]].min - @safe_max_bytes = @safe_max_files * 5120 # Average 5 KB per file - @all_diffs = !!options.fetch(:all_diffs, false) - @no_collapse = !!options.fetch(:no_collapse, true) + @safe_max_bytes = @safe_max_files * 5.kilobytes # Average 5 KB per file + @no_limits = !!options.fetch(:no_limits, false) + @expanded = !!options.fetch(:expanded, true) @line_count = 0 @byte_count = 0 @@ -88,23 +88,23 @@ module Gitlab @iterator.each do |raw| @empty = false - if !@all_diffs && i >= @max_files + if !@no_limits && i >= @max_files @overflow = true break end - collapse = !@all_diffs && !@no_collapse + expanded = @no_limits || @expanded - diff = Gitlab::Git::Diff.new(raw, collapse: collapse) + diff = Gitlab::Git::Diff.new(raw, expanded: expanded) - if collapse && over_safe_limits?(i) - diff.prune_collapsed_diff! + if !expanded && over_safe_limits?(i) + diff.collapse! end @line_count += diff.line_count @byte_count += diff.diff.bytesize - if !@all_diffs && (@line_count >= @max_lines || @byte_count >= @max_bytes) + if !@no_limits && (@line_count >= @max_lines || @byte_count >= @max_bytes) # This last Diff instance pushes us over the lines limit. We stop and # discard it. @overflow = true diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index 41b5df12522..366c635e8e8 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -118,8 +118,8 @@ describe BlobHelper do Class.new(BlobViewer::Base) do include BlobViewer::ServerSide - self.overridable_max_size = 1.megabyte - self.max_size = 5.megabytes + self.collapse_limit = 1.megabyte + self.size_limit = 5.megabytes self.type = :rich end end @@ -129,7 +129,7 @@ describe BlobHelper do describe '#blob_render_error_reason' do context 'for error :too_large' do - context 'when the blob size is larger than the absolute max size' do + context 'when the blob size is larger than the absolute size limit' do let(:blob) { fake_blob(size: 10.megabytes) } it 'returns an error message' do @@ -137,7 +137,7 @@ describe BlobHelper do end end - context 'when the blob size is larger than the max size' do + context 'when the blob size is larger than the size limit' do let(:blob) { fake_blob(size: 2.megabytes) } it 'returns an error message' do @@ -169,7 +169,7 @@ describe BlobHelper do end context 'for error :too_large' do - context 'when the max size can be overridden' do + context 'when the size limit can be overridden' do let(:blob) { fake_blob(size: 2.megabytes) } it 'includes a "load it anyway" link' do @@ -177,7 +177,7 @@ describe BlobHelper do end end - context 'when the max size cannot be overridden' do + context 'when the size limit cannot be overridden' do let(:blob) { fake_blob(size: 10.megabytes) } it 'does not include a "load it anyway" link' do diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index dd6566d25bb..84341a51f2d 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -33,17 +33,17 @@ describe DiffHelper do describe 'diff_options' do it 'returns no collapse false' do - expect(diff_options).to include(no_collapse: false) + expect(diff_options).to include(expanded: false) end - it 'returns no collapse true if expand_all_diffs' do - allow(controller).to receive(:params) { { expand_all_diffs: true } } - expect(diff_options).to include(no_collapse: true) + it 'returns no collapse true if expanded' do + allow(controller).to receive(:params) { { expanded: true } } + expect(diff_options).to include(expanded: true) end it 'returns no collapse true if action name diff_for_path' do allow(controller).to receive(:action_name) { 'diff_for_path' } - expect(diff_options).to include(no_collapse: true) + expect(diff_options).to include(expanded: true) end it 'returns paths if action name diff_for_path and param old path' do diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index ae617b313c5..d4da3db3dae 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -6,8 +6,8 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do iterator, max_files: max_files, max_lines: max_lines, - all_diffs: all_diffs, - no_collapse: no_collapse + no_limits: no_limits, + expanded: expanded ) end let(:iterator) { MutatingConstantIterator.new(file_count, fake_diff(line_length, line_count)) } @@ -16,8 +16,8 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do let(:line_count) { 1 } let(:max_files) { 10 } let(:max_lines) { 100 } - let(:all_diffs) { false } - let(:no_collapse) { true } + let(:no_limits) { false } + let(:expanded) { true } describe '#to_a' do subject { super().to_a } @@ -75,7 +75,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do end context 'when limiting is disabled' do - let(:all_diffs) { true } + let(:no_limits) { true } describe '#overflow?' do subject { super().overflow? } @@ -123,7 +123,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do it { expect(subject.size).to eq(0) } context 'when limiting is disabled' do - let(:all_diffs) { true } + let(:no_limits) { true } describe '#overflow?' do subject { super().overflow? } @@ -167,7 +167,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do it { expect(subject.size).to eq(10) } context 'when limiting is disabled' do - let(:all_diffs) { true } + let(:no_limits) { true } describe '#overflow?' do subject { super().overflow? } @@ -207,7 +207,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do it { expect(subject.size).to eq(3) } context 'when limiting is disabled' do - let(:all_diffs) { true } + let(:no_limits) { true } describe '#overflow?' do subject { super().overflow? } @@ -273,7 +273,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do it { expect(subject.size).to eq(9) } context 'when limiting is disabled' do - let(:all_diffs) { true } + let(:no_limits) { true } describe '#overflow?' do subject { super().overflow? } @@ -344,7 +344,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do let(:iterator) { [{ diff: 'a' * 20480 }] } context 'when no collapse is set' do - let(:no_collapse) { true } + let(:expanded) { true } it 'yields Diff instances even when they are quite big' do expect { |b| subject.each(&b) }. @@ -363,7 +363,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do end context 'when no collapse is unset' do - let(:no_collapse) { false } + let(:expanded) { false } it 'yields Diff instances even when they are quite big' do expect { |b| subject.each(&b) }. @@ -450,7 +450,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do end context 'when limiting is disabled' do - let(:all_diffs) { true } + let(:no_limits) { true } it 'yields Diff instances even when they are quite big' do expect { |b| subject.each(&b) }. diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 4189aaef643..1a71e3728a1 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -85,8 +85,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}::DIFF_SIZE_LIMIT", 150) - stub_const("#{described_class}::DIFF_COLLAPSE_LIMIT", 100) + stub_const("#{described_class}::MAX_SIZE", 150) + stub_const("#{described_class}::OVERRIDABLE_MAX_SIZE", 100) end it 'prunes the diff as a large diff instead of as a collapsed diff' do @@ -269,7 +269,7 @@ EOT it 'returns true for a diff that was explicitly marked as being too large' do diff = described_class.new(diff: 'a') - diff.prune_large_diff! + diff.too_large! expect(diff.too_large?).to eq(true) end @@ -291,13 +291,13 @@ EOT it 'returns true for a diff that was explicitly marked as being collapsed' do diff = described_class.new(diff: 'a') - diff.prune_collapsed_diff! + diff.collapse! expect(diff).to be_collapsed end end - describe '#collapsible?' do + describe '#collapsed?' do it 'returns true for a diff that is quite large' do diff = described_class.new(diff: 'a' * 20480) @@ -311,11 +311,11 @@ EOT end end - describe '#prune_collapsed_diff!' do + describe '#collapse!' do it 'prunes the diff' do diff = described_class.new(diff: "foo\nbar") - diff.prune_collapsed_diff! + diff.collapse! expect(diff.diff).to eq('') expect(diff.line_count).to eq(0) diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb index 92fbf64a6b7..0749ff78777 100644 --- a/spec/models/blob_viewer/base_spec.rb +++ b/spec/models/blob_viewer/base_spec.rb @@ -11,8 +11,8 @@ describe BlobViewer::Base, model: true do self.extensions = %w(pdf) self.binary = true - self.overridable_max_size = 1.megabyte - self.max_size = 5.megabytes + self.collapse_limit = 1.megabyte + self.size_limit = 5.megabytes end end @@ -69,77 +69,77 @@ describe BlobViewer::Base, model: true do end end - describe '#exceeds_overridable_max_size?' do - context 'when the blob size is larger than the overridable max size' do + describe '#collapsed?' do + context 'when the blob size is larger than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns true' do - expect(viewer.exceeds_overridable_max_size?).to be_truthy + expect(viewer.collapsed?).to be_truthy end end - context 'when the blob size is smaller than the overridable max size' do + context 'when the blob size is smaller than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) } it 'returns false' do - expect(viewer.exceeds_overridable_max_size?).to be_falsey + expect(viewer.collapsed?).to be_falsey end end end - describe '#exceeds_max_size?' do - context 'when the blob size is larger than the max size' do + describe '#too_large?' do + context 'when the blob size is larger than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) } it 'returns true' do - expect(viewer.exceeds_max_size?).to be_truthy + expect(viewer.too_large?).to be_truthy end end - context 'when the blob size is smaller than the max size' do + context 'when the blob size is smaller than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns false' do - expect(viewer.exceeds_max_size?).to be_falsey + expect(viewer.too_large?).to be_falsey end end end - describe '#can_override_max_size?' do - context 'when the blob size is larger than the overridable max size' do - context 'when the blob size is larger than the max size' do + describe '#can_expanded?' do + context 'when the blob size is larger than the collapse limit' do + context 'when the blob size is larger than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) } it 'returns false' do - expect(viewer.can_override_max_size?).to be_falsey + expect(viewer.can_expanded?).to be_falsey end end - context 'when the blob size is smaller than the max size' do + context 'when the blob size is smaller than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns true' do - expect(viewer.can_override_max_size?).to be_truthy + expect(viewer.can_expanded?).to be_truthy end end end - context 'when the blob size is smaller than the overridable max size' do + context 'when the blob size is smaller than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) } it 'returns false' do - expect(viewer.can_override_max_size?).to be_falsey + expect(viewer.can_expanded?).to be_falsey end end end describe '#render_error' do - context 'when the max size is overridden' do + context 'when the size limit is overridden' do before do - viewer.override_max_size = true + viewer.expanded = true end - context 'when the blob size is larger than the max size' do + context 'when the blob size is larger than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) } it 'returns :too_large' do @@ -147,7 +147,7 @@ describe BlobViewer::Base, model: true do end end - context 'when the blob size is smaller than the max size' do + context 'when the blob size is smaller than the size limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns nil' do @@ -156,8 +156,8 @@ describe BlobViewer::Base, model: true do end end - context 'when the max size is not overridden' do - context 'when the blob size is larger than the overridable max size' do + context 'when the size limit is not overridden' do + context 'when the blob size is larger than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) } it 'returns :too_large' do @@ -165,7 +165,7 @@ describe BlobViewer::Base, model: true do end end - context 'when the blob size is smaller than the overridable max size' do + context 'when the blob size is smaller than the collapse limit' do let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) } it 'returns nil' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 712470d6bf5..58f273ade28 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -238,10 +238,10 @@ describe MergeRequest, models: true do end context 'when there are no MR diffs' do - it 'delegates to the compare object, setting no_collapse: true' do + it 'delegates to the compare object, setting expanded: true' do merge_request.compare = double(:compare) - expect(merge_request.compare).to receive(:diffs).with(options.merge(no_collapse: true)) + expect(merge_request.compare).to receive(:diffs).with(options.merge(expanded: true)) merge_request.diffs(options) end diff --git a/spec/views/projects/blob/_viewer.html.haml_spec.rb b/spec/views/projects/blob/_viewer.html.haml_spec.rb index c6b0ed8da3c..bbd7f98fa8d 100644 --- a/spec/views/projects/blob/_viewer.html.haml_spec.rb +++ b/spec/views/projects/blob/_viewer.html.haml_spec.rb @@ -10,8 +10,8 @@ describe 'projects/blob/_viewer.html.haml', :view do include BlobViewer::Rich self.partial_name = 'text' - self.overridable_max_size = 1.megabyte - self.max_size = 5.megabytes + self.collapse_limit = 1.megabyte + self.size_limit = 5.megabytes self.load_async = true end end |