From e5c49b57c35e5fe923b0fe29e1863cc29d6cf619 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 10 Jan 2018 15:48:53 +0100 Subject: [PATCH] Added tests for removing soft removed objects --- ...71207150343_remove_soft_removed_objects.rb | 22 +++--- .../remove_soft_removed_objects_spec.rb | 77 +++++++++++++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) create mode 100644 spec/migrations/remove_soft_removed_objects_spec.rb diff --git a/db/post_migrate/20171207150343_remove_soft_removed_objects.rb b/db/post_migrate/20171207150343_remove_soft_removed_objects.rb index 542cfb42fdc..3e2dedfdd6a 100644 --- a/db/post_migrate/20171207150343_remove_soft_removed_objects.rb +++ b/db/post_migrate/20171207150343_remove_soft_removed_objects.rb @@ -156,6 +156,15 @@ class RemoveSoftRemovedObjects < ActiveRecord::Migration end def remove_group_namespaces + admin_id = id_for_admin_user + + unless admin_id + say 'Not scheduling soft removed groups for removal as no admin user ' \ + 'could be found. You will need to remove any such groups manually.' + + return + end + # Left over groups can't be easily removed because we may also need to # remove memberships, repositories, and other associated data. As a result # we'll just schedule a Sidekiq job to remove these. @@ -163,10 +172,6 @@ class RemoveSoftRemovedObjects < ActiveRecord::Migration # As of January 5th, 2018 there are 36 groups that will be removed using # this code. Namespace.select(:id).soft_removed_group.each_batch(of: 10) do |batch, index| - # We need the ID of an admin user as the owners of the group may no longer - # exist (or might not even be set in `namespaces.owner_id`). - admin_id = id_for_admin_user - batch.each do |ns| schedule_group_removal(index * 5.minutes, ns.id, admin_id) end @@ -194,14 +199,7 @@ class RemoveSoftRemovedObjects < ActiveRecord::Migration end def id_for_admin_user - return @id_for_admin_user if @id_for_admin_user - - if (admin_id = User.where(admin: true).limit(1).pluck(:id).first) - @id_for_admin_user = admin_id - else - raise 'Can not remove soft removed groups as no admin user exists. ' \ - 'Please make sure at least one user with `admin` set to TRUE exists before proceeding.' - end + User.where(admin: true).limit(1).pluck(:id).first end def migrate_inline? diff --git a/spec/migrations/remove_soft_removed_objects_spec.rb b/spec/migrations/remove_soft_removed_objects_spec.rb new file mode 100644 index 00000000000..ec089f9106d --- /dev/null +++ b/spec/migrations/remove_soft_removed_objects_spec.rb @@ -0,0 +1,77 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171207150343_remove_soft_removed_objects.rb') + +describe RemoveSoftRemovedObjects, :migration do + describe '#up' do + it 'removes various soft removed objects' do + 5.times do + create_with_deleted_at(:issue) + end + + regular_issue = create(:issue) + + run_migration + + expect(Issue.count).to eq(1) + expect(Issue.first).to eq(regular_issue) + end + + it 'removes the temporary indexes once soft removed data has been removed' do + migration = described_class.new + + run_migration + + disable_migrations_output do + expect(migration.temporary_index_exists?(Issue)).to eq(false) + end + end + + it 'removes routes of soft removed personal namespaces' do + namespace = create_with_deleted_at(:namespace) + group = create(:group) + + expect(Route.where(source: namespace).exists?).to eq(true) + expect(Route.where(source: group).exists?).to eq(true) + + run_migration + + expect(Route.where(source: namespace).exists?).to eq(false) + expect(Route.where(source: group).exists?).to eq(true) + end + + it 'schedules the removal of soft removed groups' do + group = create_with_deleted_at(:group) + admin = create(:user, admin: true) + + expect_any_instance_of(GroupDestroyWorker) + .to receive(:perform) + .with(group.id, admin.id) + + run_migration + end + + it 'does not remove soft removed groups when no admin user could be found' do + create_with_deleted_at(:group) + + expect_any_instance_of(GroupDestroyWorker) + .not_to receive(:perform) + + run_migration + end + end + + def run_migration + disable_migrations_output do + migrate! + end + end + + def create_with_deleted_at(*args) + row = create(*args) + + # We set "deleted_at" this way so we don't run into any column cache issues. + row.class.where(id: row.id).update_all(deleted_at: 1.year.ago) + + row + end +end