Refactor changing files in web UI

6 files changed, 43 insertions, 249 deletions

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