From 83fc1464ddbbce004ef310653a7caec441cf0200 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 22 Mar 2019 17:01:48 +0000 Subject: Rename GitPushService -> Git::BranchPushService --- app/services/after_branch_delete_service.rb | 5 +- app/services/git/branch_push_service.rb | 244 ++++++ app/services/git_push_service.rb | 243 ------ app/workers/create_gpg_signature_worker.rb | 4 +- app/workers/post_receive.rb | 2 +- .../projects/environments/environment_spec.rb | 2 +- spec/services/git/branch_push_service_spec.rb | 837 +++++++++++++++++++++ spec/services/git_push_service_spec.rb | 837 --------------------- spec/support/helpers/cycle_analytics_helpers.rb | 2 +- spec/workers/post_receive_spec.rb | 14 +- 10 files changed, 1094 insertions(+), 1096 deletions(-) create mode 100644 app/services/git/branch_push_service.rb delete mode 100644 app/services/git_push_service.rb create mode 100644 spec/services/git/branch_push_service_spec.rb delete mode 100644 spec/services/git_push_service_spec.rb diff --git a/app/services/after_branch_delete_service.rb b/app/services/after_branch_delete_service.rb index e7eb74d3e7d..ece9fbbef43 100644 --- a/app/services/after_branch_delete_service.rb +++ b/app/services/after_branch_delete_service.rb @@ -1,9 +1,6 @@ # frozen_string_literal: true -## -# Branch can be deleted either by DeleteBranchService -# or by GitPushService. -# +# Branch can be deleted either by DeleteBranchService or by Git::BranchPushService. class AfterBranchDeleteService < BaseService attr_reader :branch_name diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb new file mode 100644 index 00000000000..b55aeb5f2b9 --- /dev/null +++ b/app/services/git/branch_push_service.rb @@ -0,0 +1,244 @@ +# frozen_string_literal: true + +module Git + class BranchPushService < BaseService + attr_accessor :push_data, :push_commits + include Gitlab::Access + include Gitlab::Utils::StrongMemoize + + # The N most recent commits to process in a single push payload. + PROCESS_COMMIT_LIMIT = 100 + + # This method will be called after each git update + # and only if the provided user and project are present in GitLab. + # + # All callbacks for post receive action should be placed here. + # + # Next, this method: + # 1. Creates the push event + # 2. Updates merge requests + # 3. Recognizes cross-references from commit messages + # 4. Executes the project's webhooks + # 5. Executes the project's services + # 6. Checks if the project's main language has changed + # + def execute + update_commits + execute_related_hooks + perform_housekeeping + + update_remote_mirrors + update_caches + + update_signatures + end + + def update_commits + project.repository.after_create if project.empty_repo? + project.repository.after_push_commit(branch_name) + + if push_remove_branch? + project.repository.after_remove_branch + @push_commits = [] + elsif push_to_new_branch? + project.repository.after_create_branch + + # Re-find the pushed commits. + if default_branch? + # Initial push to the default branch. Take the full history of that branch as "newly pushed". + process_default_branch + else + # Use the pushed commits that aren't reachable by the default branch + # as a heuristic. This may include more commits than are actually pushed, but + # that shouldn't matter because we check for existing cross-references later. + @push_commits = project.repository.commits_between(project.default_branch, params[:newrev]) + + # don't process commits for the initial push to the default branch + process_commit_messages + end + elsif push_to_existing_branch? + # Collect data for this git push + @push_commits = project.repository.commits_between(params[:oldrev], params[:newrev]) + + process_commit_messages + + # Update the bare repositories info/attributes file using the contents of the default branches + # .gitattributes file + update_gitattributes if default_branch? + end + end + + def update_gitattributes + project.repository.copy_gitattributes(params[:ref]) + end + + def update_caches + if default_branch? + if push_to_new_branch? + # If this is the initial push into the default branch, the file type caches + # will already be reset as a result of `Project#change_head`. + types = [] + else + paths = Set.new + + last_pushed_commits.each do |commit| + commit.raw_deltas.each do |diff| + paths << diff.new_path + end + end + + types = Gitlab::FileDetector.types_in_paths(paths.to_a) + end + + DetectRepositoryLanguagesWorker.perform_async(@project.id, current_user.id) + else + types = [] + end + + ProjectCacheWorker.perform_async(project.id, types, [:commit_count, :repository_size]) + end + + # rubocop: disable CodeReuse/ActiveRecord + def update_signatures + commit_shas = last_pushed_commits.map(&:sha) + + return if commit_shas.empty? + + shas_with_cached_signatures = GpgSignature.where(commit_sha: commit_shas).pluck(:commit_sha) + commit_shas -= shas_with_cached_signatures + + return if commit_shas.empty? + + commit_shas = Gitlab::Git::Commit.shas_with_signatures(project.repository, commit_shas) + + CreateGpgSignatureWorker.perform_async(commit_shas, project.id) + end + # rubocop: enable CodeReuse/ActiveRecord + + # Schedules processing of commit messages. + def process_commit_messages + default = default_branch? + + last_pushed_commits.each do |commit| + if commit.matches_cross_reference_regex? + ProcessCommitWorker + .perform_async(project.id, current_user.id, commit.to_hash, default) + end + end + end + + def update_remote_mirrors + return unless project.has_remote_mirror? + + project.mark_stuck_remote_mirrors_as_failed! + project.update_remote_mirrors + end + + def execute_related_hooks + # Update merge requests that may be affected by this push. A new branch + # could cause the last commit of a merge request to change. + # + UpdateMergeRequestsWorker + .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) + + EventCreateService.new.push(project, current_user, build_push_data) + Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push, pipeline_options) + + project.execute_hooks(build_push_data.dup, :push_hooks) + project.execute_services(build_push_data.dup, :push_hooks) + + if push_remove_branch? + AfterBranchDeleteService + .new(project, current_user) + .execute(branch_name) + end + end + + def perform_housekeeping + housekeeping = Projects::HousekeepingService.new(project) + housekeeping.increment! + housekeeping.execute if housekeeping.needed? + rescue Projects::HousekeepingService::LeaseTaken + end + + def process_default_branch + offset = [push_commits_count_for_ref - PROCESS_COMMIT_LIMIT, 0].max + @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) + + project.after_create_default_branch + end + + def build_push_data + @push_data ||= Gitlab::DataBuilder::Push.build( + project, + current_user, + params[:oldrev], + params[:newrev], + params[:ref], + @push_commits, + commits_count: commits_count, + push_options: params[:push_options] || [] + ) + end + + def push_to_existing_branch? + # Return if this is not a push to a branch (e.g. new commits) + branch_ref? && !Gitlab::Git.blank_ref?(params[:oldrev]) + end + + def push_to_new_branch? + strong_memoize(:push_to_new_branch) do + branch_ref? && Gitlab::Git.blank_ref?(params[:oldrev]) + end + end + + def push_remove_branch? + strong_memoize(:push_remove_branch) do + branch_ref? && Gitlab::Git.blank_ref?(params[:newrev]) + end + end + + def default_branch? + branch_ref? && + (branch_name == project.default_branch || project.default_branch.nil?) + end + + def commit_user(commit) + commit.author || current_user + end + + def branch_name + strong_memoize(:branch_name) do + Gitlab::Git.ref_name(params[:ref]) + end + end + + def branch_ref? + strong_memoize(:branch_ref) do + Gitlab::Git.branch_ref?(params[:ref]) + end + end + + def commits_count + return push_commits_count_for_ref if default_branch? && push_to_new_branch? + + Array(@push_commits).size + end + + def push_commits_count_for_ref + strong_memoize(:push_commits_count_for_ref) do + project.repository.commit_count_for_ref(params[:ref]) + end + end + + def last_pushed_commits + @last_pushed_commits ||= @push_commits.last(PROCESS_COMMIT_LIMIT) + end + + private + + def pipeline_options + {} # to be overridden in EE + end + end +end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb deleted file mode 100644 index 15f68fbdf8f..00000000000 --- a/app/services/git_push_service.rb +++ /dev/null @@ -1,243 +0,0 @@ -# frozen_string_literal: true - -class GitPushService < BaseService - attr_accessor :push_data, :push_commits - include Gitlab::Access - include Gitlab::Utils::StrongMemoize - - # The N most recent commits to process in a single push payload. - PROCESS_COMMIT_LIMIT = 100 - - # This method will be called after each git update - # and only if the provided user and project are present in GitLab. - # - # All callbacks for post receive action should be placed here. - # - # Next, this method: - # 1. Creates the push event - # 2. Updates merge requests - # 3. Recognizes cross-references from commit messages - # 4. Executes the project's webhooks - # 5. Executes the project's services - # 6. Checks if the project's main language has changed - # - def execute - update_commits - execute_related_hooks - perform_housekeeping - - update_remote_mirrors - update_caches - - update_signatures - end - - protected - - def update_commits - project.repository.after_create if project.empty_repo? - project.repository.after_push_commit(branch_name) - - if push_remove_branch? - project.repository.after_remove_branch - @push_commits = [] - elsif push_to_new_branch? - project.repository.after_create_branch - - # Re-find the pushed commits. - if default_branch? - # Initial push to the default branch. Take the full history of that branch as "newly pushed". - process_default_branch - else - # Use the pushed commits that aren't reachable by the default branch - # as a heuristic. This may include more commits than are actually pushed, but - # that shouldn't matter because we check for existing cross-references later. - @push_commits = project.repository.commits_between(project.default_branch, params[:newrev]) - - # don't process commits for the initial push to the default branch - process_commit_messages - end - elsif push_to_existing_branch? - # Collect data for this git push - @push_commits = project.repository.commits_between(params[:oldrev], params[:newrev]) - - process_commit_messages - - # Update the bare repositories info/attributes file using the contents of the default branches - # .gitattributes file - update_gitattributes if default_branch? - end - end - - def update_gitattributes - project.repository.copy_gitattributes(params[:ref]) - end - - def update_caches - if default_branch? - if push_to_new_branch? - # If this is the initial push into the default branch, the file type caches - # will already be reset as a result of `Project#change_head`. - types = [] - else - paths = Set.new - - last_pushed_commits.each do |commit| - commit.raw_deltas.each do |diff| - paths << diff.new_path - end - end - - types = Gitlab::FileDetector.types_in_paths(paths.to_a) - end - - DetectRepositoryLanguagesWorker.perform_async(@project.id, current_user.id) - else - types = [] - end - - ProjectCacheWorker.perform_async(project.id, types, [:commit_count, :repository_size]) - end - - # rubocop: disable CodeReuse/ActiveRecord - def update_signatures - commit_shas = last_pushed_commits.map(&:sha) - - return if commit_shas.empty? - - shas_with_cached_signatures = GpgSignature.where(commit_sha: commit_shas).pluck(:commit_sha) - commit_shas -= shas_with_cached_signatures - - return if commit_shas.empty? - - commit_shas = Gitlab::Git::Commit.shas_with_signatures(project.repository, commit_shas) - - CreateGpgSignatureWorker.perform_async(commit_shas, project.id) - end - # rubocop: enable CodeReuse/ActiveRecord - - # Schedules processing of commit messages. - def process_commit_messages - default = default_branch? - - last_pushed_commits.each do |commit| - if commit.matches_cross_reference_regex? - ProcessCommitWorker - .perform_async(project.id, current_user.id, commit.to_hash, default) - end - end - end - - def update_remote_mirrors - return unless project.has_remote_mirror? - - project.mark_stuck_remote_mirrors_as_failed! - project.update_remote_mirrors - end - - def execute_related_hooks - # Update merge requests that may be affected by this push. A new branch - # could cause the last commit of a merge request to change. - # - UpdateMergeRequestsWorker - .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) - - EventCreateService.new.push(project, current_user, build_push_data) - Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push, pipeline_options) - - project.execute_hooks(build_push_data.dup, :push_hooks) - project.execute_services(build_push_data.dup, :push_hooks) - - if push_remove_branch? - AfterBranchDeleteService - .new(project, current_user) - .execute(branch_name) - end - end - - def perform_housekeeping - housekeeping = Projects::HousekeepingService.new(project) - housekeeping.increment! - housekeeping.execute if housekeeping.needed? - rescue Projects::HousekeepingService::LeaseTaken - end - - def process_default_branch - offset = [push_commits_count_for_ref - PROCESS_COMMIT_LIMIT, 0].max - @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) - - project.after_create_default_branch - end - - def build_push_data - @push_data ||= Gitlab::DataBuilder::Push.build( - project, - current_user, - params[:oldrev], - params[:newrev], - params[:ref], - @push_commits, - commits_count: commits_count, - push_options: params[:push_options] || []) - end - - def push_to_existing_branch? - # Return if this is not a push to a branch (e.g. new commits) - branch_ref? && !Gitlab::Git.blank_ref?(params[:oldrev]) - end - - def push_to_new_branch? - strong_memoize(:push_to_new_branch) do - branch_ref? && Gitlab::Git.blank_ref?(params[:oldrev]) - end - end - - def push_remove_branch? - strong_memoize(:push_remove_branch) do - branch_ref? && Gitlab::Git.blank_ref?(params[:newrev]) - end - end - - def default_branch? - branch_ref? && - (branch_name == project.default_branch || project.default_branch.nil?) - end - - def commit_user(commit) - commit.author || current_user - end - - def branch_name - strong_memoize(:branch_name) do - Gitlab::Git.ref_name(params[:ref]) - end - end - - def branch_ref? - strong_memoize(:branch_ref) do - Gitlab::Git.branch_ref?(params[:ref]) - end - end - - def commits_count - return push_commits_count_for_ref if default_branch? && push_to_new_branch? - - Array(@push_commits).size - end - - def push_commits_count_for_ref - strong_memoize(:push_commits_count_for_ref) do - project.repository.commit_count_for_ref(params[:ref]) - end - end - - def last_pushed_commits - @last_pushed_commits ||= @push_commits.last(PROCESS_COMMIT_LIMIT) - end - - private - - def pipeline_options - {} # to be overridden in EE - end -end diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index 2827529cc1c..7fac7822cf7 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -5,8 +5,8 @@ class CreateGpgSignatureWorker # rubocop: disable CodeReuse/ActiveRecord def perform(commit_shas, project_id) - # Older versions of GitPushService may push a single commit ID on the stack. - # We need this to be backwards compatible. + # Older versions of Git::BranchPushService may push a single commit ID on + # the stack. We need this to be backwards compatible. commit_shas = Array(commit_shas) return if commit_shas.empty? diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index e721f14ddf5..a40c865a5e5 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -46,7 +46,7 @@ class PostReceive ref: ref, push_options: post_received.push_options).execute elsif Gitlab::Git.branch_ref?(ref) - GitPushService.new( + Git::BranchPushService.new( post_received.project, @user, oldrev: oldrev, diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 3090f1a2131..fe71cb7661a 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -319,7 +319,7 @@ describe 'Environment' do yield - GitPushService.new(project, user, params).execute + Git::BranchPushService.new(project, user, params).execute end end diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb new file mode 100644 index 00000000000..d0e2169b4a6 --- /dev/null +++ b/spec/services/git/branch_push_service_spec.rb @@ -0,0 +1,837 @@ +require 'spec_helper' + +describe Git::BranchPushService, services: true do + include RepoHelpers + + set(:user) { create(:user) } + set(:project) { create(:project, :repository) } + let(:blankrev) { Gitlab::Git::BLANK_SHA } + let(:oldrev) { sample_commit.parent_id } + let(:newrev) { sample_commit.id } + let(:ref) { 'refs/heads/master' } + + before do + project.add_maintainer(user) + end + + describe 'with remote mirrors' do + let(:project) { create(:project, :repository, :remote_mirror) } + + subject do + described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) + end + + context 'when remote mirror feature is enabled' do + it 'fails stuck remote mirrors' do + allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors) + expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) + + subject.execute + end + + it 'updates remote mirrors' do + expect(project).to receive(:update_remote_mirrors) + + subject.execute + end + end + + context 'when remote mirror feature is disabled' do + before do + stub_application_setting(mirror_available: false) + end + + context 'with remote mirrors global setting overridden' do + before do + project.remote_mirror_available_overridden = true + end + + it 'fails stuck remote mirrors' do + allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors) + expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) + + subject.execute + end + + it 'updates remote mirrors' do + expect(project).to receive(:update_remote_mirrors) + + subject.execute + end + end + + context 'without remote mirrors global setting overridden' do + before do + project.remote_mirror_available_overridden = false + end + + it 'does not fails stuck remote mirrors' do + expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!) + + subject.execute + end + + it 'does not updates remote mirrors' do + expect(project).not_to receive(:update_remote_mirrors) + + subject.execute + end + end + end + end + + describe 'Push branches' do + subject do + execute_service(project, user, oldrev, newrev, ref) + end + + context 'new branch' do + let(:oldrev) { blankrev } + + it { is_expected.to be_truthy } + + it 'calls the after_push_commit hook' do + expect(project.repository).to receive(:after_push_commit).with('master') + + subject + end + + it 'calls the after_create_branch hook' do + expect(project.repository).to receive(:after_create_branch) + + subject + end + end + + context 'existing branch' do + it { is_expected.to be_truthy } + + it 'calls the after_push_commit hook' do + expect(project.repository).to receive(:after_push_commit).with('master') + + subject + end + end + + context 'rm branch' do + let(:newrev) { blankrev } + + it { is_expected.to be_truthy } + + it 'calls the after_push_commit hook' do + expect(project.repository).to receive(:after_push_commit).with('master') + + subject + end + + it 'calls the after_remove_branch hook' do + expect(project.repository).to receive(:after_remove_branch) + + subject + end + end + end + + describe "Git Push Data" do + let(:commit) { project.commit(newrev) } + + subject { push_data_from_service(project, user, oldrev, newrev, ref) } + + it { is_expected.to include(object_kind: 'push') } + it { is_expected.to include(before: oldrev) } + it { is_expected.to include(after: newrev) } + it { is_expected.to include(ref: ref) } + it { is_expected.to include(user_id: user.id) } + it { is_expected.to include(user_name: user.name) } + it { is_expected.to include(project_id: project.id) } + + context "with repository data" do + subject { push_data_from_service(project, user, oldrev, newrev, ref)[:repository] } + + it { is_expected.to include(name: project.name) } + it { is_expected.to include(url: project.url_to_repo) } + it { is_expected.to include(description: project.description) } + it { is_expected.to include(homepage: project.web_url) } + end + + context "with commits" do + subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits] } + + it { is_expected.to be_an(Array) } + it 'has 1 element' do + expect(subject.size).to eq(1) + end + + context "the commit" do + subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits].first } + + it { is_expected.to include(id: commit.id) } + it { is_expected.to include(message: commit.safe_message) } + it { expect(subject[:timestamp].in_time_zone).to eq(commit.date.in_time_zone) } + it do + is_expected.to include( + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + commit.id + ].join('/') + ) + end + + context "with a author" do + subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits].first[:author] } + + it { is_expected.to include(name: commit.author_name) } + it { is_expected.to include(email: commit.author_email) } + end + end + end + end + + describe "Pipelines" do + subject { execute_service(project, user, oldrev, newrev, ref) } + + before do + stub_ci_pipeline_to_return_yaml_file + end + + it "creates a new pipeline" do + expect { subject }.to change { Ci::Pipeline.count } + expect(Ci::Pipeline.last).to be_push + end + end + + describe "Push Event" do + context "with an existing branch" do + let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) } + let(:event) { Event.find_by_action(Event::PUSHED) } + + it 'generates a push event with one commit' do + expect(event).to be_an_instance_of(PushEvent) + expect(event.project).to eq(project) + expect(event.action).to eq(Event::PUSHED) + expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) + expect(event.push_event_payload.commit_from).to eq(oldrev) + expect(event.push_event_payload.commit_to).to eq(newrev) + expect(event.push_event_payload.ref).to eq('master') + expect(event.push_event_payload.commit_count).to eq(1) + end + end + + context "with a new branch" do + let!(:new_branch_data) { push_data_from_service(project, user, Gitlab::Git::BLANK_SHA, newrev, ref) } + let(:event) { Event.find_by_action(Event::PUSHED) } + + it 'generates a push event with more than one commit' do + expect(event).to be_an_instance_of(PushEvent) + expect(event.project).to eq(project) + expect(event.action).to eq(Event::PUSHED) + expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) + expect(event.push_event_payload.commit_from).to be_nil + expect(event.push_event_payload.commit_to).to eq(newrev) + expect(event.push_event_payload.ref).to eq('master') + expect(event.push_event_payload.commit_count).to be > 1 + end + end + + context "Updates merge requests" do + it "when pushing a new branch for the first time" do + expect(UpdateMergeRequestsWorker).to receive(:perform_async) + .with(project.id, user.id, blankrev, 'newrev', ref) + execute_service(project, user, blankrev, 'newrev', ref ) + end + end + + describe 'system hooks' do + let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) } + let!(:system_hooks_service) { SystemHooksService.new } + + it "sends a system hook after pushing a branch" do + allow(SystemHooksService).to receive(:new).and_return(system_hooks_service) + allow(system_hooks_service).to receive(:execute_hooks) + + execute_service(project, user, oldrev, newrev, ref) + + expect(system_hooks_service).to have_received(:execute_hooks).with(push_data, :push_hooks) + end + end + end + + describe "Updates git attributes" do + context "for default branch" do + it "calls the copy attributes method for the first push to the default branch" do + expect(project.repository).to receive(:copy_gitattributes).with('master') + + execute_service(project, user, blankrev, 'newrev', ref) + end + + it "calls the copy attributes method for changes to the default branch" do + expect(project.repository).to receive(:copy_gitattributes).with(ref) + + execute_service(project, user, 'oldrev', 'newrev', ref) + end + end + + context "for non-default branch" do + before do + # Make sure the "default" branch is different + allow(project).to receive(:default_branch).and_return('not-master') + end + + it "does not call copy attributes method" do + expect(project.repository).not_to receive(:copy_gitattributes) + + execute_service(project, user, oldrev, newrev, ref) + end + end + end + + describe "Webhooks" do + context "execute webhooks" do + it "when pushing a branch for the first time" do + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + execute_service(project, user, blankrev, 'newrev', ref) + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + end + + it "when pushing a branch for the first time with default branch protection disabled" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) + + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + execute_service(project, user, blankrev, 'newrev', ref) + expect(project.protected_branches).to be_empty + end + + it "when pushing a branch for the first time with default branch protection set to 'developers can push'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + + execute_service(project, user, blankrev, 'newrev', ref) + + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + end + + it "when pushing a branch for the first time with an existing branch permission configured" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + expect_any_instance_of(ProtectedBranches::CreateService).not_to receive(:execute) + + execute_service(project, user, blankrev, 'newrev', ref) + + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) + expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + + it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + execute_service(project, user, blankrev, 'newrev', ref) + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + + it "when pushing new commits to existing branch" do + expect(project).to receive(:execute_hooks) + execute_service(project, user, 'oldrev', 'newrev', ref) + end + end + end + + describe "cross-reference notes" do + let(:issue) { create :issue, project: project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + + before do + project.add_developer(commit_author) + project.add_developer(user) + + allow(commit).to receive_messages( + safe_message: "this commit \n mentions #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email + ) + + allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) + .and_return(commit) + + allow(project.repository).to receive(:commits_between).and_return([commit]) + end + + it "creates a note if a pushed commit mentions an issue" do + expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) + + execute_service(project, user, oldrev, newrev, ref) + end + + it "only creates a cross-reference note if one doesn't already exist" do + SystemNoteService.cross_reference(issue, commit, user) + + expect(SystemNoteService).not_to receive(:cross_reference).with(issue, commit, commit_author) + + execute_service(project, user, oldrev, newrev, ref) + end + + it "defaults to the pushing user if the commit's author is not known" do + allow(commit).to receive_messages( + author_name: 'unknown name', + author_email: 'unknown@email.com' + ) + expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, user) + + execute_service(project, user, oldrev, newrev, ref) + end + + it "finds references in the first push to a non-default branch" do + allow(project.repository).to receive(:commits_between).with(blankrev, newrev).and_return([]) + allow(project.repository).to receive(:commits_between).with("master", newrev).and_return([commit]) + + expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) + + execute_service(project, user, blankrev, newrev, 'refs/heads/other') + end + end + + describe "issue metrics" do + let(:issue) { create :issue, project: project } + let(:commit_author) { create :user } + let(:commit) { project.commit } + let(:commit_time) { Time.now } + + before do + project.add_developer(commit_author) + project.add_developer(user) + + allow(commit).to receive_messages( + safe_message: "this commit \n mentions #{issue.to_reference}", + references: [issue], + author_name: commit_author.name, + author_email: commit_author.email, + committed_date: commit_time + ) + + allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) + .and_return(commit) + + allow(project.repository).to receive(:commits_between).and_return([commit]) + end + + context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do + it 'sets the metric for referenced issues' do + execute_service(project, user, oldrev, newrev, ref) + + expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_like_time(commit_time) + end + + it 'does not set the metric for non-referenced issues' do + non_referenced_issue = create(:issue, project: project) + execute_service(project, user, oldrev, newrev, ref) + + expect(non_referenced_issue.reload.metrics.first_mentioned_in_commit_at).to be_nil + end + end + end + + describe "closing issues from pushed commits containing a closing reference" do + let(:issue) { create :issue, project: project } + let(:other_issue) { create :issue, project: project } + let(:commit_author) { create :user } + let(:closing_commit) { project.commit } + + before do + allow(closing_commit).to receive_messages( + issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/, + safe_message: "this is some work.\n\ncloses ##{issue.iid}", + author_name: commit_author.name, + author_email: commit_author.email + ) + + allow(project.repository).to receive(:commits_between) + .and_return([closing_commit]) + + allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) + .and_return(closing_commit) + + project.add_maintainer(commit_author) + end + + context "to default branches" do + it "closes issues" do + execute_service(project, commit_author, oldrev, newrev, ref) + expect(Issue.find(issue.id)).to be_closed + end + + it "adds a note indicating that the issue is now closed" do + expect(SystemNoteService).to receive(:change_status).with(issue, project, commit_author, "closed", closing_commit) + execute_service(project, commit_author, oldrev, newrev, ref) + end + + it "doesn't create additional cross-reference notes" do + expect(SystemNoteService).not_to receive(:cross_reference) + execute_service(project, commit_author, oldrev, newrev, ref) + end + end + + context "to non-default branches" do + before do + # Make sure the "default" branch is different + allow(project).to receive(:default_branch).and_return('not-master') + end + + it "creates cross-reference notes" do + expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author) + execute_service(project, user, oldrev, newrev, ref) + end + + it "doesn't close issues" do + execute_service(project, user, oldrev, newrev, ref) + expect(Issue.find(issue.id)).to be_opened + end + end + + context "for jira issue tracker" do + include JiraServiceHelper + + let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } + + before do + # project.create_jira_service doesn't seem to invalidate the cache here + project.has_external_issue_tracker = true + jira_service_settings + stub_jira_urls("JIRA-1") + + allow(closing_commit).to receive_messages({ + issue_closing_regex: Regexp.new(Gitlab.config.gitlab.issue_closing_pattern), + safe_message: message, + author_name: commit_author.name, + author_email: commit_author.email + }) + + allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) + + allow(project.repository).to receive_messages(commits_between: [closing_commit]) + end + + after do + jira_tracker.destroy! + end + + context "mentioning an issue" do + let(:message) { "this is some work.\n\nrelated to JIRA-1" } + + it "initiates one api call to jira server to mention the issue" do + execute_service(project, user, oldrev, newrev, ref) + + expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( + body: /mentioned this issue in/ + ).once + end + end + + context "closing an issue" do + let(:message) { "this is some work.\n\ncloses JIRA-1" } + let(:comment_body) do + { + body: "Issue solved with [#{closing_commit.id}|http://#{Gitlab.config.gitlab.host}/#{project.full_path}/commit/#{closing_commit.id}]." + }.to_json + end + + before do + open_issue = JIRA::Resource::Issue.new(jira_tracker.client, attrs: { "id" => "JIRA-1" }) + closed_issue = open_issue.dup + allow(open_issue).to receive(:resolution).and_return(false) + allow(closed_issue).to receive(:resolution).and_return(true) + allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue) + + allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-1") + end + + context "using right markdown" do + it "initiates one api call to jira server to close the issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + + expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once + end + + it "initiates one api call to jira server to comment on the issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + + expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( + body: comment_body + ).once + end + end + + context "using internal issue reference" do + context 'when internal issues are disabled' do + before do + project.issues_enabled = false + project.save! + end + let(:message) { "this is some work.\n\ncloses #1" } + + it "does not initiates one api call to jira server to close the issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + + expect(WebMock).not_to have_requested(:post, jira_api_transition_url('JIRA-1')) + end + + it "does not initiates one api call to jira server to comment on the issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + + expect(WebMock).not_to have_requested(:post, jira_api_comment_url('JIRA-1')).with( + body: comment_body + ).once + end + end + + context 'when internal issues are enabled' do + let(:issue) { create(:issue, project: project) } + let(:message) { "this is some work.\n\ncloses JIRA-1 \n\n closes #{issue.to_reference}" } + + it "initiates one api call to jira server to close the jira issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + + expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once + end + + it "initiates one api call to jira server to comment on the jira issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + + expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( + body: comment_body + ).once + end + + it "closes the internal issue" do + execute_service(project, commit_author, oldrev, newrev, ref) + expect(issue.reload).to be_closed + end + + it "adds a note indicating that the issue is now closed" do + expect(SystemNoteService).to receive(:change_status) + .with(issue, project, commit_author, "closed", closing_commit) + execute_service(project, commit_author, oldrev, newrev, ref) + end + end + end + end + end + end + + describe "empty project" do + let(:project) { create(:project_empty_repo) } + let(:new_ref) { 'refs/heads/feature' } + + before do + allow(project).to receive(:default_branch).and_return('feature') + expect(project).to receive(:change_head) { 'feature'} + end + + it 'push to first branch updates HEAD' do + execute_service(project, user, blankrev, newrev, new_ref) + end + end + + describe "housekeeping" do + let(:housekeeping) { Projects::HousekeepingService.new(project) } + + before do + # Flush any raw key-value data stored by the housekeeping code. + Gitlab::Redis::Cache.with { |conn| conn.flushall } + Gitlab::Redis::Queues.with { |conn| conn.flushall } + Gitlab::Redis::SharedState.with { |conn| conn.flushall } + + allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping) + end + + after do + Gitlab::Redis::Cache.with { |conn| conn.flushall } + Gitlab::Redis::Queues.with { |conn| conn.flushall } + Gitlab::Redis::SharedState.with { |conn| conn.flushall } + end + + it 'does not perform housekeeping when not needed' do + expect(housekeeping).not_to receive(:execute) + + execute_service(project, user, oldrev, newrev, ref) + end + + context 'when housekeeping is needed' do + before do + allow(housekeeping).to receive(:needed?).and_return(true) + end + + it 'performs housekeeping' do + expect(housekeeping).to receive(:execute) + + execute_service(project, user, oldrev, newrev, ref) + end + + it 'does not raise an exception' do + allow(housekeeping).to receive(:try_obtain_lease).and_return(false) + + execute_service(project, user, oldrev, newrev, ref) + end + end + + it 'increments the push counter' do + expect(housekeeping).to receive(:increment!) + + execute_service(project, user, oldrev, newrev, ref) + end + end + + describe '#update_caches' do + let(:service) do + described_class.new(project, + user, + oldrev: oldrev, + newrev: newrev, + ref: ref) + end + + context 'on the default branch' do + before do + allow(service).to receive(:default_branch?).and_return(true) + end + + it 'flushes the caches of any special files that have been changed' do + commit = double(:commit) + diff = double(:diff, new_path: 'README.md') + + expect(commit).to receive(:raw_deltas) + .and_return([diff]) + + service.push_commits = [commit] + + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, %i(readme), %i(commit_count repository_size)) + + service.update_caches + end + end + + context 'on a non-default branch' do + before do + allow(service).to receive(:default_branch?).and_return(false) + end + + it 'does not flush any conditional caches' do + expect(ProjectCacheWorker).to receive(:perform_async) + .with(project.id, [], %i(commit_count repository_size)) + .and_call_original + + service.update_caches + end + end + end + + describe '#process_commit_messages' do + let(:service) do + described_class.new(project, + user, + oldrev: oldrev, + newrev: newrev, + ref: ref) + end + + it 'only schedules a limited number of commits' do + service.push_commits = Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true)) + + expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times + + service.process_commit_messages + end + + it "skips commits which don't include cross-references" do + service.push_commits = [double(:commit, to_hash: {}, matches_cross_reference_regex?: false)] + + expect(ProcessCommitWorker).not_to receive(:perform_async) + + service.process_commit_messages + end + end + + describe '#update_signatures' do + let(:service) do + described_class.new( + project, + user, + oldrev: oldrev, + newrev: newrev, + ref: 'refs/heads/master' + ) + end + + context 'when the commit has a signature' do + context 'when the signature is already cached' do + before do + create(:gpg_signature, commit_sha: sample_commit.id) + end + + it 'does not queue a CreateGpgSignatureWorker' do + expect(CreateGpgSignatureWorker).not_to receive(:perform_async) + + execute_service(project, user, oldrev, newrev, ref) + end + end + + context 'when the signature is not yet cached' do + it 'queues a CreateGpgSignatureWorker' do + expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id], project.id) + + execute_service(project, user, oldrev, newrev, ref) + end + + it 'can queue several commits to create the gpg signature' do + allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).and_return([sample_commit.id, another_sample_commit.id]) + + expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id, another_sample_commit.id], project.id) + + execute_service(project, user, oldrev, newrev, ref) + end + end + end + + context 'when the commit does not have a signature' do + before do + allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).with(project.repository, [sample_commit.id]).and_return([]) + end + + it 'does not queue a CreateGpgSignatureWorker' do + expect(CreateGpgSignatureWorker).not_to receive(:perform_async).with(sample_commit.id, project.id) + + execute_service(project, user, oldrev, newrev, ref) + end + end + end + + def execute_service(project, user, oldrev, newrev, ref) + service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) + service.execute + service + end + + def push_data_from_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev, newrev, ref).push_data + end +end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb deleted file mode 100644 index e8fce951155..00000000000 --- a/spec/services/git_push_service_spec.rb +++ /dev/null @@ -1,837 +0,0 @@ -require 'spec_helper' - -describe GitPushService, services: true do - include RepoHelpers - - set(:user) { create(:user) } - set(:project) { create(:project, :repository) } - let(:blankrev) { Gitlab::Git::BLANK_SHA } - let(:oldrev) { sample_commit.parent_id } - let(:newrev) { sample_commit.id } - let(:ref) { 'refs/heads/master' } - - before do - project.add_maintainer(user) - end - - describe 'with remote mirrors' do - let(:project) { create(:project, :repository, :remote_mirror) } - - subject do - described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) - end - - context 'when remote mirror feature is enabled' do - it 'fails stuck remote mirrors' do - allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors) - expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) - - subject.execute - end - - it 'updates remote mirrors' do - expect(project).to receive(:update_remote_mirrors) - - subject.execute - end - end - - context 'when remote mirror feature is disabled' do - before do - stub_application_setting(mirror_available: false) - end - - context 'with remote mirrors global setting overridden' do - before do - project.remote_mirror_available_overridden = true - end - - it 'fails stuck remote mirrors' do - allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors) - expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!) - - subject.execute - end - - it 'updates remote mirrors' do - expect(project).to receive(:update_remote_mirrors) - - subject.execute - end - end - - context 'without remote mirrors global setting overridden' do - before do - project.remote_mirror_available_overridden = false - end - - it 'does not fails stuck remote mirrors' do - expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!) - - subject.execute - end - - it 'does not updates remote mirrors' do - expect(project).not_to receive(:update_remote_mirrors) - - subject.execute - end - end - end - end - - describe 'Push branches' do - subject do - execute_service(project, user, oldrev, newrev, ref) - end - - context 'new branch' do - let(:oldrev) { blankrev } - - it { is_expected.to be_truthy } - - it 'calls the after_push_commit hook' do - expect(project.repository).to receive(:after_push_commit).with('master') - - subject - end - - it 'calls the after_create_branch hook' do - expect(project.repository).to receive(:after_create_branch) - - subject - end - end - - context 'existing branch' do - it { is_expected.to be_truthy } - - it 'calls the after_push_commit hook' do - expect(project.repository).to receive(:after_push_commit).with('master') - - subject - end - end - - context 'rm branch' do - let(:newrev) { blankrev } - - it { is_expected.to be_truthy } - - it 'calls the after_push_commit hook' do - expect(project.repository).to receive(:after_push_commit).with('master') - - subject - end - - it 'calls the after_remove_branch hook' do - expect(project.repository).to receive(:after_remove_branch) - - subject - end - end - end - - describe "Git Push Data" do - let(:commit) { project.commit(newrev) } - - subject { push_data_from_service(project, user, oldrev, newrev, ref) } - - it { is_expected.to include(object_kind: 'push') } - it { is_expected.to include(before: oldrev) } - it { is_expected.to include(after: newrev) } - it { is_expected.to include(ref: ref) } - it { is_expected.to include(user_id: user.id) } - it { is_expected.to include(user_name: user.name) } - it { is_expected.to include(project_id: project.id) } - - context "with repository data" do - subject { push_data_from_service(project, user, oldrev, newrev, ref)[:repository] } - - it { is_expected.to include(name: project.name) } - it { is_expected.to include(url: project.url_to_repo) } - it { is_expected.to include(description: project.description) } - it { is_expected.to include(homepage: project.web_url) } - end - - context "with commits" do - subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits] } - - it { is_expected.to be_an(Array) } - it 'has 1 element' do - expect(subject.size).to eq(1) - end - - context "the commit" do - subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits].first } - - it { is_expected.to include(id: commit.id) } - it { is_expected.to include(message: commit.safe_message) } - it { expect(subject[:timestamp].in_time_zone).to eq(commit.date.in_time_zone) } - it do - is_expected.to include( - url: [ - Gitlab.config.gitlab.url, - project.namespace.to_param, - project.to_param, - 'commit', - commit.id - ].join('/') - ) - end - - context "with a author" do - subject { push_data_from_service(project, user, oldrev, newrev, ref)[:commits].first[:author] } - - it { is_expected.to include(name: commit.author_name) } - it { is_expected.to include(email: commit.author_email) } - end - end - end - end - - describe "Pipelines" do - subject { execute_service(project, user, oldrev, newrev, ref) } - - before do - stub_ci_pipeline_to_return_yaml_file - end - - it "creates a new pipeline" do - expect { subject }.to change { Ci::Pipeline.count } - expect(Ci::Pipeline.last).to be_push - end - end - - describe "Push Event" do - context "with an existing branch" do - let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) } - let(:event) { Event.find_by_action(Event::PUSHED) } - - it 'generates a push event with one commit' do - expect(event).to be_an_instance_of(PushEvent) - expect(event.project).to eq(project) - expect(event.action).to eq(Event::PUSHED) - expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) - expect(event.push_event_payload.commit_from).to eq(oldrev) - expect(event.push_event_payload.commit_to).to eq(newrev) - expect(event.push_event_payload.ref).to eq('master') - expect(event.push_event_payload.commit_count).to eq(1) - end - end - - context "with a new branch" do - let!(:new_branch_data) { push_data_from_service(project, user, Gitlab::Git::BLANK_SHA, newrev, ref) } - let(:event) { Event.find_by_action(Event::PUSHED) } - - it 'generates a push event with more than one commit' do - expect(event).to be_an_instance_of(PushEvent) - expect(event.project).to eq(project) - expect(event.action).to eq(Event::PUSHED) - expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) - expect(event.push_event_payload.commit_from).to be_nil - expect(event.push_event_payload.commit_to).to eq(newrev) - expect(event.push_event_payload.ref).to eq('master') - expect(event.push_event_payload.commit_count).to be > 1 - end - end - - context "Updates merge requests" do - it "when pushing a new branch for the first time" do - expect(UpdateMergeRequestsWorker).to receive(:perform_async) - .with(project.id, user.id, blankrev, 'newrev', ref) - execute_service(project, user, blankrev, 'newrev', ref ) - end - end - - describe 'system hooks' do - let!(:push_data) { push_data_from_service(project, user, oldrev, newrev, ref) } - let!(:system_hooks_service) { SystemHooksService.new } - - it "sends a system hook after pushing a branch" do - allow(SystemHooksService).to receive(:new).and_return(system_hooks_service) - allow(system_hooks_service).to receive(:execute_hooks) - - execute_service(project, user, oldrev, newrev, ref) - - expect(system_hooks_service).to have_received(:execute_hooks).with(push_data, :push_hooks) - end - end - end - - describe "Updates git attributes" do - context "for default branch" do - it "calls the copy attributes method for the first push to the default branch" do - expect(project.repository).to receive(:copy_gitattributes).with('master') - - execute_service(project, user, blankrev, 'newrev', ref) - end - - it "calls the copy attributes method for changes to the default branch" do - expect(project.repository).to receive(:copy_gitattributes).with(ref) - - execute_service(project, user, 'oldrev', 'newrev', ref) - end - end - - context "for non-default branch" do - before do - # Make sure the "default" branch is different - allow(project).to receive(:default_branch).and_return('not-master') - end - - it "does not call copy attributes method" do - expect(project.repository).not_to receive(:copy_gitattributes) - - execute_service(project, user, oldrev, newrev, ref) - end - end - end - - describe "Webhooks" do - context "execute webhooks" do - it "when pushing a branch for the first time" do - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - end - - it "when pushing a branch for the first time with default branch protection disabled" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) - expect(project.protected_branches).to be_empty - end - - it "when pushing a branch for the first time with default branch protection set to 'developers can push'" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - - execute_service(project, user, blankrev, 'newrev', ref) - - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - end - - it "when pushing a branch for the first time with an existing branch permission configured" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - expect_any_instance_of(ProtectedBranches::CreateService).not_to receive(:execute) - - execute_service(project, user, blankrev, 'newrev', ref) - - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end - - it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end - - it "when pushing new commits to existing branch" do - expect(project).to receive(:execute_hooks) - execute_service(project, user, 'oldrev', 'newrev', ref) - end - end - end - - describe "cross-reference notes" do - let(:issue) { create :issue, project: project } - let(:commit_author) { create :user } - let(:commit) { project.commit } - - before do - project.add_developer(commit_author) - project.add_developer(user) - - allow(commit).to receive_messages( - safe_message: "this commit \n mentions #{issue.to_reference}", - references: [issue], - author_name: commit_author.name, - author_email: commit_author.email - ) - - allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) - .and_return(commit) - - allow(project.repository).to receive(:commits_between).and_return([commit]) - end - - it "creates a note if a pushed commit mentions an issue" do - expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) - - execute_service(project, user, oldrev, newrev, ref) - end - - it "only creates a cross-reference note if one doesn't already exist" do - SystemNoteService.cross_reference(issue, commit, user) - - expect(SystemNoteService).not_to receive(:cross_reference).with(issue, commit, commit_author) - - execute_service(project, user, oldrev, newrev, ref) - end - - it "defaults to the pushing user if the commit's author is not known" do - allow(commit).to receive_messages( - author_name: 'unknown name', - author_email: 'unknown@email.com' - ) - expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, user) - - execute_service(project, user, oldrev, newrev, ref) - end - - it "finds references in the first push to a non-default branch" do - allow(project.repository).to receive(:commits_between).with(blankrev, newrev).and_return([]) - allow(project.repository).to receive(:commits_between).with("master", newrev).and_return([commit]) - - expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) - - execute_service(project, user, blankrev, newrev, 'refs/heads/other') - end - end - - describe "issue metrics" do - let(:issue) { create :issue, project: project } - let(:commit_author) { create :user } - let(:commit) { project.commit } - let(:commit_time) { Time.now } - - before do - project.add_developer(commit_author) - project.add_developer(user) - - allow(commit).to receive_messages( - safe_message: "this commit \n mentions #{issue.to_reference}", - references: [issue], - author_name: commit_author.name, - author_email: commit_author.email, - committed_date: commit_time - ) - - allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) - .and_return(commit) - - allow(project.repository).to receive(:commits_between).and_return([commit]) - end - - context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do - it 'sets the metric for referenced issues' do - execute_service(project, user, oldrev, newrev, ref) - - expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_like_time(commit_time) - end - - it 'does not set the metric for non-referenced issues' do - non_referenced_issue = create(:issue, project: project) - execute_service(project, user, oldrev, newrev, ref) - - expect(non_referenced_issue.reload.metrics.first_mentioned_in_commit_at).to be_nil - end - end - end - - describe "closing issues from pushed commits containing a closing reference" do - let(:issue) { create :issue, project: project } - let(:other_issue) { create :issue, project: project } - let(:commit_author) { create :user } - let(:closing_commit) { project.commit } - - before do - allow(closing_commit).to receive_messages( - issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/, - safe_message: "this is some work.\n\ncloses ##{issue.iid}", - author_name: commit_author.name, - author_email: commit_author.email - ) - - allow(project.repository).to receive(:commits_between) - .and_return([closing_commit]) - - allow_any_instance_of(ProcessCommitWorker).to receive(:build_commit) - .and_return(closing_commit) - - project.add_maintainer(commit_author) - end - - context "to default branches" do - it "closes issues" do - execute_service(project, commit_author, oldrev, newrev, ref) - expect(Issue.find(issue.id)).to be_closed - end - - it "adds a note indicating that the issue is now closed" do - expect(SystemNoteService).to receive(:change_status).with(issue, project, commit_author, "closed", closing_commit) - execute_service(project, commit_author, oldrev, newrev, ref) - end - - it "doesn't create additional cross-reference notes" do - expect(SystemNoteService).not_to receive(:cross_reference) - execute_service(project, commit_author, oldrev, newrev, ref) - end - end - - context "to non-default branches" do - before do - # Make sure the "default" branch is different - allow(project).to receive(:default_branch).and_return('not-master') - end - - it "creates cross-reference notes" do - expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author) - execute_service(project, user, oldrev, newrev, ref) - end - - it "doesn't close issues" do - execute_service(project, user, oldrev, newrev, ref) - expect(Issue.find(issue.id)).to be_opened - end - end - - context "for jira issue tracker" do - include JiraServiceHelper - - let(:jira_tracker) { project.create_jira_service if project.jira_service.nil? } - - before do - # project.create_jira_service doesn't seem to invalidate the cache here - project.has_external_issue_tracker = true - jira_service_settings - stub_jira_urls("JIRA-1") - - allow(closing_commit).to receive_messages({ - issue_closing_regex: Regexp.new(Gitlab.config.gitlab.issue_closing_pattern), - safe_message: message, - author_name: commit_author.name, - author_email: commit_author.email - }) - - allow(JIRA::Resource::Remotelink).to receive(:all).and_return([]) - - allow(project.repository).to receive_messages(commits_between: [closing_commit]) - end - - after do - jira_tracker.destroy! - end - - context "mentioning an issue" do - let(:message) { "this is some work.\n\nrelated to JIRA-1" } - - it "initiates one api call to jira server to mention the issue" do - execute_service(project, user, oldrev, newrev, ref) - - expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( - body: /mentioned this issue in/ - ).once - end - end - - context "closing an issue" do - let(:message) { "this is some work.\n\ncloses JIRA-1" } - let(:comment_body) do - { - body: "Issue solved with [#{closing_commit.id}|http://#{Gitlab.config.gitlab.host}/#{project.full_path}/commit/#{closing_commit.id}]." - }.to_json - end - - before do - open_issue = JIRA::Resource::Issue.new(jira_tracker.client, attrs: { "id" => "JIRA-1" }) - closed_issue = open_issue.dup - allow(open_issue).to receive(:resolution).and_return(false) - allow(closed_issue).to receive(:resolution).and_return(true) - allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue) - - allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-1") - end - - context "using right markdown" do - it "initiates one api call to jira server to close the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - - expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once - end - - it "initiates one api call to jira server to comment on the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - - expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( - body: comment_body - ).once - end - end - - context "using internal issue reference" do - context 'when internal issues are disabled' do - before do - project.issues_enabled = false - project.save! - end - let(:message) { "this is some work.\n\ncloses #1" } - - it "does not initiates one api call to jira server to close the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - - expect(WebMock).not_to have_requested(:post, jira_api_transition_url('JIRA-1')) - end - - it "does not initiates one api call to jira server to comment on the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - - expect(WebMock).not_to have_requested(:post, jira_api_comment_url('JIRA-1')).with( - body: comment_body - ).once - end - end - - context 'when internal issues are enabled' do - let(:issue) { create(:issue, project: project) } - let(:message) { "this is some work.\n\ncloses JIRA-1 \n\n closes #{issue.to_reference}" } - - it "initiates one api call to jira server to close the jira issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - - expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once - end - - it "initiates one api call to jira server to comment on the jira issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - - expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( - body: comment_body - ).once - end - - it "closes the internal issue" do - execute_service(project, commit_author, oldrev, newrev, ref) - expect(issue.reload).to be_closed - end - - it "adds a note indicating that the issue is now closed" do - expect(SystemNoteService).to receive(:change_status) - .with(issue, project, commit_author, "closed", closing_commit) - execute_service(project, commit_author, oldrev, newrev, ref) - end - end - end - end - end - end - - describe "empty project" do - let(:project) { create(:project_empty_repo) } - let(:new_ref) { 'refs/heads/feature' } - - before do - allow(project).to receive(:default_branch).and_return('feature') - expect(project).to receive(:change_head) { 'feature'} - end - - it 'push to first branch updates HEAD' do - execute_service(project, user, blankrev, newrev, new_ref) - end - end - - describe "housekeeping" do - let(:housekeeping) { Projects::HousekeepingService.new(project) } - - before do - # Flush any raw key-value data stored by the housekeeping code. - Gitlab::Redis::Cache.with { |conn| conn.flushall } - Gitlab::Redis::Queues.with { |conn| conn.flushall } - Gitlab::Redis::SharedState.with { |conn| conn.flushall } - - allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping) - end - - after do - Gitlab::Redis::Cache.with { |conn| conn.flushall } - Gitlab::Redis::Queues.with { |conn| conn.flushall } - Gitlab::Redis::SharedState.with { |conn| conn.flushall } - end - - it 'does not perform housekeeping when not needed' do - expect(housekeeping).not_to receive(:execute) - - execute_service(project, user, oldrev, newrev, ref) - end - - context 'when housekeeping is needed' do - before do - allow(housekeeping).to receive(:needed?).and_return(true) - end - - it 'performs housekeeping' do - expect(housekeeping).to receive(:execute) - - execute_service(project, user, oldrev, newrev, ref) - end - - it 'does not raise an exception' do - allow(housekeeping).to receive(:try_obtain_lease).and_return(false) - - execute_service(project, user, oldrev, newrev, ref) - end - end - - it 'increments the push counter' do - expect(housekeeping).to receive(:increment!) - - execute_service(project, user, oldrev, newrev, ref) - end - end - - describe '#update_caches' do - let(:service) do - described_class.new(project, - user, - oldrev: oldrev, - newrev: newrev, - ref: ref) - end - - context 'on the default branch' do - before do - allow(service).to receive(:default_branch?).and_return(true) - end - - it 'flushes the caches of any special files that have been changed' do - commit = double(:commit) - diff = double(:diff, new_path: 'README.md') - - expect(commit).to receive(:raw_deltas) - .and_return([diff]) - - service.push_commits = [commit] - - expect(ProjectCacheWorker).to receive(:perform_async) - .with(project.id, %i(readme), %i(commit_count repository_size)) - - service.update_caches - end - end - - context 'on a non-default branch' do - before do - allow(service).to receive(:default_branch?).and_return(false) - end - - it 'does not flush any conditional caches' do - expect(ProjectCacheWorker).to receive(:perform_async) - .with(project.id, [], %i(commit_count repository_size)) - .and_call_original - - service.update_caches - end - end - end - - describe '#process_commit_messages' do - let(:service) do - described_class.new(project, - user, - oldrev: oldrev, - newrev: newrev, - ref: ref) - end - - it 'only schedules a limited number of commits' do - service.push_commits = Array.new(1000, double(:commit, to_hash: {}, matches_cross_reference_regex?: true)) - - expect(ProcessCommitWorker).to receive(:perform_async).exactly(100).times - - service.process_commit_messages - end - - it "skips commits which don't include cross-references" do - service.push_commits = [double(:commit, to_hash: {}, matches_cross_reference_regex?: false)] - - expect(ProcessCommitWorker).not_to receive(:perform_async) - - service.process_commit_messages - end - end - - describe '#update_signatures' do - let(:service) do - described_class.new( - project, - user, - oldrev: oldrev, - newrev: newrev, - ref: 'refs/heads/master' - ) - end - - context 'when the commit has a signature' do - context 'when the signature is already cached' do - before do - create(:gpg_signature, commit_sha: sample_commit.id) - end - - it 'does not queue a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).not_to receive(:perform_async) - - execute_service(project, user, oldrev, newrev, ref) - end - end - - context 'when the signature is not yet cached' do - it 'queues a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id], project.id) - - execute_service(project, user, oldrev, newrev, ref) - end - - it 'can queue several commits to create the gpg signature' do - allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).and_return([sample_commit.id, another_sample_commit.id]) - - expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id, another_sample_commit.id], project.id) - - execute_service(project, user, oldrev, newrev, ref) - end - end - end - - context 'when the commit does not have a signature' do - before do - allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).with(project.repository, [sample_commit.id]).and_return([]) - end - - it 'does not queue a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).not_to receive(:perform_async).with(sample_commit.id, project.id) - - execute_service(project, user, oldrev, newrev, ref) - end - end - end - - def execute_service(project, user, oldrev, newrev, ref) - service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) - service.execute - service - end - - def push_data_from_service(project, user, oldrev, newrev, ref) - execute_service(project, user, oldrev, newrev, ref).push_data - end -end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index ecefdc23811..33648292037 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -23,7 +23,7 @@ module CycleAnalyticsHelpers return if skip_push_handler - GitPushService.new(project, + Git::BranchPushService.new(project, user, oldrev: oldrev, newrev: commit_shas.last, diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index d2881f99a84..9cddad71a51 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -33,7 +33,7 @@ describe PostReceive do describe "#process_project_changes" do context 'empty changes' do it "does not call any PushService but runs after project hooks" do - expect(GitPushService).not_to receive(:new) + expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) expect_next_instance_of(SystemHooksService) { |service| expect(service).to receive(:execute_hooks) } @@ -45,7 +45,7 @@ describe PostReceive do let!(:key_id) { "" } it 'returns false' do - expect(GitPushService).not_to receive(:new) + expect(Git::BranchPushService).not_to receive(:new) expect(Git::TagPushService).not_to receive(:new) expect(described_class.new.perform(gl_repository, key_id, base64_changes)).to be false @@ -60,8 +60,8 @@ describe PostReceive do context "branches" do let(:changes) { "123456 789012 refs/heads/tést" } - it "calls GitPushService" do - expect_any_instance_of(GitPushService).to receive(:execute).and_return(true) + it "calls Git::BranchPushService" do + expect_any_instance_of(Git::BranchPushService).to receive(:execute).and_return(true) expect_any_instance_of(Git::TagPushService).not_to receive(:execute) described_class.new.perform(gl_repository, key_id, base64_changes) end @@ -71,7 +71,7 @@ describe PostReceive do let(:changes) { "123456 789012 refs/tags/tag" } it "calls Git::TagPushService" do - expect_any_instance_of(GitPushService).not_to receive(:execute) + expect_any_instance_of(Git::BranchPushService).not_to receive(:execute) expect_any_instance_of(Git::TagPushService).to receive(:execute).and_return(true) described_class.new.perform(gl_repository, key_id, base64_changes) end @@ -81,7 +81,7 @@ describe PostReceive do let(:changes) { "123456 789012 refs/merge-requests/123" } it "does not call any of the services" do - expect_any_instance_of(GitPushService).not_to receive(:execute) + expect_any_instance_of(Git::BranchPushService).not_to receive(:execute) expect_any_instance_of(Git::TagPushService).not_to receive(:execute) described_class.new.perform(gl_repository, key_id, base64_changes) end @@ -125,7 +125,7 @@ describe PostReceive do allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data) # silence hooks so we can isolate allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true) - allow_any_instance_of(GitPushService).to receive(:execute).and_return(true) + allow_any_instance_of(Git::BranchPushService).to receive(:execute).and_return(true) end it 'calls SystemHooksService' do -- cgit v1.2.1