Merge branch 'repo-remove' into fix-group-remove

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

Conflicts:
	spec/features/projects_spec.rb

2 files changed, 59 insertions, 23 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index dc430351551..4ca5fc65459 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -97,18 +97,15 @@ class ProjectsController < ApplicationController
     return access_denied! unless can?(current_user, :remove_project, @project)
 
     ::Projects::DestroyService.new(@project, current_user, {}).execute
+    flash[:alert] = 'Project deleted.'
 
-    respond_to do |format|
-      format.html do
-        flash[:alert] = 'Project deleted.'
-
-        if request.referer.include?('/admin')
-          redirect_to admin_namespaces_projects_path
-        else
-          redirect_to dashboard_path
-        end
-      end
+    if request.referer.include?('/admin')
+      redirect_to admin_namespaces_projects_path
+    else
+      redirect_to dashboard_path
     end
+  rescue Projects::DestroyService::DestroyError => ex
+    redirect_to edit_project_path(@project), alert: ex.message
   end
 
   def autocomplete_sources
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 7e1d753b021..29e8ba347d4 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -1,28 +1,67 @@
 module Projects
   class DestroyService < BaseService
+    include Gitlab::ShellAdapter
+
+    class DestroyError < StandardError; end
+
+    DELETED_FLAG = '+deleted'
+
     def execute
       return false unless can?(current_user, :remove_project, project)
 
       project.team.truncate
       project.repository.expire_cache unless project.empty_repo?
 
-      if project.destroy
-        GitlabShellWorker.perform_async(
-          :remove_repository,
-          project.path_with_namespace
-        )
+      repo_path = project.path_with_namespace
+      wiki_path = repo_path + '.wiki'
+
+      Project.transaction do
+        project.destroy!
 
-        GitlabShellWorker.perform_async(
-          :remove_repository,
-          project.path_with_namespace + ".wiki"
-        )
+        unless remove_repository(repo_path)
+          raise_error('Failed to remove project repository. Please try again or contact administrator')
+        end
+
+        unless remove_repository(wiki_path)
+          raise_error('Failed to remove wiki repository. Please try again or contact administrator')
+        end
+      end
+
+      project.satellite.destroy
+      log_info("Project \"#{project.name}\" was removed")
+      system_hook_service.execute_hooks_for(project, :destroy)
+      true
+    end
 
-        project.satellite.destroy
+    private
 
-        log_info("Project \"#{project.name}\" was removed")
-        system_hook_service.execute_hooks_for(project, :destroy)
-        true
+    def remove_repository(path)
+      unless gitlab_shell.exists?(path + '.git')
+        return true
       end
+
+      new_path = removal_path(path)
+
+      if gitlab_shell.mv_repository(path, new_path)
+        log_info("Repository \"#{path}\" moved to \"#{new_path}\"")
+        GitlabShellWorker.perform_in(5.minutes, :remove_repository, new_path)
+      else
+        false
+      end
+    end
+
+    def raise_error(message)
+      raise DestroyError.new(message)
+    end
+
+    # Build a path for removing repositories
+    # We use `+` because its not allowed by GitLab so user can not create
+    # project with name cookies+119+deleted and capture someone stalled repository
+    #
+    # gitlab/cookies.git -> gitlab/cookies+119+deleted.git
+    #
+    def removal_path(path)
+      "#{path}+#{project.id}#{DELETED_FLAG}"
     end
   end
 end