summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatija Čupić <matteeyah@gmail.com>2019-08-08 16:25:38 +0200
committerMatija Čupić <matteeyah@gmail.com>2019-08-29 01:21:30 +0200
commit618ee3a5df84f70ed21f142d72c3d9a04d56dbbc (patch)
tree97adadf6bb4511cc5a56db2e9bb55fcfc6e1faa6
parentba13da2d4e9a0f02f54862ca061682fb6c1dfbdc (diff)
downloadgitlab-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.rb6
-rw-r--r--spec/models/ci/job_artifact_spec.rb40
-rw-r--r--spec/models/concerns/update_project_statistics_spec.rb61
-rw-r--r--spec/workers/ci/delete_stored_artifacts_worker_spec.rb71
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