WIPtc-clean-pending-delete-projects

5 files changed, 165 insertions, 74 deletions

diff --git a/app/workers/namespaceless_project_destroy_worker.rb b/app/workers/namespaceless_project_destroy_worker.rb
new file mode 100644
index 00000000000..f6d9738489a
--- /dev/null
+++ b/app/workers/namespaceless_project_destroy_worker.rb
@@ -0,0 +1,50 @@
+# Worker to destroy projects that do not have a namespace
+#
+# It destroys everything it can without having the info about the namespace it
+# used to belong to. Projects in this state should be rare.
+# The worker will reject doing anything for projects that *do* have a
+# namespace. For those use ProjectDestroyWorker instead.
+class NamespacelessProjectDestroyWorker
+  include Sidekiq::Worker
+  include DedicatedSidekiqQueue
+
+  def self.bulk_perform_async(args_list)
+    Sidekiq::Client.push_bulk('class' => self, 'queue' => sidekiq_options['queue'], 'args' => args_list)
+  end
+
+  def perform(project_id, user_id, params)
+    begin
+      project = Project.unscoped.find(project_id)
+    rescue ActiveRecord::RecordNotFound
+      return
+    end
+    return unless project.namespace_id.nil?  # Reject doing anything for projects that *do* have a namespace
+
+    user = User.find(user_id)
+    return unless user.can?(:remove_project, project)
+
+    project.team.truncate
+
+    unlink_fork(project) if project.forked?
+
+    [:events, :issues, :merge_requests, :labels, :milestones, :notes, :snippets].each do |thing|
+      project.send(thing).delete_all
+    end
+
+    # Override Project#remove_pages for this instance so it doesn't do anything
+    def project.remove_pages
+    end
+
+    project.destroy!
+  end
+
+  private
+
+  def unlink_fork(project)
+    merge_requests = project.forked_from_project.merge_requests.opened.from_project(project)
+
+    merge_requests.update_all(state: 'closed')
+
+    project.forked_project_link.destroy
+  end
+end
diff --git a/db/post_migrate/20170502101023_clean_up_pending_delete_projects.rb b/db/post_migrate/20170502101023_clean_up_pending_delete_projects.rb
index dd812964902..fe15cd69c03 100644
--- a/db/post_migrate/20170502101023_clean_up_pending_delete_projects.rb
+++ b/db/post_migrate/20170502101023_clean_up_pending_delete_projects.rb
@@ -1,6 +1,5 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
+# This is the counterpart of RequeuePendingDeleteProjects and cleans all
+# projects with `pending_delete = true` and that do not have a namespace.
 class CleanUpPendingDeleteProjects < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
@@ -9,36 +8,43 @@ class CleanUpPendingDeleteProjects < ActiveRecord::Migration
   disable_ddl_transaction!
 
   def up
-    Project.unscoped.where(pending_delete: true).each { |project| delete_project(project, admin) }
-  end
+    admin = User.find_by(admin: true)
+    return unless admin
 
-  def down
-    # noop
-  end
+    @offset = 0
 
-  private
+    loop do
+      ids = pending_delete_batch
 
-  def delete_project(project)
-    project.team.truncate
+      break if ids.rows.count.zero?
 
-    unlink_fork(project) if project.forked?
+      args = ids.map { |id| [id['id'], admin.id, {}] }
 
-    [:events, :issues, :merge_requests, :labels, :milestones, :notes, :snippets].each do |thing|
-      project.send(thing).delete_all
-    end
+      NamespacelessProjectDestroyWorker.bulk_perform_async(args)
 
-    # Override Project#remove_pages for this instance so it doesn't do anything
-    def project.remove_pages
+      @offset += 1
     end
+  end
 
-    project.destroy!
+  def down
+    # noop
   end
 
-  def unlink_fork(project)
-    merge_requests = project.forked_from_project.merge_requests.opened.from_project(project)
+  private
+
+  def pending_delete_batch
+    connection.exec_query(find_batch)
+  end
 
-    merge_requests.update_all(state: 'closed')
+  BATCH_SIZE = 5000
 
-    project.forked_project_link.destroy
+  def find_batch
+    projects = Arel::Table.new(:projects)
+    projects.project(projects[:id]).
+      where(projects[:pending_delete].eq(true)).
+      where(projects[:namespace_id].eq(nil)).
+      skip(@offset * BATCH_SIZE).
+      take(BATCH_SIZE).
+      to_sql
   end
 end
diff --git a/spec/migrations/clean_up_pending_delete_projects_spec.rb b/spec/migrations/clean_up_pending_delete_projects_spec.rb
index a7c13f91245..3e6a87ff418 100644
--- a/spec/migrations/clean_up_pending_delete_projects_spec.rb
+++ b/spec/migrations/clean_up_pending_delete_projects_spec.rb
@@ -16,57 +16,5 @@ describe CleanUpPendingDeleteProjects do
         migration.up
       end.to change { Project.unscoped.count }.by(-1)
     end
-
-    it "truncates the project's team" do
-      project.add_master(admin)
-
-      expect_any_instance_of(ProjectTeam).to receive(:truncate)
-
-      migration.up
-    end
-
-    it 'calls Project#destroy!' do
-      expect_any_instance_of(Project).to receive(:destroy!)
-
-      migration.up
-    end
-
-    it 'does not do anything in Project#remove_pages method' do
-      expect(Gitlab::PagesTransfer).not_to receive(:new)
-
-      migration.up
-    end
-
-    context 'project not a fork of another project' do
-      it "doesn't call unlink_fork" do
-        expect(migration).not_to receive(:unlink_fork)
-
-        migration.up
-      end
-    end
-
-    context 'project forked from another' do
-      let!(:parent_project) { create(:empty_project) }
-
-      before do
-        create(:forked_project_link, forked_to_project: project, forked_from_project: parent_project)
-      end
-
-      it 'closes open merge requests' do
-        project.update_attribute(:pending_delete, false) # needed to create the MR
-        merge_request = create(:merge_request, source_project: project, target_project: parent_project)
-        project.update_attribute(:pending_delete, true)
-
-        migration.up
-
-        expect(merge_request.reload).to be_closed
-      end
-
-      it 'destroys the link' do
-        migration.up
-
-        expect(parent_project.forked_project_links).to be_empty
-      end
-    end
   end
 end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index 4b8589b2736..44205040e78 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -94,6 +94,17 @@ describe Projects::DestroyService, services: true do
     it_behaves_like 'deleting the project with pipeline and build'
   end
 
+  context 'without namespace and skip_repo' do
+    it 'deletes the project' do
+      admin = create(:admin)
+      allow_any_instance_of(Project).to receive(:namespace_id).and_return(nil)
+
+      destroy_project(project, admin, {})
+
+      expect(Project.unscoped.all).not_to include(project)
+    end
+  end
+
   describe 'container registry' do
     context 'when there are regular container repositories' do
       let(:container_repository) { create(:container_repository) }
diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb
new file mode 100644
index 00000000000..4dca9d6b31c
--- /dev/null
+++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb
@@ -0,0 +1,76 @@
+require 'spec_helper'
+
+describe NamespacelessProjectDestroyWorker do
+  let!(:project) { create(:empty_project) }
+
+  subject { described_class.new }
+
+  describe "#perform" do
+    context 'project has namespace' do
+      it 'does not do anything' do
+        project = create(:empty_project)
+
+        subject.perform(project.id, project.owner.id, {})
+
+        expect(Project.unscoped.all).to include(project)
+      end
+
+      context 'project has no namespace' do
+        let(:admin) { create(:admin) }
+
+        context 'project not a fork of another project' do
+          before do
+            allow_any_instance_of(Project).to receive(:namespace_id).and_return(nil)
+          end
+
+          it "deletes the project" do
+            subject.perform(project.id, admin, {})
+
+            expect(Project.unscoped.all).not_to include(project)
+          end
+
+          it "truncates the project's team" do
+            expect_any_instance_of(ProjectTeam).to receive(:truncate)
+
+            subject.perform(project.id, admin, {})
+          end
+
+          it 'does not do anything in Project#remove_pages method' do
+            expect(Gitlab::PagesTransfer).not_to receive(:new)
+
+            subject.perform(project.id, admin, {})
+          end
+
+          it "doesn't call unlink_fork" do
+            is_expected.not_to receive(:unlink_fork)
+
+            subject.perform(project.id, admin, {})
+          end
+        end
+
+        context 'project forked from another' do
+          let!(:parent_project) { create(:empty_project) }
+
+          before do
+            create(:forked_project_link, forked_to_project: project, forked_from_project: parent_project)
+            allow_any_instance_of(Project).to receive(:namespace_id).and_return(nil) # after parent and fork link is created
+          end
+
+          it 'closes open merge requests' do
+            merge_request = create(:merge_request, source_project: project, target_project: parent_project)
+
+            subject.perform(project.id, admin, {})
+
+            expect(merge_request.reload).to be_closed
+          end
+
+          it 'destroys the link' do
+            subject.perform(project.id, admin, {})
+
+            expect(parent_project.forked_project_links).to be_empty
+          end
+        end
+      end
+    end
+  end
+end