kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Refactor Blob support of external storage in preparation of job artifact blobs

This commit is contained in:
Douwe Maan 2017-05-02 17:45:50 -05:00
parent 185fd98fd4
commit 720cc14a75
12 changed files with 309 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/controllers/projects/raw_controller.rb
View File

@ -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

24
app/helpers/blob_helper.rb
View File

@ -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

46
app/models/blob.rb
View File

@ -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

11
app/models/blob_viewer/base.rb
View File

@ -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

48
app/models/concerns/blob_like.rb Normal file
View File

@ -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

30
app/models/snippet_blob.rb
View File

@ -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

12
lib/gitlab/git/blob.rb
View File

@ -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?

43
spec/features/projects/blobs/blob_show_spec.rb
View File

@ -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

50
spec/helpers/blob_helper_spec.rb
View File

@ -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/)
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 '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
it 'includes a "download it" link' do
expect(helper.blob_render_error_options(viewer)).to include(/download it/)
end
end
context 'when the viewer is not rich' do
before do
viewer_class.type = :simple
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

124
spec/models/blob_spec.rb
View File

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

6
spec/models/blob_viewer/base_spec.rb
View File

@ -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

18
spec/support/helpers/fake_blob_helpers.rb
View File

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