diff options
author | Stan Hu <stanhu@gmail.com> | 2018-05-18 04:05:11 +0300 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2018-05-18 04:05:11 +0300 |
commit | 6c190d273d18d21e50dea65645185839bf067714 (patch) | |
tree | b1a67f0dfec96ab4ada3e5363317ce408269e46b | |
parent | d6c8a55189d62430c7ca4ffa6e5bb63f15a7efc1 (diff) |
Move API group deletion to Sidekiq
-rw-r--r-- | changelogs/unreleased/sh-move-delete-groups-api-async.yml | 5 | ||||
-rw-r--r-- | doc/api/groups.md | 3 | ||||
-rw-r--r-- | lib/api/groups.rb | 4 | ||||
-rw-r--r-- | lib/api/v3/groups.rb | 5 | ||||
-rw-r--r-- | spec/requests/api/groups_spec.rb | 9 | ||||
-rw-r--r-- | spec/requests/api/v3/groups_spec.rb | 8 |
6 files changed, 25 insertions, 9 deletions
diff --git a/changelogs/unreleased/sh-move-delete-groups-api-async.yml b/changelogs/unreleased/sh-move-delete-groups-api-async.yml new file mode 100644 index 00000000000..1b200cac5c5 --- /dev/null +++ b/changelogs/unreleased/sh-move-delete-groups-api-async.yml @@ -0,0 +1,5 @@ +--- +title: Move API group deletion to Sidekiq +merge_request: +author: +type: changed diff --git a/doc/api/groups.md b/doc/api/groups.md index 923fd662a5b..96842ef330f 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -487,6 +487,9 @@ Parameters: - `id` (required) - The ID or path of a user group +This will queue a background job to delete all projects in the group. The +response will be a 202 Accepted if the user has authorization. + ## Search for group Get all groups that match your string in their name or path. diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 0d125cd7831..03b6b30a0d8 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -167,8 +167,10 @@ module API Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285') destroy_conditionally!(group) do |group| - ::Groups::DestroyService.new(group, current_user).execute + ::Groups::DestroyService.new(group, current_user).async_execute end + + accepted! end desc 'Get a list of projects in this group.' do diff --git a/lib/api/v3/groups.rb b/lib/api/v3/groups.rb index 3844fd4810d..4fa7d196e50 100644 --- a/lib/api/v3/groups.rb +++ b/lib/api/v3/groups.rb @@ -131,8 +131,9 @@ module API delete ":id" do group = find_group!(params[:id]) authorize! :admin_group, group - Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285') - present ::Groups::DestroyService.new(group, current_user).execute, with: Entities::GroupDetail, current_user: current_user + ::Groups::DestroyService.new(group, current_user).async_execute + + accepted! end desc 'Get a list of projects in this group.' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index bb0034e3237..7d923932309 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -738,13 +738,16 @@ describe API::Groups do describe "DELETE /groups/:id" do context "when authenticated as user" do it "removes group" do - delete api("/groups/#{group1.id}", user1) + Sidekiq::Testing.fake! do + expect { delete api("/groups/#{group1.id}", user1) }.to change(GroupDestroyWorker.jobs, :size).by(1) + end - expect(response).to have_gitlab_http_status(204) + expect(response).to have_gitlab_http_status(202) end it_behaves_like '412 response' do let(:request) { api("/groups/#{group1.id}", user1) } + let(:success_status) { 202 } end it "does not remove a group if not an owner" do @@ -773,7 +776,7 @@ describe API::Groups do it "removes any existing group" do delete api("/groups/#{group2.id}", admin) - expect(response).to have_gitlab_http_status(204) + expect(response).to have_gitlab_http_status(202) end it "does not remove a non existing group" do diff --git a/spec/requests/api/v3/groups_spec.rb b/spec/requests/api/v3/groups_spec.rb index a1cdf583de3..34d4b8e9565 100644 --- a/spec/requests/api/v3/groups_spec.rb +++ b/spec/requests/api/v3/groups_spec.rb @@ -458,9 +458,11 @@ describe API::V3::Groups do describe "DELETE /groups/:id" do context "when authenticated as user" do it "removes group" do - delete v3_api("/groups/#{group1.id}", user1) + Sidekiq::Testing.fake! do + expect { delete v3_api("/groups/#{group1.id}", user1) }.to change(GroupDestroyWorker.jobs, :size).by(1) + end - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(202) end it "does not remove a group if not an owner" do @@ -489,7 +491,7 @@ describe API::V3::Groups do it "removes any existing group" do delete v3_api("/groups/#{group2.id}", admin) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(202) end it "does not remove a non existing group" do |