Commit ce604106 authored by Erick Bajao's avatar Erick Bajao

Count nil artifact size as zero when recalculating

We have to ensure to return zero when size of artifact
is nil.

Changelog: fixed
parent 6855d997
...@@ -12,7 +12,7 @@ module Projects ...@@ -12,7 +12,7 @@ module Projects
if batch.any? if batch.any?
# We are doing the sum in ruby because the query takes too long when done in SQL # We are doing the sum in ruby because the query takes too long when done in SQL
total_artifacts_size = batch.sum(&:size) total_artifacts_size = batch.sum { |artifact| artifact.size.to_i }
Projects::BuildArtifactsSizeRefresh.transaction do Projects::BuildArtifactsSizeRefresh.transaction do
# Mark the refresh ready for another worker to pick up and process the next batch # Mark the refresh ready for another worker to pick up and process the next batch
......
...@@ -10,7 +10,8 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -10,7 +10,8 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
let_it_be(:artifact_1) { create(:ci_job_artifact, project: project, size: 1, created_at: 14.days.ago) } let_it_be(:artifact_1) { create(:ci_job_artifact, project: project, size: 1, created_at: 14.days.ago) }
let_it_be(:artifact_2) { create(:ci_job_artifact, project: project, size: 2, created_at: 13.days.ago) } let_it_be(:artifact_2) { create(:ci_job_artifact, project: project, size: 2, created_at: 13.days.ago) }
let_it_be(:artifact_3) { create(:ci_job_artifact, project: project, size: 5, created_at: 12.days.ago) } let_it_be(:artifact_3) { create(:ci_job_artifact, project: project, size: nil, created_at: 13.days.ago) }
let_it_be(:artifact_4) { create(:ci_job_artifact, project: project, size: 5, created_at: 12.days.ago) }
# This should not be included in the recalculation as it is created later than the refresh start time # This should not be included in the recalculation as it is created later than the refresh start time
let_it_be(:future_artifact) { create(:ci_job_artifact, project: project, size: 8, created_at: 2.days.from_now) } let_it_be(:future_artifact) { create(:ci_job_artifact, project: project, size: 8, created_at: 2.days.from_now) }
...@@ -33,7 +34,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -33,7 +34,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
end end
before do before do
stub_const("#{described_class}::BATCH_SIZE", 2) stub_const("#{described_class}::BATCH_SIZE", 3)
stats = create(:project_statistics, project: project, build_artifacts_size: 120) stats = create(:project_statistics, project: project, build_artifacts_size: 120)
stats.increment_counter(:build_artifacts_size, 30) stats.increment_counter(:build_artifacts_size, 30)
...@@ -48,7 +49,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -48,7 +49,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
end end
it 'updates the last_job_artifact_id to the ID of the last artifact from the batch' do it 'updates the last_job_artifact_id to the ID of the last artifact from the batch' do
expect { service.execute }.to change { refresh.reload.last_job_artifact_id.to_i }.to(artifact_2.id) expect { service.execute }.to change { refresh.reload.last_job_artifact_id.to_i }.to(artifact_3.id)
end end
it 'requeues the refresh job' do it 'requeues the refresh job' do
...@@ -62,7 +63,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -62,7 +63,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
:project_build_artifacts_size_refresh, :project_build_artifacts_size_refresh,
:pending, :pending,
project: project, project: project,
last_job_artifact_id: artifact_2.id last_job_artifact_id: artifact_3.id
) )
end end
...@@ -73,7 +74,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -73,7 +74,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
end end
it 'keeps the last_job_artifact_id unchanged' do it 'keeps the last_job_artifact_id unchanged' do
expect(refresh.reload.last_job_artifact_id).to eq(artifact_2.id) expect(refresh.reload.last_job_artifact_id).to eq(artifact_3.id)
end end
it 'keeps the state of the refresh record at running' do it 'keeps the state of the refresh record at running' do
...@@ -89,7 +90,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl ...@@ -89,7 +90,7 @@ RSpec.describe Projects::RefreshBuildArtifactsSizeStatisticsService, :clean_gitl
project: project, project: project,
updated_at: 2.days.ago, updated_at: 2.days.ago,
refresh_started_at: now, refresh_started_at: now,
last_job_artifact_id: artifact_3.id last_job_artifact_id: artifact_4.id
) )
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