Merge branch 'refactor-web-editor' into 'master'

Refactor web editor

* fix problem with editing non-master branch
* before commit make sure branch exists
* dont allow user change file in one branch and commit to another existing branch
* remove a lot of code duplication
* remove outdated statellite errors

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Fixes #1761

See merge request !773

4 files changed, 84 insertions, 117 deletions

diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index 4d02752454e..f587ee266da 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -1,11 +1,34 @@
 module Files
   class BaseService < ::BaseService
-    attr_reader :ref, :path
+    class ValidationError < StandardError; end
 
-    def initialize(project, user, params, ref, path = nil)
-      @project, @current_user, @params = project, user, params.dup
-      @ref = ref
-      @path = path
+    def execute
+      @current_branch = params[:current_branch]
+      @target_branch  = params[:target_branch]
+      @commit_message = params[:commit_message]
+      @file_path      = params[:file_path]
+      @file_content   = if params[:file_content_encoding] == 'base64'
+                          Base64.decode64(params[:file_content])
+                        else
+                          params[:file_content]
+                        end
+
+      # Validate parameters
+      validate
+
+      # Create new branch if it different from current_branch
+      if @target_branch != @current_branch
+        create_target_branch
+      end
+
+      if sha = commit
+        after_commit(sha, @target_branch)
+        success
+      else
+        error("Something went wrong. Your changes were not committed")
+      end
+    rescue ValidationError => ex
+      error(ex.message)
     end
 
     private
@@ -14,11 +37,51 @@ module Files
       project.repository
     end
 
-    def after_commit(sha)
+    def after_commit(sha, branch)
       commit = repository.commit(sha)
-      full_ref = 'refs/heads/' + (params[:new_branch] || ref)
+      full_ref = 'refs/heads/' + branch
       old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA
       GitPushService.new.execute(project, current_user, old_sha, sha, full_ref)
     end
+
+    def current_branch
+      @current_branch ||= params[:current_branch]
+    end
+
+    def target_branch
+      @target_branch ||= params[:target_branch]
+    end
+
+    def raise_error(message)
+      raise ValidationError.new(message)
+    end
+
+    def validate
+      allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch)
+
+      unless allowed
+        raise_error("You are not allowed to push into this branch")
+      end
+
+      unless project.empty_repo?
+        unless repository.branch_names.include?(@current_branch)
+          raise_error("You can only create files if you are on top of a branch")
+        end
+
+        if @current_branch != @target_branch
+          if repository.branch_names.include?(@target_branch)
+            raise_error("Branch with such name already exists. You need to switch to this branch in order to make changes")
+          end
+        end
+      end
+    end
+
+    def create_target_branch
+      result = CreateBranchService.new(project, current_user).execute(@target_branch, @current_branch)
+
+      unless result[:status] == :success
+        raise_error("Something went wrong when we tried to create #{@target_branch} for you")
+      end
+    end
   end
 end
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index 0a80455bc6b..91d715b2d63 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -2,58 +2,28 @@ require_relative "base_service"
 
 module Files
   class CreateService < Files::BaseService
-    def execute
-      allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
+    def commit
+      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch)
+    end
 
-      unless allowed
-        return error("You are not allowed to create file in this branch")
-      end
+    def validate
+      super
 
-      file_name = File.basename(path)
-      file_path = path
+      file_name = File.basename(@file_path)
 
       unless file_name =~ Gitlab::Regex.file_name_regex
-        return error(
+        raise_error(
           'Your changes could not be committed, because the file name ' +
           Gitlab::Regex.file_name_regex_message
         )
       end
 
-      if project.empty_repo?
-        # everything is ok because repo does not have a commits yet
-      else
-        unless repository.branch_names.include?(ref)
-          return error("You can only create files if you are on top of a branch")
-        end
-
-        blob = repository.blob_at_branch(ref, file_path)
+      unless project.empty_repo?
+        blob = repository.blob_at_branch(@current_branch, @file_path)
 
         if blob
-          return error("Your changes could not be committed, because file with such name exists")
-        end
-      end
-
-      content =
-        if params[:encoding] == 'base64'
-          Base64.decode64(params[:content])
-        else
-          params[:content]
+          raise_error("Your changes could not be committed, because file with such name exists")
         end
-
-      sha = repository.commit_file(
-        current_user,
-        file_path,
-        content,
-        params[:commit_message],
-        params[:new_branch] || ref
-      )
-
-
-      if sha
-        after_commit(sha)
-        success
-      else
-        error("Your changes could not be committed, because the file has been changed")
       end
     end
   end
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index 2281777604c..27c881c3430 100644
--- a/app/services/files/delete_service.rb
+++ b/app/services/files/delete_service.rb
@@ -2,36 +2,8 @@ require_relative "base_service"
 
 module Files
   class DeleteService < Files::BaseService
-    def execute
-      allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
-
-      unless allowed
-        return error("You are not allowed to push into this branch")
-      end
-
-      unless repository.branch_names.include?(ref)
-        return error("You can only create files if you are on top of a branch")
-      end
-
-      blob = repository.blob_at_branch(ref, path)
-
-      unless blob
-        return error("You can only edit text files")
-      end
-
-      sha = repository.remove_file(
-        current_user,
-        path,
-        params[:commit_message],
-        ref
-      )
-
-      if sha
-        after_commit(sha)
-        success
-      else
-        error("Your changes could not be committed, because the file has been changed")
-      end
+    def commit
+      repository.remove_file(current_user, @file_path, @commit_message, @target_branch)
     end
   end
 end
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index 013cc1ee322..a20903c6f02 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -2,46 +2,8 @@ require_relative "base_service"
 
 module Files
   class UpdateService < Files::BaseService
-    def execute
-      allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
-
-      unless allowed
-        return error("You are not allowed to push into this branch")
-      end
-
-      unless repository.branch_names.include?(ref)
-        return error("You can only create files if you are on top of a branch")
-      end
-
-      blob = repository.blob_at_branch(ref, path)
-
-      unless blob
-        return error("You can only edit text files")
-      end
-
-      content =
-        if params[:encoding] == 'base64'
-          Base64.decode64(params[:content])
-        else
-          params[:content]
-        end
-
-      sha = repository.commit_file(
-        current_user,
-        path,
-        content,
-        params[:commit_message],
-        params[:new_branch] || ref
-      )
-
-      after_commit(sha)
-      success
-    rescue Gitlab::Satellite::CheckoutFailed => ex
-      error("Your changes could not be committed because ref '#{ref}' could not be checked out", 400)
-    rescue Gitlab::Satellite::CommitFailed => ex
-      error("Your changes could not be committed. Maybe there was nothing to commit?", 409)
-    rescue Gitlab::Satellite::PushFailed => ex
-      error("Your changes could not be committed. Maybe the file was changed by another process?", 409)
+    def commit
+      repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch)
     end
   end
 end