Commit a50ab5e1 authored by Micaël Bergeron's avatar Micaël Bergeron

port 42547 to ee

parent db735398
......@@ -70,7 +70,7 @@ module UploadsActions
end
def build_uploader_from_params
uploader = uploader_class.new(model, params[:secret])
uploader = uploader_class.new(model, secret: params[:secret])
uploader.retrieve_from_store!(params[:filename])
uploader
end
......
......@@ -33,7 +33,7 @@ class Upload < ActiveRecord::Base
end
def build_uploader
uploader_class.new(model).tap do |uploader|
uploader_class.new(model, mount_point, **uploader_context).tap do |uploader|
uploader.upload = self
uploader.retrieve_from_store!(identifier)
end
......@@ -43,6 +43,13 @@ class Upload < ActiveRecord::Base
File.exist?(absolute_path)
end
def uploader_context
{
identifier: identifier,
secret: secret
}.compact
end
private
def checksummable?
......@@ -67,11 +74,15 @@ class Upload < ActiveRecord::Base
!path.start_with?('/')
end
def uploader_class
Object.const_get(uploader)
end
def identifier
File.basename(path)
end
def uploader_class
Object.const_get(uploader)
def mount_point
super&.to_sym
end
end
......@@ -46,11 +46,11 @@ class FileMover
end
def uploader
@uploader ||= PersonalFileUploader.new(model, secret)
@uploader ||= PersonalFileUploader.new(model, secret: secret)
end
def temp_file_uploader
@temp_file_uploader ||= PersonalFileUploader.new(nil, secret)
@temp_file_uploader ||= PersonalFileUploader.new(nil, secret: secret)
end
def revert
......
......@@ -62,9 +62,11 @@ class FileUploader < GitlabUploader
attr_accessor :model
def initialize(model, secret = nil)
def initialize(model, mounted_as = nil, **uploader_context)
super(model, nil, **uploader_context)
@model = model
@secret = secret
apply_context!(uploader_context)
end
def base_dir
......@@ -107,15 +109,17 @@ class FileUploader < GitlabUploader
self.file.filename
end
# the upload does not hold the secret, but holds the path
# which contains the secret: extract it
def upload=(value)
super
return unless value
return if apply_context!(value.uploader_context)
# fallback to the regex based extraction
if matches = DYNAMIC_PATH_PATTERN.match(value.path)
@secret = matches[:secret]
@identifier = matches[:identifier]
end
super
end
def secret
......@@ -124,6 +128,18 @@ class FileUploader < GitlabUploader
private
def apply_context!(uploader_context)
@secret, @identifier = uploader_context.values_at(:secret, :identifier)
!!(@secret && @identifier)
end
def build_upload
super.tap do |upload|
upload.secret = secret
end
end
def markdown_name
(image_or_video? ? File.basename(filename, File.extname(filename)) : filename).gsub("]", "\\]")
end
......
......@@ -29,6 +29,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
delegate :base_dir, :file_storage?, to: :class
def initialize(model, mounted_as = nil, **uploader_context)
super(model, mounted_as)
end
def file_cache_storage?
cache_storage.is_a?(CarrierWave::Storage::File)
end
......
......@@ -24,7 +24,7 @@ module RecordsUploads
uploads.where(path: upload_path).delete_all
upload.destroy! if upload
self.upload = build_upload_from_uploader(self)
self.upload = build_upload
upload.save!
end
end
......@@ -39,12 +39,13 @@ module RecordsUploads
Upload.order(id: :desc).where(uploader: self.class.to_s)
end
def build_upload_from_uploader(uploader)
def build_upload
Upload.new(
size: uploader.file.size,
path: uploader.upload_path,
model: uploader.model,
uploader: uploader.class.to_s
uploader: self.class.to_s,
size: file.size,
path: upload_path,
model: model,
mount_point: mounted_as
)
end
......
---
title: Added uploader metadata to the uploads.
merge_request: 16779
author:
type: added
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddUploadsBuilderContext < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
def change
add_column :uploads, :mount_point, :string
add_column :uploads, :secret, :string
end
end
......@@ -2255,6 +2255,8 @@ ActiveRecord::Schema.define(version: 20180202111106) do
t.string "uploader", null: false
t.datetime "created_at", null: false
t.integer "store"
t.string "mount_point"
t.string "secret"
end
add_index "uploads", ["checksum"], name: "index_uploads_on_checksum", using: :btree
......
......@@ -46,7 +46,7 @@ module Gitlab
private
def find_file(project, secret, file)
uploader = FileUploader.new(project, secret)
uploader = FileUploader.new(project, secret: secret)
uploader.retrieve_from_store!(file)
uploader.file
end
......
......@@ -3,26 +3,29 @@ FactoryBot.define do
model { build(:project) }
size 100.kilobytes
uploader "AvatarUploader"
mount_point :avatar
secret nil
store ObjectStorage::Store::LOCAL
# we should build a mount agnostic upload by default
transient do
mounted_as :avatar
secret SecureRandom.hex
filename 'myfile.jpg'
end
# this needs to comply with RecordsUpload::Concern#upload_path
path { File.join("uploads/-/system", model.class.to_s.underscore, mounted_as.to_s, 'avatar.jpg') }
path { File.join("uploads/-/system", model.class.to_s.underscore, mount_point.to_s, 'avatar.jpg') }
trait :personal_snippet_upload do
model { build(:personal_snippet) }
path { File.join(secret, 'myfile.jpg') }
path { File.join(secret, filename) }
uploader "PersonalFileUploader"
secret SecureRandom.hex
end
trait :issuable_upload do
path { File.join(secret, 'myfile.jpg') }
path { File.join(secret, filename) }
uploader "FileUploader"
secret SecureRandom.hex
end
trait :object_storage do
......@@ -31,13 +34,14 @@ FactoryBot.define do
trait :namespace_upload do
model { build(:group) }
path { File.join(secret, 'myfile.jpg') }
path { File.join(secret, filename) }
uploader "NamespaceFileUploader"
secret SecureRandom.hex
end
trait :attachment_upload do
transient do
mounted_as :attachment
mount_point :attachment
end
model { build(:note) }
......
......@@ -2,8 +2,8 @@ require 'spec_helper'
describe FileSizeValidator do
let(:validator) { described_class.new(options) }
let(:attachment) { AttachmentUploader.new }
let(:note) { create(:note) }
let(:attachment) { AttachmentUploader.new(note) }
describe 'options uses an integer' do
let(:options) { { maximum: 10, attributes: { attachment: attachment } } }
......
......@@ -103,4 +103,10 @@ describe Upload do
expect(upload).not_to exist
end
end
describe "#uploader_context" do
subject { create(:upload, :issuable_upload, secret: 'secret', filename: 'file.txt') }
it { expect(subject.uploader_context).to match(a_hash_including(secret: 'secret', identifier: 'file.txt')) }
end
end
......@@ -51,7 +51,7 @@ describe FileUploader do
end
describe 'initialize' do
let(:uploader) { described_class.new(double, 'secret') }
let(:uploader) { described_class.new(double, secret: 'secret') }
it 'accepts a secret parameter' do
expect(described_class).not_to receive(:generate_secret)
......@@ -75,4 +75,31 @@ describe FileUploader do
it_behaves_like "migrates", to_store: described_class::Store::REMOTE
it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL
end
describe '#upload=' do
let(:secret) { SecureRandom.hex }
let(:upload) { create(:upload, :issuable_upload, secret: secret, filename: 'file.txt') }
it 'handles nil' do
expect(uploader).not_to receive(:apply_context!)
uploader.upload = nil
end
it 'extract the uploader context from it' do
expect(uploader).to receive(:apply_context!).with(a_hash_including(secret: secret, identifier: 'file.txt'))
uploader.upload = upload
end
context 'uploader_context is empty' do
it 'fallbacks to regex based extraction' do
expect(upload).to receive(:uploader_context).and_return({})
uploader.upload = upload
expect(uploader.secret).to eq(secret)
expect(uploader.instance_variable_get(:@identifier)).to eq('file.txt')
end
end
end
end
......@@ -4,7 +4,7 @@ require 'carrierwave/storage/fog'
describe GitlabUploader do
let(:uploader_class) { Class.new(described_class) }
subject { uploader_class.new }
subject { uploader_class.new(double) }
describe '#file_storage?' do
context 'when file storage is used' do
......
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