Do not return data when deleting an entityreturn-204-on-delete

30 files changed, 107 insertions, 89 deletions

diff --git a/Gemfile b/Gemfile
index 0060f122512..801dc8cb4e8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -68,7 +68,7 @@ gem 'gollum-rugged_adapter', '~> 0.4.2', require: false
 gem 'github-linguist', '~> 4.7.0', require: 'linguist'
 
 # API
-gem 'grape',        '~> 0.18.0'
+gem 'grape',        '~> 0.19.0'
 gem 'grape-entity', '~> 0.6.0'
 gem 'rack-cors',    '~> 0.4.0', require: 'rack/cors'
 
diff --git a/Gemfile.lock b/Gemfile.lock
index a3c2fad41ba..24477f3807d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -86,7 +86,7 @@ GEM
       sass (>= 3.3.4)
     brakeman (3.4.1)
     browser (2.2.0)
-    builder (3.2.2)
+    builder (3.2.3)
     bullet (5.2.0)
       activesupport (>= 3.0.0)
       uniform_notifier (~> 1.10.0)
@@ -305,7 +305,7 @@ GEM
       multi_json (~> 1.11)
       os (~> 0.9)
       signet (~> 0.7)
-    grape (0.18.0)
+    grape (0.19.1)
       activesupport
       builder
       hashie (>= 2.1.0)
@@ -329,7 +329,7 @@ GEM
       temple (~> 0.7.6)
       thor
       tilt
-    hashie (3.4.4)
+    hashie (3.5.3)
     health_check (2.2.1)
       rails (>= 4.0)
     hipchat (1.5.2)
@@ -354,8 +354,8 @@ GEM
       json (~> 1.8)
       multi_xml (>= 0.5.2)
     httpclient (2.8.2)
-    i18n (0.7.0)
-    ice_nine (0.11.1)
+    i18n (0.8.0)
+    ice_nine (0.11.2)
     influxdb (0.2.3)
       cause
       json
@@ -370,7 +370,7 @@ GEM
       thor (>= 0.14, < 2.0)
     jquery-ui-rails (5.0.5)
       railties (>= 3.2.16)
-    json (1.8.3)
+    json (1.8.6)
     json-schema (2.6.2)
       addressable (~> 2.3.8)
     jwt (1.5.6)
@@ -418,7 +418,7 @@ GEM
     minitest (5.7.0)
     mousetrap-rails (1.4.6)
     multi_json (1.12.1)
-    multi_xml (0.5.5)
+    multi_xml (0.6.0)
     multipart-post (2.0.0)
     mustermann (0.4.0)
       tool (~> 0.2)
@@ -889,7 +889,7 @@ DEPENDENCIES
   gollum-rugged_adapter (~> 0.4.2)
   gon (~> 6.1.0)
   google-api-client (~> 0.8.6)
-  grape (~> 0.18.0)
+  grape (~> 0.19.0)
   grape-entity (~> 0.6.0)
   haml_lint (~> 0.18.2)
   hamlit (~> 2.6.1)
@@ -1014,4 +1014,4 @@ DEPENDENCIES
   wikicloth (= 0.8.1)
 
 BUNDLED WITH
-   1.14.3
+   1.14.4
diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb
index 789f45489eb..3cf9df5b848 100644
--- a/lib/api/access_requests.rb
+++ b/lib/api/access_requests.rb
@@ -67,6 +67,7 @@ module API
         end
         delete ":id/access_requests/:user_id" do
           source = find_source(source_type, params[:id])
+          ressource_modified_since(source.updated_at)
 
           ::Members::DestroyService.new(source, current_user, params).
             execute(:requesters)
diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb
index 58a4df54bea..65e9ecf44b4 100644
--- a/lib/api/award_emoji.rb
+++ b/lib/api/award_emoji.rb
@@ -71,18 +71,17 @@ module API
 
           desc 'Delete a +awardables+ award emoji' do
             detail 'This feature was introduced in 8.9'
-            success Entities::AwardEmoji
           end
           params do
             requires :award_id, type: Integer, desc: 'The ID of an award emoji'
           end
           delete "#{endpoint}/:award_id" do
             award = awardable.award_emoji.find(params[:award_id])
+            ressource_modified_since(award.updated_at)
 
             unauthorized! unless award.user == current_user || current_user.admin?
 
             award.destroy
-            present award, with: Entities::AwardEmoji
           end
         end
       end
diff --git a/lib/api/boards.rb b/lib/api/boards.rb
index 13752eb4947..4a3dc1879b8 100644
--- a/lib/api/boards.rb
+++ b/lib/api/boards.rb
@@ -108,7 +108,6 @@ module API
 
         desc 'Delete a board list' do
           detail 'This feature was introduced in 8.13'
-          success Entities::List
         end
         params do
           requires :list_id, type: Integer, desc: 'The ID of a board list'
@@ -118,11 +117,11 @@ module API
 
           list = board_lists.find(params[:list_id])
 
+          ressource_modified_since(list.updated_at)
+
           service = ::Boards::Lists::DestroyService.new(user_project, current_user)
 
-          if service.execute(list)
-            present list, with: Entities::List
-          else
+          if !service.execute(list)
             render_api_error!({ error: 'List could not be deleted!' }, 400)
           end
         end
diff --git a/lib/api/branches.rb b/lib/api/branches.rb
index 9331be1f7de..b43bbace143 100644
--- a/lib/api/branches.rb
+++ b/lib/api/branches.rb
@@ -117,14 +117,12 @@ module API
       delete ":id/repository/branches/:branch", requirements: { branch: /.+/ } do
         authorize_push_project
 
+        # Todo: Branch is not a rails entity, cannot get updated_at
+
         result = DeleteBranchService.new(user_project, current_user).
                  execute(params[:branch])
 
-        if result[:status] == :success
-          {
-            branch_name: params[:branch]
-          }
-        else
+        if result[:status] != :success
           render_api_error!(result[:message], result[:return_code])
         end
       end
diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb
index 1217002bf8e..8d871b241e5 100644
--- a/lib/api/broadcast_messages.rb
+++ b/lib/api/broadcast_messages.rb
@@ -83,7 +83,6 @@ module API
 
       desc 'Delete a broadcast message' do
         detail 'This feature was introduced in GitLab 8.12.'
-        success Entities::BroadcastMessage
       end
       params do
         requires :id, type: Integer, desc: 'Broadcast message ID'
@@ -91,7 +90,9 @@ module API
       delete ':id' do
         message = find_message
 
-        present message.destroy, with: Entities::BroadcastMessage
+        ressource_modified_since(message.updated_at)
+
+        message.destroy
       end
     end
   end
diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb
index 3f5183d46a2..feb45df0924 100644
--- a/lib/api/deploy_keys.rb
+++ b/lib/api/deploy_keys.rb
@@ -87,31 +87,30 @@ module API
 
       desc 'Disable a deploy key for a project' do
         detail 'This feature was added in GitLab 8.11'
-        success Entities::SSHKey
       end
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
       end
       delete ":id/deploy_keys/:key_id/disable" do
         key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])
-        key.destroy
+        not_found!('Deploy Key') unless key
 
-        present key.deploy_key, with: Entities::SSHKey
-      end
+        ressource_modified_since(key.updated_at)
 
-      desc 'Delete deploy key for a project' do
-        success Key
+        key.destroy
       end
+
+      desc 'Delete deploy key for a project'
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
       end
       delete ":id/deploy_keys/:key_id" do
         key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])
-        if key
-          key.destroy
-        else
-          not_found!('Deploy Key')
-        end
+        not_found!('Deploy Key') unless key
+
+        ressource_modified_since(key.updated_at)
+
+        key.destroy
       end
     end
   end
diff --git a/lib/api/environments.rb b/lib/api/environments.rb
index 1a7e68f0528..b9527561117 100644
--- a/lib/api/environments.rb
+++ b/lib/api/environments.rb
@@ -69,7 +69,6 @@ module API
 
       desc 'Deletes an existing environment' do
         detail 'This feature was introduced in GitLab 8.11.'
-        success Entities::Environment
       end
       params do
         requires :environment_id, type: Integer,  desc: 'The environment ID'
@@ -78,8 +77,9 @@ module API
         authorize! :update_environment, user_project
 
         environment = user_project.environments.find(params[:environment_id])
+        ressource_modified_since(environment.updated_at)
 
-        present environment.destroy, with: Entities::Environment
+        environment.destroy
       end
     end
   end
diff --git a/lib/api/files.rb b/lib/api/files.rb
index 2ecdd747c8e..7ef32ad0b43 100644
--- a/lib/api/files.rb
+++ b/lib/api/files.rb
@@ -116,13 +116,12 @@ module API
       delete ":id/repository/files" do
         authorize! :push_code, user_project
 
+        # TODO: Modified since?
+
         file_params = declared_params(include_missing: false)
         result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute
 
-        if result[:status] == :success
-          status(200)
-          commit_response(file_params)
-        else
+        if result[:status] != :success
           render_api_error!(result[:message], 400)
         end
       end
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 9f29c4466ab..4ff1df7a5e8 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -126,6 +126,9 @@ module API
       delete ":id" do
         group = find_group!(params[:id])
         authorize! :admin_group, group
+
+        ressource_modified_since(group.updated_at)
+
         ::Groups::DestroyService.new(group, current_user).execute
       end
 
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 13896dd91b9..b96cfc876f1 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -180,6 +180,13 @@ module API
       items.where(iid: iid)
     end
 
+    def ressource_modified_since(last_modified)
+      if_unmodified_since = Time.parse(headers['If-Unmodified-Since']) if headers.key?('If-Unmodified-Since') rescue nil
+      if if_unmodified_since && last_modified > if_unmodified_since
+        conflict!('Ressource Modified Since')
+      end
+    end
+
     # error helpers
 
     def forbidden!(reason = nil)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 90fca20d4fa..ffea0b132d0 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -219,6 +219,8 @@ module API
         not_found!('Issue') unless issue
 
         authorize!(:destroy_issue, issue)
+        ressource_modified_since(issue.updated_at)
+        
         issue.destroy
       end
     end
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index 652786d4e3e..471a0d72854 100644
--- a/lib/api/labels.rb
+++ b/lib/api/labels.rb
@@ -52,7 +52,9 @@ module API
         label = user_project.labels.find_by(title: params[:name])
         not_found!('Label') unless label
 
-        present label.destroy, with: Entities::Label, current_user: current_user, project: user_project
+        ressource_modified_since(label.updated_at)
+
+        label.destroy
       end
 
       desc 'Update an existing label. At least one optional parameter is required.' do
diff --git a/lib/api/members.rb b/lib/api/members.rb
index d1d78775c6d..69a4ef6ad3b 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -105,6 +105,7 @@ module API
           # This is to ensure back-compatibility but find_by! should be used
           # in that casse in 9.0!
           member = source.members.find_by(user_id: params[:user_id])
+          ressource_modified_since(member.updated_at)
 
           # This is to ensure back-compatibility but this should be removed in
           # favor of find_by! in 9.0!
@@ -116,8 +117,6 @@ module API
             { message: "Access revoked", id: params[:user_id].to_i }
           else
             ::Members::DestroyService.new(source, current_user, declared_params).execute
-
-            present member.user, with: Entities::Member, member: member
           end
         end
       end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 8e09a6f7354..000899c07ad 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -99,6 +99,8 @@ module API
         merge_request = find_project_merge_request(params[:merge_request_id])
 
         authorize!(:destroy_merge_request, merge_request)
+        ressource_modified_since(merge_request.updated_at)
+
         merge_request.destroy
       end
 
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 8beccaaabd1..28b4b1f2b88 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -120,9 +120,7 @@ module API
           end
         end
 
-        desc 'Delete a +noteable+ note' do
-          success Entities::Note
-        end
+        desc 'Delete a +noteable+ note'
         params do
           requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
           requires :note_id, type: Integer, desc: 'The ID of a note'
@@ -131,9 +129,8 @@ module API
           note = user_project.notes.find(params[:note_id])
           authorize! :admin_note, note
 
+          ressource_modified_since(note.updated_at)
           ::Notes::DestroyService.new(user_project, current_user).execute(note)
-
-          present note, with: Entities::Note
         end
       end
     end
diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb
index cb679e6658a..2d9bb3def62 100644
--- a/lib/api/project_hooks.rb
+++ b/lib/api/project_hooks.rb
@@ -85,19 +85,16 @@ module API
         end
       end
 
-      desc 'Deletes project hook' do
-        success Entities::ProjectHook
-      end
+      desc 'Deletes project hook'
       params do
         requires :hook_id, type: Integer, desc: 'The ID of the hook to delete'
       end
       delete ":id/hooks/:hook_id" do
-        begin
-          present user_project.hooks.destroy(params[:hook_id]), with: Entities::ProjectHook
-        rescue
-          # ProjectHook can raise Error if hook_id not found
-          not_found!("Error deleting hook #{params[:hook_id]}")
-        end
+        hook = user_project.hooks.find_by(id: params[:hook_id])
+        not_found!('Project Hook') unless label
+
+        ressource_modified_since(hook.updated_at)
+        hook.destroy
       end
     end
   end
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index dcc0c82ee27..0339d8347dc 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -113,6 +113,8 @@ module API
         not_found!('Snippet') unless snippet
 
         authorize! :admin_project_snippet, snippet
+        ressource_modified_since(snippet.updated_at)
+
         snippet.destroy
       end
 
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 68c2732ec80..0ca71765f58 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -267,6 +267,7 @@ module API
         success Entities::Project
       end
       delete ':id/star' do
+        # Todo should be pist
         if current_user.starred?(user_project)
           current_user.toggle_star(user_project)
           user_project.reload
@@ -280,6 +281,8 @@ module API
       desc 'Remove a project'
       delete ":id" do
         authorize! :remove_project, user_project
+        ressource_modified_since(user_project.updated_at)
+
         ::Projects::DestroyService.new(user_project, current_user, {}).async_execute
       end
 
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index 4816b5ed1b7..1e223f870a7 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -76,10 +76,11 @@ module API
       end
       delete ':id' do
         runner = get_runner(params[:id])
+
         authenticate_delete_runner!(runner)
-        runner.destroy!
+        ressource_modified_since(runner.updated_at)
 
-        present runner, with: Entities::Runner
+        runner.destroy
       end
     end
 
@@ -133,10 +134,9 @@ module API
 
         runner = runner_project.runner
         forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.projects.count == 1
+        ressource_modified_since(runner_project.updated_at)
 
         runner_project.destroy
-
-        present runner, with: Entities::Runner
       end
     end
 
diff --git a/lib/api/services.rb b/lib/api/services.rb
index 1456fe4688b..eb7ce2f89b3 100644
--- a/lib/api/services.rb
+++ b/lib/api/services.rb
@@ -636,6 +636,8 @@ module API
       end
       delete ":id/services/:service_slug" do
         service = user_project.find_or_initialize_service(params[:service_slug].underscore)
+        # Todo: do we really neeed to call find_or_initialize_service?
+        ressource_modified_since(service.updated_at)
 
         attrs = service_attributes(service).inject({}) do |hash, key|
           hash.merge!(key => nil)
diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb
index eb9ece49e7f..7d14241df3c 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -113,9 +113,11 @@ module API
       delete ':id' do
         snippet = snippets_for_current_user.find_by(id: params.delete(:id))
         return not_found!('Snippet') unless snippet
+
         authorize! :destroy_personal_snippet, snippet
+        ressource_modified_since(snippet.updated_at)
+
         snippet.destroy
-        no_content!
       end
 
       desc 'Get a raw snippet' do
diff --git a/lib/api/subscriptions.rb b/lib/api/subscriptions.rb
index e11d7537cc9..51d8b7361a8 100644
--- a/lib/api/subscriptions.rb
+++ b/lib/api/subscriptions.rb
@@ -36,6 +36,7 @@ module API
           success entity_class
         end
         delete ":id/#{type}/:subscribable_id/subscription" do
+          # Todo: This should be a post
           resource = instance_exec(params[:subscribable_id], &finder)
 
           if !resource.subscribed?(current_user, user_project)
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 708ec8cfe70..18c02d1b0f6 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -54,9 +54,7 @@ module API
         data
       end
 
-      desc 'Delete a hook' do
-        success Entities::Hook
-      end
+      desc 'Delete a hook'
       params do
         requires :id, type: Integer, desc: 'The ID of the system hook'
       end
@@ -64,7 +62,9 @@ module API
         hook = SystemHook.find_by(id: params[:id])
         not_found!('System hook') unless hook
 
-        present hook.destroy, with: Entities::Hook
+        ressource_modified_since(hook.updated_at)
+
+        hook.destroy
       end
     end
   end
diff --git a/lib/api/tags.rb b/lib/api/tags.rb
index b6fd8f569a9..3afb833fa2c 100644
--- a/lib/api/tags.rb
+++ b/lib/api/tags.rb
@@ -59,14 +59,12 @@ module API
       delete ":id/repository/tags/:tag_name", requirements: { tag_name: /.+/ } do
         authorize_push_project
 
+        #Todo: Modified since
+
         result = ::Tags::DestroyService.new(user_project, current_user).
           execute(params[:tag_name])
 
-        if result[:status] == :success
-          {
-            tag_name: params[:tag_name]
-          }
-        else
+        if result[:status] != :success
           render_api_error!(result[:message], result[:return_code])
         end
       end
diff --git a/lib/api/todos.rb b/lib/api/todos.rb
index 9bd077263a7..02c9b3b47ee 100644
--- a/lib/api/todos.rb
+++ b/lib/api/todos.rb
@@ -59,6 +59,7 @@ module API
         requires :id, type: Integer, desc: 'The ID of the todo being marked as done'
       end
       delete ':id' do
+        # should be a post
         todo = current_user.todos.find(params[:id])
         TodoService.new.mark_todos_as_done([todo], current_user)
 
@@ -67,6 +68,7 @@ module API
 
       desc 'Mark all todos as done'
       delete do
+        # should be a post
         todos = find_todos
         TodoService.new.mark_todos_as_done(todos, current_user)
       end
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 87a717ba751..88590299717 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -84,9 +84,7 @@ module API
         present trigger, with: Entities::Trigger
       end
 
-      desc 'Delete a trigger' do
-        success Entities::Trigger
-      end
+      desc 'Delete a trigger'
       params do
         requires :token, type: String, desc: 'The unique token of trigger'
       end
@@ -97,9 +95,9 @@ module API
         trigger = user_project.triggers.find_by(token: params[:token].to_s)
         return not_found!('Trigger') unless trigger
 
-        trigger.destroy
+        ressource_modified_since(trigger.updated_at)
 
-        present trigger, with: Entities::Trigger
+        trigger.destroy
       end
     end
   end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 82ac3886ac3..cd7c8375ab1 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -219,9 +219,7 @@ module API
         present user.keys, with: Entities::SSHKey
       end
 
-      desc 'Delete an existing SSH key from a specified user. Available only for admins.' do
-        success Entities::SSHKey
-      end
+      desc 'Delete an existing SSH key from a specified user. Available only for admins.'
       params do
         requires :id, type: Integer, desc: 'The ID of the user'
         requires :key_id, type: Integer, desc: 'The ID of the SSH key'
@@ -235,7 +233,9 @@ module API
         key = user.keys.find_by(id: params[:key_id])
         not_found!('Key') unless key
 
-        present key.destroy, with: Entities::SSHKey
+        ressource_modified_since(key.updated_at)
+
+        key.destroy
       end
 
       desc 'Add an email address to a specified user. Available only for admins.' do
@@ -290,6 +290,8 @@ module API
         email = user.emails.find_by(id: params[:email_id])
         not_found!('Email') unless email
 
+        ressource_modified_since(email.updated_at)
+
         email.destroy
         user.update_secondary_emails!
       end
@@ -305,6 +307,8 @@ module API
         user = User.find_by(id: params[:id])
         not_found!('User') unless user
 
+        ressource_modified_since(user.updated_at)
+
         ::Users::DestroyService.new(current_user).execute(user)
       end
 
@@ -407,9 +411,7 @@ module API
         end
       end
 
-      desc 'Delete an SSH key from the currently authenticated user' do
-        success Entities::SSHKey
-      end
+      desc 'Delete an SSH key from the currently authenticated user'
       params do
         requires :key_id, type: Integer, desc: 'The ID of the SSH key'
       end
@@ -417,7 +419,9 @@ module API
         key = current_user.keys.find_by(id: params[:key_id])
         not_found!('Key') unless key
 
-        present key.destroy, with: Entities::SSHKey
+        ressource_modified_since(key.updated_at)
+
+        key.destroy
       end
 
       desc "Get the currently authenticated user's email addresses" do
@@ -465,6 +469,8 @@ module API
         email = current_user.emails.find_by(id: params[:email_id])
         not_found!('Email') unless email
 
+        ressource_modified_since(email.updated_at)
+
         email.destroy
         current_user.update_secondary_emails!
       end
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index f623b1dfe9f..c42220e1406 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -73,18 +73,17 @@ module API
         end
       end
 
-      desc 'Delete an existing variable from a project' do
-        success Entities::Variable
-      end
+      desc 'Delete an existing variable from a project'
       params do
         requires :key, type: String, desc: 'The key of the variable'
       end
       delete ':id/variables/:key' do
         variable = user_project.variables.find_by(key: params[:key])
-
         return not_found!('Variable') unless variable
 
-        present variable.destroy, with: Entities::Variable
+        ressource_modified_since(variable.updated_at)
+
+        variable.destroy
       end
     end
   end