Add a `Blob` model that wraps `Gitlab::Git::Blob`
This allows us to take advantage of Rails' `to_partial_path` to render the correct partial based on the Blob type, rather than cluttering the view with conditionals. It also allows (and will allow in the future) better encapsulation for Blob-related logic which makes sense for our Rails app but might not make as much sense for the core `gitlab_git` library, such as detecting if the blob is an SVG.
This commit is contained in:
parent
ea4d2741a2
commit
8c454b3624
|
@ -87,7 +87,7 @@ class Projects::BlobController < Projects::ApplicationController
|
|||
private
|
||||
|
||||
def blob
|
||||
@blob ||= @repository.blob_at(@commit.id, @path)
|
||||
@blob ||= Blob.decorate(@repository.blob_at(@commit.id, @path))
|
||||
|
||||
if @blob
|
||||
@blob
|
||||
|
|
|
@ -127,10 +127,6 @@ module BlobHelper
|
|||
end
|
||||
end
|
||||
|
||||
def blob_svg?(blob)
|
||||
blob.language && blob.language.name == 'SVG'
|
||||
end
|
||||
|
||||
# SVGs can contain malicious JavaScript; only include whitelisted
|
||||
# elements and attributes. Note that this whitelist is by no means complete
|
||||
# and may omit some elements.
|
||||
|
|
|
@ -0,0 +1,34 @@
|
|||
# Blob is a Rails-specific wrapper around Gitlab::Git::Blob objects
|
||||
class Blob < SimpleDelegator
|
||||
# Wrap a Gitlab::Git::Blob object, or return nil when given nil
|
||||
#
|
||||
# This method prevents the decorated object from evaluating to "truthy" when
|
||||
# given a nil value. For example:
|
||||
#
|
||||
# blob = Blob.new(nil)
|
||||
# puts "truthy" if blob # => "truthy"
|
||||
#
|
||||
# blob = Blob.decorate(nil)
|
||||
# puts "truthy" if blob # No output
|
||||
def self.decorate(blob)
|
||||
return if blob.nil?
|
||||
|
||||
new(blob)
|
||||
end
|
||||
|
||||
def svg?
|
||||
text? && language && language.name == 'SVG'
|
||||
end
|
||||
|
||||
def to_partial_path
|
||||
if lfs_pointer?
|
||||
'download'
|
||||
elsif image? || svg?
|
||||
'image'
|
||||
elsif text?
|
||||
'text'
|
||||
else
|
||||
'download'
|
||||
end
|
||||
end
|
||||
end
|
|
@ -32,14 +32,4 @@
|
|||
= number_to_human_size(blob_size(blob))
|
||||
.file-actions.hidden-xs
|
||||
= render "actions"
|
||||
- if blob.lfs_pointer?
|
||||
= render "download", blob: blob
|
||||
- elsif blob.text?
|
||||
- if blob_svg?(blob)
|
||||
= render "image", blob: blob
|
||||
- else
|
||||
= render "text", blob: blob
|
||||
- elsif blob.image?
|
||||
= render "image", blob: blob
|
||||
- else
|
||||
= render "download", blob: blob
|
||||
= render blob, blob: blob
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
.file-content.image_file
|
||||
- if blob_svg?(blob)
|
||||
- if blob.svg?
|
||||
- # We need to scrub SVG but we cannot do so in the RawController: it would
|
||||
- # be wrong/strange if RawController modified the data.
|
||||
- blob.load_all_data!(@repository)
|
||||
|
|
|
@ -0,0 +1,81 @@
|
|||
require 'rails_helper'
|
||||
|
||||
describe Blob do
|
||||
describe '.decorate' do
|
||||
it 'returns NilClass when given nil' do
|
||||
expect(described_class.decorate(nil)).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
describe '#svg?' do
|
||||
it 'is falsey when not text' do
|
||||
git_blob = double(text?: false)
|
||||
|
||||
expect(described_class.decorate(git_blob)).not_to be_svg
|
||||
end
|
||||
|
||||
it 'is falsey when no language is detected' do
|
||||
git_blob = double(text?: true, language: nil)
|
||||
|
||||
expect(described_class.decorate(git_blob)).not_to be_svg
|
||||
end
|
||||
|
||||
it' is falsey when language is not SVG' do
|
||||
git_blob = double(text?: true, language: double(name: 'XML'))
|
||||
|
||||
expect(described_class.decorate(git_blob)).not_to be_svg
|
||||
end
|
||||
|
||||
it 'is truthy when language is SVG' do
|
||||
git_blob = double(text?: true, language: double(name: 'SVG'))
|
||||
|
||||
expect(described_class.decorate(git_blob)).to be_svg
|
||||
end
|
||||
end
|
||||
|
||||
describe '#to_partial_path' do
|
||||
def stubbed_blob(overrides = {})
|
||||
overrides.reverse_merge!(
|
||||
image?: false,
|
||||
language: nil,
|
||||
lfs_pointer?: false,
|
||||
svg?: false,
|
||||
text?: false
|
||||
)
|
||||
|
||||
described_class.decorate(double).tap do |blob|
|
||||
allow(blob).to receive_messages(overrides)
|
||||
end
|
||||
end
|
||||
|
||||
it 'handles LFS pointers' do
|
||||
blob = stubbed_blob(lfs_pointer?: true)
|
||||
|
||||
expect(blob.to_partial_path).to eq 'download'
|
||||
end
|
||||
|
||||
it 'handles SVGs' do
|
||||
blob = stubbed_blob(text?: true, svg?: true)
|
||||
|
||||
expect(blob.to_partial_path).to eq 'image'
|
||||
end
|
||||
|
||||
it 'handles images' do
|
||||
blob = stubbed_blob(image?: true)
|
||||
|
||||
expect(blob.to_partial_path).to eq 'image'
|
||||
end
|
||||
|
||||
it 'handles text' do
|
||||
blob = stubbed_blob(text?: true)
|
||||
|
||||
expect(blob.to_partial_path).to eq 'text'
|
||||
end
|
||||
|
||||
it 'defaults to download' do
|
||||
blob = stubbed_blob
|
||||
|
||||
expect(blob.to_partial_path).to eq 'download'
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue