Skip repo removing whem remove user or group

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

app/models/namespace.rb

@@ -102,11 +102,15 @@ class Namespace < ActiveRecord::Base
     # Move namespace directory into trash.
     # We will remove it later async
     new_path = "#{path}+#{id}+deleted"
-    gitlab_shell.mv_namespace(path, new_path)
 
-    # Remove namespace directroy async with delay so
+    if gitlab_shell.mv_namespace(path, new_path)
-    # GitLab has time to remove all projects first
+      message = "Namespace directory \"#{path}\" moved to \"#{new_path}\""
-    GitlabShellWorker.perform_in(5.minutes, :rm_namespace, new_path)
+      Gitlab::AppLogger.info message
+
+      # Remove namespace directroy async with delay so
+      # GitLab has time to remove all projects first
+      GitlabShellWorker.perform_in(5.minutes, :rm_namespace, new_path)
+    end
   end
 
   def move_dir

app/services/delete_user_service.rb

@@ -4,9 +4,10 @@ class DeleteUserService
       user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
       user
     else
-      # TODO: Skip remove repository so Namespace#rm_dir works
       user.personal_projects.each do |project|
-        ::Projects::DestroyService.new(project, current_user, {}).execute
+        # Skip repository removal because we remove directory with namespace
+        # that contain all this repositories
+        ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
       end
 
       user.destroy

app/services/destroy_group_service.rb

@@ -6,9 +6,10 @@ class DestroyGroupService
   end
 
   def execute
-    # TODO: Skip remove repository so Namespace#rm_dir works
     @group.projects.each do |project|
-      ::Projects::DestroyService.new(project, current_user, {}).execute
+      # Skip repository removal because we remove directory with namespace
+      # that contain all this repositories
+      ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute
     end
 
     @group.destroy

app/services/projects/destroy_service.rb

@@ -36,9 +36,11 @@ module Projects
     private
 
     def remove_repository(path)
-      unless gitlab_shell.exists?(path + '.git')
+      # Skip repository removal. We use this flag when remove user or group
-        return true
+      return true if params[:skip_repo] == true
-      end
+
+      # There is a possibility project does not have repository or wiki
+      return true unless gitlab_shell.exists?(path + '.git')
 
       new_path = removal_path(path)
 

spec/services/destroy_group_service_spec.rb

@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe DestroyGroupService do
+  let!(:user) { create(:user) }
+  let!(:group) { create(:group) }
+  let!(:project) { create(:project, namespace: group) }
+  let!(:gitlab_shell) { Gitlab::Shell.new }
+  let!(:remove_path) { group.path + "+#{group.id}+deleted" }
+
+  context 'database records' do
+    before do
+      destroy_group(group, user)
+    end
+
+    it { Group.all.should_not include(group) }
+    it { Project.all.should_not include(project) }
+  end
+
+  context 'file system' do
+    context 'Sidekiq inline' do
+      before do
+        # Run sidekiq immediatly to check that renamed dir will be removed
+        Sidekiq::Testing.inline! { destroy_group(group, user) }
+      end
+
+      it { gitlab_shell.exists?(group.path).should be_falsey }
+      it { gitlab_shell.exists?(remove_path).should be_falsey }
+    end
+
+    context 'Sidekiq fake' do
+      before do
+        # Dont run sidekiq to check if renamed repository exists
+        Sidekiq::Testing.fake! { destroy_group(group, user) }
+      end
+
+      it { gitlab_shell.exists?(group.path).should be_falsey }
+      it { gitlab_shell.exists?(remove_path).should be_truthy }
+    end
+  end
+
+  def destroy_group(group, user)
+    DestroyGroupService.new(group, user).execute
+  end
+end