diff options
| -rw-r--r-- | app/models/ci/pipeline.rb | 6 | ||||
| -rw-r--r-- | app/models/concerns/atomic_internal_id.rb | 16 | ||||
| -rw-r--r-- | app/models/concerns/iid_routes.rb | 9 | ||||
| -rw-r--r-- | app/models/deployment.rb | 1 | ||||
| -rw-r--r-- | app/models/internal_id.rb | 2 | ||||
| -rw-r--r-- | app/models/issue.rb | 1 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 1 | ||||
| -rw-r--r-- | app/models/milestone.rb | 1 | ||||
| -rw-r--r-- | changelogs/unreleased/per-project-pipeline-iid.yml | 5 | ||||
| -rw-r--r-- | db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb | 15 | ||||
| -rw-r--r-- | db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb | 15 | ||||
| -rw-r--r-- | db/schema.rb | 2 | ||||
| -rw-r--r-- | doc/ci/variables/README.md | 1 | ||||
| -rw-r--r-- | lib/gitlab/ci/pipeline/chain/create.rb | 3 | ||||
| -rw-r--r-- | spec/models/ci/pipeline_spec.rb | 12 | ||||
| -rw-r--r-- | spec/support/shared_examples/models/atomic_internal_id_spec.rb | 13 |
16 files changed, 91 insertions, 12 deletions
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 0b90834d415..e9a56525fde 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -7,12 +7,17 @@ module Ci include Presentable include Gitlab::OptimisticLocking include Gitlab::Utils::StrongMemoize + include AtomicInternalId belongs_to :project, inverse_of: :pipelines belongs_to :user belongs_to :auto_canceled_by, class_name: 'Ci::Pipeline' belongs_to :pipeline_schedule, class_name: 'Ci::PipelineSchedule' + has_internal_id :iid, scope: :project, presence: false, init: ->(s) do + s&.project&.pipelines&.maximum(:iid) || s&.project&.pipelines.count + end + has_many :stages has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline @@ -511,6 +516,7 @@ module Ci def predefined_variables Gitlab::Ci::Variables::Collection.new .append(key: 'CI_PIPELINE_ID', value: id.to_s) + .append(key: 'CI_PIPELINE_IID', value: iid.to_s) .append(key: 'CI_CONFIG_PATH', value: ci_yaml_file_path) .append(key: 'CI_PIPELINE_SOURCE', value: source.to_s) .append(key: 'CI_COMMIT_MESSAGE', value: git_commit_message) diff --git a/app/models/concerns/atomic_internal_id.rb b/app/models/concerns/atomic_internal_id.rb index 22f516a172f..3d867df544f 100644 --- a/app/models/concerns/atomic_internal_id.rb +++ b/app/models/concerns/atomic_internal_id.rb @@ -25,9 +25,13 @@ module AtomicInternalId extend ActiveSupport::Concern module ClassMethods - def has_internal_id(column, scope:, init:) # rubocop:disable Naming/PredicateName - before_validation(on: :create) do + def has_internal_id(column, scope:, init:, presence: true) # rubocop:disable Naming/PredicateName + before_validation :"ensure_#{column}!", on: :create + validates column, presence: presence, numericality: true + + define_method("ensure_#{column}!") do scope_value = association(scope).reader + if read_attribute(column).blank? && scope_value scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value } usage = self.class.table_name.to_sym @@ -35,13 +39,9 @@ module AtomicInternalId new_iid = InternalId.generate_next(self, scope_attrs, usage, init) write_attribute(column, new_iid) end - end - validates column, presence: true, numericality: true + read_attribute(column) + end end end - - def to_param - iid.to_s - end end diff --git a/app/models/concerns/iid_routes.rb b/app/models/concerns/iid_routes.rb new file mode 100644 index 00000000000..50957e0230d --- /dev/null +++ b/app/models/concerns/iid_routes.rb @@ -0,0 +1,9 @@ +module IIDRoutes + ## + # This automagically enforces all related routes to use `iid` instead of `id` + # If you want to use `iid` for some routes and `id` for other routes, this module should not to be included, + # instead you should define `iid` or `id` explictly at each route generators. e.g. pipeline_path(project.id, pipeline.iid) + def to_param + iid.to_s + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 254764eefde..dac97676348 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -1,5 +1,6 @@ class Deployment < ActiveRecord::Base include AtomicInternalId + include IIDRoutes belongs_to :project, required: true belongs_to :environment, required: true diff --git a/app/models/internal_id.rb b/app/models/internal_id.rb index 189942c5ad8..dbd82dda06e 100644 --- a/app/models/internal_id.rb +++ b/app/models/internal_id.rb @@ -14,7 +14,7 @@ class InternalId < ActiveRecord::Base belongs_to :project belongs_to :namespace - enum usage: { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4 } + enum usage: { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4, ci_pipelines: 5 } validates :usage, presence: true diff --git a/app/models/issue.rb b/app/models/issue.rb index 0332bfa9371..6d33a67845f 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -2,6 +2,7 @@ require 'carrierwave/orm/activerecord' class Issue < ActiveRecord::Base include AtomicInternalId + include IIDRoutes include Issuable include Noteable include Referable diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 628c61d5d69..a14681897fd 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1,5 +1,6 @@ class MergeRequest < ActiveRecord::Base include AtomicInternalId + include IIDRoutes include Issuable include Noteable include Referable diff --git a/app/models/milestone.rb b/app/models/milestone.rb index d14e3a4ded5..f143b8452a2 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -9,6 +9,7 @@ class Milestone < ActiveRecord::Base include CacheMarkdownField include AtomicInternalId + include IIDRoutes include Sortable include Referable include StripAttribute diff --git a/changelogs/unreleased/per-project-pipeline-iid.yml b/changelogs/unreleased/per-project-pipeline-iid.yml new file mode 100644 index 00000000000..78a513a9986 --- /dev/null +++ b/changelogs/unreleased/per-project-pipeline-iid.yml @@ -0,0 +1,5 @@ +--- +title: Add per-project pipeline id +merge_request: 18558 +author: +type: added diff --git a/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb b/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb new file mode 100644 index 00000000000..d732116e9ce --- /dev/null +++ b/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb @@ -0,0 +1,15 @@ +class AddPipelineIidToCiPipelines < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column :ci_pipelines, :iid, :integer + end + + def down + remove_column :ci_pipelines, :iid, :integer + end +end diff --git a/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb b/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb new file mode 100644 index 00000000000..3fa59b44d5d --- /dev/null +++ b/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb @@ -0,0 +1,15 @@ +class AddIndexConstraintsToPipelineIid < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_pipelines, [:project_id, :iid], unique: true, where: 'iid IS NOT NULL' + end + + def down + remove_concurrent_index :ci_pipelines, [:project_id, :iid] + end +end diff --git a/db/schema.rb b/db/schema.rb index 6fd10785d77..2cf6ff3da9f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -451,10 +451,12 @@ ActiveRecord::Schema.define(version: 20180508055821) do t.integer "config_source" t.boolean "protected" t.integer "failure_reason" + t.integer "iid" end add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree add_index "ci_pipelines", ["pipeline_schedule_id"], name: "index_ci_pipelines_on_pipeline_schedule_id", using: :btree + add_index "ci_pipelines", ["project_id", "iid"], name: "index_ci_pipelines_on_project_id_and_iid", unique: true, where: "(iid IS NOT NULL)", using: :btree add_index "ci_pipelines", ["project_id", "ref", "status", "id"], name: "index_ci_pipelines_on_project_id_and_ref_and_status_and_id", using: :btree add_index "ci_pipelines", ["project_id", "sha"], name: "index_ci_pipelines_on_project_id_and_sha", using: :btree add_index "ci_pipelines", ["project_id"], name: "index_ci_pipelines_on_project_id", using: :btree diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 42367bf13f7..4a83d4fbe33 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -63,6 +63,7 @@ future GitLab releases.** | **CI_RUNNER_REVISION** | all | 10.6 | GitLab Runner revision that is executing the current job | | **CI_RUNNER_EXECUTABLE_ARCH** | all | 10.6 | The OS/architecture of the GitLab Runner executable (note that this is not necessarily the same as the environment of the executor) | | **CI_PIPELINE_ID** | 8.10 | 0.5 | The unique id of the current pipeline that GitLab CI uses internally | +| **CI_PIPELINE_IID** | 11.0 | all | The unique id of the current pipeline scoped to project | | **CI_PIPELINE_TRIGGERED** | all | all | The flag to indicate that job was [triggered] | | **CI_PIPELINE_SOURCE** | 10.0 | all | Indicates how the pipeline was triggered. Possible options are: `push`, `web`, `trigger`, `schedule`, `api`, and `pipeline`. For pipelines created before GitLab 9.5, this will show as `unknown` | | **CI_PROJECT_DIR** | all | all | The full path where the repository is cloned and where the job is run | diff --git a/lib/gitlab/ci/pipeline/chain/create.rb b/lib/gitlab/ci/pipeline/chain/create.rb index f4c8d5342c1..5967a7a6a58 100644 --- a/lib/gitlab/ci/pipeline/chain/create.rb +++ b/lib/gitlab/ci/pipeline/chain/create.rb @@ -6,6 +6,9 @@ module Gitlab include Chain::Helpers def perform! + # TODO: allocate next IID outside of transaction + pipeline.ensure_iid! + ::Ci::Pipeline.transaction do pipeline.save! diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ddd66a6be87..d3e0389cc72 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -35,6 +35,16 @@ describe Ci::Pipeline, :mailer do end end + describe 'modules' do + it_behaves_like 'AtomicInternalId' do + let(:internal_id_attribute) { :iid } + let(:instance) { build(:ci_pipeline) } + let(:scope_attrs) { { project: instance.project } } + let(:usage) { :ci_pipelines } + let(:validate_presence) { false } + end + end + describe '#source' do context 'when creating new pipeline' do let(:pipeline) do @@ -173,7 +183,7 @@ describe Ci::Pipeline, :mailer do it 'includes all predefined variables in a valid order' do keys = subject.map { |variable| variable[:key] } - expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION] + expect(keys).to eq %w[CI_PIPELINE_ID CI_PIPELINE_IID CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION] end end diff --git a/spec/support/shared_examples/models/atomic_internal_id_spec.rb b/spec/support/shared_examples/models/atomic_internal_id_spec.rb index 6a6e13418a9..1bccb578d79 100644 --- a/spec/support/shared_examples/models/atomic_internal_id_spec.rb +++ b/spec/support/shared_examples/models/atomic_internal_id_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' -shared_examples_for 'AtomicInternalId' do +shared_examples_for 'AtomicInternalId' do + let(:validate_presence) { true } + describe '.has_internal_id' do describe 'Module inclusion' do subject { described_class } @@ -15,7 +17,14 @@ shared_examples_for 'AtomicInternalId' do allow(InternalId).to receive(:generate_next).and_return(nil) end - it { is_expected.to validate_presence_of(internal_id_attribute) } + it 'checks presence' do + if validate_presence + is_expected.to validate_presence_of(internal_id_attribute) + else + is_expected.not_to validate_presence_of(internal_id_attribute) + end + end + it { is_expected.to validate_numericality_of(internal_id_attribute) } end |