From 998cd3cb63d56a0058c8e519d7c20e3d6e540899 Mon Sep 17 00:00:00 2001
From: jubianchi <contact@jubianchi.fr>
Date: Mon, 18 Aug 2014 20:09:09 +0200
Subject: [PATCH] Improve error reporting on users API

* users (#6878, #3526, #4209): Validation error messages are now exposed through 400 responses, 409 response are sent in case of duplicate email or username
* MRs (#5335): 409 responses are sent in case of duplicate merge request (source/target branches), 422 responses are sent when submiting MR fo/from unrelated forks
* issues
* labels
* projects
---
 app/models/merge_request.rb              |   9 +-
 doc/api/README.md                        |  50 +++++++
 lib/api/deploy_keys.rb                   |   2 +-
 lib/api/helpers.rb                       |  12 +-
 lib/api/issues.rb                        |   4 +-
 lib/api/labels.rb                        |  31 ++---
 lib/api/merge_requests.rb                |   9 +-
 lib/api/project_snippets.rb              |   5 +-
 lib/api/projects.rb                      |   4 +-
 lib/api/users.rb                         |  58 ++++++---
 spec/requests/api/issues_spec.rb         |  18 +++
 spec/requests/api/labels_spec.rb         |  17 ++-
 spec/requests/api/merge_requests_spec.rb |  55 ++++++--
 spec/requests/api/projects_spec.rb       |  39 +++++-
 spec/requests/api/users_spec.rb          | 158 +++++++++++++++++++----
 15 files changed, 369 insertions(+), 102 deletions(-)

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 10bd76b1c35..4894c617674 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -122,9 +122,11 @@ class MergeRequest < ActiveRecord::Base
     if opened? || reopened?
       similar_mrs = self.target_project.merge_requests.where(source_branch: source_branch, target_branch: target_branch, source_project_id: source_project.id).opened
       similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id
-
       if similar_mrs.any?
-        errors.add :base, "Cannot Create: This merge request already exists: #{similar_mrs.pluck(:title)}"
+        errors.add :validate_branches,
+                   "Cannot Create: This merge request already exists: #{
+                   similar_mrs.pluck(:title)
+                   }"
       end
     end
   end
@@ -140,7 +142,8 @@ class MergeRequest < ActiveRecord::Base
       if source_project.forked_from?(target_project)
         true
       else
-        errors.add :base, "Source project is not a fork of target project"
+        errors.add :validate_fork,
+                   'Source project is not a fork of target project'
       end
     end
   end
diff --git a/doc/api/README.md b/doc/api/README.md
index ababb7b6999..f76a253083f 100644
--- a/doc/api/README.md
+++ b/doc/api/README.md
@@ -80,6 +80,7 @@ Return values:
 - `404 Not Found` - A resource could not be accessed, e.g. an ID for a resource could not be found
 - `405 Method Not Allowed` - The request is not supported
 - `409 Conflict` - A conflicting resource already exists, e.g. creating a project with a name that already exists
+- `422 Unprocessable` - The entity could not be processed
 - `500 Server Error` - While handling the request something went wrong on the server side
 
 ## Sudo
@@ -144,3 +145,52 @@ Issue:
 - iid - is unique only in scope of a single project. When you browse issues or merge requests with Web UI, you see iid.
 
 So if you want to get issue with api you use `http://host/api/v3/.../issues/:id.json`. But when you want to create a link to web page - use  `http:://host/project/issues/:iid.json`
+
+## Data validation and error reporting
+
+When working with the API you may encounter validation errors. In such case, the API will answer with an HTTP `400` status.
+Such errors appear in two cases:
+
+* A required attribute of the API request is missing, e.g. the title of an issue is not given
+* An attribute did not pass the validation, e.g. user bio is too long
+
+When an attribute is missing, you will get something like:
+
+    HTTP/1.1 400 Bad Request
+    Content-Type: application/json
+    
+    {
+        "message":"400 (Bad request) \"title\" not given"
+    }
+
+When a validation error occurs, error messages will be different. They will hold all details of validation errors:
+
+    HTTP/1.1 400 Bad Request
+    Content-Type: application/json
+    
+    {
+        "message": {
+            "bio": [
+                "is too long (maximum is 255 characters)"
+            ]
+        }
+    }
+
+This makes error messages more machine-readable. The format can be described as follow:
+
+    {
+        "message": {
+            "<property-name>": [
+                "<error-message>",
+                "<error-message>",
+                ...
+            ],
+            "<embed-entity>": {
+                "<property-name>": [
+                    "<error-message>",
+                    "<error-message>",
+                    ...
+                ],
+            }
+        }
+    }
diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb
index 7f5a125038c..06eb7756841 100644
--- a/lib/api/deploy_keys.rb
+++ b/lib/api/deploy_keys.rb
@@ -58,7 +58,7 @@ module API
         if key.valid? && user_project.deploy_keys << key
           present key, with: Entities::SSHKey
         else
-          not_found!
+          render_validation_error!(key)
         end
       end
 
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 6af0f6d1b25..3a619169eca 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -155,7 +155,17 @@ module API
     end
 
     def not_allowed!
-      render_api_error!('Method Not Allowed', 405)
+      render_api_error!('405 Method Not Allowed', 405)
+    end
+
+    def conflict!(message = nil)
+      render_api_error!(message || '409 Conflict', 409)
+    end
+
+    def render_validation_error!(model)
+      unless model.valid?
+        render_api_error!(model.errors.messages || '400 Bad Request', 400)
+      end
     end
 
     def render_api_error!(message, status)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 5369149cdfc..30170c657ba 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -109,7 +109,7 @@ module API
 
           present issue, with: Entities::Issue
         else
-          not_found!
+          render_validation_error!(issue)
         end
       end
 
@@ -149,7 +149,7 @@ module API
 
           present issue, with: Entities::Issue
         else
-          not_found!
+          render_validation_error!(issue)
         end
       end
 
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index 2fdf53ffec2..78ca58ad0d1 100644
--- a/lib/api/labels.rb
+++ b/lib/api/labels.rb
@@ -30,16 +30,14 @@ module API
         attrs = attributes_for_keys [:name, :color]
         label = user_project.find_label(attrs[:name])
 
-        if label
-          return render_api_error!('Label already exists', 409)
-        end
+        conflict!('Label already exists') if label
 
         label = user_project.labels.create(attrs)
 
         if label.valid?
           present label, with: Entities::Label
         else
-          render_api_error!(label.errors.full_messages.join(', '), 400)
+          render_validation_error!(label)
         end
       end
 
@@ -56,9 +54,7 @@ module API
         required_attributes! [:name]
 
         label = user_project.find_label(params[:name])
-        if !label
-          return render_api_error!('Label not found', 404)
-        end
+        not_found!('Label') unless label
 
         label.destroy
       end
@@ -66,10 +62,11 @@ module API
       # Updates an existing label. At least one optional parameter is required.
       #
       # Parameters:
-      #   id    (required) - The ID of a project
-      #   name  (optional) - The name of the label to be deleted
-      #   color (optional) - Color of the label given in 6-digit hex
-      #                      notation with leading '#' sign (e.g. #FFAABB)
+      #   id        (required) - The ID of a project
+      #   name      (required) - The name of the label to be deleted
+      #   new_name  (optional) - The new name of the label
+      #   color     (optional) - Color of the label given in 6-digit hex
+      #                          notation with leading '#' sign (e.g. #FFAABB)
       # Example Request:
       #   PUT /projects/:id/labels
       put ':id/labels' do
@@ -77,16 +74,14 @@ module API
         required_attributes! [:name]
 
         label = user_project.find_label(params[:name])
-        if !label
-          return render_api_error!('Label not found', 404)
-        end
+        not_found!('Label not found') unless label
 
         attrs = attributes_for_keys [:new_name, :color]
 
         if attrs.empty?
-          return render_api_error!('Required parameters "name" or "color" ' \
-                                   'missing',
-                                   400)
+          render_api_error!('Required parameters "new_name" or "color" ' \
+                            'missing',
+                            400)
         end
 
         # Rename new name to the actual label attribute name
@@ -95,7 +90,7 @@ module API
         if label.update(attrs)
           present label, with: Entities::Label
         else
-          render_api_error!(label.errors.full_messages.join(', '), 400)
+          render_validation_error!(label)
         end
       end
     end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 8726379bf3c..40111014032 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -10,8 +10,13 @@ module API
             error!(errors[:project_access], 422)
           elsif errors[:branch_conflict].any?
             error!(errors[:branch_conflict], 422)
+          elsif errors[:validate_fork].any?
+            error!(errors[:validate_fork], 422)
+          elsif errors[:validate_branches].any?
+            conflict!(errors[:validate_branches])
           end
-          not_found!
+
+          render_api_error!(errors, 400)
         end
       end
 
@@ -214,7 +219,7 @@ module API
         if note.save
           present note, with: Entities::MRNote
         else
-          not_found!
+          render_validation_error!(note)
         end
       end
     end
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index 8e09fff6843..0c2d282f785 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -56,7 +56,7 @@ module API
         if @snippet.save
           present @snippet, with: Entities::ProjectSnippet
         else
-          not_found!
+          render_validation_error!(@snippet)
         end
       end
 
@@ -80,7 +80,7 @@ module API
         if @snippet.update_attributes attrs
           present @snippet, with: Entities::ProjectSnippet
         else
-          not_found!
+          render_validation_error!(@snippet)
         end
       end
 
@@ -97,6 +97,7 @@ module API
           authorize! :modify_project_snippet, @snippet
           @snippet.destroy
         rescue
+          not_found!('Snippet')
         end
       end
 
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 55f7975bbf7..f555819df1b 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -111,7 +111,7 @@ module API
           if @project.errors[:limit_reached].present?
             error!(@project.errors[:limit_reached], 403)
           end
-          not_found!
+          render_validation_error!(@project)
         end
       end
 
@@ -149,7 +149,7 @@ module API
         if @project.saved?
           present @project, with: Entities::Project
         else
-          not_found!
+          render_validation_error!(@project)
         end
       end
 
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 69553f16397..d07815a8a97 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -42,7 +42,8 @@ module API
       # Parameters:
       #   email (required)                  - Email
       #   password (required)               - Password
-      #   name                              - Name
+      #   name (required)                   - Name
+      #   username (required)               - Name
       #   skype                             - Skype ID
       #   linkedin                          - Linkedin
       #   twitter                           - Twitter account
@@ -65,7 +66,15 @@ module API
         if user.save
           present user, with: Entities::UserFull
         else
-          not_found!
+          conflict!('Email has already been taken') if User.
+              where(email: user.email).
+              count > 0
+
+          conflict!('Username has already been taken') if User.
+              where(username: user.username).
+              count > 0
+
+          render_validation_error!(user)
         end
       end
 
@@ -92,14 +101,23 @@ module API
 
         attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :extern_uid, :provider, :bio, :can_create_group, :admin]
         user = User.find(params[:id])
-        not_found!("User not found") unless user
+        not_found!('User') unless user
 
         admin = attrs.delete(:admin)
         user.admin = admin unless admin.nil?
+
+        conflict!('Email has already been taken') if attrs[:email] &&
+            User.where(email: attrs[:email]).
+                where.not(id: user.id).count > 0
+
+        conflict!('Username has already been taken') if attrs[:username] &&
+            User.where(username: attrs[:username]).
+                where.not(id: user.id).count > 0
+
         if user.update_attributes(attrs)
           present user, with: Entities::UserFull
         else
-          not_found!
+          render_validation_error!(user)
         end
       end
 
@@ -113,13 +131,15 @@ module API
       # POST /users/:id/keys
       post ":id/keys" do
         authenticated_as_admin!
+        required_attributes! [:title, :key]
+
         user = User.find(params[:id])
         attrs = attributes_for_keys [:title, :key]
         key = user.keys.new attrs
         if key.save
           present key, with: Entities::SSHKey
         else
-          not_found!
+          render_validation_error!(key)
         end
       end
 
@@ -132,11 +152,9 @@ module API
       get ':uid/keys' do
         authenticated_as_admin!
         user = User.find_by(id: params[:uid])
-        if user
-          present user.keys, with: Entities::SSHKey
-        else
-          not_found!
-        end
+        not_found!('User') unless user
+
+        present user.keys, with: Entities::SSHKey
       end
 
       # Delete existing ssh key of a specified user. Only available to admin
@@ -150,15 +168,13 @@ module API
       delete ':uid/keys/:id' do
         authenticated_as_admin!
         user = User.find_by(id: params[:uid])
-        if user
-          begin
-            key = user.keys.find params[:id]
-            key.destroy
-          rescue ActiveRecord::RecordNotFound
-            not_found!
-          end
-        else
-          not_found!
+        not_found!('User') unless user
+
+        begin
+          key = user.keys.find params[:id]
+          key.destroy
+        rescue ActiveRecord::RecordNotFound
+          not_found!('Key')
         end
       end
 
@@ -173,7 +189,7 @@ module API
         if user
           user.destroy
         else
-          not_found!
+          not_found!('User')
         end
       end
     end
@@ -219,7 +235,7 @@ module API
         if key.save
           present key, with: Entities::SSHKey
         else
-          not_found!
+          render_validation_error!(key)
         end
       end
 
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index e8eebda95b4..9876452f81d 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -169,6 +169,15 @@ describe API::API, api: true  do
       response.status.should == 400
       json_response['message']['labels']['?']['title'].should == ['is invalid']
     end
+
+    it 'should return 400 if title is too long' do
+      post api("/projects/#{project.id}/issues", user),
+           title: 'g' * 256
+      response.status.should == 400
+      json_response['message']['title'].should == [
+        'is too long (maximum is 255 characters)'
+      ]
+    end
   end
 
   describe "PUT /projects/:id/issues/:issue_id to update only title" do
@@ -237,6 +246,15 @@ describe API::API, api: true  do
       json_response['labels'].should include 'label_bar'
       json_response['labels'].should include 'label/bar'
     end
+
+    it 'should return 400 if title is too long' do
+      put api("/projects/#{project.id}/issues/#{issue.id}", user),
+          title: 'g' * 256
+      response.status.should == 400
+      json_response['message']['title'].should == [
+        'is too long (maximum is 255 characters)'
+      ]
+    end
   end
 
   describe "PUT /projects/:id/issues/:issue_id to update state and label" do
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index ee9088933a1..dbddc8a7da4 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -47,7 +47,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAA'
       response.status.should == 400
-      json_response['message'].should == 'Color is invalid'
+      json_response['message']['color'].should == ['is invalid']
     end
 
     it 'should return 400 for too long color code' do
@@ -55,7 +55,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAAFFFF'
       response.status.should == 400
-      json_response['message'].should == 'Color is invalid'
+      json_response['message']['color'].should == ['is invalid']
     end
 
     it 'should return 400 for invalid name' do
@@ -63,7 +63,7 @@ describe API::API, api: true  do
            name: '?',
            color: '#FFAABB'
       response.status.should == 400
-      json_response['message'].should == 'Title is invalid'
+      json_response['message']['title'].should == ['is invalid']
     end
 
     it 'should return 409 if label already exists' do
@@ -84,7 +84,7 @@ describe API::API, api: true  do
     it 'should return 404 for non existing label' do
       delete api("/projects/#{project.id}/labels", user), name: 'label2'
       response.status.should == 404
-      json_response['message'].should == 'Label not found'
+      json_response['message'].should == '404 Label Not Found'
     end
 
     it 'should return 400 for wrong parameters' do
@@ -132,11 +132,14 @@ describe API::API, api: true  do
     it 'should return 400 if no label name given' do
       put api("/projects/#{project.id}/labels", user), new_name: 'label2'
       response.status.should == 400
+      json_response['message'].should == '400 (Bad request) "name" not given'
     end
 
     it 'should return 400 if no new parameters given' do
       put api("/projects/#{project.id}/labels", user), name: 'label1'
       response.status.should == 400
+      json_response['message'].should == 'Required parameters '\
+                                         '"new_name" or "color" missing'
     end
 
     it 'should return 400 for invalid name' do
@@ -145,7 +148,7 @@ describe API::API, api: true  do
           new_name: '?',
           color: '#FFFFFF'
       response.status.should == 400
-      json_response['message'].should == 'Title is invalid'
+      json_response['message']['title'].should == ['is invalid']
     end
 
     it 'should return 400 for invalid name' do
@@ -153,7 +156,7 @@ describe API::API, api: true  do
           name: 'label1',
           color: '#FF'
       response.status.should == 400
-      json_response['message'].should == 'Color is invalid'
+      json_response['message']['color'].should == ['is invalid']
     end
 
     it 'should return 400 for too long color code' do
@@ -161,7 +164,7 @@ describe API::API, api: true  do
            name: 'Foo',
            color: '#FFAAFFFF'
       response.status.should == 400
-      json_response['message'].should == 'Color is invalid'
+      json_response['message']['color'].should == ['is invalid']
     end
   end
 end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 06a25c5e3a5..2684c1f9e5a 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -123,6 +123,28 @@ describe API::API, api: true  do
         json_response['message']['labels']['?']['title'].should ==
           ['is invalid']
       end
+
+      context 'with existing MR' do
+        before do
+          post api("/projects/#{project.id}/merge_requests", user),
+               title: 'Test merge_request',
+               source_branch: 'stable',
+               target_branch: 'master',
+               author: user
+          @mr = MergeRequest.all.last
+        end
+
+        it 'should return 409 when MR already exists for source/target' do
+          expect do
+            post api("/projects/#{project.id}/merge_requests", user),
+                 title: 'New test merge_request',
+                 source_branch: 'stable',
+                 target_branch: 'master',
+                 author: user
+          end.to change { MergeRequest.count }.by(0)
+          response.status.should == 409
+        end
+      end
     end
 
     context 'forked projects' do
@@ -170,16 +192,26 @@ describe API::API, api: true  do
         response.status.should == 400
       end
 
-      it "should return 404 when target_branch is specified and not a forked project" do
-        post api("/projects/#{project.id}/merge_requests", user),
-        title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user, target_project_id: fork_project.id
-        response.status.should == 404
-      end
+      context 'when target_branch is specified' do
+        it 'should return 422 if not a forked project' do
+          post api("/projects/#{project.id}/merge_requests", user),
+               title: 'Test merge_request',
+               target_branch: 'master',
+               source_branch: 'stable',
+               author: user,
+               target_project_id: fork_project.id
+          response.status.should == 422
+        end
 
-      it "should return 404 when target_branch is specified and for a different fork" do
-        post api("/projects/#{fork_project.id}/merge_requests", user2),
-        title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: unrelated_project.id
-        response.status.should == 404
+        it 'should return 422 if targeting a different fork' do
+          post api("/projects/#{fork_project.id}/merge_requests", user2),
+               title: 'Test merge_request',
+               target_branch: 'master',
+               source_branch: 'stable',
+               author: user2,
+               target_project_id: unrelated_project.id
+          response.status.should == 422
+        end
       end
 
       it "should return 201 when target_branch is specified and for the same project" do
@@ -216,7 +248,7 @@ describe API::API, api: true  do
       merge_request.close
       put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user)
       response.status.should == 405
-      json_response['message'].should == 'Method Not Allowed'
+      json_response['message'].should == '405 Method Not Allowed'
     end
 
     it "should return 401 if user has no permissions to merge" do
@@ -276,7 +308,8 @@ describe API::API, api: true  do
     end
 
     it "should return 404 if note is attached to non existent merge request" do
-      post api("/projects/#{project.id}/merge_request/111/comments", user), note: "My comment"
+      post api("/projects/#{project.id}/merge_request/404/comments", user),
+           note: 'My comment'
       response.status.should == 404
     end
   end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 058b831e783..31c07a12eda 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -188,9 +188,24 @@ describe API::API, api: true  do
       response.status.should == 201
     end
 
-    it "should respond with 404 on failure" do
+    it 'should respond with 400 on failure' do
       post api("/projects/user/#{user.id}", admin)
-      response.status.should == 404
+      response.status.should == 400
+      json_response['message']['creator'].should == ['can\'t be blank']
+      json_response['message']['namespace'].should == ['can\'t be blank']
+      json_response['message']['name'].should == [
+        'can\'t be blank',
+        'is too short (minimum is 0 characters)',
+        'can contain only letters, digits, \'_\', \'-\' and \'.\' and '\
+        'space. It must start with letter, digit or \'_\'.'
+      ]
+      json_response['message']['path'].should == [
+        'can\'t be blank',
+        'is too short (minimum is 0 characters)',
+        'can contain only letters, digits, \'_\', \'-\' and \'.\'. It must '\
+        'start with letter, digit or \'_\', optionally preceeded by \'.\'. '\
+        'It must not end in \'.git\'.'
+      ]
     end
 
     it "should assign attributes to project" do
@@ -407,9 +422,9 @@ describe API::API, api: true  do
       response.status.should == 200
     end
 
-    it "should return success when deleting unknown snippet id" do
+    it 'should return 404 when deleting unknown snippet id' do
       delete api("/projects/#{project.id}/snippets/1234", user)
-      response.status.should == 200
+      response.status.should == 404
     end
   end
 
@@ -456,7 +471,21 @@ describe API::API, api: true  do
     describe "POST /projects/:id/keys" do
       it "should not create an invalid ssh key" do
         post api("/projects/#{project.id}/keys", user), { title: "invalid key" }
-        response.status.should == 404
+        response.status.should == 400
+        json_response['message']['key'].should == [
+          'can\'t be blank',
+          'is too short (minimum is 0 characters)',
+          'is invalid'
+        ]
+      end
+
+      it 'should not create a key without title' do
+        post api("/projects/#{project.id}/keys", user), key: 'some key'
+        response.status.should == 400
+        json_response['message']['title'].should == [
+          'can\'t be blank',
+          'is too short (minimum is 0 characters)'
+        ]
       end
 
       it "should create new ssh key" do
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 8bbe9b5b736..b0752ebe43c 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -51,6 +51,7 @@ describe API::API, api: true  do
     it "should return a 404 error if user id not found" do
       get api("/users/9999", user)
       response.status.should == 404
+      json_response['message'].should == '404 Not found'
     end
   end
 
@@ -98,40 +99,85 @@ describe API::API, api: true  do
     end
 
     it "should not create user with invalid email" do
-      post api("/users", admin), { email: "invalid email", password: 'password' }
+      post api('/users', admin),
+           email: 'invalid email',
+           password: 'password',
+           name: 'test'
       response.status.should == 400
     end
 
-    it "should return 400 error if password not given" do
-      post api("/users", admin), { email: 'test@example.com' }
+    it 'should return 400 error if name not given' do
+      post api('/users', admin), email: 'test@example.com', password: 'pass1234'
+      response.status.should == 400
+    end
+
+    it 'should return 400 error if password not given' do
+      post api('/users', admin), email: 'test@example.com', name: 'test'
       response.status.should == 400
     end
 
     it "should return 400 error if email not given" do
-      post api("/users", admin), { password: 'pass1234' }
+      post api('/users', admin), password: 'pass1234', name: 'test'
       response.status.should == 400
     end
 
+    it 'should return 400 error if user does not validate' do
+      post api('/users', admin),
+           password: 'pass',
+           email: 'test@example.com',
+           username: 'test!',
+           name: 'test',
+           bio: 'g' * 256,
+           projects_limit: -1
+      response.status.should == 400
+      json_response['message']['password'].
+          should == ['is too short (minimum is 8 characters)']
+      json_response['message']['bio'].
+          should == ['is too long (maximum is 255 characters)']
+      json_response['message']['projects_limit'].
+          should == ['must be greater than or equal to 0']
+      json_response['message']['username'].
+          should == ['can contain only letters, digits, '\
+          '\'_\', \'-\' and \'.\'. It must start with letter, digit or '\
+          '\'_\', optionally preceeded by \'.\'. It must not end in \'.git\'.']
+    end
+
     it "shouldn't available for non admin users" do
       post api("/users", user), attributes_for(:user)
       response.status.should == 403
     end
 
-    context "with existing user" do
-      before { post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test' } }
+    context 'with existing user' do
+      before do
+        post api('/users', admin),
+             email: 'test@example.com',
+             password: 'password',
+             username: 'test',
+             name: 'foo'
+      end
 
-      it "should not create user with same email" do
+      it 'should return 409 conflict error if user with same email exists' do
         expect {
-          post api("/users", admin), { email: 'test@example.com', password: 'password' }
+          post api('/users', admin),
+               name: 'foo',
+               email: 'test@example.com',
+               password: 'password',
+               username: 'foo'
         }.to change { User.count }.by(0)
+        response.status.should == 409
+        json_response['message'].should == 'Email has already been taken'
       end
 
-      it "should return 409 conflict error if user with email exists" do
-        post api("/users", admin), { email: 'test@example.com', password: 'password' }
-      end
-
-      it "should return 409 conflict error if same username exists" do
-        post api("/users", admin), { email: 'foo@example.com', password: 'pass', username: 'test' }
+      it 'should return 409 conflict error if same username exists' do
+        expect do
+          post api('/users', admin),
+               name: 'foo',
+               email: 'foo@example.com',
+               password: 'password',
+               username: 'test'
+        end.to change { User.count }.by(0)
+        response.status.should == 409
+        json_response['message'].should == 'Username has already been taken'
       end
     end
   end
@@ -173,6 +219,20 @@ describe API::API, api: true  do
       user.reload.bio.should == 'new test bio'
     end
 
+    it 'should update user with his own email' do
+      put api("/users/#{user.id}", admin), email: user.email
+      response.status.should == 200
+      json_response['email'].should == user.email
+      user.reload.email.should == user.email
+    end
+
+    it 'should update user with his own username' do
+      put api("/users/#{user.id}", admin), username: user.username
+      response.status.should == 200
+      json_response['username'].should == user.username
+      user.reload.username.should == user.username
+    end
+
     it "should update admin status" do
       put api("/users/#{user.id}", admin), {admin: true}
       response.status.should == 200
@@ -190,7 +250,7 @@ describe API::API, api: true  do
 
     it "should not allow invalid update" do
       put api("/users/#{user.id}", admin), {email: 'invalid email'}
-      response.status.should == 404
+      response.status.should == 400
       user.reload.email.should_not == 'invalid email'
     end
 
@@ -202,25 +262,49 @@ describe API::API, api: true  do
     it "should return 404 for non-existing user" do
       put api("/users/999999", admin), {bio: 'update should fail'}
       response.status.should == 404
+      json_response['message'].should == '404 Not found'
+    end
+
+    it 'should return 400 error if user does not validate' do
+      put api("/users/#{user.id}", admin),
+          password: 'pass',
+          email: 'test@example.com',
+          username: 'test!',
+          name: 'test',
+          bio: 'g' * 256,
+          projects_limit: -1
+      response.status.should == 400
+      json_response['message']['password'].
+          should == ['is too short (minimum is 8 characters)']
+      json_response['message']['bio'].
+          should == ['is too long (maximum is 255 characters)']
+      json_response['message']['projects_limit'].
+          should == ['must be greater than or equal to 0']
+      json_response['message']['username'].
+          should == ['can contain only letters, digits, '\
+          '\'_\', \'-\' and \'.\'. It must start with letter, digit or '\
+          '\'_\', optionally preceeded by \'.\'. It must not end in \'.git\'.']
     end
 
     context "with existing user" do
       before {
         post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test', name: 'test' }
         post api("/users", admin), { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' }
-        @user_id = User.all.last.id
+        @user = User.all.last
       }
 
-#      it "should return 409 conflict error if email address exists" do
-#        put api("/users/#{@user_id}", admin), { email: 'test@example.com' }
-#        response.status.should == 409
-#      end
-#
-#      it "should return 409 conflict error if username taken" do
-#        @user_id = User.all.last.id
-#        put api("/users/#{@user_id}", admin), { username: 'test' }
-#        response.status.should == 409
-#      end
+      it 'should return 409 conflict error if email address exists' do
+        put api("/users/#{@user.id}", admin), email: 'test@example.com'
+        response.status.should == 409
+        @user.reload.email.should == @user.email
+      end
+
+      it 'should return 409 conflict error if username taken' do
+        @user_id = User.all.last.id
+        put api("/users/#{@user.id}", admin), username: 'test'
+        response.status.should == 409
+        @user.reload.username.should == @user.username
+      end
     end
   end
 
@@ -229,7 +313,14 @@ describe API::API, api: true  do
 
     it "should not create invalid ssh key" do
       post api("/users/#{user.id}/keys", admin), { title: "invalid key" }
-      response.status.should == 404
+      response.status.should == 400
+      json_response['message'].should == '400 (Bad request) "key" not given'
+    end
+
+    it 'should not create key without title' do
+      post api("/users/#{user.id}/keys", admin), key: 'some key'
+      response.status.should == 400
+      json_response['message'].should == '400 (Bad request) "title" not given'
     end
 
     it "should create ssh key" do
@@ -254,6 +345,7 @@ describe API::API, api: true  do
       it 'should return 404 for non-existing user' do
         get api('/users/999999/keys', admin)
         response.status.should == 404
+        json_response['message'].should == '404 User Not Found'
       end
 
       it 'should return array of ssh keys' do
@@ -292,11 +384,13 @@ describe API::API, api: true  do
         user.save
         delete api("/users/999999/keys/#{key.id}", admin)
         response.status.should == 404
+        json_response['message'].should == '404 User Not Found'
       end
 
       it 'should return 404 error if key not foud' do
         delete api("/users/#{user.id}/keys/42", admin)
         response.status.should == 404
+        json_response['message'].should == '404 Key Not Found'
       end
     end
   end
@@ -324,6 +418,7 @@ describe API::API, api: true  do
     it "should return 404 for non-existing user" do
       delete api("/users/999999", admin)
       response.status.should == 404
+      json_response['message'].should == '404 User Not Found'
     end
   end
 
@@ -375,6 +470,7 @@ describe API::API, api: true  do
     it "should return 404 Not Found within invalid ID" do
       get api("/user/keys/42", user)
       response.status.should == 404
+      json_response['message'].should == '404 Not found'
     end
 
     it "should return 404 error if admin accesses user's ssh key" do
@@ -383,6 +479,7 @@ describe API::API, api: true  do
       admin
       get api("/user/keys/#{key.id}", admin)
       response.status.should == 404
+      json_response['message'].should == '404 Not found'
     end
   end
 
@@ -403,6 +500,13 @@ describe API::API, api: true  do
     it "should not create ssh key without key" do
       post api("/user/keys", user), title: 'title'
       response.status.should == 400
+      json_response['message'].should == '400 (Bad request) "key" not given'
+    end
+
+    it 'should not create ssh key without title' do
+      post api('/user/keys', user), key: 'some key'
+      response.status.should == 400
+      json_response['message'].should == '400 (Bad request) "title" not given'
     end
 
     it "should not create ssh key without title" do