Commit 5c41338f authored by Robert Speicher's avatar Robert Speicher

Handle relative and absolute Upload paths in the Uploaders

parent c4c2ac10
......@@ -6,6 +6,26 @@ class FileUploader < GitlabUploader
storage :file
def self.absolute_path(upload_record)
File.join(
self.dynamic_path_segment(upload_record.model),
upload_record.path
)
end
# Returns the part of `store_dir` that can change based on the model's current
# path
#
# This is used to build Upload paths dynamically based on the model's current
# namespace and path, allowing us to ignore renames or transfers.
#
# model - Object that responds to `path_with_namespace`
#
# Returns a String without a trailing slash
def self.dynamic_path_segment(model)
File.join(CarrierWave.root, base_dir, model.path_with_namespace)
end
attr_accessor :project
attr_reader :secret
......@@ -15,7 +35,7 @@ class FileUploader < GitlabUploader
end
def store_dir
File.join(base_dir, @project.path_with_namespace, @secret)
File.join(dynamic_path_segment, @secret)
end
def cache_dir
......@@ -26,6 +46,10 @@ class FileUploader < GitlabUploader
project
end
def relative_path
self.file.path.sub("#{dynamic_path_segment}/", '')
end
def to_markdown
to_h[:markdown]
end
......@@ -46,6 +70,10 @@ class FileUploader < GitlabUploader
private
def dynamic_path_segment
self.class.dynamic_path_segment(model)
end
def generate_secret
SecureRandom.hex
end
......
class GitlabUploader < CarrierWave::Uploader::Base
def self.absolute_path(upload_record)
File.join(CarrierWave.root, upload_record.path)
end
def self.base_dir
'uploads'
end
......@@ -18,4 +22,15 @@ class GitlabUploader < CarrierWave::Uploader::Base
def move_to_store
true
end
# Designed to be overridden by child uploaders that have a dynamic path
# segment -- that is, a path that changes based on mutable attributes of its
# associated model
#
# For example, `FileUploader` builds the storage path based on the associated
# project model's `path_with_namespace` value, which can change when the
# project or its containing namespace is moved or renamed.
def relative_path
self.file.path.sub("#{root}/", '')
end
end
......@@ -22,11 +22,7 @@ module RecordsUploads
Upload.record(self)
end
# When removing an attachment, destroy any Upload records at the same path
#
# Note: this _will not work_ for Uploaders which relativize paths, such as
# `FileUploader`, but because that uploader uses different paths for every
# upload, that's an acceptable caveat.
# Before removing an attachment, destroy any Upload records at the same path
#
# Called `before :remove`
def destroy_upload(*args)
......
......@@ -3,6 +3,18 @@ require 'spec_helper'
describe FileUploader do
let(:uploader) { described_class.new(build_stubbed(:empty_project)) }
describe '.absolute_path' do
it 'returns the correct absolute path by building it dynamically' do
project = build_stubbed(:project)
upload = double(model: project, path: 'secret/foo.jpg')
dynamic_segment = project.path_with_namespace
expect(described_class.absolute_path(upload))
.to end_with("#{dynamic_segment}/secret/foo.jpg")
end
end
describe 'initialize' do
it 'generates a secret if none is provided' do
expect(SecureRandom).to receive(:hex).and_return('secret')
......@@ -32,4 +44,13 @@ describe FileUploader do
expect(uploader.move_to_store).to eq(true)
end
end
describe '#relative_path' do
it 'removes the leading dynamic path segment' do
fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')
uploader.store!(fixture_file_upload(fixture))
expect(uploader.relative_path).to match(/\A\h{32}\/rails_sample.jpg\z/)
end
end
end
......@@ -2,7 +2,7 @@ require 'rails_helper'
describe RecordsUploads do
let(:uploader) do
example_uploader = Class.new(GitlabUploader) do
class RecordsUploadsExampleUploader < GitlabUploader
include RecordsUploads
storage :file
......@@ -12,7 +12,7 @@ describe RecordsUploads do
end
end
example_uploader.new
RecordsUploadsExampleUploader.new
end
def upload_fixture(filename)
......@@ -59,10 +59,10 @@ describe RecordsUploads do
it 'it destroys Upload records at the same path before recording' do
existing = Upload.create!(
path: File.join(CarrierWave.root, 'uploads', 'rails_sample.jpg'),
path: File.join('uploads', 'rails_sample.jpg'),
size: 512.kilobytes,
model: build_stubbed(:user),
uploader: described_class.to_s
uploader: uploader.class.to_s
)
uploader.store!(upload_fixture('rails_sample.jpg'))
......
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