Commit a8c62dfe authored by Robert Speicher's avatar Robert Speicher

Minor refactoring of Uploaders

- Moves a duplicate `file_storage?` definition into the common
  `GitlabUploader` ancestor.
- Get the `uploads` base directory from a class method rather than
  hard-coding it where it's needed. This will be used in a subsequent MR
  to store Uploads in the database.
- Improves the specs for uploaders.
parent 1f8d6c79
......@@ -27,10 +27,6 @@ class ArtifactUploader < GitlabUploader
File.join(self.class.artifacts_cache_path, @build.artifacts_path)
end
def file_storage?
self.class.storage == CarrierWave::Storage::File
end
def filename
file.try(:filename)
end
......
......@@ -4,6 +4,6 @@ class AttachmentUploader < GitlabUploader
storage :file
def store_dir
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
"#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
end
end
......@@ -4,7 +4,7 @@ class AvatarUploader < GitlabUploader
storage :file
def store_dir
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
"#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
end
def exists?
......
......@@ -4,15 +4,12 @@ class FileUploader < GitlabUploader
storage :file
attr_accessor :project, :secret
attr_accessor :project
attr_reader :secret
def initialize(project, secret = nil)
@project = project
@secret = secret || self.class.generate_secret
end
def base_dir
"uploads"
@secret = secret || generate_secret
end
def store_dir
......@@ -23,10 +20,6 @@ class FileUploader < GitlabUploader
File.join(base_dir, 'tmp', @project.path_with_namespace, @secret)
end
def secure_url
File.join("/uploads", @secret, file.filename)
end
def to_markdown
to_h[:markdown]
end
......@@ -35,17 +28,23 @@ class FileUploader < GitlabUploader
filename = image_or_video? ? self.file.basename : self.file.filename
escaped_filename = filename.gsub("]", "\\]")
markdown = "[#{escaped_filename}](#{self.secure_url})"
markdown = "[#{escaped_filename}](#{secure_url})"
markdown.prepend("!") if image_or_video? || dangerous?
{
alt: filename,
url: self.secure_url,
url: secure_url,
markdown: markdown
}
end
def self.generate_secret
private
def generate_secret
SecureRandom.hex
end
def secure_url
File.join('/uploads', @secret, file.filename)
end
end
class GitlabUploader < CarrierWave::Uploader::Base
def self.base_dir
'uploads'
end
delegate :base_dir, to: :class
def file_storage?
self.class.storage == CarrierWave::Storage::File
end
# Reduce disk IO
def move_to_cache
true
......
......@@ -27,6 +27,8 @@ module UploaderHelper
extension_match?(DANGEROUS_EXT)
end
private
def extension_match?(extensions)
return false unless file
......@@ -40,8 +42,4 @@ module UploaderHelper
extensions.include?(extension.downcase)
end
def file_storage?
self.class.storage == CarrierWave::Storage::File
end
end
require 'spec_helper'
describe AttachmentUploader do
let(:issue) { build(:issue) }
subject { described_class.new(issue) }
let(:uploader) { described_class.new(build_stubbed(:user)) }
describe '#move_to_cache' do
it 'is true' do
expect(subject.move_to_cache).to eq(true)
expect(uploader.move_to_cache).to eq(true)
end
end
describe '#move_to_store' do
it 'is true' do
expect(subject.move_to_store).to eq(true)
expect(uploader.move_to_store).to eq(true)
end
end
end
require 'spec_helper'
describe AvatarUploader do
let(:user) { build(:user) }
subject { described_class.new(user) }
let(:uploader) { described_class.new(build_stubbed(:user)) }
describe '#move_to_cache' do
it 'is false' do
expect(subject.move_to_cache).to eq(false)
expect(uploader.move_to_cache).to eq(false)
end
end
describe '#move_to_store' do
it 'is false' do
expect(subject.move_to_store).to eq(false)
expect(uploader.move_to_store).to eq(false)
end
end
end
require 'spec_helper'
describe FileUploader do
let(:project) { create(:project) }
let(:uploader) { described_class.new(build_stubbed(:project)) }
before do
@previous_enable_processing = FileUploader.enable_processing
FileUploader.enable_processing = false
@uploader = FileUploader.new(project)
end
describe 'initialize' do
it 'generates a secret if none is provided' do
expect(SecureRandom).to receive(:hex).and_return('secret')
after do
FileUploader.enable_processing = @previous_enable_processing
@uploader.remove!
end
uploader = described_class.new(double)
describe '#image_or_video?' do
context 'given an image file' do
before do
@uploader.store!(fixture_file_upload(Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')))
end
it 'detects an image based on file extension' do
expect(@uploader.image_or_video?).to be true
end
expect(uploader.secret).to eq 'secret'
end
context 'given an video file' do
before do
video_file = fixture_file_upload(Rails.root.join('spec', 'fixtures', 'video_sample.mp4'))
@uploader.store!(video_file)
end
it 'detects a video based on file extension' do
expect(@uploader.image_or_video?).to be true
end
end
it 'accepts a secret parameter' do
expect(SecureRandom).not_to receive(:hex)
it 'does not return image_or_video? for other types' do
@uploader.store!(fixture_file_upload(Rails.root.join('spec', 'fixtures', 'doc_sample.txt')))
uploader = described_class.new(double, 'secret')
expect(@uploader.image_or_video?).to be false
expect(uploader.secret).to eq 'secret'
end
end
describe '#move_to_cache' do
it 'is true' do
expect(@uploader.move_to_cache).to eq(true)
expect(uploader.move_to_cache).to eq(true)
end
end
describe '#move_to_store' do
it 'is true' do
expect(@uploader.move_to_store).to eq(true)
expect(uploader.move_to_store).to eq(true)
end
end
end
require 'rails_helper'
describe UploaderHelper do
class ExampleUploader < CarrierWave::Uploader::Base
include UploaderHelper
storage :file
end
def upload_fixture(filename)
fixture_file_upload(Rails.root.join('spec', 'fixtures', filename))
end
describe '#image_or_video?' do
let(:uploader) { ExampleUploader.new }
it 'returns true for an image file' do
uploader.store!(upload_fixture('dk.png'))
expect(uploader).to be_image_or_video
end
it 'it returns true for a video file' do
uploader.store!(upload_fixture('video_sample.mp4'))
expect(uploader).to be_image_or_video
end
it 'returns false for other extensions' do
uploader.store!(upload_fixture('doc_sample.txt'))
expect(uploader).not_to be_image_or_video
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment