Merge branch 'create-merge-request-create-pipeline-service-ee' into 'master'

Port to EE: Introduce service for merge request pipeline creation

See merge request gitlab-org/gitlab-ee!13950

app/services/merge_requests/base_service.rb

@@ -60,31 +60,7 @@ module MergeRequests
     end
 
     def create_pipeline_for(merge_request, user)
-      return unless can_create_pipeline_for?(merge_request)
-
-      create_detached_merge_request_pipeline(merge_request, user)
-    end
-
-    def create_detached_merge_request_pipeline(merge_request, user)
-      if can_use_merge_request_ref?(merge_request)
-        Ci::CreatePipelineService.new(merge_request.source_project, user,
-                                      ref: merge_request.ref_path)
-          .execute(:merge_request_event, merge_request: merge_request)
-      else
-        Ci::CreatePipelineService.new(merge_request.source_project, user,
-                                      ref: merge_request.source_branch)
-          .execute(:merge_request_event, merge_request: merge_request)
-      end
-    end
-
-    def can_create_pipeline_for?(merge_request)
-      ##
-      # UpdateMergeRequestsWorker could be retried by an exception.
-      # pipelines for merge request should not be recreated in such case.
-      return false if merge_request.find_actual_head_pipeline&.triggered_by_merge_request?
-      return false if merge_request.has_no_commits?
-
-      true
+      MergeRequests::CreatePipelineService.new(project, user).execute(merge_request)
     end
 
     def can_use_merge_request_ref?(merge_request)

app/services/merge_requests/create_pipeline_service.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  class CreatePipelineService < MergeRequests::BaseService
+    def execute(merge_request)
+      return unless can_create_pipeline_for?(merge_request)
+
+      create_detached_merge_request_pipeline(merge_request)
+    end
+
+    def create_detached_merge_request_pipeline(merge_request)
+      if can_use_merge_request_ref?(merge_request)
+        Ci::CreatePipelineService.new(merge_request.source_project, current_user,
+                                      ref: merge_request.ref_path)
+          .execute(:merge_request_event, merge_request: merge_request)
+      else
+        Ci::CreatePipelineService.new(merge_request.source_project, current_user,
+                                      ref: merge_request.source_branch)
+          .execute(:merge_request_event, merge_request: merge_request)
+      end
+    end
+
+    def can_create_pipeline_for?(merge_request)
+      ##
+      # UpdateMergeRequestsWorker could be retried by an exception.
+      # pipelines for merge request should not be recreated in such case.
+      return false if !allow_duplicate && merge_request.find_actual_head_pipeline&.triggered_by_merge_request?
+      return false if merge_request.has_no_commits?
+
+      true
+    end
+
+    def allow_duplicate
+      params[:allow_duplicate]
+    end
+  end
+end
+
+MergeRequests::CreatePipelineService.prepend(EE::MergeRequests::CreatePipelineService)

ee/app/services/ee/merge_requests/base_service.rb

@@ -3,8 +3,6 @@
 module EE
   module MergeRequests
     module BaseService
-      extend ::Gitlab::Utils::Override
-
       private
 
       def filter_params(merge_request)
@@ -18,36 +16,6 @@ module EE
 
         super
       end
-
-      override :create_pipeline_for
-      def create_pipeline_for(merge_request, user)
-        create_merge_request_pipeline_for(merge_request, user) || super
-      end
-
-      def create_merge_request_pipeline_for(merge_request, user)
-        return unless can_create_merge_request_pipeline_for?(merge_request)
-
-        result = ::MergeRequests::MergeabilityCheckService.new(merge_request).execute
-
-        if result.success?
-          merge_ref_head_payload = result.payload.fetch(:merge_ref_head, {})
-          commit_id, target_id, source_id = merge_ref_head_payload.values_at(:commit_id, :target_id, :source_id)
-
-          ::Ci::CreatePipelineService.new(merge_request.source_project, user,
-                                          ref: merge_request.merge_ref_path,
-                                          checkout_sha: commit_id,
-                                          target_sha: target_id,
-                                          source_sha: source_id)
-            .execute(:merge_request_event, merge_request: merge_request)
-        end
-      end
-
-      def can_create_merge_request_pipeline_for?(merge_request)
-        return false unless merge_request.project.merge_pipelines_enabled?
-        return false unless can_use_merge_request_ref?(merge_request)
-
-        can_create_pipeline_for?(merge_request)
-      end
     end
   end
 end

ee/app/services/ee/merge_requests/create_pipeline_service.rb

+# frozen_string_literal: true
+
+module EE
+  module MergeRequests
+    module CreatePipelineService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(merge_request)
+        create_merge_request_pipeline_for(merge_request) || super
+      end
+
+      def create_merge_request_pipeline_for(merge_request)
+        return unless can_create_merge_request_pipeline_for?(merge_request)
+
+        result = ::MergeRequests::MergeabilityCheckService.new(merge_request).execute
+
+        if result.success? && merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
+          merge_ref_head_payload = result.payload.fetch(:merge_ref_head, {})
+          commit_id, target_id, source_id = merge_ref_head_payload.values_at(:commit_id, :target_id, :source_id)
+
+          ::Ci::CreatePipelineService.new(merge_request.source_project, current_user,
+                                          ref: merge_request.merge_ref_path,
+                                          checkout_sha: commit_id,
+                                          target_sha: target_id,
+                                          source_sha: source_id)
+            .execute(:merge_request_event, merge_request: merge_request)
+        end
+      end
+
+      def can_create_merge_request_pipeline_for?(merge_request)
+        return false unless merge_request.project.merge_pipelines_enabled?
+        return false unless can_use_merge_request_ref?(merge_request)
+
+        can_create_pipeline_for?(merge_request)
+      end
+    end
+  end
+end

ee/spec/services/ee/merge_requests/base_service_spec.rb

@@ -36,108 +36,4 @@ describe MergeRequests::BaseService do
       end
     end
   end
-
-  describe '#create_pipeline_for' do
-    subject { service.execute }
-
-    let(:service) { MergeRequests::CreateService.new(source_project, user, opts) }
-    let(:user) { create(:user) }
-    let(:title) { 'Awesome merge request' }
-    let(:merge_pipelines_enabled) { true }
-    let(:merge_pipelines_license) { true }
-    let(:source_project) { project }
-    let(:source_branch) { 'feature' }
-    let(:target_project) { project }
-    let(:target_branch) { 'master' }
-
-    let(:opts) do
-      {
-        title: title,
-        description: 'A new fix',
-        source_branch: source_branch,
-        source_project: source_project,
-        target_branch: target_branch,
-        target_project: target_project
-      }
-    end
-
-    let(:ci_yaml) do
-      {
-        test: {
-          stage: 'test',
-          script: 'echo',
-          only: ['merge_requests']
-        }
-      }
-    end
-
-    before do
-      source_project.add_developer(user)
-      target_project.add_developer(user)
-      source_project.merge_pipelines_enabled = merge_pipelines_enabled
-      stub_licensed_features(merge_pipelines: merge_pipelines_license)
-      stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
-    end
-
-    shared_examples_for 'creates a merge requst pipeline' do
-      it do
-        expect(subject).to be_persisted
-        expect(subject.all_pipelines.count).to eq(1)
-        expect(subject.all_pipelines.last).to be_merge_request_pipeline
-        expect(subject.all_pipelines.last).not_to be_detached_merge_request_pipeline
-      end
-    end
-
-    shared_examples_for 'creates a detached merge requst pipeline' do
-      it do
-        expect(subject).to be_persisted
-        expect(subject.all_pipelines.count).to eq(1)
-        expect(subject.all_pipelines.last).not_to be_merge_request_pipeline
-        expect(subject.all_pipelines.last).to be_detached_merge_request_pipeline
-      end
-    end
-
-    it_behaves_like 'creates a merge requst pipeline'
-
-    context 'when project setting for merge request pipelines is disabled' do
-      let(:merge_pipelines_enabled) { false }
-
-      it_behaves_like 'creates a detached merge requst pipeline'
-    end
-
-    context 'when ci_use_merge_request_ref feature flag is disabled' do
-      before do
-        stub_feature_flags(ci_use_merge_request_ref: false)
-      end
-
-      it_behaves_like 'creates a detached merge requst pipeline'
-    end
-
-    context 'when merge request is submitted from fork' do
-      let(:source_project) { fork_project(project, nil, repository: true) }
-
-      it_behaves_like 'creates a detached merge requst pipeline'
-    end
-
-    context 'when the CreateService is retried' do
-      it 'does not create a merge request pipeline twice' do
-        expect do
-          2.times { MergeRequests::CreateService.new(source_project, user, opts).execute }
-        end.to change { Ci::Pipeline.count }.by(1)
-      end
-    end
-
-    context 'when merge request has no commit' do
-      let(:source_branch) { 'empty-branch' }
-
-      it_behaves_like 'creates a detached merge requst pipeline'
-    end
-
-    context 'when merge request has a conflict' do
-      let(:source_branch) { 'feature' }
-      let(:target_branch) { 'feature_conflict' }
-
-      it_behaves_like 'creates a detached merge requst pipeline'
-    end
-  end
 end

ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequests::CreatePipelineService do
+  include ProjectForksHelper
+
+  describe '#execute' do
+    subject { service.execute(merge_request) }
+
+    let(:service) { described_class.new(source_project, user) }
+    let(:project) { create(:project, :repository) }
+    let(:user) { create(:user) }
+    let(:title) { 'Awesome merge request' }
+    let(:merge_pipelines_enabled) { true }
+    let(:merge_pipelines_license) { true }
+    let(:source_project) { project }
+    let(:source_branch) { 'feature' }
+    let(:target_project) { project }
+    let(:target_branch) { 'master' }
+
+    let(:merge_request) do
+      create(:merge_request,
+        source_project: source_project, source_branch: source_branch,
+        target_project: target_project, target_branch: target_branch,
+        merge_status: 'unchecked')
+    end
+
+    let(:ci_yaml) do
+      {
+        test: {
+          stage: 'test',
+          script: 'echo',
+          only: ['merge_requests']
+        }
+      }
+    end
+
+    before do
+      source_project.add_developer(user)
+      target_project.add_developer(user)
+      source_project.merge_pipelines_enabled = merge_pipelines_enabled
+      stub_licensed_features(merge_pipelines: merge_pipelines_license)
+      stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
+    end
+
+    shared_examples_for 'creates a merge request pipeline' do
+      it do
+        subject
+
+        expect(merge_request.all_pipelines.count).to eq(1)
+        expect(merge_request.all_pipelines.last).to be_merge_request_pipeline
+        expect(merge_request.all_pipelines.last).not_to be_detached_merge_request_pipeline
+      end
+    end
+
+    shared_examples_for 'creates a detached merge request pipeline' do
+      it do
+        subject
+
+        expect(merge_request.all_pipelines.count).to eq(1)
+        expect(merge_request.all_pipelines.last).not_to be_merge_request_pipeline
+        expect(merge_request.all_pipelines.last).to be_detached_merge_request_pipeline
+      end
+    end
+
+    it_behaves_like 'creates a merge request pipeline'
+
+    context 'when merge request is WIP' do
+      before do
+        merge_request.update!(title: merge_request.wip_title)
+      end
+
+      it_behaves_like 'creates a detached merge request pipeline'
+    end
+
+    context 'when project setting for merge request pipelines is disabled' do
+      let(:merge_pipelines_enabled) { false }
+
+      it_behaves_like 'creates a detached merge request pipeline'
+    end
+
+    context 'when ci_use_merge_request_ref feature flag is disabled' do
+      before do
+        stub_feature_flags(ci_use_merge_request_ref: false)
+      end
+
+      it_behaves_like 'creates a detached merge request pipeline'
+    end
+
+    context 'when merge request is submitted from fork' do
+      let(:source_project) { fork_project(project, nil, repository: true) }
+
+      it_behaves_like 'creates a detached merge request pipeline'
+    end
+
+    context 'when the CreateService is retried' do
+      it 'does not create a merge request pipeline twice' do
+        expect do
+          2.times { service.execute(merge_request) }
+        end.to change { Ci::Pipeline.count }.by(1)
+      end
+    end
+
+    context 'when merge request has no commit' do
+      let(:source_branch) { 'empty-branch' }
+
+      it_behaves_like 'creates a detached merge request pipeline'
+    end
+
+    context 'when merge request has a conflict' do
+      let(:source_branch) { 'feature' }
+      let(:target_branch) { 'feature_conflict' }
+
+      it_behaves_like 'creates a detached merge request pipeline'
+    end
+  end
+end

spec/services/merge_requests/create_pipeline_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe MergeRequests::CreatePipelineService do
+  set(:project) { create(:project, :repository) }
+  set(:user) { create(:user) }
+  let(:service) { described_class.new(project, user, params) }
+  let(:params) { {} }
+
+  before do
+    project.add_developer(user)
+  end
+
+  describe '#execute' do
+    subject { service.execute(merge_request) }
+
+    before do
+      stub_ci_pipeline_yaml_file(YAML.dump(config))
+    end
+
+    let(:config) do
+      { rspec: { script: 'echo', only: ['merge_requests'] } }
+    end
+
+    let(:merge_request) do
+      create(:merge_request,
+        source_branch: 'feature',
+        source_project: project,
+        target_branch: 'master',
+        target_project: project)
+    end
+
+    it 'creates a detached merge request pipeline' do
+      expect { subject }.to change { Ci::Pipeline.count }.by(1)
+
+      expect(subject).to be_persisted
+      expect(subject).to be_detached_merge_request_pipeline
+    end
+
+    context 'when service is called multiple times' do
+      it 'creates a pipeline once' do
+        expect do
+          service.execute(merge_request)
+          service.execute(merge_request)
+        end.to change { Ci::Pipeline.count }.by(1)
+      end
+
+      context 'when allow_duplicate option is true' do
+        let(:params) { { allow_duplicate: true } }
+
+        it 'creates pipelines multiple times' do
+          expect do
+            service.execute(merge_request)
+            service.execute(merge_request)
+          end.to change { Ci::Pipeline.count }.by(2)
+        end
+      end
+    end
+
+    context 'when .gitlab-ci.yml does not have only: [merge_requests] keyword' do
+      let(:config) do
+        { rspec: { script: 'echo' } }
+      end
+
+      it 'does not create a pipeline' do
+        expect { subject }.not_to change { Ci::Pipeline.count }
+      end
+    end
+  end
+end