summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2017-03-31 16:59:46 -0700
committerStan Hu <stanhu@gmail.com>2017-04-02 05:37:04 -0700
commit8a71d40e60c799f969af0ed255e931b6c9907634 (patch)
tree2737ce98e11f3c8c612d5fa6266232757cc194c0
parent9fc17f6f4abdb04f3cf1b60b87bd67b894a19c39 (diff)
downloadgitlab-ce-8a71d40e60c799f969af0ed255e931b6c9907634.tar.gz
Fix race condition where a namespace would be deleted before a project was deleted
When deleting a user, the following sequence could happen: 1. Project `mygroup/myproject` is scheduled for deletion 2. The group `mygroup` is deleted 3. Sidekiq worker runs to delete `mygroup/myproject`, but the namespace and routes have been destroyed. Closes #30334
-rw-r--r--app/services/users/destroy_service.rb4
-rw-r--r--changelogs/unreleased/sh-fix-destroy-user-race.yml5
-rw-r--r--spec/services/users/destroy_spec.rb19
3 files changed, 24 insertions, 4 deletions
diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb
index 833da5bc5d1..a3b32a71a64 100644
--- a/app/services/users/destroy_service.rb
+++ b/app/services/users/destroy_service.rb
@@ -20,10 +20,10 @@ module Users
Groups::DestroyService.new(group, current_user).execute
end
- user.personal_projects.each do |project|
+ user.personal_projects.with_deleted.each do |project|
# Skip repository removal because we remove directory with namespace
# that contain all this repositories
- ::Projects::DestroyService.new(project, current_user, skip_repo: true).async_execute
+ ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
end
move_issues_to_ghost_user(user)
diff --git a/changelogs/unreleased/sh-fix-destroy-user-race.yml b/changelogs/unreleased/sh-fix-destroy-user-race.yml
new file mode 100644
index 00000000000..4d650b51ada
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-destroy-user-race.yml
@@ -0,0 +1,5 @@
+---
+title: Fix race condition where a namespace would be deleted before a project was
+ deleted
+merge_request:
+author:
diff --git a/spec/services/users/destroy_spec.rb b/spec/services/users/destroy_spec.rb
index 9a28c03d968..66c61b7f8ff 100644
--- a/spec/services/users/destroy_spec.rb
+++ b/spec/services/users/destroy_spec.rb
@@ -17,13 +17,28 @@ describe Users::DestroyService, services: true do
expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound)
end
- it 'will delete the project in the near future' do
- expect_any_instance_of(Projects::DestroyService).to receive(:async_execute).once
+ it 'will delete the project' do
+ expect_any_instance_of(Projects::DestroyService).to receive(:execute).once
service.execute(user)
end
end
+ context 'projects in pending_delete' do
+ before do
+ project.pending_delete = true
+ project.save
+ end
+
+ it 'destroys a project in pending_delete' do
+ expect_any_instance_of(Projects::DestroyService).to receive(:execute).once
+
+ service.execute(user)
+
+ expect { Project.find(project.id) }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+
context "a deleted user's issues" do
let(:project) { create(:project) }