summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-04-18 05:10:21 -0700
committerStan Hu <stanhu@gmail.com>2019-04-19 14:07:04 -0700
commita872832d46dab9ea119118409bca45589263f3bf (patch)
tree72f7ce94dda1d6f92f4d911e1d4572dbaad63788
parentce02daea08c4cc7bc5e65e56f9b3d744a2e1faa6 (diff)
downloadgitlab-ce-sh-cleanup-import-export.tar.gz
Clean up CarrierWave's import/export filessh-cleanup-import-export
Unlike uploads that have been uploaded with Tempfile, the project import/export archives are stored in a temporary cache directory and remain there if: 1. Object storage is enabled 2. `move_to_store` is set to `true`. CarrierWave will leave these files there until disk space runs out or a clean step is run manually. If `move_to_store` is set to `false`, CarrierWave will remove the files after storing them. However, unlike a local file, with object storage, the file is still copied, so setting `move_to_store` to `true` doesn't buy us anything. To ensure files are cleaned up, we can just inherit from the GitlabUploader implementation of `move_to_store`, which returns `true` if it's a local file, `false` otherwise. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/60656
-rw-r--r--app/uploaders/import_export_uploader.rb4
-rw-r--r--changelogs/unreleased/sh-cleanup-import-export.yml5
-rw-r--r--spec/uploaders/import_export_uploader_spec.rb32
3 files changed, 37 insertions, 4 deletions
diff --git a/app/uploaders/import_export_uploader.rb b/app/uploaders/import_export_uploader.rb
index 716922bc017..104d5d3b3dd 100644
--- a/app/uploaders/import_export_uploader.rb
+++ b/app/uploaders/import_export_uploader.rb
@@ -7,10 +7,6 @@ class ImportExportUploader < AttachmentUploader
EXTENSION_WHITELIST
end
- def move_to_store
- true
- end
-
def move_to_cache
false
end
diff --git a/changelogs/unreleased/sh-cleanup-import-export.yml b/changelogs/unreleased/sh-cleanup-import-export.yml
new file mode 100644
index 00000000000..3d5d6f3c907
--- /dev/null
+++ b/changelogs/unreleased/sh-cleanup-import-export.yml
@@ -0,0 +1,5 @@
+---
+title: Clean up CarrierWave's import/export files
+merge_request: 27487
+author:
+type: fixed
diff --git a/spec/uploaders/import_export_uploader_spec.rb b/spec/uploaders/import_export_uploader_spec.rb
index 825c1cabc14..502ee1797db 100644
--- a/spec/uploaders/import_export_uploader_spec.rb
+++ b/spec/uploaders/import_export_uploader_spec.rb
@@ -3,9 +3,18 @@ require 'spec_helper'
describe ImportExportUploader do
let(:model) { build_stubbed(:import_export_upload) }
let(:upload) { create(:upload, model: model) }
+ let(:import_export_upload) { ImportExportUpload.new }
subject { described_class.new(model, :import_file) }
+ context 'local store' do
+ describe '#move_to_store' do
+ it 'returns true' do
+ expect(subject.move_to_store).to be_truthy
+ end
+ end
+ end
+
context "object_store is REMOTE" do
before do
stub_uploads_object_storage
@@ -16,5 +25,28 @@ describe ImportExportUploader do
it_behaves_like 'builds correct paths',
store_dir: %r[import_export_upload/import_file/],
upload_path: %r[import_export_upload/import_file/]
+
+ describe '#move_to_store' do
+ it 'returns false' do
+ expect(subject.move_to_store).to be_falsey
+ end
+ end
+
+ describe 'with an export file directly uploaded' do
+ let(:tempfile) { Tempfile.new(['test', '.gz']) }
+
+ before do
+ stub_uploads_object_storage(described_class, direct_upload: true)
+ import_export_upload.export_file = tempfile
+ end
+
+ it 'cleans up cached file' do
+ cache_dir = File.join(import_export_upload.export_file.cache_path(nil), '*')
+
+ import_export_upload.save!
+
+ expect(Dir[cache_dir]).to be_empty
+ end
+ end
end
end