summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-02-20 01:12:52 +0000
committerDouwe Maan <douwe@gitlab.com>2016-02-20 01:12:52 +0000
commit3a97a5ddfd4618fa675a416a00e0c807edb974ca (patch)
treeaa5ca733af7138936d9924ffcfcb2f2bc96afb76
parentf81b55bd350761709b27b4668223305563be4afe (diff)
parent8c454b362425228ab14bb4ed5320f6ba2f505679 (diff)
downloadgitlab-ce-3a97a5ddfd4618fa675a416a00e0c807edb974ca.tar.gz
Merge branch 'rs-blob' into 'master'
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. See merge request !2887
-rw-r--r--app/controllers/projects/blob_controller.rb2
-rw-r--r--app/helpers/blob_helper.rb4
-rw-r--r--app/models/blob.rb34
-rw-r--r--app/views/projects/blob/_blob.html.haml12
-rw-r--r--app/views/projects/blob/_image.html.haml2
-rw-r--r--spec/models/blob_spec.rb81
6 files changed, 118 insertions, 17 deletions
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 495a432347e..cd8b2911674 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -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
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 16967927922..7143a744869 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -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.
diff --git a/app/models/blob.rb b/app/models/blob.rb
new file mode 100644
index 00000000000..8ee9f3006b2
--- /dev/null
+++ b/app/models/blob.rb
@@ -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
diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml
index f3bfe0a18b0..3ffc3fcb7ac 100644
--- a/app/views/projects/blob/_blob.html.haml
+++ b/app/views/projects/blob/_blob.html.haml
@@ -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
diff --git a/app/views/projects/blob/_image.html.haml b/app/views/projects/blob/_image.html.haml
index 113dba5d832..3c11b97921f 100644
--- a/app/views/projects/blob/_image.html.haml
+++ b/app/views/projects/blob/_image.html.haml
@@ -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)
diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb
new file mode 100644
index 00000000000..78e95c8fac5
--- /dev/null
+++ b/spec/models/blob_spec.rb
@@ -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