diff options
| -rw-r--r-- | app/uploaders/attachment_uploader.rb | 1 | ||||
| -rw-r--r-- | app/uploaders/avatar_uploader.rb | 1 | ||||
| -rw-r--r-- | app/uploaders/file_uploader.rb | 6 | ||||
| -rw-r--r-- | app/uploaders/records_uploads.rb | 38 | ||||
| -rw-r--r-- | spec/uploaders/file_uploader_spec.rb | 2 | ||||
| -rw-r--r-- | spec/uploaders/records_uploads_spec.rb | 97 | ||||
| -rw-r--r-- | spec/uploaders/uploader_helper_spec.rb | 12 |
7 files changed, 151 insertions, 6 deletions
diff --git a/app/uploaders/attachment_uploader.rb b/app/uploaders/attachment_uploader.rb index 6aa1f5a8c50..109eb2fea0b 100644 --- a/app/uploaders/attachment_uploader.rb +++ b/app/uploaders/attachment_uploader.rb @@ -1,4 +1,5 @@ class AttachmentUploader < GitlabUploader + include RecordsUploads include UploaderHelper storage :file diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index b4c393c6f2c..66d3bcb998a 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -1,4 +1,5 @@ class AvatarUploader < GitlabUploader + include RecordsUploads include UploaderHelper storage :file diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 0d2edaeff3b..2cf97a1b6fd 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -1,5 +1,7 @@ class FileUploader < GitlabUploader + include RecordsUploads include UploaderHelper + MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)} storage :file @@ -20,6 +22,10 @@ class FileUploader < GitlabUploader File.join(base_dir, 'tmp', @project.path_with_namespace, @secret) end + def model + project + end + def to_markdown to_h[:markdown] end diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb new file mode 100644 index 00000000000..7a0424b5adf --- /dev/null +++ b/app/uploaders/records_uploads.rb @@ -0,0 +1,38 @@ +module RecordsUploads + extend ActiveSupport::Concern + + included do + after :store, :record_upload + before :remove, :destroy_upload + end + + private + + # After storing an attachment, create a corresponding Upload record + # + # NOTE: We're ignoring the argument passed to this callback because we want + # the `SanitizedFile` object from `CarrierWave::Uploader::Base#file`, not the + # `Tempfile` object the callback gets. + # + # Called `after :store` + def record_upload(_tempfile) + return unless file_storage? + return unless file.exists? + + Upload.record(self) + end + + # When removing an attachment, destroy any Upload records at the same path + # + # Note: this _will not work_ for Uploaders which relativize paths, such as + # `FileUploader`, but because that uploader uses different paths for every + # upload, that's an acceptable caveat. + # + # Called `before :remove` + def destroy_upload(*args) + return unless file_storage? + return unless file + + Upload.remove_path(relative_path) + end +end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index b0f5be55c33..396b0abdaef 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe FileUploader do - let(:uploader) { described_class.new(build_stubbed(:project)) } + let(:uploader) { described_class.new(build_stubbed(:empty_project)) } describe 'initialize' do it 'generates a secret if none is provided' do diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb new file mode 100644 index 00000000000..a104dc4257f --- /dev/null +++ b/spec/uploaders/records_uploads_spec.rb @@ -0,0 +1,97 @@ +require 'rails_helper' + +describe RecordsUploads do + let(:uploader) do + example_uploader = Class.new(GitlabUploader) do + include RecordsUploads + + storage :file + + def model + FactoryGirl.build_stubbed(:user) + end + end + + example_uploader.new + end + + def upload_fixture(filename) + fixture_file_upload(Rails.root.join('spec', 'fixtures', filename)) + end + + describe 'callbacks' do + it 'calls `record_upload` after `store`' do + expect(uploader).to receive(:record_upload).once + + uploader.store!(upload_fixture('doc_sample.txt')) + end + + it 'calls `destroy_upload` after `remove`' do + expect(uploader).to receive(:destroy_upload).once + + uploader.store!(upload_fixture('doc_sample.txt')) + + uploader.remove! + end + end + + describe '#record_upload callback' do + it 'returns early when not using file storage' do + allow(uploader).to receive(:file_storage?).and_return(false) + expect(Upload).not_to receive(:record) + + uploader.store!(upload_fixture('rails_sample.jpg')) + end + + it "returns early when the file doesn't exist" do + allow(uploader).to receive(:file).and_return(double(exists?: false)) + expect(Upload).not_to receive(:record) + + uploader.store!(upload_fixture('rails_sample.jpg')) + end + + it 'creates an Upload record after store' do + expect(Upload).to receive(:record) + .with(uploader) + + uploader.store!(upload_fixture('rails_sample.jpg')) + end + + it 'it destroys Upload records at the same path before recording' do + existing = Upload.create!( + path: File.join(CarrierWave.root, 'uploads', 'rails_sample.jpg'), + size: 512.kilobytes, + model: build_stubbed(:user), + uploader: described_class.to_s + ) + + uploader.store!(upload_fixture('rails_sample.jpg')) + + expect { existing.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(Upload.count).to eq 1 + end + end + + describe '#destroy_upload callback' do + it 'returns early when not using file storage' do + uploader.store!(upload_fixture('rails_sample.jpg')) + + allow(uploader).to receive(:file_storage?).and_return(false) + expect(Upload).not_to receive(:remove_path) + + uploader.remove! + end + + it 'returns early when file is nil' do + expect(Upload).not_to receive(:remove_path) + + uploader.remove! + end + + it 'it destroys Upload records at the same path after removal' do + uploader.store!(upload_fixture('rails_sample.jpg')) + + expect { uploader.remove! }.to change { Upload.count }.from(1).to(0) + end + end +end diff --git a/spec/uploaders/uploader_helper_spec.rb b/spec/uploaders/uploader_helper_spec.rb index e9efd13b9aa..c47f09adb6d 100644 --- a/spec/uploaders/uploader_helper_spec.rb +++ b/spec/uploaders/uploader_helper_spec.rb @@ -1,10 +1,14 @@ require 'rails_helper' describe UploaderHelper do - class ExampleUploader < CarrierWave::Uploader::Base - include UploaderHelper + let(:uploader) do + example_uploader = Class.new(CarrierWave::Uploader::Base) do + include UploaderHelper - storage :file + storage :file + end + + example_uploader.new end def upload_fixture(filename) @@ -12,8 +16,6 @@ describe UploaderHelper do end describe '#image_or_video?' do - let(:uploader) { ExampleUploader.new } - it 'returns true for an image file' do uploader.store!(upload_fixture('dk.png')) |