diff --git a/app/controllers/concerns/renders_blob.rb b/app/controllers/concerns/renders_blob.rb
index 4a6630dfd907d854a709bc203cc3195233c01d61..1d37e4cb3bd5bcad2c5168d3eeb5688d0a66d09e 100644
--- a/app/controllers/concerns/renders_blob.rb
+++ b/app/controllers/concerns/renders_blob.rb
@@ -14,7 +14,7 @@ module RendersBlob
return render_404 unless viewer
render json: {
- html: view_to_html_string("projects/blob/_viewer", viewer: viewer, load_asynchronously: false)
+ html: view_to_html_string("projects/blob/_viewer", viewer: viewer, load_async: false)
}
end
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index eb37f2e02677323e716d53a49774a8f3bd1b091a..7eb3512378c3c8623f071437188554959dfea1b5 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -226,7 +226,7 @@ module BlobHelper
def open_raw_blob_button(blob)
return if blob.empty?
-
+
if blob.raw_binary? || blob.stored_externally?
icon = icon('download')
title = 'Download'
@@ -242,9 +242,9 @@ module BlobHelper
case viewer.render_error
when :too_large
max_size =
- if viewer.absolutely_too_large?
- viewer.absolute_max_size
- elsif viewer.too_large?
+ 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)}"
diff --git a/app/models/blob_viewer/auxiliary.rb b/app/models/blob_viewer/auxiliary.rb
index db124397b274ea2636fb82ff9aac98cc2decae34..cd6e596ed608a27913508b6e7cef3b9250b90a07 100644
--- a/app/models/blob_viewer/auxiliary.rb
+++ b/app/models/blob_viewer/auxiliary.rb
@@ -5,8 +5,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.absolute_max_size = 100.kilobytes
end
end
end
diff --git a/app/models/blob_viewer/base.rb b/app/models/blob_viewer/base.rb
index 4f38c31714b7e392a99a478d588141d6b9d464a3..c7b8fbfc56a4ad82a06c1c990928566bd76b832c 100644
--- a/app/models/blob_viewer/base.rb
+++ b/app/models/blob_viewer/base.rb
@@ -2,11 +2,11 @@ module BlobViewer
class Base
PARTIAL_PATH_PREFIX = 'projects/blob/viewers'.freeze
- class_attribute :partial_name, :loading_partial_name, :type, :extensions, :file_type, :client_side, :binary, :switcher_icon, :switcher_title, :max_size, :absolute_max_size
+ class_attribute :partial_name, :loading_partial_name, :type, :extensions, :file_types, :load_async, :binary, :switcher_icon, :switcher_title, :overridable_max_size, :max_size
self.loading_partial_name = 'loading'
- delegate :partial_path, :loading_partial_path, :rich?, :simple?, :client_side?, :server_side?, :text?, :binary?, to: :class
+ delegate :partial_path, :loading_partial_path, :rich?, :simple?, :text?, :binary?, to: :class
attr_reader :blob
attr_accessor :override_max_size
@@ -35,12 +35,8 @@ module BlobViewer
type == :auxiliary
end
- def self.client_side?
- client_side
- end
-
- def self.server_side?
- !client_side?
+ def self.load_async?
+ load_async
end
def self.binary?
@@ -54,21 +50,33 @@ module BlobViewer
def self.can_render?(blob, verify_binary: true)
return false if verify_binary && binary? != blob.binary?
return true if extensions&.include?(blob.extension)
- return true if file_type && Gitlab::FileDetector.type_of(blob.path) == file_type
+ return true if file_types&.include?(Gitlab::FileDetector.type_of(blob.path))
false
end
- def too_large?
- blob.raw_size > max_size
+ def load_async?
+ self.class.load_async? && render_error.nil?
end
- def absolutely_too_large?
- blob.raw_size > absolute_max_size
+ 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 can_override_max_size?
- too_large? && !absolutely_too_large?
+ exceeds_overridable_max_size? && !exceeds_max_size?
+ end
+
+ def too_large?
+ if override_max_size
+ exceeds_max_size?
+ else
+ exceeds_overridable_max_size?
+ end
end
# This method is used on the server side to check whether we can attempt to
@@ -83,29 +91,13 @@ module BlobViewer
# binary from `blob_raw_url` and does its own format validation and error
# rendering, especially for potentially large binary formats.
def render_error
- return @render_error if defined?(@render_error)
-
- @render_error =
- if server_side_but_stored_externally?
- # Files that are not stored in the repository, like LFS files and
- # build artifacts, can only be rendered using a client-side viewer,
- # since we do not want to read large amounts of data into memory on the
- # server side. Client-side viewers use JS and can fetch the file from
- # `blob_raw_url` using AJAX.
- :server_side_but_stored_externally
- elsif override_max_size ? absolutely_too_large? : too_large?
- :too_large
- end
+ if too_large?
+ :too_large
+ end
end
def prepare!
# To be overridden by subclasses
end
-
- private
-
- def server_side_but_stored_externally?
- server_side? && blob.stored_externally?
- end
end
end
diff --git a/app/models/blob_viewer/client_side.rb b/app/models/blob_viewer/client_side.rb
index 42ec68f864bcb9b5fe50db20fc9312fba9fce0c6..cc68236f92babf5ca1b0742f2cc3c9d8df177089 100644
--- a/app/models/blob_viewer/client_side.rb
+++ b/app/models/blob_viewer/client_side.rb
@@ -3,9 +3,9 @@ module BlobViewer
extend ActiveSupport::Concern
included do
- self.client_side = true
- self.max_size = 10.megabytes
- self.absolute_max_size = 50.megabytes
+ self.load_async = false
+ self.overridable_max_size = 10.megabytes
+ self.max_size = 50.megabytes
end
end
end
diff --git a/app/models/blob_viewer/download.rb b/app/models/blob_viewer/download.rb
index adc06587f6981d3a40d3b6d66cc9f7220904ea11..074e7204814049ec3c4c9592bbd7d7fc90f7efd8 100644
--- a/app/models/blob_viewer/download.rb
+++ b/app/models/blob_viewer/download.rb
@@ -1,17 +1,9 @@
module BlobViewer
class Download < Base
include Simple
- # We treat the Download viewer as if it renders the content client-side,
- # so that it doesn't attempt to load the entire blob contents and is
- # rendered synchronously instead of loaded asynchronously.
- include ClientSide
+ include Static
self.partial_name = 'download'
self.binary = true
-
- # We can always render the Download viewer, even if the blob is in LFS or too large.
- def render_error
- nil
- end
end
end
diff --git a/app/models/blob_viewer/gitlab_ci_yml.rb b/app/models/blob_viewer/gitlab_ci_yml.rb
index 81afab2f49b5315a67c3b22aae54b04b5825b8dc..7267c3965d3360f233f9cb249790373ff2d001ff 100644
--- a/app/models/blob_viewer/gitlab_ci_yml.rb
+++ b/app/models/blob_viewer/gitlab_ci_yml.rb
@@ -5,7 +5,7 @@ module BlobViewer
self.partial_name = 'gitlab_ci_yml'
self.loading_partial_name = 'gitlab_ci_yml_loading'
- self.file_type = :gitlab_ci
+ self.file_types = %i(gitlab_ci)
self.binary = false
def validation_message
diff --git a/app/models/blob_viewer/license.rb b/app/models/blob_viewer/license.rb
index 3ad49570c88f0dd8c04031c2266a02fe206b32cd..6751ea15a5d68bb9c2ccea149bcb7878761c4176 100644
--- a/app/models/blob_viewer/license.rb
+++ b/app/models/blob_viewer/license.rb
@@ -1,13 +1,10 @@
module BlobViewer
class License < Base
- # We treat the License viewer as if it renders the content client-side,
- # so that it doesn't attempt to load the entire blob contents and is
- # rendered synchronously instead of loaded asynchronously.
- include ClientSide
include Auxiliary
+ include Static
self.partial_name = 'license'
- self.file_type = :license
+ self.file_types = %i(license)
self.binary = false
def license
diff --git a/app/models/blob_viewer/route_map.rb b/app/models/blob_viewer/route_map.rb
index 1ca730c1ea0169e65626b6f924e53e5ee093a1f3..153b4eeb2c9e7b3ebe15a423028621498b0738a4 100644
--- a/app/models/blob_viewer/route_map.rb
+++ b/app/models/blob_viewer/route_map.rb
@@ -5,7 +5,7 @@ module BlobViewer
self.partial_name = 'route_map'
self.loading_partial_name = 'route_map_loading'
- self.file_type = :route_map
+ self.file_types = %i(route_map)
self.binary = false
def validation_message
diff --git a/app/models/blob_viewer/server_side.rb b/app/models/blob_viewer/server_side.rb
index e8c5c17b824be9db94410506abd298c65bceacaa..87884dcd6bf7da0db0f3cca18c958a010718b1db 100644
--- a/app/models/blob_viewer/server_side.rb
+++ b/app/models/blob_viewer/server_side.rb
@@ -3,9 +3,9 @@ module BlobViewer
extend ActiveSupport::Concern
included do
- self.client_side = false
- self.max_size = 2.megabytes
- self.absolute_max_size = 5.megabytes
+ self.load_async = true
+ self.overridable_max_size = 2.megabytes
+ self.max_size = 5.megabytes
end
def prepare!
@@ -13,5 +13,18 @@ module BlobViewer
blob.load_all_data!(blob.project.repository)
end
end
+
+ def render_error
+ if blob.stored_externally?
+ # Files that are not stored in the repository, like LFS files and
+ # build artifacts, can only be rendered using a client-side viewer,
+ # since we do not want to read large amounts of data into memory on the
+ # server side. Client-side viewers use JS and can fetch the file from
+ # `blob_raw_url` using AJAX.
+ return :server_side_but_stored_externally
+ end
+
+ super
+ end
end
end
diff --git a/app/models/blob_viewer/static.rb b/app/models/blob_viewer/static.rb
new file mode 100644
index 0000000000000000000000000000000000000000..c9e257e53885bad87500ffbf2158356dbc8679ba
--- /dev/null
+++ b/app/models/blob_viewer/static.rb
@@ -0,0 +1,14 @@
+module BlobViewer
+ module Static
+ extend ActiveSupport::Concern
+
+ included do
+ self.load_async = false
+ end
+
+ # We can always render a static viewer, even if the blob is too large.
+ def render_error
+ nil
+ end
+ end
+end
diff --git a/app/models/blob_viewer/text.rb b/app/models/blob_viewer/text.rb
index e27b2c2b4932fe24bd306d7fd582069f006f79ba..eddca50b4d4827e3e609f140559552fe764b1f6a 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.max_size = 1.megabyte
- self.absolute_max_size = 10.megabytes
+ self.overridable_max_size = 1.megabyte
+ self.max_size = 10.megabytes
end
end
diff --git a/app/views/projects/blob/_viewer.html.haml b/app/views/projects/blob/_viewer.html.haml
index 3d9c3a5998082ddf3723509c7d726cd7445bb3cf..4252f27d0078bd9694685cdeb9ae175af3398ec2 100644
--- a/app/views/projects/blob/_viewer.html.haml
+++ b/app/views/projects/blob/_viewer.html.haml
@@ -1,10 +1,10 @@
- hidden = local_assigns.fetch(:hidden, false)
- render_error = viewer.render_error
-- load_asynchronously = local_assigns.fetch(:load_asynchronously, viewer.server_side?) && render_error.nil?
+- load_async = local_assigns.fetch(:load_async, viewer.load_async?)
-- viewer_url = local_assigns.fetch(:viewer_url) { url_for(params.merge(viewer: viewer.type, format: :json)) } if load_asynchronously
+- viewer_url = local_assigns.fetch(:viewer_url) { url_for(params.merge(viewer: viewer.type, format: :json)) } if load_async
.blob-viewer{ data: { type: viewer.type, url: viewer_url }, class: ('hidden' if hidden) }
- - if load_asynchronously
+ - if load_async
= render viewer.loading_partial_path, viewer: viewer
- elsif render_error
= render 'projects/blob/render_error', viewer: viewer
diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb
index 1b4393e6167483bdb8404575112c086171913773..41b5df12522777d157bd5d1095fdb6b99fe36edb 100644
--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -116,10 +116,11 @@ describe BlobHelper do
let(:viewer_class) do
Class.new(BlobViewer::Base) do
- self.max_size = 1.megabyte
- self.absolute_max_size = 5.megabytes
+ include BlobViewer::ServerSide
+
+ self.overridable_max_size = 1.megabyte
+ self.max_size = 5.megabytes
self.type = :rich
- self.client_side = false
end
end
diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb
index a6641970e1b529ffe5f5a14ad7a4af15bf643ad1..92fbf64a6b74235ce1e0f1dbddd608861a702b35 100644
--- a/spec/models/blob_viewer/base_spec.rb
+++ b/spec/models/blob_viewer/base_spec.rb
@@ -7,11 +7,12 @@ describe BlobViewer::Base, model: true do
let(:viewer_class) do
Class.new(described_class) do
+ include BlobViewer::ServerSide
+
self.extensions = %w(pdf)
self.binary = true
- self.max_size = 1.megabyte
- self.absolute_max_size = 5.megabytes
- self.client_side = false
+ self.overridable_max_size = 1.megabyte
+ self.max_size = 5.megabytes
end
end
@@ -38,10 +39,10 @@ describe BlobViewer::Base, model: true do
context 'when the file type is supported' do
before do
- viewer_class.file_type = :license
+ viewer_class.file_types = %i(license)
viewer_class.binary = false
end
-
+
context 'when the binaryness matches' do
let(:blob) { fake_blob(path: 'LICENSE', binary: false) }
@@ -68,45 +69,45 @@ describe BlobViewer::Base, model: true do
end
end
- describe '#too_large?' do
- context 'when the blob size is larger than the max size' do
+ describe '#exceeds_overridable_max_size?' do
+ context 'when the blob size is larger than the overridable max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) }
it 'returns true' do
- expect(viewer.too_large?).to be_truthy
+ expect(viewer.exceeds_overridable_max_size?).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 overridable max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) }
it 'returns false' do
- expect(viewer.too_large?).to be_falsey
+ expect(viewer.exceeds_overridable_max_size?).to be_falsey
end
end
end
- describe '#absolutely_too_large?' do
- context 'when the blob size is larger than the absolute max size' do
+ describe '#exceeds_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 true' do
- expect(viewer.absolutely_too_large?).to be_truthy
+ expect(viewer.exceeds_max_size?).to be_truthy
end
end
- context 'when the blob size is smaller than the absolute max size' do
+ context 'when the blob size is smaller than the max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) }
it 'returns false' do
- expect(viewer.absolutely_too_large?).to be_falsey
+ 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 max size' do
- context 'when the blob size is larger than the absolute 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
@@ -114,7 +115,7 @@ describe BlobViewer::Base, model: true do
end
end
- context 'when the blob size is smaller than the absolute max size' do
+ 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
@@ -123,7 +124,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 overridable max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) }
it 'returns false' do
@@ -138,7 +139,7 @@ describe BlobViewer::Base, model: true do
viewer.override_max_size = true
end
- context 'when the blob size is larger than the absolute 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 :too_large' do
@@ -146,7 +147,7 @@ describe BlobViewer::Base, model: true do
end
end
- context 'when the blob size is smaller than the absolute max size' do
+ context 'when the blob size is smaller than the max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) }
it 'returns nil' do
@@ -156,7 +157,7 @@ describe BlobViewer::Base, model: true do
end
context 'when the max size is not overridden' do
- context 'when the blob size is larger than the max size' do
+ context 'when the blob size is larger than the overridable max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 2.megabytes) }
it 'returns :too_large' do
@@ -164,7 +165,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 overridable max size' do
let(:blob) { fake_blob(path: 'file.pdf', size: 10.kilobytes) }
it 'returns nil' do
@@ -172,19 +173,5 @@ describe BlobViewer::Base, model: true do
end
end
end
-
- context 'when the viewer is server side but the blob is stored externally' do
- let(:project) { build(:empty_project, lfs_enabled: true) }
-
- let(:blob) { fake_blob(path: 'file.pdf', lfs: true) }
-
- before do
- allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
- end
-
- it 'return :server_side_but_stored_externally' do
- expect(viewer.render_error).to eq(:server_side_but_stored_externally)
- end
- end
end
end
diff --git a/spec/models/blob_viewer/server_side_spec.rb b/spec/models/blob_viewer/server_side_spec.rb
index ddca9b7939067e04b63f859472b083ae2f53ca42..f047953d540aa871dda614a8065dd8545c77daa6 100644
--- a/spec/models/blob_viewer/server_side_spec.rb
+++ b/spec/models/blob_viewer/server_side_spec.rb
@@ -22,4 +22,20 @@ describe BlobViewer::ServerSide, model: true do
subject.prepare!
end
end
+
+ describe '#render_error' do
+ context 'when the blob is stored externally' do
+ let(:project) { build(:empty_project, lfs_enabled: true) }
+
+ let(:blob) { fake_blob(path: 'file.pdf', lfs: true) }
+
+ before do
+ allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
+ end
+
+ it 'return :server_side_but_stored_externally' do
+ expect(subject.render_error).to eq(:server_side_but_stored_externally)
+ end
+ end
+ end
end
diff --git a/spec/views/projects/blob/_viewer.html.haml_spec.rb b/spec/views/projects/blob/_viewer.html.haml_spec.rb
index 08018767624d738cc7023085627e40204cbef50d..c6b0ed8da3cd1dd37beaee07870eb38e81c6eee8 100644
--- a/spec/views/projects/blob/_viewer.html.haml_spec.rb
+++ b/spec/views/projects/blob/_viewer.html.haml_spec.rb
@@ -10,9 +10,9 @@ describe 'projects/blob/_viewer.html.haml', :view do
include BlobViewer::Rich
self.partial_name = 'text'
- self.max_size = 1.megabyte
- self.absolute_max_size = 5.megabytes
- self.client_side = false
+ self.overridable_max_size = 1.megabyte
+ self.max_size = 5.megabytes
+ self.load_async = true
end
end
@@ -35,9 +35,9 @@ describe 'projects/blob/_viewer.html.haml', :view do
render partial: 'projects/blob/viewer', locals: { viewer: viewer }
end
- context 'when the viewer is server side' do
+ context 'when the viewer is loaded asynchronously' do
before do
- viewer_class.client_side = false
+ viewer_class.load_async = true
end
context 'when there is no render error' do
@@ -65,9 +65,9 @@ describe 'projects/blob/_viewer.html.haml', :view do
end
end
- context 'when the viewer is client side' do
+ context 'when the viewer is loaded synchronously' do
before do
- viewer_class.client_side = true
+ viewer_class.load_async = false
end
context 'when there is no render error' do