Commit 2aec9d32 authored by Micaël Bergeron's avatar Micaël Bergeron

fix the failing specs

parent 06cd10a3
...@@ -54,16 +54,6 @@ class Upload < ActiveRecord::Base ...@@ -54,16 +54,6 @@ class Upload < ActiveRecord::Base
}.compact }.compact
end end
private
def delete_file!
build_uploader.remove!
end
def checksummable?
checksum.nil? && local? && exist?
end
def local? def local?
return true if store.nil? return true if store.nil?
...@@ -72,6 +62,10 @@ class Upload < ActiveRecord::Base ...@@ -72,6 +62,10 @@ class Upload < ActiveRecord::Base
private private
def delete_file!
build_uploader.remove!
end
def checksummable? def checksummable?
checksum.nil? && local? && exist? checksum.nil? && local? && exist?
end end
......
...@@ -72,19 +72,29 @@ module ObjectStorage ...@@ -72,19 +72,29 @@ module ObjectStorage
end end
end end
# Add support for automatic background uploading after the file is stored.
#
module BackgroundUpload module BackgroundUpload
extend ActiveSupport::Concern extend ActiveSupport::Concern
def background_upload(mount_point) def background_upload(mount_points = [])
run_after_commit { send(mount_point).schedule_background_upload } return unless mount_points.any?
run_after_commit do
mount_points.each { |mount| send(mount).schedule_background_upload } # rubocop:disable GitlabSecurity/PublicSend
end
end
def changed_mounts
self.class.uploaders.select do |mount, uploader_class|
mounted_as = uploader_class.serialization_column(self.class, mount)
mount if send(:"#{mounted_as}_changed?") # rubocop:disable GitlabSecurity/PublicSend
end.keys
end end
included do included do
after_save on: [:create, :update] do after_save on: [:create, :update] do
self.class.uploaders.each do |mount, uploader_class| background_upload(changed_mounts)
mounted_as = uploader_class.serialization_column(self.class, mount)
background_upload(mount) if send(:"#{mounted_as}_changed?")
end
end end
end end
end end
......
...@@ -124,7 +124,7 @@ module ObjectStorage ...@@ -124,7 +124,7 @@ module ObjectStorage
Rails.logger.info header(success, failures) Rails.logger.info header(success, failures)
Rails.logger.warn failures(failures) Rails.logger.warn failures(failures)
raise MigrationFailures.new(failures.map(&:error)) raise MigrationFailures.new(failures.map(&:error)) if failures.any?
end end
def header(success, failures) def header(success, failures)
......
...@@ -12,10 +12,10 @@ namespace :gitlab do ...@@ -12,10 +12,10 @@ namespace :gitlab do
.where.not(store: @to_store) .where.not(store: @to_store)
.where(uploader: uploader_class.to_s, .where(uploader: uploader_class.to_s,
model_type: model_class.base_class.sti_name) model_type: model_class.base_class.sti_name)
.in_batches(of: batch_size, &method(:process)) # rubocop: disable Cop/InBatches .in_batches(of: batch_size, &method(:enqueue_batch)) # rubocop: disable Cop/InBatches
end end
def process(batch) def enqueue_batch(batch)
job = ObjectStorage::MigrateUploadsWorker.enqueue!(batch, job = ObjectStorage::MigrateUploadsWorker.enqueue!(batch,
@mounted_as, @mounted_as,
@to_store) @to_store)
......
...@@ -44,7 +44,8 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do ...@@ -44,7 +44,8 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
let(:mount_point) { nil } let(:mount_point) { nil }
it do it do
expect { described_class.sanity_check!(uploads, mount_point) }.to raise_error(described_class::SanityCheckError) expect { described_class.sanity_check!(uploads, mount_point) }
.to raise_error(described_class::SanityCheckError)
end end
end end
...@@ -69,7 +70,9 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do ...@@ -69,7 +70,9 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
describe '#perform' do describe '#perform' do
def perform def perform
described_class.enqueue!(uploads, mounted_as, to_store) described_class.new.perform(uploads.ids, mounted_as, to_store)
rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures
# swallow
end end
# rubocop:disable Style/MultilineIfModifier # rubocop:disable Style/MultilineIfModifier
...@@ -104,7 +107,6 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do ...@@ -104,7 +107,6 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
context 'migration is unsuccessful' do context 'migration is unsuccessful' do
before do before do
expect { described_class.perform }.to raise_error(described_class::Report::MigrationFailures)
allow_any_instance_of(ObjectStorage::Concern).to receive(:migrate!).and_raise(CarrierWave::UploadError, "I am a teapot.") allow_any_instance_of(ObjectStorage::Concern).to receive(:migrate!).and_raise(CarrierWave::UploadError, "I am a teapot.")
end end
......
require 'spec_helper'
describe ObjectStorage::BackgroundUploadWorker do
let(:local) { ObjectStorage::Store::LOCAL }
let(:remote) { ObjectStorage::Store::REMOTE }
def perform
described_class.perform_async(uploader_class.name, subject_class, file_field, subject_id)
end
context 'for LFS' do
let!(:lfs_object) { create(:lfs_object, :with_file, file_store: local) }
let(:uploader_class) { LfsObjectUploader }
let(:subject_class) { LfsObject }
let(:file_field) { :file }
let(:subject_id) { lfs_object.id }
context 'when object storage is enabled' do
before do
stub_lfs_object_storage(background_upload: true)
end
it 'uploads object to storage' do
expect { perform }.to change { lfs_object.reload.file_store }.from(local).to(remote)
end
context 'when background upload is disabled' do
before do
allow(Gitlab.config.lfs.object_store).to receive(:background_upload) { false }
end
it 'is skipped' do
expect { perform }.not_to change { lfs_object.reload.file_store }
end
end
end
context 'when object storage is disabled' do
before do
stub_lfs_object_storage(enabled: false)
end
it "doesn't migrate files" do
perform
expect(lfs_object.reload.file_store).to eq(local)
end
end
end
context 'for legacy artifacts' do
let(:build) { create(:ci_build, :legacy_artifacts) }
let(:uploader_class) { LegacyArtifactUploader }
let(:subject_class) { Ci::Build }
let(:file_field) { :artifacts_file }
let(:subject_id) { build.id }
context 'when local storage is used' do
let(:store) { local }
context 'and remote storage is defined' do
before do
stub_artifacts_object_storage(background_upload: true)
end
it "migrates file to remote storage" do
perform
expect(build.reload.artifacts_file_store).to eq(remote)
end
context 'for artifacts_metadata' do
let(:file_field) { :artifacts_metadata }
it 'migrates metadata to remote storage' do
perform
expect(build.reload.artifacts_metadata_store).to eq(remote)
end
end
end
end
end
context 'for job artifacts' do
let(:artifact) { create(:ci_job_artifact, :archive) }
let(:uploader_class) { JobArtifactUploader }
let(:subject_class) { Ci::JobArtifact }
let(:file_field) { :file }
let(:subject_id) { artifact.id }
context 'when local storage is used' do
let(:store) { local }
context 'and remote storage is defined' do
before do
stub_artifacts_object_storage(background_upload: true)
end
it "migrates file to remote storage" do
perform
expect(artifact.reload.file_store).to eq(remote)
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment