Revert "Merge branch 'per-project-pipeline-iid' into 'master'"

This reverts commit 7b00bc5b2f51a66b8dc8e63c47b13940010dd68d, reversing
changes made to 5f0a5367c8081a788f03a8e14907838481128fae.

23 files changed, 28 insertions, 165 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 5eb30f4aaa0..53af87a271a 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -7,17 +7,12 @@ 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
@@ -536,7 +531,6 @@ module Ci
 
     def predefined_variables
       Gitlab::Ci::Variables::Collection.new
-        .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 164c704260e..22f516a172f 100644
--- a/app/models/concerns/atomic_internal_id.rb
+++ b/app/models/concerns/atomic_internal_id.rb
@@ -25,13 +25,9 @@ module AtomicInternalId
   extend ActiveSupport::Concern
 
   module ClassMethods
-    def has_internal_id(column, scope:, init:, presence: true) # rubocop:disable Naming/PredicateName
-      before_validation :"ensure_#{scope}_#{column}!", on: :create
-      validates column, presence: presence
-
-      define_method("ensure_#{scope}_#{column}!") do
+    def has_internal_id(column, scope:, init:) # rubocop:disable Naming/PredicateName
+      before_validation(on: :create) 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
@@ -39,9 +35,13 @@ module AtomicInternalId
           new_iid = InternalId.generate_next(self, scope_attrs, usage, init)
           write_attribute(column, new_iid)
         end
-
-        read_attribute(column)
       end
+
+      validates column, presence: true, numericality: true
     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
deleted file mode 100644
index 246748cf52c..00000000000
--- a/app/models/concerns/iid_routes.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-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 ac86e9e8de0..254764eefde 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -1,6 +1,5 @@
 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 f50f28deffe..f7f930e86ed 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, ci_pipelines: 5 }
+  enum usage: { issues: 0, merge_requests: 1, deployments: 2, milestones: 3, epics: 4 }
 
   validates :usage, presence: true
 
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d136700836d..41a290f34b4 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -2,7 +2,6 @@ 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 4c1628d2bdb..79fc155fd3c 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -1,6 +1,5 @@
 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 d05dcfd083a..d14e3a4ded5 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -9,7 +9,6 @@ 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
deleted file mode 100644
index 78a513a9986..00000000000
--- a/changelogs/unreleased/per-project-pipeline-iid.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-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
deleted file mode 100644
index e8f0c91d612..00000000000
--- a/db/migrate/20180424160449_add_pipeline_iid_to_ci_pipelines.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-class AddPipelineIidToCiPipelines < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  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
deleted file mode 100644
index 3fa59b44d5d..00000000000
--- a/db/migrate/20180425205249_add_index_constraints_to_pipeline_iid.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-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 0d6b44d1b92..97247387bc7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -451,12 +451,10 @@ ActiveRecord::Schema.define(version: 20180529093006) 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 aa4395b01a9..f10423b92cf 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -72,7 +72,6 @@ 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 |
@@ -353,8 +352,6 @@ Running on runner-8a2f473d-project-1796893-concurrent-0 via runner-8a2f473d-mach
 ++ CI_PROJECT_URL=https://example.com/gitlab-examples/ci-debug-trace
 ++ export CI_PIPELINE_ID=52666
 ++ CI_PIPELINE_ID=52666
-++ export CI_PIPELINE_IID=123
-++ CI_PIPELINE_IID=123
 ++ export CI_RUNNER_ID=1337
 ++ CI_RUNNER_ID=1337
 ++ export CI_RUNNER_DESCRIPTION=shared-runners-manager-1.example.com
@@ -442,7 +439,6 @@ export CI_JOB_MANUAL="true"
 export CI_JOB_TRIGGERED="true"
 export CI_JOB_TOKEN="abcde-1234ABCD5678ef"
 export CI_PIPELINE_ID="1000"
-export CI_PIPELINE_IID="10"
 export CI_PROJECT_ID="34"
 export CI_PROJECT_DIR="/builds/gitlab-org/gitlab-ce"
 export CI_PROJECT_NAME="gitlab-ce"
diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb
index f34c11ca3c2..69b8a8fc68f 100644
--- a/lib/gitlab/ci/pipeline/chain/populate.rb
+++ b/lib/gitlab/ci/pipeline/chain/populate.rb
@@ -8,9 +8,6 @@ module Gitlab
           PopulateError = Class.new(StandardError)
 
           def perform!
-            # Allocate next IID. This operation must be outside of transactions of pipeline creations.
-            pipeline.ensure_project_iid!
-
             ##
             # Populate pipeline with block argument of CreatePipelineService#execute.
             #
diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
index c5a4d9b4778..4d7d6951a51 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
@@ -42,10 +42,6 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
     it 'correctly assigns user' do
       expect(pipeline.builds).to all(have_attributes(user: user))
     end
-
-    it 'has pipeline iid' do
-      expect(pipeline.iid).to be > 0
-    end
   end
 
   context 'when pipeline is empty' do
@@ -72,10 +68,6 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
       expect(pipeline.errors.to_a)
         .to include 'No stages / jobs for this pipeline.'
     end
-
-    it 'wastes pipeline iid' do
-      expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0
-    end
   end
 
   context 'when pipeline has validation errors' do
@@ -95,10 +87,6 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
       expect(pipeline.errors.to_a)
         .to include 'Failed to build the pipeline!'
     end
-
-    it 'wastes pipeline iid' do
-      expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0
-    end
   end
 
   context 'when there is a seed blocks present' do
@@ -123,12 +111,6 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
         expect(pipeline.variables.first.key).to eq 'VAR'
         expect(pipeline.variables.first.value).to eq '123'
       end
-
-      it 'has pipeline iid' do
-        step.perform!
-
-        expect(pipeline.iid).to be > 0
-      end
     end
 
     context 'when seeds block tries to persist some resources' do
@@ -139,12 +121,6 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
       it 'raises exception' do
         expect { step.perform! }.to raise_error(ActiveRecord::RecordNotSaved)
       end
-
-      it 'wastes pipeline iid' do
-        expect { step.perform! }.to raise_error
-
-        expect(InternalId.ci_pipelines.where(project_id: project.id).last.last_value).to be > 0
-      end
     end
   end
 
@@ -156,39 +132,22 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
     end
   end
 
-  context 'when variables policy is specified' do
-    shared_examples_for 'a correct pipeline' do
-      it 'populates pipeline according to used policies' do
-        step.perform!
-
-        expect(pipeline.stages.size).to eq 1
-        expect(pipeline.stages.first.builds.size).to eq 1
-        expect(pipeline.stages.first.builds.first.name).to eq 'rspec'
-      end
+  context 'when using only/except build policies' do
+    let(:config) do
+      { rspec: { script: 'rspec', stage: 'test', only: ['master'] },
+        prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } }
     end
 
-    context 'when using only/except build policies' do
-      let(:config) do
-        { rspec: { script: 'rspec', stage: 'test', only: ['master'] },
-          prod: { script: 'cap prod', stage: 'deploy', only: ['tags'] } }
-      end
-
-      let(:pipeline) do
-        build(:ci_pipeline, ref: 'master', config: config)
-      end
-
-      it_behaves_like 'a correct pipeline'
+    let(:pipeline) do
+      build(:ci_pipeline, ref: 'master', config: config)
+    end
 
-      context 'when variables expression is specified' do
-        context 'when pipeline iid is the subject' do
-          let(:config) do
-            { rspec: { script: 'rspec', only: { variables: ["$CI_PIPELINE_IID == '1'"] } },
-              prod: { script: 'cap prod', only: { variables: ["$CI_PIPELINE_IID == '1000'"] } } }
-          end
+    it 'populates pipeline according to used policies' do
+      step.perform!
 
-          it_behaves_like 'a correct pipeline'
-        end
-      end
+      expect(pipeline.stages.size).to eq 1
+      expect(pipeline.stages.first.builds.size).to eq 1
+      expect(pipeline.stages.first.builds.first.name).to eq 'rspec'
     end
   end
 end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 2aebdc57f7c..74e7a45fd6c 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -242,7 +242,6 @@ Ci::Pipeline:
 - config_source
 - failure_reason
 - protected
-- iid
 Ci::Stage:
 - id
 - name
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 66c9708b4cf..b5eac913639 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1559,7 +1559,6 @@ describe Ci::Build do
         { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true },
         { key: 'CI_PROJECT_URL', value: project.web_url, public: true },
         { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true },
-        { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true },
         { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true },
         { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true },
         { key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true },
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 24692ebb9a3..f5295bec65b 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -35,16 +35,6 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
-  describe 'modules' do
-    it_behaves_like 'AtomicInternalId', validate_presence: false do
-      let(:internal_id_attribute) { :iid }
-      let(:instance) { build(:ci_pipeline) }
-      let(:scope) { :project }
-      let(:scope_attrs) { { project: instance.project } }
-      let(:usage) { :ci_pipelines }
-    end
-  end
-
   describe '#source' do
     context 'when creating new pipeline' do
       let(:pipeline) do
@@ -205,8 +195,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_IID
-                            CI_CONFIG_PATH
+      expect(keys).to eq %w[CI_CONFIG_PATH
                             CI_PIPELINE_SOURCE
                             CI_COMMIT_MESSAGE
                             CI_COMMIT_TITLE
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index e01906f4b6c..aee70bcfb29 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -20,7 +20,6 @@ describe Deployment do
     it_behaves_like 'AtomicInternalId' do
       let(:internal_id_attribute) { :iid }
       let(:instance) { build(:deployment) }
-      let(:scope) { :project }
       let(:scope_attrs) { { project: instance.project } }
       let(:usage) { :deployments }
     end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index e818fbeb9cf..128acf83686 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -17,7 +17,6 @@ describe Issue do
     it_behaves_like 'AtomicInternalId' do
       let(:internal_id_attribute) { :iid }
       let(:instance) { build(:issue) }
-      let(:scope) { :project }
       let(:scope_attrs) { { project: instance.project } }
       let(:usage) { :issues }
     end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 65cc9372cbe..9ffa91fc265 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -84,7 +84,6 @@ describe MergeRequest do
     it_behaves_like 'AtomicInternalId' do
       let(:internal_id_attribute) { :iid }
       let(:instance) { build(:merge_request) }
-      let(:scope) { :target_project }
       let(:scope_attrs) { { project: instance.target_project } }
       let(:usage) { :merge_requests }
     end
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 204d6b47832..4bb9717d33e 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -6,7 +6,6 @@ describe Milestone do
       it_behaves_like 'AtomicInternalId' do
         let(:internal_id_attribute) { :iid }
         let(:instance) { build(:milestone, project: build(:project), group: nil) }
-        let(:scope) { :project }
         let(:scope_attrs) { { project: instance.project } }
         let(:usage) { :milestones }
       end
@@ -16,7 +15,6 @@ describe Milestone do
       it_behaves_like 'AtomicInternalId' do
         let(:internal_id_attribute) { :iid }
         let(:instance) { build(:milestone, project: nil, group: build(:group)) }
-        let(:scope) { :group }
         let(:scope_attrs) { { namespace: instance.group } }
         let(:usage) { :milestones }
       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 7ab1041d17c..6a6e13418a9 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,6 @@
 require 'spec_helper'
 
-shared_examples_for 'AtomicInternalId' do |validate_presence: true|
+shared_examples_for 'AtomicInternalId' do
   describe '.has_internal_id' do
     describe 'Module inclusion' do
       subject { described_class }
@@ -9,31 +9,14 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true|
     end
 
     describe 'Validation' do
-      before do
-        allow_any_instance_of(described_class).to receive(:"ensure_#{scope}_#{internal_id_attribute}!")
-
-        instance.valid?
-      end
-
-      context 'when presence validation is required' do
-        before do
-          skip unless validate_presence
-        end
+      subject { instance }
 
-        it 'validates presence' do
-          expect(instance.errors[internal_id_attribute]).to include("can't be blank")
-        end
+      before do
+        allow(InternalId).to receive(:generate_next).and_return(nil)
       end
 
-      context 'when presence validation is not required' do
-        before do
-          skip if validate_presence
-        end
-
-        it 'does not validate presence' do
-          expect(instance.errors[internal_id_attribute]).to be_empty
-        end
-      end
+      it { is_expected.to validate_presence_of(internal_id_attribute) }
+      it { is_expected.to validate_numericality_of(internal_id_attribute) }
     end
 
     describe 'Creating an instance' do