diff options
-rw-r--r-- | app/models/ci/build.rb | 7 | ||||
-rw-r--r-- | app/models/ci/job_artifact.rb | 11 | ||||
-rw-r--r-- | db/migrate/20170918072948_create_job_artifacts.rb | 3 | ||||
-rw-r--r-- | db/schema.rb | 3 | ||||
-rw-r--r-- | lib/api/runner.rb | 9 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 14 | ||||
-rw-r--r-- | spec/models/ci/job_artifact_spec.rb | 44 |
7 files changed, 81 insertions, 10 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d5c7cd0bb6a..a19273484ef 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_artifacts_archive, -> () { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id - has_one :job_artifacts_metadata, -> () { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', foreign_key: :job_id + 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 @@ -38,7 +38,7 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } 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')) + where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.build_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) } @@ -386,6 +386,7 @@ module Ci def keep_artifacts! self.update(artifacts_expire_at: nil) + self.job_artifacts.update_all(expire_at: nil) end def coverage_regex diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index e02fa021409..84fc6863567 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -21,5 +21,16 @@ module Ci 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 diff --git a/db/migrate/20170918072948_create_job_artifacts.rb b/db/migrate/20170918072948_create_job_artifacts.rb index 90152127ed5..d6c6a228ed1 100644 --- a/db/migrate/20170918072948_create_job_artifacts.rb +++ b/db/migrate/20170918072948_create_job_artifacts.rb @@ -7,11 +7,12 @@ class CreateJobArtifacts < ActiveRecord::Migration 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.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 diff --git a/db/schema.rb b/db/schema.rb index 3ec183f1c05..64f67574b3f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -322,10 +322,11 @@ ActiveRecord::Schema.define(version: 20171130145523) do 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.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 diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 8de0868c063..80feb629d54 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -222,12 +222,13 @@ module API bad_request!('Missing artifacts file!') unless artifacts file_to_large! unless artifacts.size < max_artifacts_size - 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'] || + 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 diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 212ff36633b..9070692abfe 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1008,11 +1008,23 @@ describe Ci::Build do describe '#keep_artifacts!' do let(:build) { create(:ci_build, artifacts_expire_at: Time.now + 7.days) } + subject { build.keep_artifacts! } + it 'to reset expire_at' do - build.keep_artifacts! + subject expect(build.artifacts_expire_at).to be_nil end + + context 'when having artifacts files' do + let!(:artifact) { create(:ci_job_artifact, job: build, expire_in: '7 days') } + + it 'to reset dependent objects' do + subject + + expect(artifact.reload.expire_at).to be_nil + end + end end describe '#merge_request' do diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 9dd5cba150d..cdde1ed207a 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -27,4 +27,48 @@ describe Ci::JobArtifact do it { is_expected.to respond_to(:work_dir) } end end + + describe '#expire_in' do + subject { artifact.expire_in } + + it { is_expected.to be_nil } + + context 'when expire_at is specified' do + let(:expire_at) { Time.now + 7.days } + + before do + artifact.expire_at = expire_at + end + + it { is_expected.to be_within(5).of(expire_at - Time.now) } + end + end + + describe '#expire_in=' do + subject { artifact.expire_in } + + it 'when assigning valid duration' do + artifact.expire_in = '7 days' + + is_expected.to be_within(10).of(7.days.to_i) + end + + it 'when assigning invalid duration' do + expect { artifact.expire_in = '7 elephants' }.to raise_error(ChronicDuration::DurationParseError) + + is_expected.to be_nil + end + + it 'when resetting value' do + artifact.expire_in = nil + + is_expected.to be_nil + end + + it 'when setting to 0' do + artifact.expire_in = '0' + + is_expected.to be_nil + end + end end |