Merge branch 'sh-fix-project-destroy-in-namespace' into 'master'

Defer project destroys within a namespace in Groups::DestroyService#async_execute

See merge request !12435

2 files changed, 43 insertions, 20 deletions

diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index d4f898f6d9f..62c4ea01ce1 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -342,6 +342,17 @@ describe Namespace, models: true do
     end
   end
 
+  describe '#soft_delete_without_removing_associations' do
+    let(:project1) { create(:project_empty_repo, namespace: namespace) }
+
+    it 'updates the deleted_at timestamp but preserves projects' do
+      namespace.soft_delete_without_removing_associations
+
+      expect(Project.all).to include(project1)
+      expect(namespace.deleted_at).not_to be_nil
+    end
+  end
+
   describe '#user_ids_for_project_authorizations' do
     it 'returns the user IDs for which to refresh authorizations' do
       expect(namespace.user_ids_for_project_authorizations)
diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index a37257d1bf4..d59b37bee36 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -15,6 +15,14 @@ describe Groups::DestroyService, services: true do
     group.add_user(user, Gitlab::Access::OWNER)
   end
 
+  def destroy_group(group, user, async)
+    if async
+      Groups::DestroyService.new(group, user).async_execute
+    else
+      Groups::DestroyService.new(group, user).execute
+    end
+  end
+
   shared_examples 'group destruction' do |async|
     context 'database records' do
       before do
@@ -30,30 +38,14 @@ describe Groups::DestroyService, services: true do
     context 'file system' do
       context 'Sidekiq inline' do
         before do
-          # Run sidekiq immediatly to check that renamed dir will be removed
+          # Run sidekiq immediately to check that renamed dir will be removed
           Sidekiq::Testing.inline! { destroy_group(group, user, async) }
         end
 
-        it { expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey }
-        it { expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey }
-      end
-
-      context 'Sidekiq fake' do
-        before do
-          # Don't run sidekiq to check if renamed repository exists
-          Sidekiq::Testing.fake! { destroy_group(group, user, async) }
+        it 'verifies that paths have been deleted' do
+          expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey
+          expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey
         end
-
-        it { expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey }
-        it { expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_truthy }
-      end
-    end
-
-    def destroy_group(group, user, async)
-      if async
-        Groups::DestroyService.new(group, user).async_execute
-      else
-        Groups::DestroyService.new(group, user).execute
       end
     end
   end
@@ -61,6 +53,26 @@ describe Groups::DestroyService, services: true do
   describe 'asynchronous delete' do
     it_behaves_like 'group destruction', true
 
+    context 'Sidekiq fake' do
+      before do
+        # Don't run Sidekiq to verify that group and projects are not actually destroyed
+        Sidekiq::Testing.fake! { destroy_group(group, user, true) }
+      end
+
+      after do
+        # Clean up stale directories
+        gitlab_shell.rm_namespace(project.repository_storage_path, group.path)
+        gitlab_shell.rm_namespace(project.repository_storage_path, remove_path)
+      end
+
+      it 'verifies original paths and projects still exist' do
+        expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_truthy
+        expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey
+        expect(Project.unscoped.count).to eq(1)
+        expect(Group.unscoped.count).to eq(2)
+      end
+    end
+
     context 'potential race conditions' do
       context "when the `GroupDestroyWorker` task runs immediately" do
         it "deletes the group" do