Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/actions/app_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def delete(apps, record_event: true)
end
logger.info("Deleted app: #{app.guid}")
end

[]
end

def delete_without_event(apps)
Expand Down
2 changes: 2 additions & 0 deletions app/actions/organization_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ def delete(org_dataset)
Repositories::OrganizationEventRepository.new.record_organization_delete_request(org, @user_audit_info, { recursive: true })
end
end

[]
end

def timeout_error(dataset)
Expand Down
1 change: 1 addition & 0 deletions app/actions/v2/organization_delete.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def delete(org_dataset)
org.destroy
end
end
[]
end

def timeout_error(dataset)
Expand Down
20 changes: 15 additions & 5 deletions app/jobs/delete_action_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,23 @@ def perform
logger.info("Deleting model class '#{model_class}' with guid '#{resource_guid}'")

dataset = model_class.where(guid: resource_guid)
if delete_action_can_return_warnings?
errors, warnings = delete_action.delete(dataset)
else
errors = delete_action.delete(dataset)
errors = []

begin
if delete_action_can_return_warnings?
errors, warnings = delete_action.delete(dataset)
else
errors = delete_action.delete(dataset)
end
rescue StandardError => e
errors << e
end

raise errors.first unless errors&.empty?
# Ignore errors if the target resource has already been deleted (e.g., by a parallel job)
quoted_table_name = model_class.db.quote_identifier(model_class.table_name)
errors.reject! { |err| err.is_a?(Sequel::NoExistingObject) && err.message.include?("DELETE FROM #{quoted_table_name}") } unless errors.frozen?

raise errors.first unless errors.empty?

warnings
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/runtime/route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ def internal_route_vip_range

def destroy_route_bindings
errors = RouteBindingDelete.new.delete(route_binding_dataset)

quoted_table_name = RouteBinding.db.quote_identifier(RouteBinding.table_name)
errors.reject! { |e| e.is_a?(Sequel::NoExistingObject) && e.message.include?("DELETE FROM #{quoted_table_name}") }

raise errors.first unless errors.empty?
end

Expand Down
39 changes: 39 additions & 0 deletions spec/unit/jobs/delete_action_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@

module VCAP::CloudController
module Jobs
RSpec.shared_examples 'a delete action handling external deletion' do
before do
allow_any_instance_of(resource.class).to receive(:destroy).and_wrap_original do |original_method, *args|
Sequel::Model.db.run("DELETE FROM #{resource.class.table_name} WHERE id = #{resource.id}") # Simulate external deletion
original_method.call(*args)
end
end

it 'still attempts to delete the resource even if it was already deleted externally' do
expect { delete_job.perform }.not_to raise_error
end
end

RSpec.describe DeleteActionJob, job_context: :worker do
let(:user) { User.make(admin: true) }
let(:delete_action) { instance_double(SpaceDelete, delete: []) }
Expand Down Expand Up @@ -127,6 +140,32 @@ module Jobs
expect(job.resource_guid).to eq(space.guid)
end
end

context 'when the resource is deleted externally before destroy' do
it_behaves_like 'a delete action handling external deletion' do
let(:resource) { PackageModel.make }
let(:delete_action) { PackageDelete.new(nil) }
let(:delete_job) { DeleteActionJob.new(PackageModel, resource.guid, delete_action) }
end

it_behaves_like 'a delete action handling external deletion' do
let(:resource) { Space.make }
let(:delete_action) { SpaceDelete.new(nil, nil) }
let(:delete_job) { DeleteActionJob.new(Space, resource.guid, delete_action) }
end

it_behaves_like 'a delete action handling external deletion' do
let(:resource) { Route.make }
let(:delete_action) { RouteDeleteAction.new(nil) }
let(:delete_job) { DeleteActionJob.new(Route, resource.guid, delete_action) }
end

it_behaves_like 'a delete action handling external deletion' do
let(:resource) { User.make }
let(:delete_action) { UserDeleteAction.new }
let(:delete_job) { DeleteActionJob.new(User, resource.guid, delete_action) }
end
end
end
end
end
17 changes: 17 additions & 0 deletions spec/unit/models/runtime/route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,23 @@ module VCAP::CloudController
expect(process.reload.routes[0]).to eq route
end
end

context 'when route_binding is deleted externally before destroy' do
before do
allow_any_instance_of(ServiceKeyDelete).to receive(:delete_service_binding).and_wrap_original do |original_method, *args|
service_binding = args.first
service_binding.destroy
original_method.call(*args)
end
end

it 'does not raise a Sequel::NoExistingObject error' do
route_binding = RouteBinding.make
route = route_binding.route
stub_unbind(route_binding)
expect { route.destroy }.not_to raise_error
end
end
end
end

Expand Down