Clean up CarrierWave's import/export files

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
This commit is contained in:
Stan Hu 2019-04-18 05:10:21 -07:00
parent bd73925b85
commit ec341a2bbd
3 changed files with 37 additions and 4 deletions

View file

@ -7,10 +7,6 @@ class ImportExportUploader < AttachmentUploader
EXTENSION_WHITELIST EXTENSION_WHITELIST
end end
def move_to_store
true
end
def move_to_cache def move_to_cache
false false
end end

View file

@ -0,0 +1,5 @@
---
title: Clean up CarrierWave's import/export files
merge_request: 27487
author:
type: fixed

View file

@ -3,9 +3,18 @@ require 'spec_helper'
describe ImportExportUploader do describe ImportExportUploader do
let(:model) { build_stubbed(:import_export_upload) } let(:model) { build_stubbed(:import_export_upload) }
let(:upload) { create(:upload, model: model) } let(:upload) { create(:upload, model: model) }
let(:import_export_upload) { ImportExportUpload.new }
subject { described_class.new(model, :import_file) } 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 true
end
end
end
context "object_store is REMOTE" do context "object_store is REMOTE" do
before do before do
stub_uploads_object_storage stub_uploads_object_storage
@ -16,5 +25,28 @@ describe ImportExportUploader do
it_behaves_like 'builds correct paths', it_behaves_like 'builds correct paths',
store_dir: %r[import_export_upload/import_file/], store_dir: %r[import_export_upload/import_file/],
upload_path: %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 false
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
end end