diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/models/project.rb | 5 | ||||
| -rw-r--r-- | app/services/git_push_service.rb | 15 | ||||
| -rw-r--r-- | app/workers/update_merge_requests_worker.rb | 16 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 21 | ||||
| -rw-r--r-- | spec/services/git_push_service_spec.rb | 4 | ||||
| -rw-r--r-- | spec/services/merge_requests/refresh_service_spec.rb | 3 | ||||
| -rw-r--r-- | spec/workers/post_receive_spec.rb | 8 | ||||
| -rw-r--r-- | spec/workers/update_merge_requests_worker_spec.rb | 38 |
9 files changed, 68 insertions, 43 deletions
diff --git a/CHANGELOG b/CHANGELOG index 4b1ac9d2c3c..472601fa336 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -24,6 +24,7 @@ v 8.13.0 (unreleased) - Create a new /templates namespace for the /licenses, /gitignores and /gitlab_ci_ymls API endpoints. !5717 (tbalthazar) - Speed-up group milestones show page - Fix inconsistent options dropdown caret on mobile viewports (ClemMakesApps) + - Extract project#update_merge_requests and SystemHooks to its own worker from GitPushService - Don't include archived projects when creating group milestones. !4940 (Jeroen Jacobs) - Add tag shortcut from the Commit page. !6543 - Keep refs for each deployment diff --git a/app/models/project.rb b/app/models/project.rb index 74d54e69648..8c488f4aeef 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -827,11 +827,6 @@ class Project < ActiveRecord::Base end end - def update_merge_requests(oldrev, newrev, ref, user) - MergeRequests::RefreshService.new(self, user). - execute(oldrev, newrev, ref) - end - def valid_repo? repository.exists? rescue diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index c499427605a..e8415862de5 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -63,13 +63,12 @@ class GitPushService < BaseService protected def update_merge_requests - @project.update_merge_requests(params[:oldrev], params[:newrev], params[:ref], current_user) + UpdateMergeRequestsWorker.perform_async(@project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) EventCreateService.new.push(@project, current_user, build_push_data) - SystemHooksService.new.execute_hooks(build_push_data_system_hook.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) - Ci::CreatePipelineService.new(project, current_user, build_push_data).execute + Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute ProjectCacheWorker.perform_async(@project.id) end @@ -148,16 +147,6 @@ class GitPushService < BaseService push_commits) end - def build_push_data_system_hook - @push_data_system ||= Gitlab::DataBuilder::Push.build( - @project, - current_user, - params[:oldrev], - params[:newrev], - params[:ref], - []) - end - def push_to_existing_branch? # Return if this is not a push to a branch (e.g. new commits) Gitlab::Git.branch_ref?(params[:ref]) && !Gitlab::Git.blank_ref?(params[:oldrev]) diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb new file mode 100644 index 00000000000..03f0528cdae --- /dev/null +++ b/app/workers/update_merge_requests_worker.rb @@ -0,0 +1,16 @@ +class UpdateMergeRequestsWorker + include Sidekiq::Worker + + def perform(project_id, user_id, oldrev, newrev, ref) + project = Project.find_by(id: project_id) + return unless project + + user = User.find_by(id: user_id) + return unless user + + MergeRequests::RefreshService.new(project, user).execute(oldrev, newrev, ref) + + push_data = Gitlab::DataBuilder::Push.build(project, user, oldrev, newrev, ref, []) + SystemHooksService.new.execute_hooks(push_data, :push_hooks) + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dae546a0cdc..7435713d4c0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -219,7 +219,6 @@ describe Project, models: true do describe 'Respond to' do it { is_expected.to respond_to(:url_to_repo) } it { is_expected.to respond_to(:repo_exists?) } - it { is_expected.to respond_to(:update_merge_requests) } it { is_expected.to respond_to(:execute_hooks) } it { is_expected.to respond_to(:owner) } it { is_expected.to respond_to(:path_with_namespace) } @@ -380,26 +379,6 @@ describe Project, models: true do end end - describe '#update_merge_requests' do - let(:project) { create(:project) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:key) { create(:key, user_id: project.owner.id) } - let(:prev_commit_id) { merge_request.commits.last.id } - let(:commit_id) { merge_request.commits.first.id } - - it 'closes merge request if last commit from source branch was pushed to target branch' do - project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.target_branch}", key.user) - merge_request.reload - expect(merge_request.merged?).to be_truthy - end - - it 'updates merge request commits with new one if pushed to source branch' do - project.update_merge_requests(prev_commit_id, commit_id, "refs/heads/#{merge_request.source_branch}", key.user) - merge_request.reload - expect(merge_request.diff_head_sha).to eq(commit_id) - end - end - describe '.find_with_namespace' do context 'with namespace' do before do diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 8e3e12114f2..dd2a9e9903a 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -184,8 +184,8 @@ describe GitPushService, services: true do context "Updates merge requests" do it "when pushing a new branch for the first time" do - expect(project).to receive(:update_merge_requests). - with(@blankrev, 'newrev', 'refs/heads/master', user) + expect(UpdateMergeRequestsWorker).to receive(:perform_async). + with(project.id, user.id, @blankrev, 'newrev', 'refs/heads/master') execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 5b4e4908add..e515bc9f89c 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -62,7 +62,8 @@ describe MergeRequests::RefreshService, services: true do it { expect(@merge_request.notes).not_to be_empty } it { expect(@merge_request).to be_open } - it { expect(@merge_request.merge_when_build_succeeds).to be_falsey} + it { expect(@merge_request.merge_when_build_succeeds).to be_falsey } + it { expect(@merge_request.diff_head_sha).to eq(@newrev) } it { expect(@fork_merge_request).to be_open } it { expect(@fork_merge_request.notes).to be_empty } it { expect(@build_failed_todo).to be_done } diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index ffeaafe654a..984acdade36 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -92,7 +92,13 @@ describe PostReceive do allow(Project).to receive(:find_with_namespace).and_return(project) expect(project).to receive(:execute_hooks).twice expect(project).to receive(:execute_services).twice - expect(project).to receive(:update_merge_requests) + + PostReceive.new.perform(pwd(project), key_id, base64_changes) + end + + it "enqueues a UpdateMergeRequestsWorker job" do + allow(Project).to receive(:find_with_namespace).and_return(project) + expect(UpdateMergeRequestsWorker).to receive(:perform_async).with(project.id, project.owner.id, any_args) PostReceive.new.perform(pwd(project), key_id, base64_changes) end diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb new file mode 100644 index 00000000000..c78a69eda67 --- /dev/null +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe UpdateMergeRequestsWorker do + include RepoHelpers + + let(:project) { create(:project) } + let(:user) { create(:user) } + + subject { described_class.new } + + describe '#perform' do + let(:oldrev) { "123456" } + let(:newrev) { "789012" } + let(:ref) { "refs/heads/test" } + + def perform + subject.perform(project.id, user.id, oldrev, newrev, ref) + end + + it 'executes MergeRequests::RefreshService with expected values' do + expect(MergeRequests::RefreshService).to receive(:new).with(project, user).and_call_original + expect_any_instance_of(MergeRequests::RefreshService).to receive(:execute).with(oldrev, newrev, ref) + + perform + end + + it 'executes SystemHooksService with expected values' do + push_data = double('push_data') + expect(Gitlab::DataBuilder::Push).to receive(:build).with(project, user, oldrev, newrev, ref, []).and_return(push_data) + + system_hook_service = double('system_hook_service') + expect(SystemHooksService).to receive(:new).and_return(system_hook_service) + expect(system_hook_service).to receive(:execute_hooks).with(push_data, :push_hooks) + + perform + end + end +end |