diff --git a/.rubocop_todo/rails/save_bang.yml b/.rubocop_todo/rails/save_bang.yml index d7c9366d85b..f83bb1fddef 100644 --- a/.rubocop_todo/rails/save_bang.yml +++ b/.rubocop_todo/rails/save_bang.yml @@ -1,7 +1,6 @@ --- Rails/SaveBang: Exclude: - - ee/spec/initializers/fog_google_https_private_urls_spec.rb - ee/spec/lib/analytics/merge_request_metrics_calculator_spec.rb - ee/spec/lib/gitlab/auth/ldap/access_spec.rb - ee/spec/lib/gitlab/auth/o_auth/user_spec.rb diff --git a/app/services/events/destroy_service.rb b/app/services/events/destroy_service.rb index fdb718f0fcb..3a0a7b339bf 100644 --- a/app/services/events/destroy_service.rb +++ b/app/services/events/destroy_service.rb @@ -2,20 +2,29 @@ module Events class DestroyService + BATCH_SIZE = 50 + def initialize(project) @project = project end def execute - project.events.all.delete_all + loop do + count = delete_events_in_batches + break if count < BATCH_SIZE + end ServiceResponse.success(message: 'Events were deleted.') - rescue StandardError - ServiceResponse.error(message: 'Failed to remove events.') + rescue StandardError => e + ServiceResponse.error(message: e.message) end private attr_reader :project + + def delete_events_in_batches + project.events.limit(BATCH_SIZE).delete_all + end end end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index aef92b8adee..936c2da1e71 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -80,8 +80,14 @@ module Projects end def remove_events + log_info("Attempting to destroy events from #{project.full_path} (#{project.id})") + response = ::Events::DestroyService.new(project).execute + if response.error? + log_error("Event deletion failed on #{project.full_path} with the following message: #{response.message}") + end + response.success? end diff --git a/spec/services/events/destroy_service_spec.rb b/spec/services/events/destroy_service_spec.rb index 8dcbb83eb1d..8b07852c040 100644 --- a/spec/services/events/destroy_service_spec.rb +++ b/spec/services/events/destroy_service_spec.rb @@ -30,16 +30,28 @@ RSpec.describe Events::DestroyService do expect(unrelated_event.reload).to be_present end + context 'batch delete' do + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + end + + it 'splits delete queries into batches' do + expect(project).to receive(:events).twice.and_call_original + + subject.execute + end + end + context 'when an error is raised while deleting the records' do before do - allow(project).to receive_message_chain(:events, :all, :delete_all).and_raise(ActiveRecord::ActiveRecordError) + allow(project).to receive_message_chain(:events, :limit, :delete_all).and_raise(ActiveRecord::ActiveRecordError, 'custom error') end it 'returns error' do response = subject.execute expect(response).to be_error - expect(response.message).to eq 'Failed to remove events.' + expect(response.message).to eq 'custom error' end it 'does not delete events' do