diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb
index a0b08ad130fe9524bf6a52750a2e58ce624b947a..a02cc477e081964766148fa2d519544fd152c418 100644
--- a/app/controllers/projects/raw_controller.rb
+++ b/app/controllers/projects/raw_controller.rb
@@ -15,7 +15,7 @@ class Projects::RawController < Projects::ApplicationController
return if cached_blob?
- if @blob.valid_lfs_pointer?
+ if @blob.stored_externally?
send_lfs_object
else
send_git_blob @repository, @blob
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 377b080b3c6a3d4657c484cbd6b43a7f190bcbbd..37b6f4ad5cc6babc379dabe32f7e31268d0531ca 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -52,7 +52,7 @@ module BlobHelper
if !on_top_of_branch?(project, ref)
button_tag label, class: "#{common_classes} disabled has-tooltip", title: "You can only #{action} files when you are on a branch", data: { container: 'body' }
- elsif blob.valid_lfs_pointer?
+ elsif blob.stored_externally?
button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' }
elsif can_modify_blob?(blob, project, ref)
button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal'
@@ -95,7 +95,7 @@ module BlobHelper
end
def can_modify_blob?(blob, project = @project, ref = @ref)
- !blob.valid_lfs_pointer? && can_edit_tree?(project, ref)
+ !blob.stored_externally? && can_edit_tree?(project, ref)
end
def leave_edit_message
@@ -223,7 +223,9 @@ module BlobHelper
end
def open_raw_blob_button(blob)
- if blob.raw_binary?
+ return if blob.empty?
+
+ if blob.raw_binary? || blob.stored_externally?
icon = icon('download')
title = 'Download'
else
@@ -244,19 +246,27 @@ module BlobHelper
viewer.max_size
end
"it is larger than #{number_to_human_size(max_size)}"
- when :server_side_but_stored_in_lfs
- "it is stored in LFS"
+ when :server_side_but_stored_externally
+ case viewer.blob.external_storage
+ when :lfs
+ 'it is stored in LFS'
+ else
+ 'it is stored externally'
+ end
end
end
def blob_render_error_options(viewer)
+ error = viewer.render_error
options = []
- if viewer.render_error == :too_large && viewer.can_override_max_size?
+ 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)))
end
- if viewer.rich? && viewer.blob.rendered_as_text?
+ # If the error is `:server_side_but_stored_externally`, the simple viewer will show the same error,
+ # so don't bother switching.
+ if viewer.rich? && viewer.blob.rendered_as_text? && error != :server_side_but_stored_externally
options << link_to('view the source', '#', class: 'js-blob-viewer-switch-btn', data: { viewer: 'simple' })
end
diff --git a/app/models/blob.rb b/app/models/blob.rb
index 1cdb8811cfff3f401ae1c714b42f05ed0ace0757..a4fae22a0c459407d04b7dd1e83f64aeaf54faef 100644
--- a/app/models/blob.rb
+++ b/app/models/blob.rb
@@ -28,7 +28,7 @@ class Blob < SimpleDelegator
BlobViewer::Sketch,
BlobViewer::Video,
-
+
BlobViewer::PDF,
BlobViewer::BinarySTL,
@@ -75,19 +75,37 @@ class Blob < SimpleDelegator
end
def no_highlighting?
- size && size > MAXIMUM_TEXT_HIGHLIGHT_SIZE
+ raw_size && raw_size > MAXIMUM_TEXT_HIGHLIGHT_SIZE
+ end
+
+ def empty?
+ raw_size == 0
end
def too_large?
size && truncated?
end
+ def external_storage_error?
+ if external_storage == :lfs
+ !project&.lfs_enabled?
+ else
+ false
+ end
+ end
+
+ def stored_externally?
+ return @stored_externally if defined?(@stored_externally)
+
+ @stored_externally = external_storage && !external_storage_error?
+ end
+
# Returns the size of the file that this blob represents. If this blob is an
# LFS pointer, this is the size of the file stored in LFS. Otherwise, this is
# the size of the blob itself.
def raw_size
- if valid_lfs_pointer?
- lfs_size
+ if stored_externally?
+ external_size
else
size
end
@@ -98,9 +116,13 @@ class Blob < SimpleDelegator
# text-based rich blob viewer matched on the file's extension. Otherwise, this
# depends on the type of the blob itself.
def raw_binary?
- if valid_lfs_pointer?
+ if stored_externally?
if rich_viewer
rich_viewer.binary?
+ elsif Linguist::Language.find_by_filename(name).any?
+ false
+ elsif _mime_type
+ _mime_type.binary?
else
true
end
@@ -118,15 +140,7 @@ class Blob < SimpleDelegator
end
def readable_text?
- text? && !valid_lfs_pointer? && !too_large?
- end
-
- def valid_lfs_pointer?
- lfs_pointer? && project&.lfs_enabled?
- end
-
- def invalid_lfs_pointer?
- lfs_pointer? && !project&.lfs_enabled?
+ text? && !stored_externally? && !too_large?
end
def simple_viewer
@@ -165,10 +179,10 @@ class Blob < SimpleDelegator
end
def rich_viewer_class
- return if invalid_lfs_pointer? || empty?
+ return if empty? || external_storage_error?
classes =
- if valid_lfs_pointer?
+ if stored_externally?
BINARY_VIEWERS + TEXT_VIEWERS
elsif binary?
BINARY_VIEWERS
diff --git a/app/models/blob_viewer/base.rb b/app/models/blob_viewer/base.rb
index f944b00c9d3105c80ed8c897368c0a68a5b04a07..a8b91d8d6bcf16fc4eea9cbc3e194b6661ac587d 100644
--- a/app/models/blob_viewer/base.rb
+++ b/app/models/blob_viewer/base.rb
@@ -70,12 +70,13 @@ module BlobViewer
return @render_error if defined?(@render_error)
@render_error =
- if server_side_but_stored_in_lfs?
- # Files stored in LFS can only be rendered using a client-side viewer,
+ 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_in_lfs
+ :server_side_but_stored_externally
elsif override_max_size ? absolutely_too_large? : too_large?
:too_large
end
@@ -89,8 +90,8 @@ module BlobViewer
private
- def server_side_but_stored_in_lfs?
- server_side? && blob.valid_lfs_pointer?
+ def server_side_but_stored_externally?
+ server_side? && blob.stored_externally?
end
end
end
diff --git a/app/models/concerns/blob_like.rb b/app/models/concerns/blob_like.rb
new file mode 100644
index 0000000000000000000000000000000000000000..adb81561000677bda65f4d8c8f1d604935b395d1
--- /dev/null
+++ b/app/models/concerns/blob_like.rb
@@ -0,0 +1,48 @@
+module BlobLike
+ extend ActiveSupport::Concern
+ include Linguist::BlobHelper
+
+ def id
+ raise NotImplementedError
+ end
+
+ def name
+ raise NotImplementedError
+ end
+
+ def path
+ raise NotImplementedError
+ end
+
+ def size
+ 0
+ end
+
+ def data
+ nil
+ end
+
+ def mode
+ nil
+ end
+
+ def binary?
+ false
+ end
+
+ def load_all_data!(repository)
+ # No-op
+ end
+
+ def truncated?
+ false
+ end
+
+ def external_storage
+ nil
+ end
+
+ def external_size
+ nil
+ end
+end
diff --git a/app/models/snippet_blob.rb b/app/models/snippet_blob.rb
index d6cab74eb1a861bc4b6e0b950c133764cf526faf..fa5fa151607af09742fff1c74e60fb10932dafc1 100644
--- a/app/models/snippet_blob.rb
+++ b/app/models/snippet_blob.rb
@@ -1,5 +1,5 @@
class SnippetBlob
- include Linguist::BlobHelper
+ include BlobLike
attr_reader :snippet
@@ -28,32 +28,4 @@ class SnippetBlob
Banzai.render_field(snippet, :content)
end
-
- def mode
- nil
- end
-
- def binary?
- false
- end
-
- def load_all_data!(repository)
- # No-op
- end
-
- def lfs_pointer?
- false
- end
-
- def lfs_oid
- nil
- end
-
- def lfs_size
- nil
- end
-
- def truncated?
- false
- end
end
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index e8bb9e1f805bb513df0c3887b296d1a6a5b45909..12458f9f41000bab8684cc2461b553e7a8a9b6ba 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -128,6 +128,10 @@ module Gitlab
encode! @name
end
+ def truncated?
+ size && (size > loaded_size)
+ end
+
# Valid LFS object pointer is a text file consisting of
# version
# oid
@@ -155,10 +159,14 @@ module Gitlab
nil
end
- def truncated?
- size && (size > loaded_size)
+ def external_storage
+ return unless lfs_pointer?
+
+ :lfs
end
+ alias_method :external_size, :lfs_size
+
private
def has_lfs_version_key?
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb
index 8dba2ccbafaedff4af6e82871e8c33efafe8c51c..5955623f565cb594652ba4c24206c9353cb57336 100644
--- a/spec/features/projects/blobs/blob_show_spec.rb
+++ b/spec/features/projects/blobs/blob_show_spec.rb
@@ -159,7 +159,7 @@ feature 'File blob', :js, feature: true do
expect(page).to have_selector('.blob-viewer[data-type="rich"]')
# shows an error message
- expect(page).to have_content('The rendered file could not be displayed because it is stored in LFS. You can view the source or download it instead.')
+ expect(page).to have_content('The rendered file could not be displayed because it is stored in LFS. You can download it instead.')
# shows a viewer switcher
expect(page).to have_selector('.js-blob-viewer-switcher')
@@ -167,8 +167,8 @@ feature 'File blob', :js, feature: true do
# does not show a copy button
expect(page).not_to have_selector('.js-copy-blob-source-btn')
- # shows a raw button
- expect(page).to have_link('Open raw')
+ # shows a download button
+ expect(page).to have_link('Download')
end
end
@@ -332,4 +332,41 @@ feature 'File blob', :js, feature: true do
end
end
end
+
+ context 'empty file' do
+ before do
+ project.add_master(project.creator)
+
+ Files::CreateService.new(
+ project,
+ project.creator,
+ start_branch: 'master',
+ branch_name: 'master',
+ commit_message: "Add empty file",
+ file_path: 'files/empty.md',
+ file_content: ''
+ ).execute
+
+ visit_blob('files/empty.md')
+
+ wait_for_ajax
+ end
+
+ it 'displays an error' do
+ aggregate_failures do
+ # shows an error message
+ expect(page).to have_content('Empty file')
+
+ # does not show a viewer switcher
+ expect(page).not_to have_selector('.js-blob-viewer-switcher')
+
+ # does not show a copy button
+ expect(page).not_to have_selector('.js-copy-blob-source-btn')
+
+ # does not show a download or raw button
+ expect(page).not_to have_link('Download')
+ expect(page).not_to have_link('Open raw')
+ end
+ end
+ end
end
diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb
index 075f1887d9189287e876daf72808c27a691fd7c3..1b4393e6167483bdb8404575112c086171913773 100644
--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -145,7 +145,7 @@ describe BlobHelper do
end
end
- context 'for error :server_side_but_stored_in_lfs' do
+ context 'for error :server_side_but_stored_externally' do
let(:blob) { fake_blob(lfs: true) }
it 'returns an error message' do
@@ -183,40 +183,56 @@ describe BlobHelper do
expect(helper.blob_render_error_options(viewer)).not_to include(/load it anyway/)
end
end
- end
- context 'when the viewer is rich' do
- context 'the blob is rendered as text' do
- let(:blob) { fake_blob(path: 'file.md', lfs: true) }
+ context 'when the viewer is rich' do
+ context 'the blob is rendered as text' do
+ let(:blob) { fake_blob(path: 'file.md', size: 2.megabytes) }
+
+ it 'includes a "view the source" link' do
+ expect(helper.blob_render_error_options(viewer)).to include(/view the source/)
+ end
+ end
+
+ context 'the blob is not rendered as text' do
+ let(:blob) { fake_blob(path: 'file.pdf', binary: true, size: 2.megabytes) }
- it 'includes a "view the source" link' do
- expect(helper.blob_render_error_options(viewer)).to include(/view the source/)
+ it 'does not include a "view the source" link' do
+ expect(helper.blob_render_error_options(viewer)).not_to include(/view the source/)
+ end
end
end
- context 'the blob is not rendered as text' do
- let(:blob) { fake_blob(path: 'file.pdf', binary: true, lfs: true) }
+ context 'when the viewer is not rich' do
+ before do
+ viewer_class.type = :simple
+ end
+
+ let(:blob) { fake_blob(path: 'file.md', size: 2.megabytes) }
it 'does not include a "view the source" link' do
expect(helper.blob_render_error_options(viewer)).not_to include(/view the source/)
end
end
- end
- context 'when the viewer is not rich' do
- before do
- viewer_class.type = :simple
+ it 'includes a "download it" link' do
+ expect(helper.blob_render_error_options(viewer)).to include(/download it/)
end
+ end
+ context 'for error :server_side_but_stored_externally' do
let(:blob) { fake_blob(path: 'file.md', lfs: true) }
+ 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 "view the source" link' do
expect(helper.blob_render_error_options(viewer)).not_to include(/view the source/)
end
- end
- it 'includes a "download it" link' do
- expect(helper.blob_render_error_options(viewer)).to include(/download it/)
+ it 'includes a "download it" link' do
+ expect(helper.blob_render_error_options(viewer)).to include(/download it/)
+ end
end
end
end
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb
index 7e8a1c8add79c81a514b9156d340b5eed0d0a5e4..f84c6b481732d4162a84eb5f48447f34f706bcca 100644
--- a/spec/models/blob_spec.rb
+++ b/spec/models/blob_spec.rb
@@ -35,8 +35,68 @@ describe Blob do
end
end
+ describe '#external_storage_error?' do
+ context 'if the blob is stored in LFS' do
+ let(:blob) { fake_blob(path: 'file.pdf', lfs: true) }
+
+ context 'when the project has LFS enabled' do
+ it 'returns false' do
+ expect(blob.external_storage_error?).to be_falsey
+ end
+ end
+
+ context 'when the project does not have LFS enabled' do
+ before do
+ project.lfs_enabled = false
+ end
+
+ it 'returns true' do
+ expect(blob.external_storage_error?).to be_truthy
+ end
+ end
+ end
+
+ context 'if the blob is not stored in LFS' do
+ let(:blob) { fake_blob(path: 'file.md') }
+
+ it 'returns false' do
+ expect(blob.external_storage_error?).to be_falsey
+ end
+ end
+ end
+
+ describe '#stored_externally?' do
+ context 'if the blob is stored in LFS' do
+ let(:blob) { fake_blob(path: 'file.pdf', lfs: true) }
+
+ context 'when the project has LFS enabled' do
+ it 'returns true' do
+ expect(blob.stored_externally?).to be_truthy
+ end
+ end
+
+ context 'when the project does not have LFS enabled' do
+ before do
+ project.lfs_enabled = false
+ end
+
+ it 'returns false' do
+ expect(blob.stored_externally?).to be_falsey
+ end
+ end
+ end
+
+ context 'if the blob is not stored in LFS' do
+ let(:blob) { fake_blob(path: 'file.md') }
+
+ it 'returns false' do
+ expect(blob.stored_externally?).to be_falsey
+ end
+ end
+ end
+
describe '#raw_binary?' do
- context 'if the blob is a valid LFS pointer' do
+ context 'if the blob is stored externally' do
context 'if the extension has a rich viewer' do
context 'if the viewer is binary' do
it 'returns true' do
@@ -56,15 +116,63 @@ describe Blob do
end
context "if the extension doesn't have a rich viewer" do
- it 'returns true' do
- blob = fake_blob(path: 'file.exe', lfs: true)
+ context 'if the extension has a text mime type' do
+ context 'if the extension is for a programming language' do
+ it 'returns false' do
+ blob = fake_blob(path: 'file.txt', lfs: true)
- expect(blob.raw_binary?).to be_truthy
+ expect(blob.raw_binary?).to be_falsey
+ end
+ end
+
+ context 'if the extension is not for a programming language' do
+ it 'returns false' do
+ blob = fake_blob(path: 'file.ics', lfs: true)
+
+ expect(blob.raw_binary?).to be_falsey
+ end
+ end
+ end
+
+ context 'if the extension has a binary mime type' do
+ context 'if the extension is for a programming language' do
+ it 'returns false' do
+ blob = fake_blob(path: 'file.rb', lfs: true)
+
+ expect(blob.raw_binary?).to be_falsey
+ end
+ end
+
+ context 'if the extension is not for a programming language' do
+ it 'returns true' do
+ blob = fake_blob(path: 'file.exe', lfs: true)
+
+ expect(blob.raw_binary?).to be_truthy
+ end
+ end
+ end
+
+ context 'if the extension has an unknown mime type' do
+ context 'if the extension is for a programming language' do
+ it 'returns false' do
+ blob = fake_blob(path: 'file.ini', lfs: true)
+
+ expect(blob.raw_binary?).to be_falsey
+ end
+ end
+
+ context 'if the extension is not for a programming language' do
+ it 'returns true' do
+ blob = fake_blob(path: 'file.wtf', lfs: true)
+
+ expect(blob.raw_binary?).to be_truthy
+ end
+ end
end
end
end
- context 'if the blob is not an LFS pointer' do
+ context 'if the blob is not stored externally' do
context 'if the blob is binary' do
it 'returns true' do
blob = fake_blob(path: 'file.pdf', binary: true)
@@ -94,7 +202,7 @@ describe Blob do
describe '#simple_viewer' do
context 'when the blob is empty' do
it 'returns an empty viewer' do
- blob = fake_blob(data: '')
+ blob = fake_blob(data: '', size: 0)
expect(blob.simple_viewer).to be_a(BlobViewer::Empty)
end
@@ -118,7 +226,7 @@ describe Blob do
end
describe '#rich_viewer' do
- context 'when the blob is an invalid LFS pointer' do
+ context 'when the blob has an external storage error' do
before do
project.lfs_enabled = false
end
@@ -138,7 +246,7 @@ describe Blob do
end
end
- context 'when the blob is a valid LFS pointer' do
+ context 'when the blob is stored externally' do
it 'returns a matching viewer' do
blob = fake_blob(path: 'file.pdf', lfs: true)
diff --git a/spec/models/blob_viewer/base_spec.rb b/spec/models/blob_viewer/base_spec.rb
index a3e598de56d9819fe6a0a6f4b70f524d56af27fb..740ad9d275e94fd99f3581affe57109cd6ef06d9 100644
--- a/spec/models/blob_viewer/base_spec.rb
+++ b/spec/models/blob_viewer/base_spec.rb
@@ -139,7 +139,7 @@ describe BlobViewer::Base, model: true do
end
end
- context 'when the viewer is server side but the blob is stored in LFS' do
+ 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) }
@@ -148,8 +148,8 @@ describe BlobViewer::Base, model: true do
allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
end
- it 'return :server_side_but_stored_in_lfs' do
- expect(viewer.render_error).to eq(:server_side_but_stored_in_lfs)
+ it 'return :server_side_but_stored_externally' do
+ expect(viewer.render_error).to eq(:server_side_but_stored_externally)
end
end
end
diff --git a/spec/support/helpers/fake_blob_helpers.rb b/spec/support/helpers/fake_blob_helpers.rb
index b29af732ad320440c6b3b70f7ba418c05338ee5f..bc9686ed9cfc31884b1a99fe7c902c2bf9a42bad 100644
--- a/spec/support/helpers/fake_blob_helpers.rb
+++ b/spec/support/helpers/fake_blob_helpers.rb
@@ -1,6 +1,6 @@
module FakeBlobHelpers
class FakeBlob
- include Linguist::BlobHelper
+ include BlobLike
attr_reader :path, :size, :data, :lfs_oid, :lfs_size
@@ -19,10 +19,6 @@ module FakeBlobHelpers
alias_method :name, :path
- def mode
- nil
- end
-
def id
0
end
@@ -31,17 +27,11 @@ module FakeBlobHelpers
@binary
end
- def load_all_data!(repository)
- # No-op
+ def external_storage
+ :lfs if @lfs_pointer
end
- def lfs_pointer?
- @lfs_pointer
- end
-
- def truncated?
- false
- end
+ alias_method :external_size, :lfs_size
end
def fake_blob(**kwargs)