Merge pull request #1818 from riyad/refactor-satellite-actions

Refactor Satellite Code

app/controllers/tree_controller.rb

@@ -26,15 +26,14 @@ class TreeController < ProjectResourceController
   end
 
   def update
-    file_editor = Gitlab::FileEditor.new(current_user, @project, @ref)
-    update_status = file_editor.update(
-      @path,
+    edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, @project, @ref, @path)
+    updated_successfully = edit_file_action.commit!(
       params[:content],
       params[:commit_message],
       params[:last_commit]
     )
 
-    if update_status
+    if updated_successfully
       redirect_to project_tree_path(@project, @id), notice: "Your changes have been successfully commited"
     else
       flash[:notice] = "Your changes could not be commited, because the file has been changed"

app/models/merge_request.rb

@@ -60,7 +60,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def check_if_can_be_merged
-    self.state = if Gitlab::Merge.new(self, self.author).can_be_merged?
+    self.state = if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
                    CAN_BE_MERGED
                  else
                    CANNOT_BE_MERGED
@@ -167,7 +167,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def automerge!(current_user)
-    if Gitlab::Merge.new(self, current_user).merge! && self.unmerged_commits.empty?
+    if Gitlab::Satellite::MergeAction.new(current_user, self).merge! && self.unmerged_commits.empty?
       self.merge!(current_user.id)
       true
     end

app/roles/repository.rb

@@ -41,7 +41,7 @@ module Repository
   end
 
   def satellite
-    @satellite ||= Gitlab::Satellite.new(self)
+    @satellite ||= Gitlab::Satellite::Satellite.new(self)
   end
 
   def has_post_receive_file?

lib/gitlab/file_editor.rb

-module Gitlab
-  # GitLab file editor
-  #
-  # It gives you ability to make changes to files
-  # & commit this changes from GitLab UI.
-  class FileEditor
-    attr_accessor :user, :project, :ref
-
-    def initialize(user, project, ref)
-      self.user = user
-      self.project = project
-      self.ref = ref
-    end
-
-    def update(path, content, commit_message, last_commit)
-      return false unless can_edit?(path, last_commit)
-
-      Grit::Git.with_timeout(10.seconds) do
-        lock_file = Rails.root.join("tmp", "#{project.path}.lock")
-
-        File.open(lock_file, "w+") do |f|
-          f.flock(File::LOCK_EX)
-
-          unless project.satellite.exists?
-            raise "Satellite doesn't exist"
-          end
-
-          project.satellite.clear
-
-          Dir.chdir(project.satellite.path) do
-            r = Grit::Repo.new('.')
-            r.git.sh "git reset --hard"
-            r.git.sh "git fetch origin"
-            r.git.sh "git config user.name \"#{user.name}\""
-            r.git.sh "git config user.email \"#{user.email}\""
-            r.git.sh "git checkout -b #{ref} origin/#{ref}"
-            File.open(path, 'w'){|f| f.write(content)}
-            r.git.sh "git add ."
-            r.git.sh "git commit -am '#{commit_message}'"
-            output = r.git.sh "git push origin #{ref}"
-
-            if output =~ /reject/
-              return false
-            end
-          end
-        end
-      end
-      true
-    end
-
-    protected
-
-    def can_edit?(path, last_commit)
-      current_last_commit = @project.last_commit_for(ref, path).sha
-      last_commit == current_last_commit
-    end
-  end
-end

lib/gitlab/merge.rb

-module Gitlab
-  class Merge
-    attr_accessor :merge_request, :project, :user
-
-    def initialize(merge_request, user)
-      @merge_request = merge_request
-      @project = merge_request.project
-      @user = user
-    end
-
-    def can_be_merged?
-      in_locked_and_timed_satellite do |merge_repo|
-        merge_in_satellite!(merge_repo)
-      end
-    end
-
-    # Merges the source branch into the target branch in the satellite and
-    # pushes it back to Gitolite.
-    # It also removes the source branch if requested in the merge request.
-    #
-    # Returns false if the merge produced conflicts
-    # Returns false if pushing from the satellite to Gitolite failed or was rejected
-    # Returns true otherwise
-    def merge!
-      in_locked_and_timed_satellite do |merge_repo|
-        if merge_in_satellite!(merge_repo)
-          # push merge back to Gitolite
-          # will raise CommandFailed when push fails
-          merge_repo.git.push({raise: true}, :origin, merge_request.target_branch)
-
-          # remove source branch
-          if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch)
-            # will raise CommandFailed when push fails
-            merge_repo.git.push({raise: true}, :origin, ":#{merge_request.source_branch}")
-          end
-
-          # merge, push and branch removal successful
-          true
-        end
-      end
-    rescue Grit::Git::CommandFailed
-      false
-    end
-
-    private
-
-    # * Sets a 30s timeout for Git
-    # * Locks the satellite repo
-    # * Yields the prepared satellite repo
-    def in_locked_and_timed_satellite
-      Grit::Git.with_timeout(30.seconds) do
-        lock_file = Rails.root.join("tmp", "#{project.path}.lock")
-
-        File.open(lock_file, "w+") do |f|
-          f.flock(File::LOCK_EX)
-
-          unless project.satellite.exists?
-            raise "Satellite doesn't exist"
-          end
-
-          Dir.chdir(project.satellite.path) do
-            repo = Grit::Repo.new('.')
-
-            return yield repo
-          end
-        end
-      end
-    rescue Errno::ENOMEM => ex
-      Gitlab::GitLogger.error(ex.message)
-    rescue Grit::Git::GitTimeout
-      return false
-    end
-
-    # Merges the source_branch into the target_branch in the satellite.
-    #
-    # Note: it will clear out the satellite before doing anything
-    #
-    # Returns false if the merge produced conflicts
-    # Returns true otherwise
-    def merge_in_satellite!(repo)
-      prepare_satellite!(repo)
-
-      # create target branch in satellite at the corresponding commit from Gitolite
-      repo.git.checkout({b: true}, merge_request.target_branch, "origin/#{merge_request.target_branch}")
-
-      # merge the source branch from Gitolite into the satellite
-      # will raise CommandFailed when merge fails
-      repo.git.pull({no_ff: true, raise: true}, :origin, merge_request.source_branch)
-    rescue Grit::Git::CommandFailed
-      false
-    end
-
-    # * Clears the satellite
-    # * Updates the satellite from Gitolite
-    # * Sets up Git variables for the user
-    def prepare_satellite!(repo)
-      project.satellite.clear
-
-      repo.git.reset(hard: true)
-      repo.git.fetch({}, :origin)
-
-      repo.git.config({}, "user.name", user.name)
-      repo.git.config({}, "user.email", user.email)
-    end
-  end
-end

lib/gitlab/satellite.rb

-module Gitlab
-  class Satellite
-
-    PARKING_BRANCH = "__parking_branch"
-
-    attr_accessor :project
-
-    def initialize project
-      self.project = project
-    end
-
-    def create
-      `git clone #{project.url_to_repo} #{path}`
-    end
-
-    def path
-      Rails.root.join("tmp", "repo_satellites", project.path)
-    end
-
-    def exists?
-      File.exists? path
-    end
-
-    #will be deleted all branches except PARKING_BRANCH
-    def clear
-      Dir.chdir(path) do
-        heads = Grit::Repo.new(".").heads.map{|head| head.name}
-        if heads.include? PARKING_BRANCH
-          `git checkout #{PARKING_BRANCH}`
-        else
-          `git checkout -b #{PARKING_BRANCH}`
-        end
-        heads.delete(PARKING_BRANCH)
-        heads.each do |head|
-          `git branch -D #{head}`
-        end
-      end
-    end
-
-  end
-end

lib/gitlab/satellite/action.rb

+module Gitlab
+  module Satellite
+    class Action
+      DEFAULT_OPTIONS = { git_timeout: 30.seconds }
+
+      attr_accessor :options, :project, :user
+
+      def initialize(user, project, options = {})
+        @options = DEFAULT_OPTIONS.merge(options)
+        @project = project
+        @user = user
+      end
+
+      protected
+
+      # * Sets a 30s timeout for Git
+      # * Locks the satellite repo
+      # * Yields the prepared satellite repo
+      def in_locked_and_timed_satellite
+        Grit::Git.with_timeout(options[:git_timeout]) do
+          project.satellite.lock do
+            return yield project.satellite.repo
+          end
+        end
+      rescue Errno::ENOMEM => ex
+        Gitlab::GitLogger.error(ex.message)
+        return false
+      rescue Grit::Git::GitTimeout => ex
+        Gitlab::GitLogger.error(ex.message)
+        return false
+      end
+
+      # * Clears the satellite
+      # * Updates the satellite from Gitolite
+      # * Sets up Git variables for the user
+      #
+      # Note: use this within #in_locked_and_timed_satellite
+      def prepare_satellite!(repo)
+        project.satellite.clear_and_update!
+
+        repo.git.config({}, "user.name", user.name)
+        repo.git.config({}, "user.email", user.email)
+      end
+    end
+  end
+end

lib/gitlab/satellite/edit_file_action.rb

+module Gitlab
+  module Satellite
+    # GitLab server-side file update and commit
+    class EditFileAction < Action
+      attr_accessor :file_path, :ref
+
+      def initialize(user, project, ref, file_path)
+        super user, project, git_timeout: 10.seconds
+        @file_path = file_path
+        @ref = ref
+      end
+
+      # Updates the files content and creates a new commit for it
+      #
+      # Returns false if the ref has been updated while editing the file
+      # Returns false if commiting the change fails
+      # Returns false if pushing from the satellite to Gitolite failed or was rejected
+      # Returns true otherwise
+      def commit!(content, commit_message, last_commit)
+        return false unless can_edit?(last_commit)
+
+        in_locked_and_timed_satellite do |repo|
+          prepare_satellite!(repo)
+
+          # create target branch in satellite at the corresponding commit from Gitolite
+          repo.git.checkout({raise: true, timeout: true, b: true}, ref, "origin/#{ref}")
+
+          # update the file in the satellite's working dir
+          file_path_in_satellite = File.join(repo.working_dir, file_path)
+          File.open(file_path_in_satellite, 'w') { |f| f.write(content) }
+
+          # commit the changes
+          # will raise CommandFailed when commit fails
+          repo.git.commit(raise: true, timeout: true, a: true, m: commit_message)
+
+
+          # push commit back to Gitolite
+          # will raise CommandFailed when push fails
+          repo.git.push({raise: true, timeout: true}, :origin, ref)
+
+          # everything worked
+          true
+        end
+      rescue Grit::Git::CommandFailed => ex
+        Gitlab::GitLogger.error(ex.message)
+        false
+      end
+
+      protected
+
+      def can_edit?(last_commit)
+        current_last_commit = @project.last_commit_for(ref, file_path).sha
+        last_commit == current_last_commit
+      end
+    end
+  end
+end

lib/gitlab/satellite/merge_action.rb

+module Gitlab
+  module Satellite
+    # GitLab server-side merge
+    class MergeAction < Action
+      attr_accessor :merge_request
+
+      def initialize(user, merge_request)
+        super user, merge_request.project
+        @merge_request = merge_request
+      end
+
+      # Checks if a merge request can be executed without user interaction
+      def can_be_merged?
+        in_locked_and_timed_satellite do |merge_repo|
+          merge_in_satellite!(merge_repo)
+        end
+      end
+
+      # Merges the source branch into the target branch in the satellite and
+      # pushes it back to Gitolite.
+      # It also removes the source branch if requested in the merge request.
+      #
+      # Returns false if the merge produced conflicts
+      # Returns false if pushing from the satellite to Gitolite failed or was rejected
+      # Returns true otherwise
+      def merge!
+        in_locked_and_timed_satellite do |merge_repo|
+          if merge_in_satellite!(merge_repo)
+            # push merge back to Gitolite
+            # will raise CommandFailed when push fails
+            merge_repo.git.push({raise: true, timeout: true}, :origin, merge_request.target_branch)
+
+            # remove source branch
+            if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch)
+              # will raise CommandFailed when push fails
+              merge_repo.git.push({raise: true, timeout: true}, :origin, ":#{merge_request.source_branch}")
+            end
+
+            # merge, push and branch removal successful
+            true
+          end
+        end
+      rescue Grit::Git::CommandFailed => ex
+        Gitlab::GitLogger.error(ex.message)
+        false
+      end
+
+      private
+
+      # Merges the source_branch into the target_branch in the satellite.
+      #
+      # Note: it will clear out the satellite before doing anything
+      #
+      # Returns false if the merge produced conflicts
+      # Returns true otherwise
+      def merge_in_satellite!(repo)
+        prepare_satellite!(repo)
+
+        # create target branch in satellite at the corresponding commit from Gitolite
+        repo.git.checkout({raise: true, timeout: true, b: true}, merge_request.target_branch, "origin/#{merge_request.target_branch}")
+
+        # merge the source branch from Gitolite into the satellite
+        # will raise CommandFailed when merge fails
+        repo.git.pull({raise: true, timeout: true, no_ff: true}, :origin, merge_request.source_branch)
+      rescue Grit::Git::CommandFailed => ex
+        Gitlab::GitLogger.error(ex.message)
+        false
+      end
+    end
+  end
+end

lib/gitlab/satellite/satellite.rb

+module Gitlab
+  module Satellite
+    class Satellite
+      PARKING_BRANCH = "__parking_branch"
+
+      attr_accessor :project
+
+      def initialize(project)
+        @project = project
+      end
+
+      def clear_and_update!
+        raise "Satellite doesn't exist" unless exists?
+
+        delete_heads!
+        clear_working_dir!
+        update_from_source!
+      end
+
+      def create
+        `git clone #{project.url_to_repo} #{path}`
+      end
+
+      def exists?
+        File.exists? path
+      end
+
+      # * Locks the satellite
+      # * Changes the current directory to the satellite's working dir
+      # * Yields
+      def lock
+        raise "Satellite doesn't exist" unless exists?
+
+        File.open(lock_file, "w+") do |f|
+          f.flock(File::LOCK_EX)
+
+          Dir.chdir(path) do
+            return yield
+          end
+        end
+      end
+
+      def lock_file
+        Rails.root.join("tmp", "#{project.path}.lock")
+      end
+
+      def path
+        Rails.root.join("tmp", "repo_satellites", project.path)
+      end
+
+      def repo
+        raise "Satellite doesn't exist" unless exists?
+
+        @repo ||= Grit::Repo.new(path)
+      end
+
+      private
+
+      # Clear the working directory
+      def clear_working_dir!
+        repo.git.reset(hard: true)
+      end
+
+      # Deletes all branches except the parking branch
+      #
+      # This ensures we have no name clashes or issues updating branches when
+      # working with the satellite.
+      def delete_heads!
+        heads = repo.heads.map(&:name)
+
+        # update or create the parking branch
+        if heads.include? PARKING_BRANCH
+          repo.git.checkout({}, PARKING_BRANCH)
+        else
+          repo.git.checkout({b: true}, PARKING_BRANCH)
+        end
+
+        # remove the parking branch from the list of heads ...
+        heads.delete(PARKING_BRANCH)
+        # ... and delete all others
+        heads.each { |head| repo.git.branch({D: true}, head) }
+      end
+
+      # Updates the satellite from Gitolite
+      #
+      # Note: this will only update remote branches (i.e. origin/*)
+      def update_from_source!
+        repo.git.fetch({timeout: true}, :origin)
+      end
+    end
+  end
+end