Merge branch 'zj-multiple-artifacts' into 'master'

Multiple artifacts

See merge request gitlab-org/gitlab-ce!14367

app/models/ci/build.rb

 module Ci
   class Build < CommitStatus
+    prepend ArtifactMigratable
     include TokenAuthenticatable
     include AfterCommitQueue
     include Presentable
@@ -10,9 +11,14 @@ module Ci
     belongs_to :erased_by, class_name: 'User'
 
     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_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
+    has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, 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(
@@ -31,15 +37,18 @@ 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 :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
     scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) }
     scope :ref_protected, -> { where(protected: true) }
 
-    mount_uploader :artifacts_file, ArtifactUploader
-    mount_uploader :artifacts_metadata, ArtifactUploader
+    mount_uploader :legacy_artifacts_file, LegacyArtifactUploader, mount_on: :artifacts_file
+    mount_uploader :legacy_artifacts_metadata, LegacyArtifactUploader, mount_on: :artifacts_metadata
 
     acts_as_taggable
 
@@ -326,14 +335,6 @@ module Ci
       project.running_or_pending_build_count(force: true)
     end
 
-    def artifacts?
-      !artifacts_expired? && artifacts_file.exists?
-    end
-
-    def artifacts_metadata?
-      artifacts? && artifacts_metadata.exists?
-    end
-
     def artifacts_metadata_entry(path, **options)
       metadata = Gitlab::Ci::Build::Artifacts::Metadata.new(
         artifacts_metadata.path,
@@ -386,6 +387,7 @@ module Ci
 
     def keep_artifacts!
       self.update(artifacts_expire_at: nil)
+      self.job_artifacts.update_all(expire_at: nil)
     end
 
     def coverage_regex
@@ -473,11 +475,7 @@ module Ci
     private
 
     def update_artifacts_size
-      self.artifacts_size = if artifacts_file.exists?
-                              artifacts_file.size
-                            else
-                              nil
-                            end
+      self.artifacts_size = legacy_artifacts_file&.size
     end
 
     def erase_trace!

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?
+
+    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
+
+    def expire_in
+      expire_at - Time.now if expire_at
+    end
+
+    def expire_in=(value)
+      self.expire_at =
+        if value
+          ChronicDuration.parse(value)&.seconds&.from_now
+        end
+    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_artifacts_archive&.file || legacy_artifacts_file
+  end
+
+  def artifacts_metadata
+    job_artifacts_metadata&.file || legacy_artifacts_metadata
+  end
+
+  def artifacts?
+    !artifacts_expired? && artifacts_file.exists?
+  end
+
+  def artifacts_metadata?
+    artifacts? && artifacts_metadata.exists?
+  end
+
+  def artifacts_file_changed?
+    job_artifacts_archive&.file_changed? || attribute_changed?(:artifacts_file)
+  end
+
+  def remove_artifacts_file!
+    if job_artifacts_archive
+      job_artifacts_archive.destroy
+    else
+      remove_legacy_artifacts_file!
+    end
+  end
+
+  def remove_artifacts_metadata!
+    if job_artifacts_metadata
+      job_artifacts_metadata.destroy
+    else
+      remove_legacy_artifacts_metadata!
+    end
+  end
+
+  def artifacts_size
+    read_attribute(:artifacts_size).to_i +
+      job_artifacts_archive&.size.to_i + job_artifacts_metadata&.size.to_i
+  end
+end

app/models/project_statistics.rb

@@ -35,7 +35,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/services/projects/update_pages_service.rb

@@ -18,7 +18,7 @@ module Projects
       @status.enqueue!
       @status.run!
 
-      raise 'missing pages artifacts' unless build.artifacts_file?
+      raise 'missing pages artifacts' unless build.artifacts?
       raise 'pages are outdated' unless latest?
 
       # Create temporary directory in which we will extract the artifacts

app/uploaders/job_artifact_uploader.rb

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

app/uploaders/artifact_uploader.rb → app/uploaders/legacy_artifact_uploader.rb

-class ArtifactUploader < GitlabUploader
+class LegacyArtifactUploader < GitlabUploader
   storage :file
 
-  attr_reader :job, :field
-
-  def self.local_artifacts_store
+  def self.local_store_path
     Gitlab.config.artifacts.path
   end
 
   def self.artifacts_upload_path
-    File.join(self.local_artifacts_store, 'tmp/uploads/')
-  end
-
-  def initialize(job, field)
-    @job, @field = job, field
+    File.join(self.local_store_path, 'tmp/uploads/')
   end
 
   def store_dir
@@ -20,20 +14,20 @@ class ArtifactUploader < GitlabUploader
   end
 
   def cache_dir
-    File.join(self.class.local_artifacts_store, 'tmp/cache')
+    File.join(self.class.local_store_path, 'tmp/cache')
   end
 
   def work_dir
-    File.join(self.class.local_artifacts_store, 'tmp/work')
+    File.join(self.class.local_store_path, 'tmp/work')
   end
 
   private
 
   def default_local_path
-    File.join(self.class.local_artifacts_store, default_path)
+    File.join(self.class.local_store_path, default_path)
   end
 
   def default_path
-    File.join(job.created_at.utc.strftime('%Y_%m'), job.project_id.to_s, job.id.to_s)
+    File.join(model.created_at.utc.strftime('%Y_%m'), model.project_id.to_s, model.id.to_s)
   end
 end

config/gitlab.yml.example

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

db/fixtures/development/14_pipelines.rb

@@ -124,11 +124,11 @@ class Gitlab::Seeder::Pipelines
     return unless %w[build test].include?(build.stage)
 
     artifacts_cache_file(artifacts_archive_path) do |file|
-      build.artifacts_file = file
+      build.job_artifacts.build(project: build.project, file_type: :archive, file: file)
     end
 
     artifacts_cache_file(artifacts_metadata_path) do |file|
-      build.artifacts_metadata = file
+      build.job_artifacts.build(project: build.project, file_type: :metadata, file: file)
     end
   end
 

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
+      t.integer :file_type, null: false
+      t.integer :size, limit: 8
+
+      t.datetime_with_timezone :created_at, null: false
+      t.datetime_with_timezone :updated_at, null: false
+      t.datetime_with_timezone :expire_at
+
+      t.string :file
+
+      t.foreign_key :ci_builds, column: :job_id, on_delete: :cascade
+      t.index [:job_id, :file_type], unique: true
+    end
+  end
+end

db/schema.rb

@@ -319,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 "job_id", null: false
+    t.integer "file_type", null: false
+    t.integer "size", limit: 8
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.datetime_with_timezone "expire_at"
+    t.string "file"
+  end
+
+  add_index "ci_job_artifacts", ["job_id", "file_type"], name: "index_ci_job_artifacts_on_job_id_and_file_type", unique: true, 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"
@@ -1909,6 +1923,8 @@ 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", "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

features/steps/project/pages.rb

@@ -44,8 +44,8 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps
                   project: @project,
                   pipeline: pipeline,
                   ref: 'HEAD',
-                  artifacts_file: fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip'),
-                  artifacts_metadata: fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip.meta')
+                  legacy_artifacts_file: fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip'),
+                  legacy_artifacts_metadata: fixture_file_upload(Rails.root + 'spec/fixtures/pages.zip.meta')
                  )
 
     result = ::Projects::UpdatePagesService.new(@project, build).execute

features/steps/shared/builds.rb

@@ -37,13 +37,13 @@ module SharedBuilds
   step 'recent build has artifacts available' do
     artifacts = Rails.root + 'spec/fixtures/ci_build_artifacts.zip'
     archive = fixture_file_upload(artifacts, 'application/zip')
-    @build.update_attributes(artifacts_file: archive)
+    @build.update_attributes(legacy_artifacts_file: archive)
   end
 
   step 'recent build has artifacts metadata available' do
     metadata = Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz'
     gzip = fixture_file_upload(metadata, 'application/x-gzip')
-    @build.update_attributes(artifacts_metadata: gzip)
+    @build.update_attributes(legacy_artifacts_metadata: gzip)
   end
 
   step 'recent build has a build trace' do

lib/api/entities.rb

@@ -1050,13 +1050,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,18 +215,20 @@ 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.artifacts_expire_in = params['expire_in'] ||
+        expire_in = params['expire_in'] ||
           Gitlab::CurrentSettings.current_application_settings.default_artifacts_expire_in
 
+        job.build_job_artifacts_archive(project: job.project, file_type: :archive, file: artifacts, expire_in: expire_in)
+        job.build_job_artifacts_metadata(project: job.project, file_type: :metadata, file: metadata, expire_in: expire_in) if metadata
+        job.artifacts_expire_in = expire_in
+
         if job.save
           present job, with: Entities::JobRequest::Response
         else

lib/backup/artifacts.rb

@@ -3,7 +3,7 @@ require 'backup/files'
 module Backup
   class Artifacts < Files
     def initialize
-      super('artifacts', ArtifactUploader.local_artifacts_store)
+      super('artifacts', LegacyArtifactUploader.local_store_path)
     end
 
     def create_files_dir

lib/gitlab/workhorse.rb

@@ -58,7 +58,7 @@ module Gitlab
       end
 
       def artifact_upload_ok
-        { TempPath: ArtifactUploader.artifacts_upload_path }
+        { TempPath: JobArtifactUploader.artifacts_upload_path }
       end
 
       def send_git_blob(repository, blob)

spec/factories/ci/builds.rb

@@ -154,36 +154,29 @@ FactoryGirl.define do
       runner factory: :ci_runner
     end
 
-    trait :artifacts do
+    trait :legacy_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!
+        build.update!(
+          legacy_artifacts_file: fixture_file_upload(
+            Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip'),
+          legacy_artifacts_metadata: fixture_file_upload(
+            Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
+        )
       end
     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!
+    trait :artifacts do
+      after(:create) do |build|
+        create(:ci_job_artifact, :archive, job: build)
+        create(:ci_job_artifact, :metadata, job: build)
+        build.reload
       end
     end
 
+    trait :expired do
+      artifacts_expire_at 1.minute.ago
+    end
+
     trait :with_commit do
       after(:build) do |build|
         allow(build).to receive(:commit).and_return build(:commit, :without_author)

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
+
+    trait :archive do
+      file_type :archive
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip')
+      end
+    end
+
+    trait :metadata do
+      file_type :metadata
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
+      end
+    end
+  end
+end

spec/features/commits_spec.rb

@@ -89,7 +89,7 @@ describe 'Commits' do
 
         context 'Download artifacts' do
           before do
-            build.update_attributes(artifacts_file: artifacts_file)
+            build.update_attributes(legacy_artifacts_file: artifacts_file)
           end
 
           it do
@@ -146,7 +146,7 @@ describe 'Commits' do
       context "when logged as reporter" do
         before do
           project.team << [user, :reporter]
-          build.update_attributes(artifacts_file: artifacts_file)
+          build.update_attributes(legacy_artifacts_file: artifacts_file)
           visit pipeline_path(pipeline)
         end
 
@@ -168,7 +168,7 @@ describe 'Commits' do
           project.update(
             visibility_level: Gitlab::VisibilityLevel::INTERNAL,
             public_builds: false)
-          build.update_attributes(artifacts_file: artifacts_file)
+          build.update_attributes(legacy_artifacts_file: artifacts_file)
           visit pipeline_path(pipeline)
         end
 

spec/features/merge_requests/mini_pipeline_graph_spec.rb

@@ -28,14 +28,14 @@ feature 'Mini Pipeline Graph', :js do
     let(:artifacts_file2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png') }
 
     before do
-      create(:ci_build, pipeline: pipeline, artifacts_file: artifacts_file1)
+      create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file1)
       create(:ci_build, pipeline: pipeline, when: 'manual')
     end
 
     it 'avoids repeated database queries' do
       before = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) }
 
-      create(:ci_build, pipeline: pipeline, artifacts_file: artifacts_file2)
+      create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file2)
       create(:ci_build, pipeline: pipeline, when: 'manual')
 
       after = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) }

spec/features/projects/jobs_spec.rb

@@ -187,7 +187,7 @@ feature 'Jobs' do
 
     context "Download artifacts" do
       before do
-        job.update_attributes(artifacts_file: artifacts_file)
+        job.update_attributes(legacy_artifacts_file: artifacts_file)
         visit project_job_path(project, job)
       end
 
@@ -198,7 +198,7 @@ feature 'Jobs' do
 
     context 'Artifacts expire date' do
       before do
-        job.update_attributes(artifacts_file: artifacts_file,
+        job.update_attributes(legacy_artifacts_file: artifacts_file,
                               artifacts_expire_at: expire_at)
 
         visit project_job_path(project, job)
@@ -422,14 +422,14 @@ feature 'Jobs' do
 
   describe "GET /:project/jobs/:id/download" do
     before do
-      job.update_attributes(artifacts_file: artifacts_file)
+      job.update_attributes(legacy_artifacts_file: artifacts_file)
       visit project_job_path(project, job)
       click_link 'Download'
     end
 
     context "Build from other project" do
       before do
-        job2.update_attributes(artifacts_file: artifacts_file)
+        job2.update_attributes(legacy_artifacts_file: artifacts_file)
         visit download_project_job_artifacts_path(project, job2)
       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