summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/concerns/uploads_actions.rb18
-rw-r--r--app/uploaders/file_uploader.rb48
-rw-r--r--app/uploaders/namespace_file_uploader.rb4
-rw-r--r--app/uploaders/personal_file_uploader.rb4
-rw-r--r--ee/changelogs/unreleased/poc-upload-hashing-path.yml5
-rw-r--r--spec/uploaders/file_uploader_spec.rb38
6 files changed, 70 insertions, 47 deletions
diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb
index 3dbfabcae8a..f4e19615760 100644
--- a/app/controllers/concerns/uploads_actions.rb
+++ b/app/controllers/concerns/uploads_actions.rb
@@ -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
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 0e2da64de6a..133fdf6684d 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -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
diff --git a/app/uploaders/namespace_file_uploader.rb b/app/uploaders/namespace_file_uploader.rb
index 269415b1926..1085ecb1700 100644
--- a/app/uploaders/namespace_file_uploader.rb
+++ b/app/uploaders/namespace_file_uploader.rb
@@ -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
diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb
index d225002ad2e..e3898b07730 100644
--- a/app/uploaders/personal_file_uploader.rb
+++ b/app/uploaders/personal_file_uploader.rb
@@ -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
diff --git a/ee/changelogs/unreleased/poc-upload-hashing-path.yml b/ee/changelogs/unreleased/poc-upload-hashing-path.yml
new file mode 100644
index 00000000000..7970405bea1
--- /dev/null
+++ b/ee/changelogs/unreleased/poc-upload-hashing-path.yml
@@ -0,0 +1,5 @@
+---
+title: File uploads in remote storage now support project renaming.
+merge_request: 4597
+author:
+type: fixed
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index 4b7dcbeda33..db2810bbe1d 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -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