Merge pull request #7807 from cirosantilli/factor-service-error

Factor error and success methods from services.

10 files changed, 57 insertions, 67 deletions

diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index 6845fc5e6e6..faa0ce67ca8 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -17,10 +17,8 @@ class Projects::BranchesController < Projects::ApplicationController
   end
 
   def create
-    result = CreateBranchService.new.execute(project,
-                                             params[:branch_name],
-                                             params[:ref],
-                                             current_user)
+    result = CreateBranchService.new(project, current_user).
+        execute(params[:branch_name], params[:ref])
     if result[:status] == :success
       @branch = result[:branch]
       redirect_to project_tree_path(@project, @branch.name)
@@ -31,7 +29,7 @@ class Projects::BranchesController < Projects::ApplicationController
   end
 
   def destroy
-    DeleteBranchService.new.execute(project, params[:id], current_user)
+    DeleteBranchService.new(project, current_user).execute(params[:id])
     @branch_name = params[:id]
 
     respond_to do |format|
diff --git a/app/controllers/projects/edit_tree_controller.rb b/app/controllers/projects/edit_tree_controller.rb
index 72a41f771c0..8976d7c7be8 100644
--- a/app/controllers/projects/edit_tree_controller.rb
+++ b/app/controllers/projects/edit_tree_controller.rb
@@ -10,7 +10,8 @@ class Projects::EditTreeController < Projects::BaseTreeController
   end
 
   def update
-    result = Files::UpdateService.new(@project, current_user, params, @ref, @path).execute
+    result = Files::UpdateService.
+      new(@project, current_user, params, @ref, @path).execute
 
     if result[:status] == :success
       flash[:notice] = "Your changes have been successfully committed"
diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index c80ad8355d5..537c94bda20 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/app/controllers/projects/tags_controller.rb
@@ -13,9 +13,8 @@ class Projects::TagsController < Projects::ApplicationController
   end
 
   def create
-    result = CreateTagService.new.execute(@project, params[:tag_name],
-                                          params[:ref], params[:message],
-                                          current_user)
+    result = CreateTagService.new(@project, current_user).
+      execute(params[:tag_name], params[:ref], params[:message])
     if result[:status] == :success
       @tag = result[:tag]
       redirect_to project_tags_path(@project)
diff --git a/app/services/base_service.rb b/app/services/base_service.rb
index 31b38aca530..051612633cd 100644
--- a/app/services/base_service.rb
+++ b/app/services/base_service.rb
@@ -1,7 +1,7 @@
 class BaseService
   attr_accessor :project, :current_user, :params
 
-  def initialize(project, user, params)
+  def initialize(project, user, params = {})
     @project, @current_user, @params = project, user, params.dup
   end
 
@@ -32,4 +32,19 @@ class BaseService
   def system_hook_service
     SystemHooksService.new
   end
+
+  private
+
+  def error(message)
+    {
+      message: message,
+      status: :error
+    }
+  end
+
+  def success
+    {
+      status: :success
+    }
+  end
 end
diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb
index 79b8239602e..901f67bafb3 100644
--- a/app/services/create_branch_service.rb
+++ b/app/services/create_branch_service.rb
@@ -1,5 +1,7 @@
-class CreateBranchService
-  def execute(project, branch_name, ref, current_user)
+require_relative 'base_service'
+
+class CreateBranchService < BaseService
+  def execute(branch_name, ref)
     valid_branch = Gitlab::GitRefValidator.validate(branch_name)
     if valid_branch == false
       return error('Branch name invalid')
@@ -22,17 +24,9 @@ class CreateBranchService
     end
   end
 
-  def error(message)
-    {
-      message: message,
-      status: :error
-    }
-  end
-
   def success(branch)
-    {
-      branch: branch,
-      status: :success
-    }
+    out = super()
+    out[:branch] = branch
+    out
   end
 end
diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb
index 3716abd4b2b..9b2a2270233 100644
--- a/app/services/create_tag_service.rb
+++ b/app/services/create_tag_service.rb
@@ -1,5 +1,7 @@
-class CreateTagService
-  def execute(project, tag_name, ref, message, current_user)
+require_relative 'base_service'
+
+class CreateTagService < BaseService
+  def execute(tag_name, ref, message)
     valid_tag = Gitlab::GitRefValidator.validate(tag_name)
     if valid_tag == false
       return error('Tag name invalid')
@@ -26,17 +28,9 @@ class CreateTagService
     end
   end
 
-  def error(message)
-    {
-      message: message,
-      status: :error
-    }
-  end
-
   def success(branch)
-    {
-      tag: branch,
-      status: :success
-    }
+    out = super()
+    out[:tag] = branch
+    out
   end
 end
diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb
index a94dabcdfc0..cae6327fe72 100644
--- a/app/services/delete_branch_service.rb
+++ b/app/services/delete_branch_service.rb
@@ -1,5 +1,7 @@
-class DeleteBranchService
-  def execute(project, branch_name, current_user)
+require_relative 'base_service'
+
+class DeleteBranchService < BaseService
+  def execute(branch_name)
     repository = project.repository
     branch = repository.find_branch(branch_name)
 
@@ -31,17 +33,14 @@ class DeleteBranchService
   end
 
   def error(message, return_code = 400)
-    {
-      message: message,
-      return_code: return_code,
-      state: :error
-    }
+    out = super(message)
+    out[:return_code] = return_code
+    out
   end
 
   def success(message)
-    {
-      message: message,
-      state: :success
-    }
+    out = super()
+    out[:message] = message
+    out
   end
 end
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index f1765d38976..db6f0831f8b 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -10,18 +10,10 @@ module Files
 
     private
 
-    def error(message)
-      {
-        error: message,
-        status: :error
-      }
-    end
-
     def success
-      {
-        error: '',
-        status: :success
-      }
+      out = super()
+      out[:error] = ''
+      out
     end
 
     def repository
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index 4db5f61dd28..75783628e3d 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -80,10 +80,8 @@ module API
       #   POST /projects/:id/repository/branches
       post ":id/repository/branches" do
         authorize_push_project
-        result = CreateBranchService.new.execute(user_project,
-                                                 params[:branch_name],
-                                                 params[:ref],
-                                                 current_user)
+        result = CreateBranchService.new(user_project, current_user).
+          execute(params[:branch_name], params[:ref])
         if result[:status] == :success
           present result[:branch],
                   with: Entities::RepoObject,
@@ -102,9 +100,10 @@ module API
       #   DELETE /projects/:id/repository/branches/:branch
       delete ":id/repository/branches/:branch" do
         authorize_push_project
-        result = DeleteBranchService.new.execute(user_project, params[:branch], current_user)
+        result = DeleteBranchService.new(user_project, current_user).
+          execute(params[:branch])
 
-        if result[:state] == :success
+        if result[:status] == :success
           true
         else
           render_api_error!(result[:message], result[:return_code])
diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index 07c29aa7b4c..626d99c2649 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -38,9 +38,8 @@ module API
       post ':id/repository/tags' do
         authorize_push_project
         message = params[:message] || nil
-        result = CreateTagService.new.execute(user_project, params[:tag_name],
-                                              params[:ref], message,
-                                              current_user)
+        result = CreateTagService.new(user_project, current_user).
+          execute(params[:tag_name], params[:ref], message)
 
         if result[:status] == :success
           present result[:tag],