From 83fc1464ddbbce004ef310653a7caec441cf0200 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 22 Mar 2019 17:01:48 +0000 Subject: [PATCH] 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 +- .../branch_push_service_spec.rb} | 2 +- .../helpers/cycle_analytics_helpers.rb | 2 +- spec/workers/post_receive_spec.rb | 14 +- 9 files changed, 258 insertions(+), 260 deletions(-) create mode 100644 app/services/git/branch_push_service.rb delete mode 100644 app/services/git_push_service.rb rename spec/services/{git_push_service_spec.rb => git/branch_push_service_spec.rb} (99%) 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_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb similarity index 99% rename from spec/services/git_push_service_spec.rb rename to spec/services/git/branch_push_service_spec.rb index e8fce951155..d0e2169b4a6 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe GitPushService, services: true do +describe Git::BranchPushService, services: true do include RepoHelpers set(:user) { create(:user) } 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