Refactor changing files in web UI

11 files changed, 125 insertions, 302 deletions

diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index 1297a792259..a48d6a976f0 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -1,69 +1,27 @@
 module Commits
-  class ChangeService < ::BaseService
-    ValidationError = Class.new(StandardError)
-    ChangeError = Class.new(StandardError)
+  class ChangeService < Commits::CreateService
+    def initialize(*args)
+      super
 
-    def execute
-      @start_project = params[:start_project] || @project
-      @start_branch = params[:start_branch]
-      @target_branch = params[:target_branch]
       @commit = params[:commit]
-
-      check_push_permissions
-
-      commit
-    rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
-           ValidationError, ChangeError => ex
-      error(ex.message)
     end
 
     private
 
-    def commit
-      raise NotImplementedError
-    end
-
     def commit_change(action)
       raise NotImplementedError unless repository.respond_to?(action)
 
-      validate_target_branch if different_branch?
-
       repository.public_send(
         action,
         current_user,
         @commit,
-        @target_branch,
+        @branch_name,
         start_project: @start_project,
         start_branch_name: @start_branch)
-
-      success
     rescue Repository::CreateTreeError
       error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
-                     A #{action.to_s.dasherize} may have already been performed with this #{@commit.change_type_title(current_user)}, or a more recent commit may have updated some of its content."
+                   This #{@commit.change_type_title(current_user)} may already have been #{action.to_s.dasherize}ed, or a more recent commit may have updated some of its content."
       raise ChangeError, error_msg
     end
-
-    def check_push_permissions
-      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
-
-      unless allowed
-        raise ValidationError.new('You are not allowed to push into this branch')
-      end
-
-      true
-    end
-
-    def validate_target_branch
-      result = ValidateNewBranchService.new(@project, current_user)
-        .execute(@target_branch)
-
-      if result[:status] == :error
-        raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
-      end
-    end
-
-    def different_branch?
-      @start_branch != @target_branch || @start_project != @project
-    end
   end
 end
diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb
index 605cca36f9c..320e229560d 100644
--- a/app/services/commits/cherry_pick_service.rb
+++ b/app/services/commits/cherry_pick_service.rb
@@ -1,6 +1,6 @@
 module Commits
   class CherryPickService < ChangeService
-    def commit
+    def create_commit!
       commit_change(:cherry_pick)
     end
   end
diff --git a/app/services/commits/create_service.rb b/app/services/commits/create_service.rb
new file mode 100644
index 00000000000..c58f04a252b
--- /dev/null
+++ b/app/services/commits/create_service.rb
@@ -0,0 +1,74 @@
+module Commits
+  class CreateService < ::BaseService
+    ValidationError = Class.new(StandardError)
+    ChangeError = Class.new(StandardError)
+
+    def initialize(*args)
+      super
+
+      @start_project = params[:start_project] || @project
+      @start_branch = params[:start_branch]
+      @branch_name = params[:branch_name]
+    end
+
+    def execute
+      validate!
+
+      new_commit = create_commit!
+
+      success(result: new_commit)
+    rescue ValidationError, ChangeError, Gitlab::Git::Index::IndexError, Repository::CommitError, GitHooksService::PreReceiveError => ex
+      error(ex.message)
+    end
+
+    private
+
+    def create_commit!
+      raise NotImplementedError
+    end
+
+    def raise_error(message)
+      raise ValidationError, message
+    end
+
+    def different_branch?
+      @start_branch != @branch_name || @start_project != @project
+    end
+
+    def validate!
+      validate_permissions!
+      validate_on_branch!
+      validate_branch_existance!
+
+      validate_new_branch_name! if different_branch?
+    end
+
+    def validate_permissions!
+      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@branch_name)
+
+      unless allowed
+        raise_error("You are not allowed to push into this branch")
+      end
+    end
+
+    def validate_on_branch!
+      if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch)
+        raise_error('You can only create or edit files when you are on a branch')
+      end
+    end
+
+    def validate_branch_existance!
+      if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name)
+        raise_error("A branch called '#{@branch_name}' already exists. Switch to that branch in order to make changes")
+      end
+    end
+
+    def validate_new_branch_name!
+      result = ValidateNewBranchService.new(project, current_user).execute(@branch_name)
+
+      if result[:status] == :error
+        raise_error("Something went wrong when we tried to create '#{@branch_name}' for you: #{result[:message]}")
+      end
+    end
+  end
+end
diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb
index addd55cb32f..dc27399e047 100644
--- a/app/services/commits/revert_service.rb
+++ b/app/services/commits/revert_service.rb
@@ -1,6 +1,6 @@
 module Commits
   class RevertService < ChangeService
-    def commit
+    def create_commit!
       commit_change(:revert)
     end
   end
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index c8a60422bf4..38231f66009 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -1,79 +1,17 @@
 module Files
-  class BaseService < ::BaseService
-    ValidationError = Class.new(StandardError)
-
-    def execute
-      @start_project = params[:start_project] || @project
-      @start_branch = params[:start_branch]
-      @target_branch = params[:target_branch]
+  class BaseService < Commits::CreateService
+    def initialize(*args)
+      super
 
+      @author_email = params[:author_email]
+      @author_name = params[:author_name]
       @commit_message = params[:commit_message]
-      @file_path      = params[:file_path]
-      @previous_path  = params[:previous_path]
-      @file_content   = if params[:file_content_encoding] == 'base64'
-                          Base64.decode64(params[:file_content])
-                        else
-                          params[:file_content]
-                        end
-      @last_commit_sha = params[:last_commit_sha]
-      @author_email    = params[:author_email]
-      @author_name     = params[:author_name]
-
-      # Validate parameters
-      validate
-
-      # Create new branch if it different from start_branch
-      validate_target_branch if different_branch?
-
-      result = commit
-      if result
-        success(result: result)
-      else
-        error('Something went wrong. Your changes were not committed')
-      end
-    rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, ValidationError => ex
-      error(ex.message)
-    end
-
-    private
-
-    def different_branch?
-      @start_branch != @target_branch || @start_project != @project
-    end
-
-    def file_has_changed?
-      return false unless @last_commit_sha && last_commit
-
-      @last_commit_sha != last_commit.sha
-    end
-
-    def raise_error(message)
-      raise ValidationError.new(message)
-    end
-
-    def validate
-      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
-
-      unless allowed
-        raise_error("You are not allowed to push into this branch")
-      end
-
-      if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch)
-        raise ValidationError, 'You can only create or edit files when you are on a branch'
-      end
-
-      if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name)
-        raise ValidationError, "A branch called #{@branch_name} already exists. Switch to that branch in order to make changes"
-      end
-    end
 
-    def validate_target_branch
-      result = ValidateNewBranchService.new(project, current_user).
-        execute(@target_branch)
+      @file_path = params[:file_path]
+      @previous_path = params[:previous_path]
 
-      if result[:status] == :error
-        raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}")
-      end
+      @file_content = params[:file_content]
+      @file_content = Base64.decode64(@file_content) if params[:file_content_encoding] == 'base64'
     end
   end
 end
diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb
index 083ffdc634c..8ecac6115bd 100644
--- a/app/services/files/create_dir_service.rb
+++ b/app/services/files/create_dir_service.rb
@@ -1,26 +1,15 @@
 module Files
   class CreateDirService < Files::BaseService
-    def commit
+    def create_commit!
       repository.create_dir(
         current_user,
         @file_path,
         message: @commit_message,
-        branch_name: @target_branch,
+        branch_name: @branch_name,
         author_email: @author_email,
         author_name: @author_name,
         start_project: @start_project,
         start_branch_name: @start_branch)
     end
-
-    def validate
-      super
-
-      unless @file_path =~ Gitlab::Regex.file_path_regex
-        raise_error(
-          'Your changes could not be committed, because the file path ' +
-          Gitlab::Regex.file_path_regex_message
-        )
-      end
-    end
   end
 end
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index 65b5537fb68..00a8dcf0934 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -1,48 +1,16 @@
 module Files
   class CreateService < Files::BaseService
-    def commit
+    def create_commit!
       repository.create_file(
         current_user,
         @file_path,
         @file_content,
         message: @commit_message,
-        branch_name: @target_branch,
+        branch_name: @branch_name,
         author_email: @author_email,
         author_name: @author_name,
         start_project: @start_project,
         start_branch_name: @start_branch)
     end
-
-    def validate
-      super
-
-      if @file_content.nil?
-        raise_error("You must provide content.")
-      end
-
-      if @file_path =~ Gitlab::Regex.directory_traversal_regex
-        raise_error(
-          'Your changes could not be committed, because the file name ' +
-          Gitlab::Regex.directory_traversal_regex_message
-        )
-      end
-
-      unless @file_path =~ Gitlab::Regex.file_path_regex
-        raise_error(
-          'Your changes could not be committed, because the file name ' +
-          Gitlab::Regex.file_path_regex_message
-        )
-      end
-
-      unless project.empty_repo?
-        @file_path.slice!(0) if @file_path.start_with?('/')
-
-        blob = repository.blob_at_branch(@start_branch, @file_path)
-
-        if blob
-          raise_error('Your changes could not be committed because a file with the same name already exists')
-        end
-      end
-    end
   end
 end
diff --git a/app/services/files/destroy_service.rb b/app/services/files/delete_service.rb
index e294659bc98..7952e5c95d4 100644
--- a/app/services/files/destroy_service.rb
+++ b/app/services/files/delete_service.rb
@@ -1,11 +1,11 @@
 module Files
-  class DestroyService < Files::BaseService
-    def commit
+  class DeleteService < Files::BaseService
+    def create_commit!
       repository.delete_file(
         current_user,
         @file_path,
         message: @commit_message,
-        branch_name: @target_branch,
+        branch_name: @branch_name,
         author_email: @author_email,
         author_name: @author_name,
         start_project: @start_project,
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index 700f9f4f6f0..bfacc462847 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -1,14 +1,10 @@
 module Files
   class MultiService < Files::BaseService
-    FileChangedError = Class.new(StandardError)
-
-    ACTIONS = %w[create update delete move].freeze
-
-    def commit
+    def create_commit!
       repository.multi_action(
         user: current_user,
         message: @commit_message,
-        branch_name: @target_branch,
+        branch_name: @branch_name,
         actions: params[:actions],
         author_email: @author_email,
         author_name: @author_name,
@@ -19,122 +15,17 @@ module Files
 
     private
 
-    def validate
+    def validate!
       super
 
-      params[:actions].each_with_index do |action, index|
-        if ACTIONS.include?(action[:action].to_s)
-          action[:action] = action[:action].to_sym
-        else
-          raise_error("Unknown action type `#{action[:action]}`.")
-        end
-
-        unless action[:file_path].present?
-          raise_error("You must specify a file_path.")
-        end
-
-        action[:file_path].slice!(0) if action[:file_path] && action[:file_path].start_with?('/')
-        action[:previous_path].slice!(0) if action[:previous_path] && action[:previous_path].start_with?('/')
-
-        regex_check(action[:file_path])
-        regex_check(action[:previous_path]) if action[:previous_path]
-
-        if project.empty_repo? && action[:action] != :create
-          raise_error("No files to #{action[:action]}.")
-        end
-
-        validate_file_exists(action)
-
-        case action[:action]
-        when :create
-          validate_create(action)
-        when :update
-          validate_update(action)
-        when :delete
-          validate_delete(action)
-        when :move
-          validate_move(action, index)
-        end
-      end
-    end
-
-    def validate_file_exists(action)
-      return if action[:action] == :create
-
-      file_path = action[:file_path]
-      file_path = action[:previous_path] if action[:action] == :move
-
-      blob = repository.blob_at_branch(params[:branch], file_path)
-
-      unless blob
-        raise_error("File to be #{action[:action]}d `#{file_path}` does not exist.")
+      params[:actions].each do |action|
+        validate_action!(action)
       end
     end
 
-    def last_commit
-      Gitlab::Git::Commit.last_for_path(repository, @start_branch, @file_path)
-    end
-
-    def regex_check(file)
-      if file =~ Gitlab::Regex.directory_traversal_regex
-        raise_error(
-          'Your changes could not be committed, because the file name, `' +
-          file +
-          '` ' +
-          Gitlab::Regex.directory_traversal_regex_message
-        )
-      end
-
-      unless file =~ Gitlab::Regex.file_path_regex
-        raise_error(
-          'Your changes could not be committed, because the file name, `' +
-          file +
-          '` ' +
-          Gitlab::Regex.file_path_regex_message
-        )
-      end
-    end
-
-    def validate_create(action)
-      return if project.empty_repo?
-
-      if repository.blob_at_branch(params[:branch], action[:file_path])
-        raise_error("Your changes could not be committed because a file with the name `#{action[:file_path]}` already exists.")
-      end
-
-      if action[:content].nil?
-        raise_error("You must provide content.")
-      end
-    end
-
-    def validate_update(action)
-      if action[:content].nil?
-        raise_error("You must provide content.")
-      end
-
-      if file_has_changed?
-        raise FileChangedError.new("You are attempting to update a file `#{action[:file_path]}` that has changed since you started editing it.")
-      end
-    end
-
-    def validate_delete(action)
-    end
-
-    def validate_move(action, index)
-      if action[:previous_path].nil?
-        raise_error("You must supply the original file path when moving file `#{action[:file_path]}`.")
-      end
-
-      blob = repository.blob_at_branch(params[:branch], action[:file_path])
-
-      if blob
-        raise_error("Move destination `#{action[:file_path]}` already exists.")
-      end
-
-      if action[:content].nil?
-        blob = repository.blob_at_branch(params[:branch], action[:previous_path])
-        blob.load_all_data!(repository) if blob.truncated?
-        params[:actions][index][:content] = blob.data
+    def validate_action!(action)
+      unless Gitlab::Git::Index::ACTIONS.include?(action[:action].to_s)
+        raise_error("Unknown action '#{action[:action]}'")
       end
     end
   end
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index fbbab97632e..f23a9f6d57c 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -2,10 +2,16 @@ module Files
   class UpdateService < Files::BaseService
     FileChangedError = Class.new(StandardError)
 
-    def commit
+    def initialize(*args)
+      super
+
+      @last_commit_sha = params[:last_commit_sha]
+    end
+
+    def create_commit!
       repository.update_file(current_user, @file_path, @file_content,
                              message: @commit_message,
-                             branch_name: @target_branch,
+                             branch_name: @branch_name,
                              previous_path: @previous_path,
                              author_email: @author_email,
                              author_name: @author_name,
@@ -15,21 +21,23 @@ module Files
 
     private
 
-    def validate
-      super
-
-      if @file_content.nil?
-        raise_error("You must provide content.")
-      end
+    def file_has_changed?
+      return false unless @last_commit_sha && last_commit
 
-      if file_has_changed?
-        raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.")
-      end
+      @last_commit_sha != last_commit.sha
     end
 
     def last_commit
       @last_commit ||= Gitlab::Git::Commit.
         last_for_path(@start_project.repository, @start_branch, @file_path)
     end
+
+    def validate!
+      super
+
+      if file_has_changed?
+        raise FileChangedError, "You are attempting to update a file that has changed since you started editing it."
+      end
+    end
   end
 end
diff --git a/app/services/validate_new_branch_service.rb b/app/services/validate_new_branch_service.rb
index 2f61be184ce..d232e85cd33 100644
--- a/app/services/validate_new_branch_service.rb
+++ b/app/services/validate_new_branch_service.rb
@@ -8,10 +8,7 @@ class ValidateNewBranchService < BaseService
       return error('Branch name is invalid')
     end
 
-    repository = project.repository
-    existing_branch = repository.find_branch(branch_name)
-
-    if existing_branch
+    if project.repository.branch_exists?(branch_name)
       return error('Branch already exists')
     end