diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb index 25474b494ff..272837aa6ce 100644 --- a/app/uploaders/personal_file_uploader.rb +++ b/app/uploaders/personal_file_uploader.rb @@ -6,8 +6,15 @@ class PersonalFileUploader < FileUploader options.storage_path end - def self.base_dir(model, _store = nil) - File.join(options.base_dir, model_path_segment(model)) + def self.base_dir(model, store = nil) + base_dirs(model)[store || Store::LOCAL] + end + + def self.base_dirs(model) + { + Store::LOCAL => File.join(options.base_dir, model_path_segment(model)), + Store::REMOTE => model_path_segment(model) + } end def self.model_path_segment(model) @@ -33,13 +40,6 @@ class PersonalFileUploader < FileUploader store_dirs[object_store] end - def store_dirs - { - Store::LOCAL => File.join(base_dir, dynamic_segment), - Store::REMOTE => File.join(self.class.model_path_segment(model), dynamic_segment) - } - end - private def secure_url diff --git a/changelogs/unreleased/sh-fix-snippet-uploads-path-lookup.yml b/changelogs/unreleased/sh-fix-snippet-uploads-path-lookup.yml new file mode 100644 index 00000000000..414c8663049 --- /dev/null +++ b/changelogs/unreleased/sh-fix-snippet-uploads-path-lookup.yml @@ -0,0 +1,5 @@ +--- +title: Fix 404s with snippet uploads in object storage +merge_request: 24550 +author: +type: fixed diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb index 2896e9a112d..97758f0243e 100644 --- a/spec/uploaders/personal_file_uploader_spec.rb +++ b/spec/uploaders/personal_file_uploader_spec.rb @@ -4,19 +4,13 @@ describe PersonalFileUploader do let(:model) { create(:personal_snippet) } let(:uploader) { described_class.new(model) } let(:upload) { create(:upload, :personal_snippet_upload) } - let(:identifier) { %r{\h+/\S+} } subject { uploader } - it_behaves_like 'builds correct paths' do - let(:patterns) do - { - store_dir: %r[uploads/-/system/personal_snippet/\d+], - upload_path: identifier, - absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet/\d+/#{identifier}] - } - end - end + it_behaves_like 'builds correct paths', + store_dir: %r[uploads/-/system/personal_snippet/\d+], + upload_path: %r[\h+/\S+], + absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet\/\d+\/\h+\/\S+$] context "object_store is REMOTE" do before do @@ -25,13 +19,17 @@ describe PersonalFileUploader do include_context 'with storage', described_class::Store::REMOTE - it_behaves_like 'builds correct paths' do - let(:patterns) do - { - store_dir: %r[\d+/\h+], - upload_path: identifier - } - end + it_behaves_like 'builds correct paths', + store_dir: %r[\d+/\h+], + upload_path: %r[^personal_snippet\/\d+\/\h+\/] + end + + describe '#upload_paths' do + it 'builds correct paths for both local and remote storage' do + paths = uploader.upload_paths('test.jpg') + + expect(paths.first).to match(%r[\h+\/test.jpg]) + expect(paths.second).to match(%r[^personal_snippet\/\d+\/\h+\/test.jpg]) end end