diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 91747da28a17e6aba4b2ef66ca9b98adb67cd006..842323e26e3a79573a70660df77395ad6e7832ca 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -16,8 +16,8 @@ module Ci has_many :trace_sections, class_name: 'Ci::BuildTraceSection' has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_one :job_archive, -> () { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id - has_one :job_metadata, -> () { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id + has_one :artifacts_archive, -> () { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id + has_one :artifacts_metadata, -> () { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id # The "environment" field for builds is a String, and is the unexpanded name def persisted_environment diff --git a/app/models/concerns/artifact_migratable.rb b/app/models/concerns/artifact_migratable.rb index 811a8252459839b19c996c38bec7593c2af65aff..b99585a0b65a9f1b7177e7bcba6c9eea13e95130 100644 --- a/app/models/concerns/artifact_migratable.rb +++ b/app/models/concerns/artifact_migratable.rb @@ -3,11 +3,11 @@ # Meant to be prepended so the interface can stay the same module ArtifactMigratable def artifacts_file - job_archive&.file || legacy_artifacts_file + artifacts_archive&.file || legacy_artifacts_file end def artifacts_metadata - job_metadata&.file || legacy_artifacts_metadata + artifacts_metadata&.file || legacy_artifacts_metadata end def artifacts? @@ -19,20 +19,20 @@ module ArtifactMigratable end def artifacts_file_changed? - job_archive&.file_changed? || attribute_changed?(:artifacts_file) + artifacts_archive&.file_changed? || attribute_changed?(:artifacts_file) end def remove_artifacts_file! - if job_archive - job_archive.destroy + if artifacts_archive + artifacts_archive.destroy else remove_legacy_artifacts_file! end end def remove_artifacts_metadata! - if job_metadata - job_metadata.destroy + if artifacts_metadata + artifacts_metadata.destroy else remove_legacy_artifacts_metadata! end @@ -40,6 +40,6 @@ module ArtifactMigratable def artifacts_size read_attribute(:artifacts_size).to_i + - job_archive&.size.to_i + job_metadata&.size.to_i + artifacts_archive&.size.to_i + artifacts_metadata&.size.to_i end end diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index 17b9d2cf7b437e70ded9ddf4bb9be1f5ea37666d..b3d4c82583c6060c9fc3ce74fea4b145f47b294f 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -35,6 +35,9 @@ class ProjectStatistics < ActiveRecord::Base end def update_build_artifacts_size + # REMARK: + # We perform dual calculation as we run SQL query: sum does not instantiate AR model. + # The Ci::Build#artifacts_size returns sum of ci_builds.artifacts_size and ci_job_artifacts.file_size self.build_artifacts_size = project.builds.sum(:artifacts_size) + Ci::JobArtifact.artifacts_size_for(self) diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 20fe80beadeddaa764dd50e4094191dc2bb55462..3078e5b35e41c54cd1003e945569864db9621528 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -17,7 +17,7 @@ describe Ci::RetryBuildService do %i[id status user token coverage trace runner artifacts_expire_at artifacts_file artifacts_metadata artifacts_size created_at updated_at started_at finished_at queued_at erased_by - erased_at auto_canceled_by job_artifacts job_archive job_metadata].freeze + erased_at auto_canceled_by job_artifacts artifacts_archive artifacts_metadata].freeze IGNORE_ACCESSORS = %i[type lock_version target_url base_tags trace_sections diff --git a/spec/workers/expire_build_instance_artifacts_worker_spec.rb b/spec/workers/expire_build_instance_artifacts_worker_spec.rb index c0d2b1b7411fa6f02fa39b6c5e96757cf1c5e1c3..05e2aa0703b5eef3ac9cd4f1b462312578f76b91 100644 --- a/spec/workers/expire_build_instance_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_instance_artifacts_worker_spec.rb @@ -23,7 +23,7 @@ describe ExpireBuildInstanceArtifactsWorker do end it 'does remove the job artifact record' do - expect(build.reload.job_archive).to be_nil + expect(build.reload.artifacts_archive).to be_nil end end end @@ -42,7 +42,7 @@ describe ExpireBuildInstanceArtifactsWorker do end it 'does not remove the job artifact record' do - expect(build.reload.job_archive).not_to be_nil + expect(build.reload.artifacts_archive).not_to be_nil end end @@ -58,7 +58,7 @@ describe ExpireBuildInstanceArtifactsWorker do end it 'does not remove the job artifact record' do - expect(build.reload.job_archive).not_to be_nil + expect(build.reload.artifacts_archive).not_to be_nil end end