diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-09-21 11:34:12 +0300 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2017-12-03 14:04:48 +0300 |
commit | 61864a5a5bb523953589c9398a431c4369fbfc76 (patch) | |
tree | 5eac32ef8155e9066d7d1488d7856e83605aa6a5 | |
parent | 25df666156279e5b392b429519b4f4ba01eefaac (diff) |
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 |