Skip to content

Commit

Permalink
Ensuring we run record_route_transfer_owner only on a successful co…
Browse files Browse the repository at this point in the history
…mmit.

Our tests include checks that ensure `record_route_transfer_owner` doesn't run on exceptions that result in a commit being rolled back.
Unfortunately the DB.transaction block does not bubble up `Sequel::RollBack` exceptions themselves--although it does bubble up all other types of exceptions that would result in a failed commit

Co-authored-by: Alex Rocha <alexr1@vmware.com>
Co-authored-by: Michael Oleske <moleske@pivotal.io>
  • Loading branch information
xandroc and moleske committed Jul 28, 2022
1 parent a0293e2 commit 50527bb
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 4 deletions.
6 changes: 4 additions & 2 deletions app/actions/route_transfer_owner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ def transfer(route, target_space, user_audit_info)

original_space = route.space
Route.db.transaction do
Route.db.after_commit {
Repositories::RouteEventRepository.new.record_route_transfer_owner(
route, user_audit_info, original_space, target_space.guid)
}
route.space = target_space
route.remove_shared_space(target_space)
route.add_shared_space(original_space)
route.save
end
Repositories::RouteEventRepository.new.record_route_transfer_owner(
route, user_audit_info, original_space, target_space.guid)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/request/routes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3187,7 +3187,7 @@
it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
end

it 'changes the route owner to the given space and logs an event' do
it 'changes the route owner to the given space and logs an event', isolation: :truncation do
api_call.call(space_dev_headers)

expect(last_response.status).to eq(200)
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/actions/route_transfer_owner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ module VCAP::CloudController
end
end

it 'records a transfer event' do
it 'records a transfer event', isolation: :truncation do
expect_any_instance_of(Repositories::RouteEventRepository).to receive(:record_route_transfer_owner).with(
route, user_audit_info, original_owning_space, target_space.guid)

Expand Down

0 comments on commit 50527bb

Please sign in to comment.