Merge pull request #1422 from AlexDenisov/api_errors_refactoring

lib/api/helpers.rb

@@ -8,7 +8,7 @@ module Gitlab
       if @project ||= current_user.projects.find_by_id(params[:id]) ||
                       current_user.projects.find_by_code(params[:id])
       else
-        error!({'message' => '404 Not found'}, 404)
+        not_found!
       end
 
       @project
@@ -19,15 +19,40 @@ module Gitlab
     end
 
     def authenticate!
-      error!({'message' => '401 Unauthorized'}, 401) unless current_user
+      unauthorized! unless current_user
     end
 
     def authorize! action, subject
       unless abilities.allowed?(current_user, action, subject)
-        error!({'message' => '403 Forbidden'}, 403)
+        forbidden!
       end
     end
 
+    # error helpers
+
+    def forbidden!
+      render_api_error!('403 Forbidden', 403)
+    end
+
+    def not_found!(resource = nil)
+      message = ["404"]
+      message << resource if resource
+      message << "Not Found"
+      render_api_error!(message.join(' '), 404)
+    end
+
+    def unauthorized!
+      render_api_error!('401 Unauthorized', 401)
+    end
+
+    def not_allowed!
+      render_api_error!('Method Not Allowed', 405)
+    end
+
+    def render_api_error!(message, status)
+      error!({'message' => message}, status)
+    end
+
     private 
 
     def abilities

lib/api/issues.rb

@@ -60,7 +60,7 @@ module Gitlab
         if @issue.save
           present @issue, with: Entities::Issue
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
 
@@ -93,7 +93,7 @@ module Gitlab
         if @issue.update_attributes(parameters)
           present @issue, with: Entities::Issue
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
 
@@ -105,7 +105,7 @@ module Gitlab
       # Example Request:
       #   DELETE /projects/:id/issues/:issue_id
       delete ":id/issues/:issue_id" do
-        error!({'message' => 'method not allowed'}, 405)
+        not_allowed!
       end
     end
   end

lib/api/milestones.rb

@@ -45,7 +45,7 @@ module Gitlab
         if @milestone.save
           present @milestone, with: Entities::Milestone
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
 
@@ -74,7 +74,7 @@ module Gitlab
         if @milestone.update_attributes(parameters)
           present @milestone, with: Entities::Milestone
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
     end

lib/api/projects.rb

@@ -50,7 +50,7 @@ module Gitlab
         if @project.saved?
           present @project, with: Entities::Project
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
 
@@ -172,7 +172,7 @@ module Gitlab
         if @snippet.save
           present @snippet, with: Entities::ProjectSnippet
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
 
@@ -201,7 +201,7 @@ module Gitlab
         if @snippet.update_attributes(parameters)
           present @snippet, with: Entities::ProjectSnippet
         else
-          error!({'message' => '404 Not found'}, 404)
+          not_found!
         end
       end
 
@@ -244,10 +244,10 @@ module Gitlab
         ref = params[:sha]
 
         commit = user_project.commit ref
-        error!('404 Commit Not Found', 404) unless commit
+        not_found! "Commit" unless commit
 
         tree = Tree.new commit.tree, user_project, ref, params[:filepath]
-        error!('404 File Not Found', 404) unless tree.try(:tree)
+        not_found! "File" unless tree.try(:tree)
 
         if tree.text?
           encoding = Gitlab::Encode.detect_encoding(tree.data)

spec/requests/api/projects_spec.rb

@@ -85,7 +85,7 @@ describe Gitlab::API do
     it "should return a 404 error if not found" do
       get api("/projects/42", user)
       response.status.should == 404
-      json_response['message'].should == '404 Not found'
+      json_response['message'].should == '404 Not Found'
     end
   end