Remove trigger request and migrate all data to pipeline, by adding trigger_id and trigger_variables.remove-trigger-requests

19 files changed, 109 insertions, 117 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 8c1b076c2d7..3df867b05e9 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -5,7 +5,6 @@ module Ci
     include Presentable
 
     belongs_to :runner
-    belongs_to :trigger_request
     belongs_to :erased_by, class_name: 'User'
 
     has_many :deployments, as: :deployable
@@ -57,7 +56,6 @@ module Ci
         new_build = build.dup
         new_build.status = 'pending'
         new_build.runner_id = nil
-        new_build.trigger_request_id = nil
         new_build.token = nil
         new_build.save
       end
@@ -75,7 +73,6 @@ module Ci
           allow_failure: build.allow_failure,
           stage: build.stage,
           stage_idx: build.stage_idx,
-          trigger_request: build.trigger_request,
           yaml_variables: build.yaml_variables,
           when: build.when,
           user: user,
@@ -231,7 +228,7 @@ module Ci
       variables += yaml_variables
       variables += user_variables
       variables += project.secret_variables
-      variables += trigger_request.user_variables if trigger_request
+      variables += pipeline.trigger_variables
       variables
     end
 
@@ -585,7 +582,7 @@ module Ci
         { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }
       ]
       variables << { key: 'CI_BUILD_TAG', value: ref, public: true } if tag?
-      variables << { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } if trigger_request
+      variables << { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } if pipeline.trigger
       variables << { key: 'CI_BUILD_MANUAL', value: 'true', public: true } if manual?
       variables
     end
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index bbc358adb83..7ee6987e0e2 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -9,10 +9,10 @@ module Ci
 
     belongs_to :project, foreign_key: :gl_project_id
     belongs_to :user
+    belongs_to :trigger, foreign_key: :trigger_id
 
     has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id
     has_many :builds, foreign_key: :commit_id
-    has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id
 
     validates_presence_of :sha, unless: :importing?
     validates_presence_of :ref, unless: :importing?
@@ -21,6 +21,8 @@ module Ci
 
     after_create :keep_around_commits, unless: :importing?
 
+    serialize :trigger_variables
+
     state_machine :status, initial: :created do
       event :enqueue do
         transition created: :pending
@@ -239,7 +241,7 @@ module Ci
     end
 
     def triggered?
-      trigger_requests.any?
+      trigger.any?
     end
 
     def retried
@@ -257,7 +259,7 @@ module Ci
       return [] unless config_processor
 
       config_processor.
-        builds_for_ref(ref, tag?, trigger_requests.first).
+        builds_for_ref(ref, tag?, trigger).
         sort_by { |build| build[:stage_idx] }
     end
 
@@ -367,6 +369,14 @@ module Ci
         .fabricate!
     end
 
+    def trigger_variables
+      return [] unless super
+
+      super.map do |key, value|
+        { key: key, value: value, public: false }
+      end
+    end
+
     private
 
     def pipeline_data
diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb
index 62889fe80d8..d6dba3ef7c8 100644
--- a/app/models/ci/trigger.rb
+++ b/app/models/ci/trigger.rb
@@ -5,7 +5,7 @@ module Ci
     acts_as_paranoid
 
     belongs_to :project, foreign_key: :gl_project_id
-    has_many :trigger_requests, dependent: :destroy
+    has_many :pipelines, dependent: :destroy
 
     validates_presence_of :token
     validates_uniqueness_of :token
@@ -16,12 +16,12 @@ module Ci
       self.token = SecureRandom.hex(15) if self.token.blank?
     end
 
-    def last_trigger_request
-      trigger_requests.last
+    def last_pipeline
+      pipelines.last
     end
 
     def last_used
-      last_trigger_request.try(:created_at)
+      pipelines.try(:created_at)
     end
 
     def short_token
diff --git a/app/models/ci/trigger_request.rb b/app/models/ci/trigger_request.rb
deleted file mode 100644
index 2b807731d0d..00000000000
--- a/app/models/ci/trigger_request.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-module Ci
-  class TriggerRequest < ActiveRecord::Base
-    extend Ci::Model
-
-    belongs_to :trigger
-    belongs_to :pipeline, foreign_key: :commit_id
-    has_many :builds
-
-    serialize :variables
-
-    def user_variables
-      return [] unless variables
-
-      variables.map do |key, value|
-        { key: key, value: value, public: false }
-      end
-    end
-  end
-end
diff --git a/app/services/ci/create_pipeline_builds_service.rb b/app/services/ci/create_pipeline_builds_service.rb
index b7da3f8e7eb..cc8c9149dd9 100644
--- a/app/services/ci/create_pipeline_builds_service.rb
+++ b/app/services/ci/create_pipeline_builds_service.rb
@@ -22,8 +22,7 @@ module Ci
         project: project,
         ref: pipeline.ref,
         tag: pipeline.tag,
-        user: current_user,
-        trigger_request: trigger_request
+        user: current_user
       )
       build = pipeline.builds.create(build_attributes)
 
@@ -43,11 +42,5 @@ module Ci
     def existing_build_names
       @existing_build_names ||= pipeline.builds.pluck(:name)
     end
-
-    def trigger_request
-      return @trigger_request if defined?(@trigger_request)
-
-      @trigger_request ||= pipeline.trigger_requests.first
-    end
   end
 end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index e3bc9847200..4223b8423b1 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -2,14 +2,15 @@ module Ci
   class CreatePipelineService < BaseService
     attr_reader :pipeline
 
-    def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil)
+    def execute(ignore_skip_ci: false, save_on_errors: true, trigger: nil, trigger_variables: nil)
       @pipeline = Ci::Pipeline.new(
         project: project,
         ref: ref,
         sha: sha,
         before_sha: before_sha,
         tag: tag?,
-        trigger_requests: Array(trigger_request),
+        trigger: trigger,
+        trigger_variables: trigger_variables,
         user: current_user
       )
 
@@ -17,7 +18,7 @@ module Ci
         return error('Pipeline is disabled')
       end
 
-      unless trigger_request || can?(current_user, :create_pipeline, project)
+      unless trigger || can?(current_user, :create_pipeline, project)
         return error('Insufficient permissions to create a new pipeline')
       end
 
diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb
deleted file mode 100644
index 6af3c1ca5b1..00000000000
--- a/app/services/ci/create_trigger_request_service.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-module Ci
-  class CreateTriggerRequestService
-    def execute(project, trigger, ref, variables = nil)
-      trigger_request = trigger.trigger_requests.create(variables: variables)
-
-      pipeline = Ci::CreatePipelineService.new(project, nil, ref: ref).
-        execute(ignore_skip_ci: true, trigger_request: trigger_request)
-      if pipeline.persisted?
-        trigger_request
-      end
-    end
-  end
-end
diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml
index 5ea85f9fd4c..9302d675ffc 100644
--- a/app/views/projects/ci/builds/_build.html.haml
+++ b/app/views/projects/ci/builds/_build.html.haml
@@ -42,7 +42,7 @@
         - build.tags.each do |tag|
           %span.label.label-primary
             = tag
-      - if build.try(:trigger_request)
+      - if build.pipeline.trigger?
         %span.label.label-info triggered
       - if build.try(:allow_failure)
         %span.label.label-danger allowed to fail
diff --git a/db/migrate/20170215155650_add_trigger_request_id_to_pipeline.rb b/db/migrate/20170215155650_add_trigger_request_id_to_pipeline.rb
new file mode 100644
index 00000000000..6c923b4eb23
--- /dev/null
+++ b/db/migrate/20170215155650_add_trigger_request_id_to_pipeline.rb
@@ -0,0 +1,14 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddTriggerRequestIdToPipeline < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_commits, :trigger_id, :integer
+    add_column :ci_commits, :trigger_variables, :text
+    add_foreign_key :ci_commits, :ci_triggers, column: "trigger_id", on_delete: :cascade
+  end
+end
diff --git a/db/migrate/20170215160640_migrate_trigger_id_to_pipeline.rb b/db/migrate/20170215160640_migrate_trigger_id_to_pipeline.rb
new file mode 100644
index 00000000000..ed70cd7cfd3
--- /dev/null
+++ b/db/migrate/20170215160640_migrate_trigger_id_to_pipeline.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class MigrateTriggerIdToPipeline < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    execute("UPDATE ci_commits SET " +
+      "trigger_id = triggers.trigger_id, " +
+      "trigger_variables = triggers.variables " +
+      "FROM (SELECT commit_id, trigger_id, variables FROM ci_trigger_requests) as triggers " +
+      "WHERE ci_commits.id = triggers.commit_id")
+  end
+end
diff --git a/db/migrate/20170215160655_add_trigger_id_index.rb b/db/migrate/20170215160655_add_trigger_id_index.rb
new file mode 100644
index 00000000000..26122d6bcbb
--- /dev/null
+++ b/db/migrate/20170215160655_add_trigger_id_index.rb
@@ -0,0 +1,11 @@
+class AddTriggerIdIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def change
+    add_concurrent_index :ci_commits, [:gl_project_id, :trigger_id]
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 3dde4b9c82b..d467164bca1 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170211073944) do
+ActiveRecord::Schema.define(version: 20170215160655) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -249,10 +249,13 @@ ActiveRecord::Schema.define(version: 20170211073944) do
     t.integer "duration"
     t.integer "user_id"
     t.integer "lock_version"
+    t.integer "trigger_id"
+    t.text "trigger_variables"
   end
 
   add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree
   add_index "ci_commits", ["gl_project_id", "status"], name: "index_ci_commits_on_gl_project_id_and_status", using: :btree
+  add_index "ci_commits", ["gl_project_id", "trigger_id"], name: "index_ci_commits_on_gl_project_id_and_trigger_id", using: :btree
   add_index "ci_commits", ["gl_project_id"], name: "index_ci_commits_on_gl_project_id", using: :btree
   add_index "ci_commits", ["status"], name: "index_ci_commits_on_status", using: :btree
   add_index "ci_commits", ["user_id"], name: "index_ci_commits_on_user_id", using: :btree
@@ -580,9 +583,9 @@ ActiveRecord::Schema.define(version: 20170211073944) do
   end
 
   add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree
-  add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
   add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
   add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
+  add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree
 
   create_table "lfs_objects", force: :cascade do |t|
     t.string "oid", null: false
@@ -1330,6 +1333,7 @@ ActiveRecord::Schema.define(version: 20170211073944) do
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
 
   add_foreign_key "boards", "projects"
+  add_foreign_key "ci_commits", "ci_triggers", column: "trigger_id", on_delete: :cascade
   add_foreign_key "issue_metrics", "issues", on_delete: :cascade
   add_foreign_key "label_priorities", "labels", on_delete: :cascade
   add_foreign_key "label_priorities", "projects", on_delete: :cascade
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 1d3767c78ff..311ba248d1a 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -32,9 +32,10 @@ module API
         end
 
         # create request and trigger builds
-        trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables)
-        if trigger_request
-          present trigger_request.pipeline, with: Entities::Pipeline
+        pipeline = Ci::CreatePipelineService.new(project, nil, ref: params[:ref].to_s).
+          execute(ignore_skip_ci: true, trigger: trigger, trigger_variables: variables)
+        if pipeline
+          present pipeline, with: Entities::Pipeline
         else
           errors = 'No pipeline created'
           render_api_error!(errors, 400)
@@ -51,7 +52,7 @@ module API
         authenticate!
         authorize! :admin_build, user_project
 
-        triggers = user_project.triggers.includes(:trigger_requests)
+        triggers = user_project.triggers
 
         present paginate(triggers), with: Entities::Trigger
       end
diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb
index 9239c7fd168..3cc0dc968a8 100644
--- a/lib/api/v3/entities.rb
+++ b/lib/api/v3/entities.rb
@@ -11,10 +11,6 @@ module API
           Gitlab::UrlBuilder.build(snippet)
         end
       end
-
-      class TriggerRequest < Grape::Entity
-        expose :id, :variables
-      end
     end
   end
 end
diff --git a/lib/api/v3/triggers.rb b/lib/api/v3/triggers.rb
index 8af182fdd2c..a7d669dfd4f 100644
--- a/lib/api/v3/triggers.rb
+++ b/lib/api/v3/triggers.rb
@@ -5,9 +5,7 @@ module API
         requires :id, type: String, desc: 'The ID of a project'
       end
       resource :projects do
-        desc 'Trigger a GitLab project build' do
-          success V3::Entities::TriggerRequest
-        end
+        desc 'Trigger a GitLab project build'
         params do
           requires :ref, type: String, desc: 'The commit sha or name of a branch or tag'
           requires :token, type: String, desc: 'The unique token of trigger'
@@ -31,9 +29,11 @@ module API
           end
 
           # create request and trigger builds
-          trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables)
-          if trigger_request
-            present trigger_request, with: Entities::TriggerRequest
+          pipeline = Ci::CreatePipelineService.new(project, nil, ref: params[:ref].to_s).
+            execute(ignore_skip_ci: true, trigger: trigger, trigger_variables: variables)
+          if pipeline
+            data = { id: pipeline.trigger_id, variables: pipeline.trigger_variables }
+            present data
           else
             errors = 'No builds created'
             render_api_error!(errors, 400)
diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb
index 792ff628b09..bf94ed43ac6 100644
--- a/lib/ci/api/entities.rb
+++ b/lib/ci/api/entities.rb
@@ -69,11 +69,6 @@ module Ci
       class WebHook < Grape::Entity
         expose :id, :project_id, :url
       end
-
-      class TriggerRequest < Grape::Entity
-        expose :id, :variables
-        expose :pipeline, using: Commit, as: :commit
-      end
     end
   end
 end
diff --git a/lib/ci/api/triggers.rb b/lib/ci/api/triggers.rb
index 63b42113513..1a82bc8fc8a 100644
--- a/lib/ci/api/triggers.rb
+++ b/lib/ci/api/triggers.rb
@@ -3,14 +3,12 @@ module Ci
     # Build Trigger API
     class Triggers < Grape::API
       resource :projects do
-        # Trigger a GitLab CI project build
-        #
-        # Parameters:
-        #   id (required) - The ID of a CI project
-        #   ref (required) - The name of project's branch or tag
-        #   token (required) - The uniq token of trigger
-        # Example Request:
-        #   POST /projects/:id/ref/:ref/trigger
+        desc 'Trigger a GitLab CI project build'
+        params do
+          requires :ref, type: String, desc: 'The commit sha or name of a branch or tag'
+          requires :token, type: String, desc: 'The unique token of trigger'
+          optional :variables, type: Hash, desc: 'The list of variables to be injected into build'
+        end
         post ":id/refs/:ref/trigger" do
           required_attributes! [:token]
 
@@ -35,9 +33,11 @@ module Ci
           end
 
           # create request and trigger builds
-          trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables)
-          if trigger_request
-            present trigger_request, with: Entities::TriggerRequest
+          pipeline = Ci::CreatePipelineService.new(project, nil, ref: params[:ref].to_s).
+            execute(ignore_skip_ci: true, trigger: trigger, trigger_variables: variables)
+          if pipeline
+            data = { id: pipeline.trigger_id, variables: pipeline.trigger_variables }
+            present data
           else
             errors = 'No builds created'
             render_api_error!(errors, 400)
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 649ee4d018b..734b7743101 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -20,26 +20,26 @@ module Ci
       raise ValidationError, e.message
     end
 
-    def jobs_for_ref(ref, tag = false, trigger_request = nil)
+    def jobs_for_ref(ref, tag = false, trigger = nil)
       @jobs.select do |_, job|
-        process?(job[:only], job[:except], ref, tag, trigger_request)
+        process?(job[:only], job[:except], ref, tag, trigger)
       end
     end
 
-    def jobs_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
-      jobs_for_ref(ref, tag, trigger_request).select do |_, job|
+    def jobs_for_stage_and_ref(stage, ref, tag = false, trigger = nil)
+      jobs_for_ref(ref, tag, trigger).select do |_, job|
         job[:stage] == stage
       end
     end
 
-    def builds_for_ref(ref, tag = false, trigger_request = nil)
-      jobs_for_ref(ref, tag, trigger_request).map do |name, _|
+    def builds_for_ref(ref, tag = false, trigger = nil)
+      jobs_for_ref(ref, tag, trigger).map do |name, _|
         build_attributes(name)
       end
     end
 
-    def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
-      jobs_for_stage_and_ref(stage, ref, tag, trigger_request).map do |name, _|
+    def builds_for_stage_and_ref(stage, ref, tag = false, trigger = nil)
+      jobs_for_stage_and_ref(stage, ref, tag, trigger).map do |name, _|
         build_attributes(name)
       end
     end
@@ -181,30 +181,30 @@ module Ci
       end
     end
 
-    def process?(only_params, except_params, ref, tag, trigger_request)
+    def process?(only_params, except_params, ref, tag, trigger)
       if only_params.present?
-        return false unless matching?(only_params, ref, tag, trigger_request)
+        return false unless matching?(only_params, ref, tag, trigger)
       end
 
       if except_params.present?
-        return false if matching?(except_params, ref, tag, trigger_request)
+        return false if matching?(except_params, ref, tag, trigger)
       end
 
       true
     end
 
-    def matching?(patterns, ref, tag, trigger_request)
+    def matching?(patterns, ref, tag, trigger)
       patterns.any? do |pattern|
-        match_ref?(pattern, ref, tag, trigger_request)
+        match_ref?(pattern, ref, tag, trigger)
       end
     end
 
-    def match_ref?(pattern, ref, tag, trigger_request)
+    def match_ref?(pattern, ref, tag, trigger)
       pattern, path = pattern.split('@', 2)
       return false if path && path != self.path
       return true if tag && pattern == 'tags'
       return true if !tag && pattern == 'branches'
-      return true if trigger_request.present? && pattern == 'triggers'
+      return true if trigger.present? && pattern == 'triggers'
 
       if pattern.first == "/" && pattern.last == "/"
         Regexp.new(pattern[1...-1]) =~ ref
diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb
deleted file mode 100644
index b8d8fab0e0b..00000000000
--- a/spec/factories/ci/trigger_requests.rb
+++ /dev/null
@@ -1,14 +0,0 @@
-FactoryGirl.define do
-  factory :ci_trigger_request, class: Ci::TriggerRequest do
-    factory :ci_trigger_request_with_variables do
-      trigger factory: :ci_trigger
-
-      variables do
-        {
-          TRIGGER_KEY_1: 'TRIGGER_VALUE_1',
-          TRIGGER_KEY_2: 'TRIGGER_VALUE_2'
-        }
-      end
-    end
-  end
-end