Merge branch '15463-rename-git-push-services' into 'master'

Rename GitTagPushService and GitPushService to be more consistent

See merge request gitlab-org/gitlab-ce!26547

app/services/after_branch_delete_service.rb

 # 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
 

app/services/git/branch_push_service.rb

+# 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

app/services/git/tag_push_service.rb

+# frozen_string_literal: true
+
+module Git
+  class TagPushService < BaseService
+    attr_accessor :push_data
+
+    def execute
+      project.repository.after_create if project.empty_repo?
+      project.repository.before_push_tag
+
+      @push_data = build_push_data
+
+      EventCreateService.new.push(project, current_user, push_data)
+      Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push, pipeline_options)
+
+      SystemHooksService.new.execute_hooks(build_system_push_data, :tag_push_hooks)
+      project.execute_hooks(push_data.dup, :tag_push_hooks)
+      project.execute_services(push_data.dup, :tag_push_hooks)
+
+      ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size])
+
+      true
+    end
+
+    private
+
+    def build_push_data
+      commits = []
+      message = nil
+
+      unless Gitlab::Git.blank_ref?(params[:newrev])
+        tag_name = Gitlab::Git.ref_name(params[:ref])
+        tag = project.repository.find_tag(tag_name)
+
+        if tag && tag.target == params[:newrev]
+          commit = project.commit(tag.dereferenced_target)
+          commits = [commit].compact
+          message = tag.message
+        end
+      end
+
+      Gitlab::DataBuilder::Push.build(
+        project,
+        current_user,
+        params[:oldrev],
+        params[:newrev],
+        params[:ref],
+        commits,
+        message,
+        push_options: params[:push_options] || [])
+    end
+
+    def build_system_push_data
+      Gitlab::DataBuilder::Push.build(
+        project,
+        current_user,
+        params[:oldrev],
+        params[:newrev],
+        params[:ref],
+        [],
+        '')
+    end
+
+    def pipeline_options
+      {} # to be overridden in EE
+    end
+  end
+end

app/services/git_push_service.rb

-# 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
-    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
-
-    execute_related_hooks
-    perform_housekeeping
-
-    update_remote_mirrors
-    update_caches
-
-    update_signatures
-  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
-
-  protected
-
-  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

app/services/git_tag_push_service.rb

-# frozen_string_literal: true
-
-class GitTagPushService < BaseService
-  attr_accessor :push_data
-
-  def execute
-    project.repository.after_create if project.empty_repo?
-    project.repository.before_push_tag
-
-    @push_data = build_push_data
-
-    EventCreateService.new.push(project, current_user, push_data)
-    Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push, pipeline_options)
-
-    SystemHooksService.new.execute_hooks(build_system_push_data, :tag_push_hooks)
-    project.execute_hooks(push_data.dup, :tag_push_hooks)
-    project.execute_services(push_data.dup, :tag_push_hooks)
-
-    ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size])
-
-    true
-  end
-
-  private
-
-  def build_push_data
-    commits = []
-    message = nil
-
-    unless Gitlab::Git.blank_ref?(params[:newrev])
-      tag_name = Gitlab::Git.ref_name(params[:ref])
-      tag = project.repository.find_tag(tag_name)
-
-      if tag && tag.target == params[:newrev]
-        commit = project.commit(tag.dereferenced_target)
-        commits = [commit].compact
-        message = tag.message
-      end
-    end
-
-    Gitlab::DataBuilder::Push.build(
-      project,
-      current_user,
-      params[:oldrev],
-      params[:newrev],
-      params[:ref],
-      commits,
-      message,
-      push_options: params[:push_options] || [])
-  end
-
-  def build_system_push_data
-    Gitlab::DataBuilder::Push.build(
-      project,
-      current_user,
-      params[:oldrev],
-      params[:newrev],
-      params[:ref],
-      [],
-      '')
-  end
-
-  def pipeline_options
-    {} # to be overridden in EE
-  end
-end

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?

app/workers/post_receive.rb

@@ -38,7 +38,7 @@ class PostReceive
 
     post_received.changes_refs do |oldrev, newrev, ref|
       if Gitlab::Git.tag_ref?(ref)
-        GitTagPushService.new(
+        Git::TagPushService.new(
           post_received.project,
           @user,
           oldrev: oldrev,
@@ -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,

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
 

spec/services/git_push_service_spec.rb → spec/services/git/branch_push_service_spec.rb

 require 'spec_helper'
 
-describe GitPushService, services: true do
+describe Git::BranchPushService, services: true do
   include RepoHelpers
 
   set(:user)     { create(:user) }

spec/services/git_tag_push_service_spec.rb → spec/services/git/tag_push_service_spec.rb

 require 'spec_helper'
 
-describe GitTagPushService do
+describe Git::TagPushService do
   include RepoHelpers
   include GitHelpers
 

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,

spec/workers/post_receive_spec.rb

@@ -33,8 +33,8 @@ 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(GitTagPushService).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) }
 
         described_class.new.perform(gl_repository, key_id, "")
@@ -45,8 +45,8 @@ describe PostReceive do
       let!(:key_id) { "" }
 
       it 'returns false' do
-        expect(GitPushService).not_to receive(:new)
-        expect(GitTagPushService).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
       end
@@ -60,9 +60,9 @@ 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)
-          expect_any_instance_of(GitTagPushService).not_to receive(:execute)
+        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
       end
@@ -70,9 +70,9 @@ describe PostReceive do
       context "tags" do
         let(:changes) { "123456 789012 refs/tags/tag" }
 
-        it "calls GitTagPushService" do
-          expect_any_instance_of(GitPushService).not_to receive(:execute)
-          expect_any_instance_of(GitTagPushService).to receive(:execute).and_return(true)
+        it "calls Git::TagPushService" do
+          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
       end
@@ -81,8 +81,8 @@ 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(GitTagPushService).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
       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