From 543d24892cf67f58e17740511bbbe4cc6ac94b62 Mon Sep 17 00:00:00 2001 From: Sriram Nookala Date: Mon, 5 May 2025 19:25:16 +0000 Subject: [PATCH 1/2] Can't move routes to other orgs if the space has the same name as owner space Compare the space id instead of name for route transfer --- app/actions/route_transfer_owner.rb | 2 +- .../unit/actions/route_transfer_owner_spec.rb | 30 ++++++++++++------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/actions/route_transfer_owner.rb b/app/actions/route_transfer_owner.rb index fb897a70f19..7981fbe5428 100644 --- a/app/actions/route_transfer_owner.rb +++ b/app/actions/route_transfer_owner.rb @@ -4,7 +4,7 @@ module VCAP::CloudController class RouteTransferOwner class << self def transfer(route, target_space, user_audit_info) - return route if target_space.name == route.space.name + return route if target_space.id == route.space.id original_space = route.space Route.db.transaction do diff --git a/spec/unit/actions/route_transfer_owner_spec.rb b/spec/unit/actions/route_transfer_owner_spec.rb index 1fc4fb7b3a4..39e74d0bb99 100644 --- a/spec/unit/actions/route_transfer_owner_spec.rb +++ b/spec/unit/actions/route_transfer_owner_spec.rb @@ -7,6 +7,7 @@ module VCAP::CloudController let(:route) { Route.make domain: SharedDomain.make, space: original_owning_space } let(:original_owning_space) { Space.make name: 'original_owning_space' } let(:target_space) { Space.make name: 'target_space' } + let(:target_space_dup_name) { Space.make name: 'original_owning_space' } let(:shared_space) { Space.make name: 'shared_space' } let(:user_audit_info) { UserAuditInfo.new(user_guid: 'user-guid-1', user_email: 'user@email.com') } @@ -17,7 +18,7 @@ module VCAP::CloudController it 'makes the target space the new owner' do RouteTransferOwner.transfer(route, target_space, user_audit_info) - expect(route.space.name).to eq target_space.name + expect(route.space.id).to eq target_space.id end context 'route was previously shared with the target space' do @@ -26,25 +27,32 @@ module VCAP::CloudController end it 'removes the target space from the list of shared spaces' do - expect(route.shared_spaces.map(&:name)).to include target_space.name + expect(route.shared_spaces.map(&:id)).to include target_space.id RouteTransferOwner.transfer(route, target_space, user_audit_info) route.reload - expect(route.shared_spaces.map(&:name)).not_to include target_space.name + expect(route.shared_spaces.map(&:id)).not_to include target_space.id end end it 'shares the route with the original owning space' do - expect(route.shared_spaces.map(&:name)).not_to include original_owning_space.name + expect(route.shared_spaces.map(&:id)).not_to include original_owning_space.id RouteTransferOwner.transfer(route, target_space, user_audit_info) route.reload - expect(route.shared_spaces.map(&:name)).to include original_owning_space.name + expect(route.shared_spaces.map(&:id)).to include original_owning_space.id end context 'target space is already the owning space' do it 'does nothing and succeeds' do expect { RouteTransferOwner.transfer(route, original_owning_space, user_audit_info) }.not_to raise_error - expect(route.shared_spaces.map(&:name)).not_to include original_owning_space.name - expect(route.space.name).to eq original_owning_space.name + expect(route.shared_spaces.map(&:id)).not_to include original_owning_space.id + expect(route.space.id).to eq original_owning_space.id + end + end + + context 'target space has the same name as the owning space' do + it 'makes the target space with the same name the new owner' do + RouteTransferOwner.transfer(route, target_space_dup_name, user_audit_info) + expect(route.space.id).to eq target_space.id end end @@ -65,12 +73,12 @@ module VCAP::CloudController expect_any_instance_of(Repositories::RouteEventRepository).not_to receive(:record_route_transfer_owner).with( route, user_audit_info, original_owning_space, target_space.guid ) - expect(route.space.name).to eq original_owning_space.name + expect(route.space.id).to eq original_owning_space.id expect do RouteTransferOwner.transfer(route, target_space, user_audit_info) end.to raise_error('db failure') route.reload - expect(route.space.name).to eq original_owning_space.name + expect(route.space.id).to eq original_owning_space.id end it 'does not change the shared spaces' do @@ -78,12 +86,12 @@ module VCAP::CloudController route, user_audit_info, original_owning_space, target_space.guid ) expect(route.shared_spaces.length).to eq 1 - expect(route.shared_spaces.map(&:name)).to include shared_space.name + expect(route.shared_spaces.map(&:id)).to include shared_space.id expect do RouteTransferOwner.transfer(route, target_space, user_audit_info) end.to raise_error('db failure') route.reload - expect(route.shared_spaces.map(&:name)).to include shared_space.name + expect(route.shared_spaces.map(&:id)).to include shared_space.id expect(route.shared_spaces.length).to eq 1 end end From 3a031c3b917efc07c2ea3677245a800e19976944 Mon Sep 17 00:00:00 2001 From: Sriram Nookala Date: Tue, 6 May 2025 13:55:43 +0000 Subject: [PATCH 2/2] Can't move routes to other orgs if the space has the same name as owner space Update spec --- spec/unit/actions/route_transfer_owner_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/actions/route_transfer_owner_spec.rb b/spec/unit/actions/route_transfer_owner_spec.rb index 39e74d0bb99..92878bbc5fb 100644 --- a/spec/unit/actions/route_transfer_owner_spec.rb +++ b/spec/unit/actions/route_transfer_owner_spec.rb @@ -52,7 +52,7 @@ module VCAP::CloudController context 'target space has the same name as the owning space' do it 'makes the target space with the same name the new owner' do RouteTransferOwner.transfer(route, target_space_dup_name, user_audit_info) - expect(route.space.id).to eq target_space.id + expect(route.space.id).to eq target_space_dup_name.id end end