CE implementation of multiple artifacts

See also: gitlab-org/gitlab-ce!14367

app/models/ci/build.rb

 module Ci
   class Build < CommitStatus
+    prepend ArtifactMigratable
     include TokenAuthenticatable
     include AfterCommitQueue
     include Presentable
@@ -13,9 +14,14 @@ module Ci
     has_many :sourced_pipelines, class_name: Ci::Sources::Pipeline, foreign_key: :source_job_id
 
     has_many :deployments, as: :deployable
+
     has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment'
     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
+
     # The "environment" field for builds is a String, and is the unexpanded name
     def persisted_environment
       @persisted_environment ||= Environment.find_by(
@@ -34,7 +40,9 @@ module Ci
 
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
-    scope :with_artifacts, ->() { where.not(artifacts_file: [nil, '']) }
+    scope :with_artifacts, ->() do
+      where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id'))
+    end
     scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
     scope :with_artifacts_stored_locally, ->() { with_artifacts.where(artifacts_file_store: [nil, ArtifactUploader::LOCAL_STORE]) }
@@ -330,18 +338,6 @@ module Ci
       project.running_or_pending_build_count(force: true)
     end
 
-    def artifacts?
-      !artifacts_expired? && artifacts_file.exists?
-    end
-
-    def browsable_artifacts?
-      artifacts_metadata?
-    end
-
-    def artifacts_metadata?
-      artifacts? && artifacts_metadata.exists?
-    end
-
     def artifacts_metadata_entry(path, **options)
       artifacts_metadata.use_file do |metadata_path|
         metadata = Gitlab::Ci::Build::Artifacts::Metadata.new(

app/models/ci/job_artifact.rb

+module Ci
+  class JobArtifact < ActiveRecord::Base
+    extend Gitlab::Ci::Model
+
+    belongs_to :project
+    belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id
+
+    before_save :set_size, if: :file_changed?
+    after_commit :remove_file!, on: :destroy
+
+    mount_uploader :file, JobArtifactUploader
+
+    enum file_type: {
+      archive: 1,
+      metadata: 2
+    }
+
+    def self.artifacts_size_for(project)
+      self.where(project: project).sum(:size)
+    end
+
+    def set_size
+      self.size = file.size
+    end
+  end
+end

app/models/concerns/artifact_migratable.rb

+# Adapter class to unify the interface between mounted uploaders and the
+# Ci::Artifact model
+# Meant to be prepended so the interface can stay the same
+module ArtifactMigratable
+  def artifacts_file
+    job_archive&.file || super
+  end
+
+  def artifacts_metadata
+    job_metadata&.file || super
+  end
+
+  def artifacts?
+    !artifacts_expired? && artifacts_file.exists?
+  end
+
+  def artifacts_metadata?
+    artifacts? && artifacts_metadata.exists?
+  end
+
+  def artifacts_file_changed?
+    job_archive&.file_changed? || super
+  end
+
+  def remove_artifacts_file!
+    if job_archive
+      job_archive.destroy
+    else
+      super
+    end
+  end
+
+  def remove_artifacts_metadata!
+    if job_metadata
+      job_metadata.destroy
+    else
+      super
+    end
+  end
+
+  def artifacts_size
+    read_attribute(:artifacts_size).to_i +
+      job_archive&.size.to_i + job_metadata&.size.to_i
+  end
+end

app/models/project_statistics.rb

@@ -39,7 +39,9 @@ class ProjectStatistics < ActiveRecord::Base
   end
 
   def update_build_artifacts_size
-    self.build_artifacts_size = project.builds.sum(:artifacts_size)
+    self.build_artifacts_size =
+      project.builds.sum(:artifacts_size) +
+      Ci::JobArtifact.artifacts_size_for(self)
   end
 
   def update_storage_size

app/uploaders/job_artifact_uploader.rb

+class JobArtifactUploader < GitlabUploader
+  storage :file
+
+  def self.local_artifacts_store
+    Gitlab.config.artifacts.path
+  end
+
+  def self.artifacts_upload_path
+    File.join(self.local_artifacts_store, 'tmp/uploads/')
+  end
+
+  def initialize(artifact, _field)
+    @artifact = artifact
+  end
+
+  def size
+    return super if @artifact.size.nil?
+
+    @artifact.size
+  end
+
+  def store_dir
+    File.join(self.class.local_artifacts_store, default_path)
+  end
+
+  private
+
+  def default_path
+    creation_date = @artifact.created_at.utc.strftime('%Y_%m_%d')
+
+    File.join(disk_hash[0..1], disk_hash[2..3], disk_hash,
+              creation_date, @artifact.job_id.to_s, @artifact.id.to_s)
+  end
+
+  def disk_hash
+    @disk_hash ||= Digest::SHA2.hexdigest(@artifact.project_id.to_s)
+  end
+end

config/gitlab.yml.example

@@ -785,6 +785,8 @@ test:
     # user: YOUR_USERNAME
   pages:
     path: tmp/tests/pages
+  artifacts:
+    path: tmp/tests/artifacts
   repositories:
     storages:
       default:

db/migrate/20170918072948_create_job_artifacts.rb

+class CreateJobArtifacts < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :ci_job_artifacts do |t|
+      t.belongs_to :project, null: false, index: true, foreign_key: { on_delete: :cascade }
+      t.integer :job_id, null: false, index: true
+      t.integer :size, limit: 8
+      t.integer :file_type, null: false, index: true
+
+      t.datetime_with_timezone :created_at, null: false
+      t.datetime_with_timezone :updated_at, null: false
+
+      t.string :file
+
+      t.foreign_key :ci_builds, column: :job_id, on_delete: :cascade
+    end
+  end
+end

db/schema.rb

@@ -396,6 +396,20 @@ ActiveRecord::Schema.define(version: 20171124165823) do
 
   add_index "ci_group_variables", ["group_id", "key"], name: "index_ci_group_variables_on_group_id_and_key", unique: true, using: :btree
 
+  create_table "ci_job_artifacts", force: :cascade do |t|
+    t.integer "project_id", null: false
+    t.integer "job_id", null: false
+    t.integer "size", limit: 8
+    t.integer "file_type", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.string "file"
+  end
+
+  add_index "ci_job_artifacts", ["file_type"], name: "index_ci_job_artifacts_on_file_type", using: :btree
+  add_index "ci_job_artifacts", ["job_id"], name: "index_ci_job_artifacts_on_job_id", using: :btree
+  add_index "ci_job_artifacts", ["project_id"], name: "index_ci_job_artifacts_on_project_id", using: :btree
+
   create_table "ci_pipeline_schedule_variables", force: :cascade do |t|
     t.string "key", null: false
     t.text "value"
@@ -456,9 +470,9 @@ ActiveRecord::Schema.define(version: 20171124165823) do
     t.integer "auto_canceled_by_id"
     t.integer "pipeline_schedule_id"
     t.integer "source"
-    t.integer "config_source"
     t.boolean "protected"
     t.integer "failure_reason"
+    t.integer "config_source"
   end
 
   add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree
@@ -2403,6 +2417,8 @@ ActiveRecord::Schema.define(version: 20171124165823) do
   add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade
   add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade
   add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade
+  add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade
+  add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade
   add_foreign_key "ci_pipeline_schedule_variables", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_41c35fda51", on_delete: :cascade
   add_foreign_key "ci_pipeline_schedules", "projects", name: "fk_8ead60fcc4", on_delete: :cascade
   add_foreign_key "ci_pipeline_schedules", "users", column: "owner_id", name: "fk_9ea99f58d2", on_delete: :nullify

lib/api/entities.rb

@@ -1134,13 +1134,9 @@ module API
         expose :type, :url, :username, :password
       end
 
-      class ArtifactFile < Grape::Entity
-        expose :filename, :size
-      end
-
       class Dependency < Grape::Entity
         expose :id, :name, :token
-        expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? }
+        expose :artifacts_file, using: JobArtifactFile, if: ->(job, _) { job.artifacts? }
       end
 
       class Response < Grape::Entity

lib/api/runner.rb

@@ -215,15 +215,16 @@ module API
         job = authenticate_job!
         forbidden!('Job is not running!') unless job.running?
 
-        artifacts_upload_path = ArtifactUploader.artifacts_upload_path
+        artifacts_upload_path = JobArtifactUploader.artifacts_upload_path
         artifacts = uploaded_file(:file, artifacts_upload_path)
         metadata = uploaded_file(:metadata, artifacts_upload_path)
 
         bad_request!('Missing artifacts file!') unless artifacts
         file_to_large! unless artifacts.size < max_artifacts_size
 
-        job.artifacts_file = artifacts
-        job.artifacts_metadata = metadata
+        job.job_artifacts.build(project: job.project, file_type: :archive, file: artifacts)
+        job.job_artifacts.build(project: job.project, file_type: :metadata, file: metadata)
+
         job.artifacts_expire_in = params['expire_in'] ||
           Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in
 

spec/factories/ci/builds.rb

@@ -155,38 +155,15 @@ FactoryGirl.define do
     end
 
     trait :artifacts do
-      after(:create) do |build, _|
-        build.artifacts_file =
-          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
-                             'application/zip')
-
-        build.artifacts_metadata =
-          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
-                             'application/x-gzip')
-
-        build.save!
+      after(:create) do |build|
+        create(:ci_job_artifact, job: build)
+        create(:ci_job_metadata, job: build)
+        build.reload
       end
     end
 
-    trait :remote_store do
-      artifacts_file_store ArtifactUploader::REMOTE_STORE
-      artifacts_metadata_store ArtifactUploader::REMOTE_STORE
-    end
-
-    trait :artifacts_expired do
-      after(:create) do |build, _|
-        build.artifacts_file =
-          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
-            'application/zip')
-
-        build.artifacts_metadata =
-          fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
-            'application/x-gzip')
-
-        build.artifacts_expire_at = 1.minute.ago
-
-        build.save!
-      end
+    trait :expired do
+      artifacts_expire_at 1.minute.ago
     end
 
     trait :with_commit do

spec/factories/ci/job_artifacts.rb

+include ActionDispatch::TestProcess
+
+FactoryGirl.define do
+  factory :ci_job_artifact, class: Ci::JobArtifact do
+    job factory: :ci_build
+    file_type :archive
+
+    after :build do |artifact|
+      artifact.project ||= artifact.job.project
+    end
+
+    after :create do |artifact|
+      if artifact.archive?
+        artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
+                                            'application/zip')
+        artifact.save
+      end
+    end
+  end
+
+  factory :ci_job_metadata, parent: :ci_job_artifact do
+    file_type :metadata
+
+    after :create do |artifact|
+      artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
+                                            'application/x-gzip')
+      artifact.save
+    end
+  end
+end

spec/features/projects/pipelines/pipelines_spec.rb

@@ -304,7 +304,7 @@ describe 'Pipelines', :js do
 
         context 'with artifacts expired' do
           let!(:with_artifacts_expired) do
-            create(:ci_build, :artifacts_expired, :success,
+            create(:ci_build, :expired, :success,
               pipeline: pipeline,
               name: 'rspec',
               stage: 'test')

spec/migrations/migrate_old_artifacts_spec.rb

@@ -16,20 +16,22 @@ describe MigrateOldArtifacts do
   end
 
   context 'with migratable data' do
-    let(:project1) { create(:project, ci_id: 2) }
-    let(:project2) { create(:project, ci_id: 3) }
-    let(:project3) { create(:project) }
+    set(:project1) { create(:project, ci_id: 2) }
+    set(:project2) { create(:project, ci_id: 3) }
+    set(:project3) { create(:project) }
 
-    let(:pipeline1) { create(:ci_empty_pipeline, project: project1) }
-    let(:pipeline2) { create(:ci_empty_pipeline, project: project2) }
-    let(:pipeline3) { create(:ci_empty_pipeline, project: project3) }
+    set(:pipeline1) { create(:ci_empty_pipeline, project: project1) }
+    set(:pipeline2) { create(:ci_empty_pipeline, project: project2) }
+    set(:pipeline3) { create(:ci_empty_pipeline, project: project3) }
 
     let!(:build_with_legacy_artifacts) { create(:ci_build, pipeline: pipeline1) }
     let!(:build_without_artifacts) { create(:ci_build, pipeline: pipeline1) }
-    let!(:build2) { create(:ci_build, :artifacts, pipeline: pipeline2) }
-    let!(:build3) { create(:ci_build, :artifacts, pipeline: pipeline3) }
+    let!(:build2) { create(:ci_build, pipeline: pipeline2) }
+    let!(:build3) { create(:ci_build, pipeline: pipeline3) }
 
     before do
+      setup_builds(build2, build3)
+
       store_artifacts_in_legacy_path(build_with_legacy_artifacts)
     end
 
@@ -113,5 +115,24 @@ describe MigrateOldArtifacts do
         build.project.ci_id.to_s,
         build.id.to_s)
     end
+
+    def new_legacy_path(build)
+      File.join(directory,
+                build.created_at.utc.strftime('%Y_%m'),
+                build.project_id.to_s,
+                build.id.to_s)
+    end
+
+    def setup_builds(*builds)
+      builds.each do |build|
+        FileUtils.mkdir_p(new_legacy_path(build))
+
+        build.update_columns(
+          artifacts_file: 'ci_build_artifacts.zip',
+          artifacts_metadata: 'ci_build_artifacts_metadata.gz')
+
+        build.reload
+      end
+    end
   end
 end

spec/models/ci/build_spec.rb

@@ -24,6 +24,8 @@ describe Ci::Build do
   it { is_expected.to respond_to(:has_trace?) }
   it { is_expected.to respond_to(:trace) }
 
+  it { is_expected.to be_a(ArtifactMigratable) }
+
   describe 'callbacks' do
     context 'when running after_create callback' do
       it 'triggers asynchronous build hooks worker' do
@@ -131,33 +133,26 @@ describe Ci::Build do
   end
 
   describe '#artifacts?' do
+    let(:build) { create(:ci_build, :artifacts) }
+
     subject { build.artifacts? }
 
     context 'artifacts archive does not exist' do
-      before do
-        build.update_attributes(artifacts_file: nil)
-      end
+      let(:build) { create(:ci_build) }
 
       it { is_expected.to be_falsy }
     end
 
     context 'artifacts archive exists' do
-      let(:build) { create(:ci_build, :artifacts) }
       it { is_expected.to be_truthy }
 
       context 'is expired' do
-        before do
-          build.update(artifacts_expire_at: Time.now - 7.days)
-        end
+        let!(:build) { create(:ci_build, :artifacts, :expired) }
 
         it { is_expected.to be_falsy }
       end
 
       context 'is not expired' do
-        before do
-          build.update(artifacts_expire_at: Time.now + 7.days)
-        end
-
         it { is_expected.to be_truthy }
       end
     end

spec/models/ci/job_artifact_spec.rb

+require 'spec_helper'
+
+describe Ci::JobArtifact do
+  set(:artifact) { create(:ci_job_artifact) }
+
+  describe "Associations" do
+    it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:job) }
+  end
+
+  it { is_expected.to respond_to(:file) }
+  it { is_expected.to respond_to(:created_at) }
+  it { is_expected.to respond_to(:updated_at) }
+
+  describe '#set_size' do
+    it 'sets the size' do
+      expect(artifact.size).to eq(106365)
+    end
+  end
+
+  describe '#file' do
+    subject { artifact.file }
+
+    context 'the uploader api' do
+      it { is_expected.to respond_to(:store_dir) }
+      it { is_expected.to respond_to(:cache_dir) }
+      it { is_expected.to respond_to(:work_dir) }
+    end
+  end
+end

spec/models/project_statistics_spec.rb

@@ -133,15 +133,17 @@ describe ProjectStatistics do
 
   describe '#update_build_artifacts_size' do
     let!(:pipeline) { create(:ci_pipeline, project: project) }
-    let!(:build1) { create(:ci_build, pipeline: pipeline, artifacts_size: 45.megabytes) }
-    let!(:build2) { create(:ci_build, pipeline: pipeline, artifacts_size: 56.megabytes) }
+    let!(:ci_build) { create(:ci_build, pipeline: pipeline, artifacts_size: 45.megabytes) }
 
     before do
+      create(:ci_build, pipeline: pipeline, artifacts_size: 56.megabytes)
+      create(:ci_job_artifact, project: pipeline.project, job: ci_build)
+
       statistics.update_build_artifacts_size
     end
 
     it "stores the size of related build artifacts" do
-      expect(statistics.build_artifacts_size).to eq 101.megabytes
+      expect(statistics.build_artifacts_size).to eq(106012541)
     end
   end
 

spec/requests/api/runner_spec.rb

@@ -948,7 +948,7 @@ describe API::Runner do
         context 'when artifacts are being stored inside of tmp path' do
           before do
             # by configuring this path we allow to pass temp file from any path
-            allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
+            allow(JobArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
           end
 
           context 'when job has been erased' do
@@ -1109,7 +1109,7 @@ describe API::Runner do
                 expect(response).to have_gitlab_http_status(201)
                 expect(stored_artifacts_file.original_filename).to eq(artifacts.original_filename)
                 expect(stored_metadata_file.original_filename).to eq(metadata.original_filename)
-                expect(stored_artifacts_size).to eq(71759)
+                expect(stored_artifacts_size).to eq(72821)
               end
             end
 

spec/serializers/pipeline_serializer_spec.rb

 require 'spec_helper'
 
 describe PipelineSerializer do
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
 
   let(:serializer) do
     described_class.new(current_user: user)
@@ -117,7 +117,8 @@ describe PipelineSerializer do
       shared_examples 'no N+1 queries' do
         it 'verifies number of queries', :request_store do
           recorded = ActiveRecord::QueryRecorder.new { subject }
-          expect(recorded.count).to be_within(1).of(61)
+
+          expect(recorded.count).to be_within(1).of(36)
           expect(recorded.cached_count).to eq(0)
         end
       end

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].freeze
+       erased_at auto_canceled_by job_artifacts job_archive job_metadata].freeze
 
   IGNORE_ACCESSORS =
     %i[type lock_version target_url base_tags trace_sections
@@ -35,7 +35,7 @@ describe Ci::RetryBuildService do
     end
 
     let(:build) do
-      create(:ci_build, :failed, :artifacts_expired, :erased,
+      create(:ci_build, :failed, :artifacts, :expired, :erased,
              :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
              :triggered, :trace, :teardown_environment,
              description: 'my-job', stage: 'test',  pipeline: pipeline,

spec/support/test_env.rb

@@ -121,6 +121,7 @@ module TestEnv
     FileUtils.mkdir_p(repos_path)
     FileUtils.mkdir_p(backup_path)
     FileUtils.mkdir_p(pages_path)
+    FileUtils.mkdir_p(artifacts_path)
   end
 
   def clean_gitlab_test_path
@@ -234,6 +235,10 @@ module TestEnv
     Gitlab.config.pages.path
   end
 
+  def artifacts_path
+    Gitlab.config.artifacts.path
+  end
+
   # When no cached assets exist, manually hit the root path to create them
   #
   # Otherwise they'd be created by the first test, often timing out and

spec/tasks/gitlab/backup_rake_spec.rb

@@ -24,6 +24,7 @@ describe 'gitlab:app namespace rake task' do
 
     # We need this directory to run `gitlab:backup:create` task
     FileUtils.mkdir_p('public/uploads')
+    FileUtils.mkdir_p(Rails.root.join('tmp/tests/artifacts'))
   end
 
   before do

spec/uploaders/artifact_uploader_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 describe ArtifactUploader do
   let(:store) { described_class::LOCAL_STORE }
-  let(:job) { create(:ci_build, artifacts_file_store: store) }
+  set(:job) { create(:ci_build, artifacts_file_store: store) }
   let(:uploader) { described_class.new(job, :artifacts_file) }
   let(:local_path) { Gitlab.config.artifacts.path }
 

spec/uploaders/job_artifact_uploader_spec.rb

+require 'spec_helper'
+
+describe JobArtifactUploader do
+  set(:job_artifact) { create(:ci_job_artifact) }
+  let(:job) { job_artifact.job }
+  let(:uploader) { described_class.new(job_artifact, :file) }
+
+  describe '#store_dir' do
+    subject { uploader.store_dir }
+
+    it { is_expected.to start_with(Gitlab.config.artifacts.path) }
+    it { is_expected.not_to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") }
+    it { is_expected.to match(/\h{2}\/\h{2}\/\h{64}\/\d{4}_\d{1,2}_\d{1,2}\/\d+\/\d+\z/) }
+  end
+end

spec/workers/expire_build_instance_artifacts_worker_spec.rb

@@ -11,12 +11,8 @@ describe ExpireBuildInstanceArtifactsWorker do
     end
 
     context 'with expired artifacts' do
-      let(:artifacts_expiry) { { artifacts_expire_at: Time.now - 7.days } }
-
       context 'when associated project is valid' do
-        let(:build) do
-          create(:ci_build, :artifacts, artifacts_expiry)
-        end
+        let(:build) { create(:ci_build, :artifacts, :expired) }
 
         it 'does expire' do
           expect(build.reload.artifacts_expired?).to be_truthy
@@ -26,14 +22,14 @@ describe ExpireBuildInstanceArtifactsWorker do
           expect(build.reload.artifacts_file.exists?).to be_falsey
         end
 
-        it 'does nullify artifacts_file column' do
-          expect(build.reload.artifacts_file_identifier).to be_nil
+        it 'does remove the job artifact record' do
+          expect(build.reload.job_archive).to be_nil
         end
       end
     end
 
     context 'with not yet expired artifacts' do
-      let(:build) do
+      set(:build) do
         create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days)
       end
 
@@ -45,8 +41,8 @@ describe ExpireBuildInstanceArtifactsWorker do
         expect(build.reload.artifacts_file.exists?).to be_truthy
       end
 
-      it 'does not nullify artifacts_file column' do
-        expect(build.reload.artifacts_file_identifier).not_to be_nil
+      it 'does not remove the job artifact record' do
+        expect(build.reload.job_archive).not_to be_nil
       end
     end
 
@@ -61,13 +57,13 @@ describe ExpireBuildInstanceArtifactsWorker do
         expect(build.reload.artifacts_file.exists?).to be_truthy
       end
 
-      it 'does not nullify artifacts_file column' do
-        expect(build.reload.artifacts_file_identifier).not_to be_nil
+      it 'does not remove the job artifact record' do
+        expect(build.reload.job_archive).not_to be_nil
       end
     end
 
     context 'for expired artifacts' do
-      let(:build) { create(:ci_build, artifacts_expire_at: Time.now - 7.days) }
+      let(:build) { create(:ci_build, :expired) }
 
       it 'is still expired' do
         expect(build.reload.artifacts_expired?).to be_truthy