summaryrefslogtreecommitdiff
path: root/spec/uploaders
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-01-21 16:03:37 -0800
committerStan Hu <stanhu@gmail.com>2019-01-21 22:13:37 -0800
commit940ad0c7a14e7ecff34300737e5802c4956bbb23 (patch)
treee6f9c10c29e6de635e7f2fc0161f89151cf68e16 /spec/uploaders
parentc141d0afb15366beb1cae8a240faf6aaeb632214 (diff)
downloadgitlab-ce-940ad0c7a14e7ecff34300737e5802c4956bbb23.tar.gz
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.rb32
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