diff options
author | Matija Čupić <matteeyah@gmail.com> | 2019-08-08 16:25:38 +0200 |
---|---|---|
committer | Matija Čupić <matteeyah@gmail.com> | 2019-08-29 01:21:30 +0200 |
commit | 618ee3a5df84f70ed21f142d72c3d9a04d56dbbc (patch) | |
tree | 97adadf6bb4511cc5a56db2e9bb55fcfc6e1faa6 | |
parent | ba13da2d4e9a0f02f54862ca061682fb6c1dfbdc (diff) | |
download | gitlab-ce-618ee3a5df84f70ed21f142d72c3d9a04d56dbbc.tar.gz |
Add specs for async deletion functionality
Adds missing specs for the async deletion functionality.
- Adds Ci::DeleteStoredArtifactsWorker specs
- Adds UpdateProjectStatistics specs
- Adds JobArtifact#finalize_fast_destroy specs
-rw-r--r-- | app/models/ci/job_artifact.rb | 6 | ||||
-rw-r--r-- | spec/models/ci/job_artifact_spec.rb | 40 | ||||
-rw-r--r-- | spec/models/concerns/update_project_statistics_spec.rb | 61 | ||||
-rw-r--r-- | spec/workers/ci/delete_stored_artifacts_worker_spec.rb | 71 |
4 files changed, 175 insertions, 3 deletions
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index c583743789a..87601ea0ac7 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -91,7 +91,7 @@ module Ci scope :scoped_project, -> { where('ci_job_artifacts.project_id = projects.id') } - delegate :filename, :exists?, :open, to: :file + delegate :filename, :exists?, :open, :store_path, to: :file enum file_type: { archive: 1, @@ -160,8 +160,8 @@ module Ci local_artifacts, remote_artifacts = artifacts.partition(&:local_store?) artifact_file_paths = { - ::JobArtifactUploader::Store::LOCAL => local_artifacts.map { |artifact| artifact.file.store_path }, - ::JobArtifactUploader::Store::REMOTE => remote_artifacts.map { |artifact| artifact.file.store_path } + ::JobArtifactUploader::Store::LOCAL => local_artifacts.map(&:store_path), + ::JobArtifactUploader::Store::REMOTE => remote_artifacts.map(&:store_path) } Ci::DeleteStoredArtifactsWorker.perform_async(artifact_file_paths) diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 1413da231e0..265afa8a998 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -95,6 +95,46 @@ describe Ci::JobArtifact do end end + describe '.finalize_fast_destroy' do + let(:project) { create(:project ) } + let(:local_artifacts) { create_list(:ci_job_artifact, 2, project: project, size: 100) } + let(:remote_artifacts) { create_list(:ci_job_artifact, 2, :remote_store, project: project, size: 100) } + + let(:artifact_list) do + { + project => local_artifacts + remote_artifacts + } + end + let(:expected_partition) do + { + ::JobArtifactUploader::Store::LOCAL => local_artifacts.map(&:store_path), + ::JobArtifactUploader::Store::REMOTE => remote_artifacts.map(&:store_path) + } + end + + before do + stub_artifacts_object_storage + end + + subject { described_class.finalize_fast_destroy(artifact_list) } + + it 'calls the async deletion worker' do + expect(Ci::DeleteStoredArtifactsWorker).to receive(:perform_async).with(expected_partition) + + subject + end + + it 'updates project statistics' do + allow(Ci::DeleteStoredArtifactsWorker).to receive(:perform_async) + + delta = local_artifacts.sum(&:size) + remote_artifacts.sum(&:size) + + expect(described_class).to receive(:update_project_statistics!).with(project, :build_artifacts_size, -delta) + + subject + end + end + describe 'callbacks' do subject { create(:ci_job_artifact, :archive) } diff --git a/spec/models/concerns/update_project_statistics_spec.rb b/spec/models/concerns/update_project_statistics_spec.rb new file mode 100644 index 00000000000..66877a6e348 --- /dev/null +++ b/spec/models/concerns/update_project_statistics_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UpdateProjectStatistics do + describe '.update_project_statistics!' do + let(:project) { create(:project) } + + subject do + Class.new(ActiveRecord::Base) do + self.table_name = 'ci_job_artifacts' + + include UpdateProjectStatistics + end.update_project_statistics!(project, :build_artifacts_size, 100) + end + + context 'when project is not pending delete' do + it 'increments project statistics' do + expect(ProjectStatistics).to receive(:increment_statistic).with(project.id, :build_artifacts_size, 100) + + subject + end + + context 'when update_statistics_namespace is enabled' do + before do + stub_feature_flags(update_statistics_namespace: true) + end + + it 'schedules the aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker).to receive(:perform_async).with(project.namespace_id) + + subject + end + end + + context 'when update_statistics_namespace is not enabled' do + before do + stub_feature_flags(update_statistics_namespace: false) + end + + it 'does not schedule the aggregation worker' do + expect(Namespaces::ScheduleAggregationWorker).not_to receive(:perform_async).with(project.namespace_id) + + subject + end + end + end + + context 'when project is pending delete' do + before do + project.pending_delete = true + end + + it 'does not increment project statistics' do + expect(ProjectStatistics).not_to receive(:increment_statistics).with(project.id, 'dummy_name', 100) + + subject + end + end + end +end diff --git a/spec/workers/ci/delete_stored_artifacts_worker_spec.rb b/spec/workers/ci/delete_stored_artifacts_worker_spec.rb new file mode 100644 index 00000000000..c47a4f23ed0 --- /dev/null +++ b/spec/workers/ci/delete_stored_artifacts_worker_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::DeleteStoredArtifactsWorker do + describe '#perform' do + let(:worker) { described_class.new } + + subject { worker.perform(file_paths) } + + context 'with a local artifact' do + let(:path) { File.join(Gitlab.config.artifacts['storage_path'], 'local_file_path') } + + let(:file_paths) do + { + ::JobArtifactUploader::Store::LOCAL.to_s => ['local_file_path'], + ::JobArtifactUploader::Store::REMOTE.to_s => [] + } + end + + before do + allow(File).to receive(:exist?).with(path).and_return(true) + end + + it 'deletes the local artifact' do + expect(File).to receive(:delete).with(path) + + subject + end + end + + context 'with a remote artifact' do + let(:file_double) { double } + + let(:file_paths) do + { + ::JobArtifactUploader::Store::LOCAL.to_s => [], + ::JobArtifactUploader::Store::REMOTE.to_s => ['remote_file_path'] + } + end + + before do + stub_artifacts_object_storage + + allow_any_instance_of(Fog::AWS::Storage::Files).to receive(:new).with(key: 'remote_file_path').and_return(file_double) + end + + it 'deletes the remote artifact' do + expect(file_double).to receive(:destroy) + + subject + end + end + + context 'with both local and remote artifacts' do + let(:file_paths) do + { + ::JobArtifactUploader::Store::LOCAL.to_s => ['local_file_path'], + ::JobArtifactUploader::Store::REMOTE.to_s => ['remote_file_path'] + } + end + + it 'calls correct delete methods' do + expect(worker).to receive(:delete_local_file).with('local_file_path').once + expect(worker).to receive(:delete_remote_file).with('remote_file_path').once + + subject + end + end + end +end |