Rename Artifact to JobArtifact, split metadata out

Two things at ones, as there was no clean way to seperate the commit and
give me feedback from the tests.

But the model Artifact is now JobArtifact, and the table does not have a
type anymore, but the metadata is now its own model:
Ci::JobArtifactMetadata.

24 files changed, 238 insertions, 203 deletions

diff --git a/app/models/ci/artifact.rb b/app/models/ci/artifact.rb
deleted file mode 100644
index 858609883ce..00000000000
--- a/app/models/ci/artifact.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-module Ci
-  class Artifact < ActiveRecord::Base
-    extend Gitlab::Ci::Model
-
-    belongs_to :project
-    belongs_to :build, class_name: "Ci::Build", foreign_key: :ci_build_id
-
-    before_save :set_size, if: :file_changed?
-
-    mount_uploader :file, ArtifactUploader
-
-    enum type: { archive: 0, metadata: 1 }
-
-    # Allow us to use `type` as column name, without Rails thinking we're using
-    # STI: https://stackoverflow.com/a/29663933
-    def self.inheritance_column
-      nil
-    end
-
-    def set_size
-      self.size = file.exists? ? file.size : 0
-    end
-  end
-end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index fae2f5590b4..6d0ebd7f932 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -11,10 +11,15 @@ module Ci
     belongs_to :erased_by, class_name: 'User'
 
     has_many :deployments, as: :deployable
-    has_many :artifacts, class_name: 'Ci::Artifact', foreign_key: :ci_build_id
+
     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: :ci_job_id
+    has_one :job_archive, -> () { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', foreign_key: :ci_job_id
+    has_one :job_metadata, -> () { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', foreign_key: :ci_job_id
+
+
     # The "environment" field for builds is a String, and is the unexpanded name
     def persisted_environment
       @persisted_environment ||= Environment.find_by(
@@ -33,7 +38,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.ci_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 :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
@@ -423,7 +430,7 @@ module Ci
       Gitlab::Ci::Build::Image.from_services(self)
     end
 
-    def artifacts_options
+    def artifacts
       [options[:artifacts]]
     end
 
@@ -464,12 +471,6 @@ module Ci
       super(options).merge(when: read_attribute(:when))
     end
 
-    def update_project_statistics
-      return unless project
-
-      ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size])
-    end
-
     private
 
     def update_artifacts_size
@@ -560,6 +561,11 @@ module Ci
       pipeline.config_processor.build_attributes(name)
     end
 
+    def update_project_statistics
+      return unless project
+
+      ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size])
+    end
 
     def update_project_statistics_after_save
       if previous_changes.include?('artifacts_size')
diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb
new file mode 100644
index 00000000000..9c709077ac6
--- /dev/null
+++ b/app/models/ci/job_artifact.rb
@@ -0,0 +1,26 @@
+module Ci
+  class JobArtifact < ActiveRecord::Base
+    extend Gitlab::Ci::Model
+
+    belongs_to :project
+    belongs_to :job, class_name: "Ci::Build", foreign_key: :ci_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
diff --git a/app/models/concerns/artifact_migratable.rb b/app/models/concerns/artifact_migratable.rb
index a14a278df9f..8e331617cfa 100644
--- a/app/models/concerns/artifact_migratable.rb
+++ b/app/models/concerns/artifact_migratable.rb
@@ -3,15 +3,14 @@
 # Meant to be prepended so the interface can stay the same
 module ArtifactMigratable
   def artifacts_file
-    artifacts.archive.first&.file || super
+    job_archive&.file || super
   end
 
   def artifacts_metadata
-    artifacts.metadata.first&.file || super
+    job_metadata&.file || super
   end
 
   def artifacts?
-    byebug
     !artifacts_expired? && artifacts_file.exists?
   end
 
@@ -19,19 +18,28 @@ module ArtifactMigratable
     artifacts? && artifacts_metadata.exists?
   end
 
-  def remove_artifacts_file!
-    artifacts_file.remove!
+  def artifacts_file_changed?
+    job_archive&.file_changed? || super
   end
 
-  def remove_artifacts_metadata!
-    artifacts_metadata.remove!
+  def remove_artifacts_file!
+    if job_archive
+      job_archive.destroy
+    else
+      super
+    end
   end
 
-  def artifacts_file=(file)
-    artifacts.create(project: project, type: :archive, file: file)
+  def remove_artifacts_metadata!
+    if job_metadata
+      job_metadata.destroy
+    else
+      super
+    end
   end
 
-  def artifacts_metadata=(file)
-    artifacts.create(project: project, type: :metadata, file: file)
+  def artifacts_size
+    read_attribute(:artifacts_size).to_i +
+      job_archive&.size.to_i + job_metadata&.size.to_i
   end
 end
diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb
index 715b215d1db..a9c22d9cf18 100644
--- a/app/models/project_statistics.rb
+++ b/app/models/project_statistics.rb
@@ -35,7 +35,10 @@ 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) +
+      Ci::JobArtifactMetadata.artifacts_size_for(self)
   end
 
   def update_storage_size
diff --git a/app/uploaders/artifact_uploader.rb b/app/uploaders/artifact_uploader.rb
index 8ac0e2fe5a2..d4dda8e9e67 100644
--- a/app/uploaders/artifact_uploader.rb
+++ b/app/uploaders/artifact_uploader.rb
@@ -12,10 +12,6 @@ class ArtifactUploader < GitlabUploader
   end
 
   def initialize(job, field)
-    # Temporairy conditional, needed to move artifacts to their own table,
-    # but keeping compat with Ci::Build for the time being
-    job = job.build if job.respond_to?(:build)
-
     @job, @field = job, field
   end
 
@@ -38,6 +34,8 @@ class ArtifactUploader < GitlabUploader
   end
 
   def default_path
-    File.join(job.created_at.utc.strftime('%Y_%m'), job.project_id.to_s, job.id.to_s)
+    File.join(job.project_id.to_s,
+              job.created_at.utc.strftime('%Y_%m'),
+              job.id.to_s)
   end
 end
diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb
new file mode 100644
index 00000000000..6ea6a85b4a2
--- /dev/null
+++ b/app/uploaders/job_artifact_uploader.rb
@@ -0,0 +1,34 @@
+class JobArtifactUploader < ArtifactUploader
+  def initialize(artifact, _field)
+    @artifact = artifact
+  end
+
+  # If this record exists, the associatied artifact is there. Every artifact
+  # persisted will have an associated file
+  def exists?
+    true
+  end
+
+  def size
+    return super unless @artifact.size
+
+    @artifact.size
+  end
+
+  private
+
+  def disk_hash
+    @disk_hash ||= Digest::SHA2.hexdigest(job.project_id.to_s)
+  end
+
+  def default_path
+    creation_date = job.created_at.utc.strftime('%Y_%m_%d')
+
+    File.join(disk_hash[0..1], disk_hash[2..3], disk_hash,
+              creation_date, job.id.to_s, @artifact.id.to_s)
+  end
+
+  def job
+    @artifact.job
+  end
+end
diff --git a/db/migrate/20170918072948_create_artifacts.rb b/db/migrate/20170918072948_create_artifacts.rb
deleted file mode 100644
index 1dd5edc3f15..00000000000
--- a/db/migrate/20170918072948_create_artifacts.rb
+++ /dev/null
@@ -1,21 +0,0 @@
-class CreateArtifacts < ActiveRecord::Migration
-  def up
-    create_table :ci_artifacts do |t|
-      t.belongs_to :project, null: false, foreign_key: { on_delete: :cascade }
-      t.belongs_to :ci_build, null: false, foreign_key: { on_delete: :cascade }
-      t.integer :type, default: 0, null: false
-      t.integer :size, limit: 8, default: 0
-
-      t.datetime_with_timezone :created_at, null: false
-      t.datetime_with_timezone :updated_at, null: false
-
-      t.text :file
-    end
-
-    add_index(:ci_artifacts, [:project_id, :ci_build_id], unique: true)
-  end
-
-  def down
-    drop_table(:ci_artifacts)
-  end
-end
diff --git a/db/migrate/20170918072948_create_job_artifacts.rb b/db/migrate/20170918072948_create_job_artifacts.rb
new file mode 100644
index 00000000000..6d1756f368c
--- /dev/null
+++ b/db/migrate/20170918072948_create_job_artifacts.rb
@@ -0,0 +1,19 @@
+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 :ci_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
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index b4048371676..46a3b147198 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -226,18 +226,6 @@ ActiveRecord::Schema.define(version: 20171124150326) do
 
   add_index "chat_teams", ["namespace_id"], name: "index_chat_teams_on_namespace_id", unique: true, using: :btree
 
-  create_table "ci_artifacts", force: :cascade do |t|
-    t.integer "project_id", null: false
-    t.integer "ci_build_id", null: false
-    t.integer "type", default: 0, null: false
-    t.integer "size", limit: 8, default: 0
-    t.datetime_with_timezone "created_at", null: false
-    t.datetime_with_timezone "updated_at", null: false
-    t.text "file"
-  end
-
-  add_index "ci_artifacts", ["project_id", "ci_build_id"], name: "index_ci_artifacts_on_project_id_and_ci_build_id", unique: true, using: :btree
-
   create_table "ci_build_trace_section_names", force: :cascade do |t|
     t.integer "project_id", null: false
     t.string "name", null: false
@@ -331,6 +319,20 @@ ActiveRecord::Schema.define(version: 20171124150326) 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 "ci_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", ["ci_job_id"], name: "index_ci_job_artifacts_on_ci_job_id", using: :btree
+  add_index "ci_job_artifacts", ["file_type"], name: "index_ci_job_artifacts_on_file_type", 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"
@@ -391,9 +393,9 @@ ActiveRecord::Schema.define(version: 20171124150326) 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
@@ -1913,8 +1915,6 @@ ActiveRecord::Schema.define(version: 20171124150326) do
 
   add_foreign_key "boards", "projects", name: "fk_f15266b5f9", on_delete: :cascade
   add_foreign_key "chat_teams", "namespaces", on_delete: :cascade
-  add_foreign_key "ci_artifacts", "ci_builds", on_delete: :cascade
-  add_foreign_key "ci_artifacts", "projects", on_delete: :cascade
   add_foreign_key "ci_build_trace_section_names", "projects", on_delete: :cascade
   add_foreign_key "ci_build_trace_sections", "ci_build_trace_section_names", column: "section_name_id", name: "fk_264e112c66", on_delete: :cascade
   add_foreign_key "ci_build_trace_sections", "ci_builds", column: "build_id", name: "fk_4ebe41f502", on_delete: :cascade
@@ -1923,6 +1923,7 @@ ActiveRecord::Schema.define(version: 20171124150326) 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", "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
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 9eb2c98c436..0964bd98fbb 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1006,13 +1006,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
@@ -1036,7 +1032,7 @@ module API
         expose :steps, using: Step
         expose :image, using: Image
         expose :services, using: Service
-        expose :artifacts_options, as: :artifacts, using: Artifacts
+        expose :artifacts, using: Artifacts
         expose :cache, using: Cache
         expose :credentials, using: Credentials
         expose :dependencies, using: Dependency
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index a3987c560dd..8de0868c063 100644
--- a/lib/api/runner.rb
+++ b/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
 
diff --git a/spec/factories/ci/artifacts.rb b/spec/factories/ci/artifacts.rb
deleted file mode 100644
index 085e707d686..00000000000
--- a/spec/factories/ci/artifacts.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-include ActionDispatch::TestProcess
-
-FactoryGirl.define do
-  factory :artifact, class: Ci::Artifact do
-    project
-    build factory: :ci_build
-
-    after :create do |artifact|
-      artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
-      artifact.save
-    end
-
-    factory :artifact_metadata do
-      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
-end
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index 69d58f367ac..6cb612a58d2 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -155,14 +155,12 @@ FactoryGirl.define do
     end
 
     trait :artifacts do
-      after(:create) do |build, _|
-        create(:artifact, build: build)
-        create(:artifact_metadata, build: build)
-      end
+      job_archive factory: :ci_job_artifact
+      job_metadata factory: :ci_job_metadata
     end
 
     trait :expired do
-      artifacts_expire_at = 1.minute.ago
+      artifacts_expire_at 1.minute.ago
     end
 
     trait :with_commit do
diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb
new file mode 100644
index 00000000000..8a7e04c747f
--- /dev/null
+++ b/spec/factories/ci/job_artifacts.rb
@@ -0,0 +1,28 @@
+include ActionDispatch::TestProcess
+
+FactoryGirl.define do
+  factory :ci_job_artifact, class: Ci::JobArtifact do
+    project
+    job factory: :ci_build
+    file_type :archive
+
+    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
diff --git a/spec/models/ci/artifact_spec.rb b/spec/models/ci/artifact_spec.rb
deleted file mode 100644
index 5e39f73763b..00000000000
--- a/spec/models/ci/artifact_spec.rb
+++ /dev/null
@@ -1,59 +0,0 @@
-require 'spec_helper'
-
-describe Ci::Artifact do
-  it { is_expected.to belong_to(:project) }
-  it { is_expected.to belong_to(:build) }
-
-  it { is_expected.to respond_to(:file) }
-  it { is_expected.to respond_to(:created_at) }
-  it { is_expected.to respond_to(:updated_at) }
-  it { is_expected.to respond_to(:type) }
-
-  let(:artifact) { create(:artifact) }
-
-  describe '#type' do
-    it 'defaults to archive' do
-      expect(artifact.type).to eq("archive")
-    end
-  end
-
-  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
-
-  describe '#remove_file' do
-    it 'removes the file from the database' do
-      artifact.remove_file!
-
-      expect(artifact.file.exists?).to be_falsy
-    end
-  end
-
-  describe '#exists?' do
-    context 'when the artifact file is present' do
-      it 'is returns true' do
-        expect(artifact.exists?).to be(true)
-      end
-    end
-
-    context 'when the file has been removed' do
-      it 'does not exist' do
-        artifact.remove_file!
-
-        expect(artifact.exists?).to be_falsy
-      end
-    end
-  end
-end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 52a3732d0cd..f8dbed91c1a 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -146,7 +146,7 @@ describe Ci::Build do
       it { is_expected.to be_truthy }
 
       context 'is expired' do
-        let(:build) { create(:ci_build, :artifacts, :expired) }
+        let!(:build) { create(:ci_build, :artifacts, :expired) }
 
         it { is_expected.to be_falsy }
       end
diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb
new file mode 100644
index 00000000000..5202a8183af
--- /dev/null
+++ b/spec/models/ci/job_artifact_spec.rb
@@ -0,0 +1,30 @@
+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
diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb
index 59e20e84c2f..95e8d519bdd 100644
--- a/spec/models/project_statistics_spec.rb
+++ b/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
 
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 47f4ccd4887..f320a366e6e 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -945,7 +945,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
@@ -1106,7 +1106,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
 
diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb
index 8fc1ceedc34..7144b66585c 100644
--- a/spec/serializers/pipeline_serializer_spec.rb
+++ b/spec/serializers/pipeline_serializer_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe PipelineSerializer do
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
 
   let(:serializer) do
     described_class.new(current_user: user)
diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb
index 2a3bd0e3bb2..9cb2c090b43 100644
--- a/spec/uploaders/artifact_uploader_spec.rb
+++ b/spec/uploaders/artifact_uploader_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 describe ArtifactUploader do
-  let(:job) { create(:ci_build) }
+  set(:job) { create(:ci_build) }
   let(:uploader) { described_class.new(job, :artifacts_file) }
   let(:path) { Gitlab.config.artifacts.path }
 
@@ -26,7 +26,7 @@ describe ArtifactUploader do
     subject { uploader.store_dir }
 
     it { is_expected.to start_with(path) }
-    it { is_expected.to end_with("#{job.project_id}/#{job.id}") }
+    it { is_expected.to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") }
   end
 
   describe '#cache_dir' do
diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb
new file mode 100644
index 00000000000..d045acf9089
--- /dev/null
+++ b/spec/uploaders/job_artifact_uploader_spec.rb
@@ -0,0 +1,15 @@
+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
diff --git a/spec/workers/expire_build_instance_artifacts_worker_spec.rb b/spec/workers/expire_build_instance_artifacts_worker_spec.rb
index bed5c5e2ecb..c0d2b1b7411 100644
--- a/spec/workers/expire_build_instance_artifacts_worker_spec.rb
+++ b/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