From 8c454b362425228ab14bb4ed5320f6ba2f505679 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 18 Feb 2016 18:19:43 -0500 Subject: [PATCH] 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. --- app/controllers/projects/blob_controller.rb | 2 +- app/helpers/blob_helper.rb | 4 - app/models/blob.rb | 34 +++++++++ app/views/projects/blob/_blob.html.haml | 12 +-- app/views/projects/blob/_image.html.haml | 2 +- spec/models/blob_spec.rb | 81 +++++++++++++++++++++ 6 files changed, 118 insertions(+), 17 deletions(-) create mode 100644 app/models/blob.rb create mode 100644 spec/models/blob_spec.rb 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