summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--lib/api/access_requests.rb11
-rw-r--r--lib/api/award_emoji.rb4
-rw-r--r--lib/api/boards.rb11
-rw-r--r--lib/api/broadcast_messages.rb4
-rw-r--r--lib/api/deploy_keys.rb5
-rw-r--r--lib/api/environments.rb4
-rw-r--r--lib/api/groups.rb7
-rw-r--r--lib/api/helpers.rb17
-rw-r--r--lib/api/issues.rb4
-rw-r--r--lib/api/labels.rb5
-rw-r--r--lib/api/members.rb7
-rw-r--r--lib/api/merge_requests.rb4
-rw-r--r--lib/api/notes.rb6
-rw-r--r--lib/api/project_hooks.rb5
-rw-r--r--lib/api/project_snippets.rb4
-rw-r--r--lib/api/projects.rb5
-rw-r--r--lib/api/runner.rb6
-rw-r--r--lib/api/runners.rb7
-rw-r--r--lib/api/services.rb4
-rw-r--r--lib/api/snippets.rb4
-rw-r--r--lib/api/system_hooks.rb5
-rw-r--r--lib/api/triggers.rb5
-rw-r--r--lib/api/users.rb47
-rw-r--r--spec/requests/api/tags_spec.rb4
24 files changed, 71 insertions, 114 deletions
diff --git a/lib/api/access_requests.rb b/lib/api/access_requests.rb
index 0c5b8862d79..4fa9b2b2494 100644
--- a/lib/api/access_requests.rb
+++ b/lib/api/access_requests.rb
@@ -67,13 +67,12 @@ module API
end
delete ":id/access_requests/:user_id" do
source = find_source(source_type, params[:id])
- member = source.public_send(:requesters).find_by!(user_id: params[:user_id])
+ member = source.requesters.find_by!(user_id: params[:user_id])
- check_unmodified_since(member.updated_at)
-
- status 204
- ::Members::DestroyService.new(source, current_user, params)
- .execute(:requesters)
+ destroy_conditionally!(member) do
+ ::Members::DestroyService.new(source, current_user, params)
+ .execute(:requesters)
+ end
end
end
end
diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb
index 51a8587d26e..8e3851640da 100644
--- a/lib/api/award_emoji.rb
+++ b/lib/api/award_emoji.rb
@@ -85,12 +85,10 @@ module API
end
delete "#{endpoint}/:award_id" do
award = awardable.award_emoji.find(params[:award_id])
- check_unmodified_since(award.updated_at)
unauthorized! unless award.user == current_user || current_user.admin?
- status 204
- award.destroy
+ destroy_conditionally!(award)
end
end
end
diff --git a/lib/api/boards.rb b/lib/api/boards.rb
index d36df77dc6c..0d11c5fc971 100644
--- a/lib/api/boards.rb
+++ b/lib/api/boards.rb
@@ -122,14 +122,13 @@ module API
end
delete "/lists/:list_id" do
authorize!(:admin_list, user_project)
-
list = board_lists.find(params[:list_id])
- check_unmodified_since(list.updated_at)
-
- service = ::Boards::Lists::DestroyService.new(user_project, current_user)
- unless service.execute(list)
- render_api_error!({ error: 'List could not be deleted!' }, 400)
+ destroy_conditionally!(list) do |list|
+ service = ::Boards::Lists::DestroyService.new(user_project, current_user)
+ unless service.execute(list)
+ render_api_error!({ error: 'List could not be deleted!' }, 400)
+ end
end
end
end
diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb
index 352972b584a..0b45621ce7b 100644
--- a/lib/api/broadcast_messages.rb
+++ b/lib/api/broadcast_messages.rb
@@ -90,10 +90,8 @@ module API
end
delete ':id' do
message = find_message
- check_unmodified_since(message.updated_at)
- status 204
- message.destroy
+ destroy_conditionally!(message)
end
end
end
diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb
index 971cc816454..f405c341398 100644
--- a/lib/api/deploy_keys.rb
+++ b/lib/api/deploy_keys.rb
@@ -125,10 +125,7 @@ module API
key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id])
not_found!('Deploy Key') unless key
- check_unmodified_since(key.updated_at)
-
- status 204
- key.destroy
+ destroy_conditionally!(key)
end
end
end
diff --git a/lib/api/environments.rb b/lib/api/environments.rb
index 3fc423ae79a..e33269f9483 100644
--- a/lib/api/environments.rb
+++ b/lib/api/environments.rb
@@ -78,10 +78,8 @@ module API
authorize! :update_environment, user_project
environment = user_project.environments.find(params[:environment_id])
- check_unmodified_since(environment.updated_at)
- status 204
- environment.destroy
+ destroy_conditionally!(environment)
end
desc 'Stops an existing environment' do
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index c9b32a85487..ee2ad27837b 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -117,11 +117,10 @@ module API
delete ":id" do
group = find_group!(params[:id])
authorize! :admin_group, group
-
- check_unmodified_since(group.updated_at)
- status 204
- ::Groups::DestroyService.new(group, current_user).execute
+ destroy_conditionally!(group) do |group|
+ ::Groups::DestroyService.new(group, current_user).execute
+ end
end
desc 'Get a list of projects in this group.' do
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 1c74a14d91c..8d4f8c01903 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -11,14 +11,25 @@ module API
declared(params, options).to_h.symbolize_keys
end
- def check_unmodified_since(last_modified)
- if_unmodified_since = Time.parse(headers['If-Unmodified-Since']) if headers.key?('If-Unmodified-Since') rescue nil
+ def check_unmodified_since!(last_modified)
+ if_unmodified_since = Time.parse(headers['If-Unmodified-Since']) rescue nil
- if if_unmodified_since && if_unmodified_since < last_modified
+ if if_unmodified_since && last_modified > if_unmodified_since
render_api_error!('412 Precondition Failed', 412)
end
end
+ def destroy_conditionally!(resource, last_update_field: :updated_at)
+ check_unmodified_since!(resource.public_send(last_update_field))
+
+ status 204
+ if block_given?
+ yield resource
+ else
+ resource.destroy
+ end
+ end
+
def current_user
return @current_user if defined?(@current_user)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index cee9898d3a6..6503629e2a2 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -230,10 +230,8 @@ module API
not_found!('Issue') unless issue
authorize!(:destroy_issue, issue)
- check_unmodified_since(issue.updated_at)
- status 204
- issue.destroy
+ destroy_conditionally!(issue)
end
desc 'List merge requests closing issue' do
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index 45fa57fdf55..c0cf618ee8d 100644
--- a/lib/api/labels.rb
+++ b/lib/api/labels.rb
@@ -56,10 +56,7 @@ module API
label = user_project.labels.find_by(title: params[:name])
not_found!('Label') unless label
- check_unmodified_since(label.updated_at)
-
- status 204
- label.destroy
+ destroy_conditionally!(label)
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 5634f123eca..a5d3d7f25a0 100644
--- a/lib/api/members.rb
+++ b/lib/api/members.rb
@@ -93,12 +93,11 @@ module API
end
delete ":id/members/:user_id" do
source = find_source(source_type, params[:id])
- # Ensure that memeber exists
member = source.members.find_by!(user_id: params[:user_id])
- check_unmodified_since(member.updated_at)
- status 204
- ::Members::DestroyService.new(source, current_user, declared_params).execute
+ destroy_conditionally!(member) do
+ ::Members::DestroyService.new(source, current_user, declared_params).execute
+ end
end
end
end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index c6fecc1aa6c..969c6064662 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -164,10 +164,8 @@ module API
merge_request = find_project_merge_request(params[:merge_request_iid])
authorize!(:destroy_merge_request, merge_request)
- check_unmodified_since(merge_request.updated_at)
- status 204
- merge_request.destroy
+ destroy_conditionally!(merge_request)
end
params do
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 58d71787aca..e116448c15b 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -131,10 +131,10 @@ module API
note = user_project.notes.find(params[:note_id])
authorize! :admin_note, note
- check_unmodified_since(note.updated_at)
- status 204
- ::Notes::DestroyService.new(user_project, current_user).execute(note)
+ destroy_conditionally!(note) do |note|
+ ::Notes::DestroyService.new(user_project, current_user).execute(note)
+ end
end
end
end
diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb
index 74d736fda59..5b457bbe639 100644
--- a/lib/api/project_hooks.rb
+++ b/lib/api/project_hooks.rb
@@ -96,10 +96,7 @@ module API
delete ":id/hooks/:hook_id" do
hook = user_project.hooks.find(params.delete(:hook_id))
- check_unmodified_since(hook.updated_at)
-
- status 204
- hook.destroy
+ destroy_conditionally!(hook)
end
end
end
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index 645162d564d..704e8c6718d 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -116,10 +116,8 @@ module API
not_found!('Snippet') unless snippet
authorize! :admin_project_snippet, snippet
- check_unmodified_since(snippet.updated_at)
- status 204
- snippet.destroy
+ destroy_conditionally!(snippet)
end
desc 'Get a raw project snippet'
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index eab0ca0b3c9..36fe3f243b9 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -334,9 +334,10 @@ module API
desc 'Remove a project'
delete ":id" do
authorize! :remove_project, user_project
- check_unmodified_since(user_project.updated_at)
- ::Projects::DestroyService.new(user_project, current_user, {}).async_execute
+ destroy_conditionally!(user_project) do
+ ::Projects::DestroyService.new(user_project, current_user, {}).async_execute
+ end
accepted!
end
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 88fc62d33df..daa8ddbe251 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -45,8 +45,10 @@ module API
end
delete '/' do
authenticate_runner!
- status 204
- Ci::Runner.find_by_token(params[:token]).destroy
+
+ runner = Ci::Runner.find_by_token(params[:token])
+
+ destroy_conditionally!(runner)
end
desc 'Validates authentication credentials' do
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index e3b2eb904b7..68c2120cc15 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -79,10 +79,8 @@ module API
runner = get_runner(params[:id])
authenticate_delete_runner!(runner)
- check_unmodified_since(runner.updated_at)
- status 204
- runner.destroy!
+ destroy_conditionally!(runner)
end
end
@@ -137,8 +135,7 @@ module API
runner = runner_project.runner
forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.projects.count == 1
- status 204
- runner_project.destroy
+ destroy_conditionally!(runner_project)
end
end
diff --git a/lib/api/services.rb b/lib/api/services.rb
index 4fef3383e5e..2b5ef75b6bf 100644
--- a/lib/api/services.rb
+++ b/lib/api/services.rb
@@ -655,8 +655,8 @@ module API
end
delete ":id/services/:service_slug" do
service = user_project.find_or_initialize_service(params[:service_slug].underscore)
- # Todo, not sure
- check_unmodified_since(service.updated_at)
+ # Todo: Check if this done the right way
+ check_unmodified_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 7107b3d669c..00eb7c60f16 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -122,10 +122,8 @@ module API
return not_found!('Snippet') unless snippet
authorize! :destroy_personal_snippet, snippet
- check_unmodified_since(snippet.updated_at)
- status 204
- snippet.destroy
+ destroy_conditionally!(snippet)
end
desc 'Get a raw snippet' do
diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb
index 64066a75b15..6b6a03e3300 100644
--- a/lib/api/system_hooks.rb
+++ b/lib/api/system_hooks.rb
@@ -66,10 +66,7 @@ module API
hook = SystemHook.find_by(id: params[:id])
not_found!('System hook') unless hook
- check_unmodified_since(hook.updated_at)
-
- status 204
- hook.destroy
+ destroy_conditionally!(hook)
end
end
end
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 4ae70c65759..c9fee7e5193 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -140,10 +140,7 @@ module API
trigger = user_project.triggers.find(params.delete(:trigger_id))
return not_found!('Trigger') unless trigger
- check_unmodified_since(trigger.updated_at)
-
- status 204
- trigger.destroy
+ destroy_conditionally!(trigger)
end
end
end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 942bb72cf97..d7c7b9ae9c1 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -230,13 +230,7 @@ module API
key = user.keys.find_by(id: params[:key_id])
not_found!('Key') unless key
-<<<<<<< HEAD
- status 204
-=======
- check_unmodified_since(key.updated_at)
-
->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints
- key.destroy
+ destroy_conditionally!(key)
end
desc 'Add an email address to a specified user. Available only for admins.' do
@@ -292,14 +286,11 @@ module API
email = user.emails.find_by(id: params[:email_id])
not_found!('Email') unless email
-<<<<<<< HEAD
- Emails::DestroyService.new(user, email: email.email).execute
-=======
- check_unmodified_since(email.updated_at)
+ destroy_conditionally!(email) do |email|
+ Emails::DestroyService.new(current_user, email: email.email).execute
+ end
- email.destroy
user.update_secondary_emails!
->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints
end
desc 'Delete a user. Available only for admins.' do
@@ -315,14 +306,9 @@ module API
user = User.find_by(id: params[:id])
not_found!('User') unless user
-<<<<<<< HEAD
- status 204
- user.delete_async(deleted_by: current_user, params: params)
-=======
- check_unmodified_since(user.updated_at)
-
- ::Users::DestroyService.new(current_user).execute(user)
->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints
+ destroy_conditionally!(user) do
+ user.delete_async(deleted_by: current_user, params: params)
+ end
end
desc 'Block a user. Available only for admins.'
@@ -500,10 +486,7 @@ module API
key = current_user.keys.find_by(id: params[:key_id])
not_found!('Key') unless key
- check_unmodified_since(key.updated_at)
-
- status 204
- key.destroy
+ destroy_conditionally!(key)
end
desc "Get the currently authenticated user's email addresses" do
@@ -554,9 +537,11 @@ module API
email = current_user.emails.find_by(id: params[:email_id])
not_found!('Email') unless email
-<<<<<<< HEAD
- status 204
- Emails::DestroyService.new(current_user, email: email.email).execute
+ destroy_conditionally!(email) do |email|
+ Emails::DestroyService.new(current_user, email: email.email).execute
+ end
+
+ current_user.update_secondary_emails!
end
desc 'Get a list of user activities'
@@ -572,12 +557,6 @@ module API
.reorder(last_activity_on: :asc)
present paginate(activities), with: Entities::UserActivity
-=======
- check_unmodified_since(email.updated_at)
-
- email.destroy
- current_user.update_secondary_emails!
->>>>>>> API: Respect the 'If-Unmodified-Since' for delete endpoints
end
end
end
diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb
index 9884c1ec206..54900c75eb8 100644
--- a/spec/requests/api/tags_spec.rb
+++ b/spec/requests/api/tags_spec.rb
@@ -283,7 +283,7 @@ describe API::Tags do
it_behaves_like '404 response' do
let(:request) { delete api(route, current_user) }
- let(:message) { 'No such tag' }
+ let(:message) { '404 Tag Not Found' }
end
end
@@ -394,7 +394,7 @@ describe API::Tags do
it_behaves_like '404 response' do
let(:request) { put api(route, current_user), description: new_description }
- let(:message) { 'Tag does not exist' }
+ let(:message) { '404 Tag Not Found' }
end
end