attempt to reduce code complexity on GitPushService#execute

1 files changed, 66 insertions, 53 deletions

diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index e3bf14966c8..491322f5ff5 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -3,6 +3,15 @@ class GitPushService
   include Gitlab::CurrentSettings
   include Gitlab::Access
 
+  def initialize(project, user, oldrev, newrev, ref)
+    # TODO: Consider passing an options hash instead https://github.com/bbatsov/ruby-style-guide#too-many-params
+    @project = project
+    @user = user
+    @oldrev = oldrev
+    @newrev = newrev
+    @ref = ref
+  end
+
   # This method will be called after each git update
   # and only if the provided user and project is present in GitLab.
   #
@@ -15,67 +24,67 @@ class GitPushService
   #  4. Executes the project's web hooks
   #  5. Executes the project's services
   #
-  def execute(project, user, oldrev, newrev, ref)
-    @project, @user = project, user
-
-    branch_name = Gitlab::Git.ref_name(ref)
-
-    project.repository.expire_cache(branch_name)
-
-    if push_remove_branch?(ref, newrev)
-      project.repository.expire_has_visible_content_cache
+  def execute
+    @project.repository.expire_cache(branch_name)
 
+    if push_remove_branch?
+      @project.repository.expire_has_visible_content_cache
       @push_commits = []
-    elsif push_to_new_branch?(ref, oldrev)
-      project.repository.expire_has_visible_content_cache
+    elsif push_to_new_branch?
+      @project.repository.expire_has_visible_content_cache
 
       # Re-find the pushed commits.
-      if is_default_branch?(ref)
+      if is_default_branch?
         # Initial push to the default branch. Take the full history of that branch as "newly pushed".
-        @push_commits = project.repository.commits(newrev)
-
-        # Ensure HEAD points to the default branch in case it is not master
-        project.change_head(branch_name)
-
-        # Set protection on the default branch if configured
-        if (current_application_settings.default_branch_protection != PROTECTION_NONE)
-          developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
-          project.protected_branches.create({ name: project.default_branch, developers_can_push: developers_can_push })
-        end
+        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, newrev)
+        # that shouldn't matter because we check for existing cross-@references later.
+        @push_commits = @project.repository.commits_between(@project.default_branch, @newrev)
 
         # don't process commits for the initial push to the default branch
-        process_commit_messages(ref)
+        process_commit_messages
       end
-    elsif push_to_existing_branch?(ref, oldrev)
+    elsif push_to_existing_branch?
       # Collect data for this git push
-      @push_commits = project.repository.commits_between(oldrev, newrev)
-      process_commit_messages(ref)
+      @push_commits = @project.repository.commits_between(@oldrev, @newrev)
+      process_commit_messages
     end
-
     # Update merge requests that may be affected by this push. A new branch
     # could cause the last commit of a merge request to change.
-    project.update_merge_requests(oldrev, newrev, ref, @user)
+    update_merge_requests
+  end
 
-    @push_data = build_push_data(oldrev, newrev, ref)
+  protected
+
+  def update_merge_requests
+    @project.update_merge_requests(@oldrev, @newrev, @ref, @user)
 
-    EventCreateService.new.push(project, user, @push_data)
-    project.execute_hooks(@push_data.dup, :push_hooks)
-    project.execute_services(@push_data.dup, :push_hooks)
-    CreateCommitBuildsService.new.execute(project, @user, @push_data)
-    ProjectCacheWorker.perform_async(project.id)
+    EventCreateService.new.push(@project, @user, build_push_data)
+    @project.execute_hooks(build_push_data.dup, :push_hooks)
+    @project.execute_services(build_push_data.dup, :push_hooks)
+    CreateCommitBuildsService.new.execute(@project, @user, build_push_data)
+    ProjectCacheWorker.perform_async(@project.id)
   end
 
-  protected
+  def process_default_branch
+    @push_commits = project.repository.commits(@newrev)
+
+    # Ensure HEAD points to the default branch in case it is not master
+    project.change_head(branch_name)
+
+    # Set protection on the default branch if configured
+    if (current_application_settings.default_branch_protection != PROTECTION_NONE)
+      developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false
+      @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push })
+    end
+  end
 
   # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched,
   # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables.
-  def process_commit_messages(ref)
-    is_default_branch = is_default_branch?(ref)
+  def process_commit_messages
+    is_default_branch = is_default_branch?
 
     authors = Hash.new do |hash, commit|
       email = commit.author_email
@@ -104,34 +113,38 @@ class GitPushService
     end
   end
 
-  def build_push_data(oldrev, newrev, ref)
-    Gitlab::PushDataBuilder.
-      build(project, user, oldrev, newrev, ref, push_commits)
+  def build_push_data
+    @push_data ||= Gitlab::PushDataBuilder.
+        build(@project, @user, @oldrev, @newrev, @ref, push_commits)
   end
 
-  def push_to_existing_branch?(ref, oldrev)
+  def push_to_existing_branch?
     # Return if this is not a push to a branch (e.g. new commits)
-    Gitlab::Git.branch_ref?(ref) && !Gitlab::Git.blank_ref?(oldrev)
+    Gitlab::Git.branch_ref?(@ref) && !Gitlab::Git.blank_ref?(@oldrev)
   end
 
-  def push_to_new_branch?(ref, oldrev)
-    Gitlab::Git.branch_ref?(ref) && Gitlab::Git.blank_ref?(oldrev)
+  def push_to_new_branch?
+    Gitlab::Git.branch_ref?(@ref) && Gitlab::Git.blank_ref?(@oldrev)
   end
 
-  def push_remove_branch?(ref, newrev)
-    Gitlab::Git.branch_ref?(ref) && Gitlab::Git.blank_ref?(newrev)
+  def push_remove_branch?
+    Gitlab::Git.branch_ref?(@ref) && Gitlab::Git.blank_ref?(@newrev)
   end
 
-  def push_to_branch?(ref)
-    Gitlab::Git.branch_ref?(ref)
+  def push_to_branch?
+    Gitlab::Git.branch_ref?(@ref)
   end
 
-  def is_default_branch?(ref)
-    Gitlab::Git.branch_ref?(ref) &&
-      (Gitlab::Git.ref_name(ref) == project.default_branch || project.default_branch.nil?)
+  def is_default_branch?
+    Gitlab::Git.branch_ref?(@ref) &&
+        (Gitlab::Git.ref_name(@ref) == project.default_branch || project.default_branch.nil?)
   end
 
   def commit_user(commit)
     commit.author || user
   end
+
+  def branch_name
+    @_branch_name ||= Gitlab::Git.ref_name(@ref)
+  end
 end