Commit 0ac89cc0 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'poc-upload-hashing-path' into 'master'

File uploads on objects storage should use hashed storage

Closes #4952

See merge request gitlab-org/gitlab-ee!4597
parents 68a13836 501ac831
......@@ -62,19 +62,27 @@ module UploadsActions
end
def build_uploader_from_upload
return nil unless params[:secret] && params[:filename]
return unless uploader = build_uploader
upload_path = uploader_class.upload_path(params[:secret], params[:filename])
upload = Upload.find_by(uploader: uploader_class.to_s, path: upload_path)
upload_paths = uploader.upload_paths(params[:filename])
upload = Upload.find_by(uploader: uploader_class.to_s, path: upload_paths)
upload&.build_uploader
end
def build_uploader_from_params
return unless uploader = build_uploader
uploader.retrieve_from_store!(params[:filename])
uploader
end
def build_uploader
return unless params[:secret] && params[:filename]
uploader = uploader_class.new(model, secret: params[:secret])
return nil unless uploader.model_valid?
return unless uploader.model_valid?
uploader.retrieve_from_store!(params[:filename])
uploader
end
......
......@@ -32,8 +32,11 @@ class FileUploader < GitlabUploader
)
end
def self.base_dir(model)
model_path_segment(model)
def self.base_dir(model, store = Store::LOCAL)
decorated_model = model
decorated_model = Storage::HashedProject.new(model) if store == Store::REMOTE
model_path_segment(decorated_model)
end
# used in migrations and import/exports
......@@ -51,21 +54,24 @@ class FileUploader < GitlabUploader
#
# Returns a String without a trailing slash
def self.model_path_segment(model)
if model.hashed_storage?(:attachments)
model.disk_path
case model
when Storage::HashedProject then model.disk_path
else
model.full_path
model.hashed_storage?(:attachments) ? model.disk_path : model.full_path
end
end
def self.upload_path(secret, identifier)
File.join(secret, identifier)
end
def self.generate_secret
SecureRandom.hex
end
def upload_paths(filename)
[
File.join(secret, filename),
File.join(base_dir(Store::REMOTE), secret, filename)
]
end
attr_accessor :model
def initialize(model, mounted_as = nil, **uploader_context)
......@@ -75,8 +81,10 @@ class FileUploader < GitlabUploader
apply_context!(uploader_context)
end
def base_dir
self.class.base_dir(@model)
# enforce the usage of Hashed storage when storing to
# remote store as the FileMover doesn't support OS
def base_dir(store = nil)
self.class.base_dir(@model, store || object_store)
end
# we don't need to know the actual path, an uploader instance should be
......@@ -86,15 +94,19 @@ class FileUploader < GitlabUploader
end
def upload_path
self.class.upload_path(dynamic_segment, identifier)
end
def model_path_segment
self.class.model_path_segment(@model)
if file_storage?
# Legacy path relative to project.full_path
File.join(dynamic_segment, identifier)
else
File.join(store_dir, identifier)
end
end
def store_dir
File.join(base_dir, dynamic_segment)
def store_dirs
{
Store::LOCAL => File.join(base_dir, dynamic_segment),
Store::REMOTE => File.join(base_dir(ObjectStorage::Store::REMOTE), dynamic_segment)
}
end
def markdown_link
......
......@@ -4,7 +4,7 @@ class NamespaceFileUploader < FileUploader
options.storage_path
end
def self.base_dir(model)
def self.base_dir(model, _store = nil)
File.join(options.base_dir, 'namespace', model_path_segment(model))
end
......@@ -20,7 +20,7 @@ class NamespaceFileUploader < FileUploader
def store_dirs
{
Store::LOCAL => File.join(base_dir, dynamic_segment),
Store::REMOTE => File.join('namespace', model_path_segment, dynamic_segment)
Store::REMOTE => File.join('namespace', self.class.model_path_segment(model), dynamic_segment)
}
end
end
......@@ -4,7 +4,7 @@ class PersonalFileUploader < FileUploader
options.storage_path
end
def self.base_dir(model)
def self.base_dir(model, _store = nil)
File.join(options.base_dir, model_path_segment(model))
end
......@@ -34,7 +34,7 @@ class PersonalFileUploader < FileUploader
def store_dirs
{
Store::LOCAL => File.join(base_dir, dynamic_segment),
Store::REMOTE => File.join(model_path_segment, dynamic_segment)
Store::REMOTE => File.join(self.class.model_path_segment(model), dynamic_segment)
}
end
......
---
title: File uploads in remote storage now support project renaming.
merge_request: 4597
author:
type: fixed
......@@ -11,34 +11,30 @@ describe FileUploader do
shared_examples 'builds correct legacy storage paths' do
include_examples 'builds correct paths',
store_dir: %r{awesome/project/\h+},
upload_path: %r{\h+/<filename>},
absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg}
end
shared_examples 'uses hashed storage' do
context 'when rolled out attachments' do
let(:project) { build_stubbed(:project, namespace: group, name: 'project') }
context 'legacy storage' do
it_behaves_like 'builds correct legacy storage paths'
before do
allow(project).to receive(:disk_path).and_return('ca/fe/fe/ed')
end
context 'uses hashed storage' do
context 'when rolled out attachments' do
let(:project) { build_stubbed(:project, namespace: group, name: 'project') }
it_behaves_like 'builds correct paths',
store_dir: %r{ca/fe/fe/ed/\h+},
absolute_path: %r{#{described_class.root}/ca/fe/fe/ed/secret/foo.jpg}
end
include_examples 'builds correct paths',
store_dir: %r{@hashed/\h{2}/\h{2}/\h+},
upload_path: %r{\h+/<filename>}
end
context 'when only repositories are rolled out' do
let(:project) { build_stubbed(:project, namespace: group, name: 'project', storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) }
context 'when only repositories are rolled out' do
let(:project) { build_stubbed(:project, namespace: group, name: 'project', storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) }
it_behaves_like 'builds correct legacy storage paths'
it_behaves_like 'builds correct legacy storage paths'
end
end
end
context 'legacy storage' do
it_behaves_like 'builds correct legacy storage paths'
include_examples 'uses hashed storage'
end
context 'object store is remote' do
before do
stub_uploads_object_storage
......@@ -46,8 +42,10 @@ describe FileUploader do
include_context 'with storage', described_class::Store::REMOTE
it_behaves_like 'builds correct legacy storage paths'
include_examples 'uses hashed storage'
# always use hashed storage path for remote uploads
it_behaves_like 'builds correct paths',
store_dir: %r{@hashed/\h{2}/\h{2}/\h+},
upload_path: %r{@hashed/\h{2}/\h{2}/\h+/\h+/<filename>}
end
describe 'initialize' 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