Merge branch 'dm-collapsed-blob' into 'master'

Consistent diff and blob size limit names

See merge request !11776

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

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

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

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

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

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,

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
 

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

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)

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
 

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

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!

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

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

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)

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')

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(limits: false).each do |diff|
             next unless diff.new_path == params[:path]
             lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
 

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(limits: false).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(limits: false).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(limits: false).to_a
       end
 
       expose :compare_timeout do |compare, options|

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(limits: false).each do |diff|
               next unless diff.new_path == params[:path]
               lines = Gitlab::Diff::Parser.new.parse(diff.diff.each_line)
 

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(limits: false).to_a
         end
       end
 

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)

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

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

lib/gitlab/git/diff.rb

@@ -15,13 +15,16 @@ module Gitlab
       alias_method :deleted_file?, :deleted_file
       alias_method :renamed_file?, :renamed_file
 
+      attr_accessor :expanded
+
+      # We need this accessor because of `to_hash` and `init_from_hash`
       attr_accessor :too_large
 
       # 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 +155,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, :limits, :expanded]
 
           if default_options
             actual_defaults = default_options.dup
@@ -177,16 +180,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
@@ -226,17 +231,13 @@ module Gitlab
 
       def too_large?
         if @too_large.nil?
-          @too_large = @diff.bytesize >= DIFF_SIZE_LIMIT
+          @too_large = @diff.bytesize >= SIZE_LIMIT
         else
           @too_large
         end
       end
 
-      def collapsible?
-        @diff.bytesize >= DIFF_COLLAPSE_LIMIT
-      end
-
-      def prune_large_diff!
+      def too_large!
         @diff = ''
         @line_count = 0
         @too_large = true
@@ -244,10 +245,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 +257,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 +274,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 +301,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
 

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
+        @enforce_limits = !!options.fetch(:limits, true)
+        @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 @enforce_limits && i >= @max_files
             @overflow = true
             break
           end
 
-          collapse = !@all_diffs && !@no_collapse
+          expanded = !@enforce_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 @enforce_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

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
@@ -168,21 +168,19 @@ describe BlobHelper do
         controller.params[:id] = File.join('master', blob.path)
       end
 
-      context 'for error :too_large' do
-        context 'when the max size can be overridden' do
-          let(:blob) { fake_blob(size: 2.megabytes) }
+      context 'for error :collapsed' do
+        let(:blob) { fake_blob(size: 2.megabytes) }
 
-          it 'includes a "load it anyway" link' do
-            expect(helper.blob_render_error_options(viewer)).to include(/load it anyway/)
-          end
+        it 'includes a "load it anyway" link' do
+          expect(helper.blob_render_error_options(viewer)).to include(/load it anyway/)
         end
+      end
 
-        context 'when the max size cannot be overridden' do
-          let(:blob) { fake_blob(size: 10.megabytes) }
+      context 'for error :too_large' do
+        let(:blob) { fake_blob(size: 10.megabytes) }
 
-          it 'does not include a "load it anyway" link' do
-            expect(helper.blob_render_error_options(viewer)).not_to include(/load it anyway/)
-          end
+        it 'does not include a "load it anyway" link' do
+          expect(helper.blob_render_error_options(viewer)).not_to include(/load it anyway/)
         end
 
         context 'when the viewer is rich' do

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

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
+      limits: 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(:limits) { true }
+  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(:limits) { false }
 
           describe '#overflow?' do
             subject { super().overflow? }
@@ -94,7 +94,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
 
           describe '#size' do
             it { expect(subject.size).to eq(3) }
-  
+
             it 'does not change after peeking' do
               subject.any?
               expect(subject.size).to eq(3)
@@ -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(:limits) { false }
 
           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(:limits) { false }
 
           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(:limits) { false }
 
           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(:limits) { false }
 
         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(:limits) { false }
 
         it 'yields Diff instances even when they are quite big' do
           expect { |b| subject.each(&b) }.

spec/lib/gitlab/git/diff_spec.rb

@@ -85,12 +85,12 @@ 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}::SIZE_LIMIT", 150)
+          stub_const("#{described_class}::COLLAPSE_LIMIT", 100)
         end
 
         it 'prunes the diff as a large diff instead of as a collapsed diff' do
-          diff = described_class.new(@rugged_diff, collapse: true)
+          diff = described_class.new(@rugged_diff, expanded: false)
 
           expect(diff.diff).to be_empty
           expect(diff).to be_too_large
@@ -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,31 +291,31 @@ 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)
+      diff = described_class.new({ diff: 'a' * 20480 }, expanded: false)
 
-      expect(diff).to be_collapsible
+      expect(diff).to be_collapsed
     end
 
     it 'returns false for a diff that is small enough' do
-      diff = described_class.new(diff: 'a')
+      diff = described_class.new({ diff: 'a' }, expanded: false)
 
-      expect(diff).not_to be_collapsible
+      expect(diff).not_to be_collapsed
     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)

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,49 @@ 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
-      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
-        let(:blob) { fake_blob(path: 'file.pdf', size: 10.megabytes) }
-
-        it 'returns false' do
-          expect(viewer.can_override_max_size?).to be_falsey
-        end
-      end
-
-      context 'when the blob size is smaller than the max size' do
-        let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) }
-
-        it 'returns true' do
-          expect(viewer.can_override_max_size?).to be_truthy
-        end
-      end
-    end
-
-    context 'when the blob size is smaller than the overridable max size' 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.too_large?).to be_falsey
       end
     end
   end
 
   describe '#render_error' do
-    context 'when the max size is overridden' do
+    context 'when expanded' 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 +119,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,16 +128,16 @@ 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 not expanded' 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
-          expect(viewer.render_error).to eq(:too_large)
+        it 'returns :collapsed' do
+          expect(viewer.render_error).to eq(:collapsed)
         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

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

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