diff options
author | Stan Hu <stanhu@gmail.com> | 2019-01-22 03:03:37 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-01-22 09:13:37 +0300 |
commit | 940ad0c7a14e7ecff34300737e5802c4956bbb23 (patch) | |
tree | e6f9c10c29e6de635e7f2fc0161f89151cf68e16 /spec/uploaders | |
parent | c141d0afb15366beb1cae8a240faf6aaeb632214 (diff) |
Fix 404s with snippet uploads in object storage
Previously, an HTTP request for
`/uploads/-/system/personal_snippet/:snippet_id/:hash/:filename` would
look for an uploader of `PersonalFileUploader` class and use
`PersonalFileUploader#upload_paths` to search the datbase for one of the
following paths:
1. `:hash/:filename`
2. `uploads/-/system/personal_snippet/:id/:hash/:filename`
However, when the upload were stored in object storage,
`PersonalFileUploader#store_dirs` stored the path as:
`personal_snippet/:snippet_id/:hash`
The extraneous `uploads/-/system` prefix prevented the path from being
matched, and uploads in object storage would return a 404 error. Uploads
in local storage would work fine.
To fix this, we set the `#base_dir` properly so that `#upload_paths`
generates the right value for object storage. Note that this also makes
`#store_dirs` do the right thing in `FileUploader`.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/52595
Diffstat (limited to 'spec/uploaders')
-rw-r--r-- | spec/uploaders/personal_file_uploader_spec.rb | 32 |
1 files changed, 15 insertions, 17 deletions
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+\/<filename>] + 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 |