Wrap space guid within data field in PATCH v3/routes/:guid/relationsh…

…ips/space

This change ensures the endpoint adheres to the v3 api style guide
https://github.com/cloudfoundry/cloud_controller_ng/blob/main/docs/v3_style_guide.md

[#182668597]

Authored-by: Merric de Launey <[email protected]>

app/controllers/v3/routes_controller.rb

@@ -172,9 +172,9 @@ def transfer_owner
     unauthorized! unless permission_queryer.can_write_to_active_space?(route.space.guid)
     suspended! unless permission_queryer.is_space_active?(route.space.guid)
 
-    target_space = Space.first(guid: message.guid)
+    target_space = Space.first(guid: message.space_guid)
     target_space_error = check_if_space_is_accessible(target_space)
-    unprocessable!("Unable to transfer owner of route '#{route.uri}' to space '#{message.guid}'. #{target_space_error}") unless target_space_error.nil?
+    unprocessable!("Unable to transfer owner of route '#{route.uri}' to space '#{message.space_guid}'. #{target_space_error}") unless target_space_error.nil?
 
     RouteTransferOwner.transfer(route, target_space, user_audit_info)
 

app/messages/route_transfer_owner_message.rb

@@ -2,9 +2,28 @@
 
 module VCAP::CloudController
   class RouteTransferOwnerMessage < BaseMessage
-    register_allowed_keys [:guid]
+    register_allowed_keys [:data]
 
     validates_with NoAdditionalKeysValidator
-    validates :guid, presence: true, string: true, allow_nil: false, allow_blank: false
+    validates :data, presence: true, hash: true, allow_nil: false
+    validate :data_content
+
+    def space_guid
+      HashUtils.dig(data, :guid)
+    end
+
+    def data_content
+      return if data.nil?
+
+      errors.add(:data, 'can only accept one key') unless data.keys.length == 1
+      errors.add(:data, "can only accept key 'guid'") unless data.key?(:guid)
+      if space_guid && !space_guid.is_a?(String)
+        errors.add(:data, "#{space_guid} must be a string")
+        return
+      end
+      if space_guid && space_guid.empty?
+        errors.add(:data, "guid can't be blank")
+      end
+    end
   end
 end

spec/request/routes_spec.rb

@@ -3214,7 +3214,7 @@
     let(:target_space) { VCAP::CloudController::Space.make(organization: org) }
     let(:request_body) do
       {
-        'guid' => target_space.guid
+        data: { 'guid' => target_space.guid }
       }
     end
     let(:space_dev_headers) do
@@ -3261,7 +3261,7 @@
         let(:suspended_space) { VCAP::CloudController::Space.make }
         let(:request_body) do
           {
-            'guid' => suspended_space.guid
+            data: { 'guid' => suspended_space.guid }
           }
         end
 
@@ -3315,7 +3315,7 @@
         let(:target_space_guid) { 'fake-target' }
         let(:request_body) do
           {
-            'guid' => target_space_guid
+            data: { 'guid' => target_space_guid }
           }
         end
 
@@ -3338,7 +3338,7 @@
         let(:no_access_target_space) { VCAP::CloudController::Space.make(organization: org) }
         let(:request_body) do
           {
-            'guid' => no_access_target_space.guid
+            data: { 'guid' => no_access_target_space.guid }
           }
         end
 
@@ -3360,7 +3360,7 @@
         let(:no_write_access_target_space) { VCAP::CloudController::Space.make(organization: org) }
         let(:request_body) do
           {
-            'guid' => no_write_access_target_space.guid
+            data: { 'guid' => no_write_access_target_space.guid }
           }
         end
 
@@ -3401,7 +3401,7 @@
       context 'when there are additional keys' do
         let(:request_body) do
           {
-            'guid' => target_space.guid,
+            data: { 'guid' => target_space.guid },
             'fake-key' => 'foo'
           }
         end

spec/unit/messages/route_transfer_owner_message_spec.rb

@@ -4,38 +4,121 @@
 
 module VCAP::CloudController
   RSpec.describe RouteTransferOwnerMessage do
-    context 'when unexpected keys are requested' do
-      let(:params) { { guid: 'some-guid', unexpected: 'foo' } }
+    describe 'validations' do
+      context 'when it contains the right content' do
+        let(:symbolized_body) do
+          {
+            data: { guid: 'some-guid' }
+          }
+        end
 
-      it 'is not valid' do
-        message = RouteTransferOwnerMessage.new(params)
+        it 'it is valid' do
+          message = RouteTransferOwnerMessage.new(symbolized_body)
 
-        expect(message).not_to be_valid
-        expect(message.errors.full_messages[0]).to include("Unknown field(s): 'unexpected'")
+          expect(message).to be_valid
+          expect(message.errors.count).to eq(0)
+        end
       end
-    end
 
-    context 'when guid is not a string' do
-      let(:params) { { guid: 5 } }
+      context 'when there is no data in the object' do
+        let(:symbolized_body) do
+          {}
+        end
 
-      it 'is not valid' do
-        message = RouteTransferOwnerMessage.new(params)
+        it 'returns an error' do
+          message = RouteTransferOwnerMessage.new(symbolized_body)
 
-        expect(message).not_to be_valid
-        expect(message.errors.count).to eq(1)
-        expect(message.errors[:guid]).to include('must be a string')
+          expect(message).to_not be_valid
+          expect(message.errors[:data]).to include("can't be blank")
+        end
+      end
+
+      context 'when there is no guid in the data' do
+        let(:symbolized_body) do
+          {
+            data: {}
+          }
+        end
+
+        it 'returns an error' do
+          message = RouteTransferOwnerMessage.new(symbolized_body)
+
+          expect(message).to_not be_valid
+          expect(message.errors[:data]).to include("can't be blank")
+        end
+      end
+
+      context 'when data is nil' do
+        let(:symbolized_body) do
+          {
+            data: nil
+          }
+        end
+
+        it 'does not error and returns the correct message' do
+          message = RouteTransferOwnerMessage.new(symbolized_body)
+
+          expect(message).not_to be_valid
+          expect(message.errors[:data]).to include("can't be blank")
+        end
+      end
+
+      context 'when unexpected keys are requested' do
+        let(:params) { { data: { guid: 'some-guid' }, unexpected: 'foo' } }
+
+        it 'is not valid' do
+          message = RouteTransferOwnerMessage.new(params)
+
+          expect(message).not_to be_valid
+          expect(message.errors.full_messages[0]).to include("Unknown field(s): 'unexpected'")
+        end
+
+        context 'when there are unexpected keys inside data hash' do
+          let(:symbolized_body) {
+            {
+              data: { blah: 'awesome-guid' },
+            }
+          }
+
+          it 'is not valid' do
+            message = RouteTransferOwnerMessage.new(symbolized_body)
+
+            expect(message).to_not be_valid
+            expect(message.errors[:data]).to include("can only accept key 'guid'")
+          end
+        end
+      end
+
+      context 'when guid is not a string' do
+        let(:symbolized_body) do
+          {
+            data: { guid: 32.77 }
+          }
+        end
+
+        it 'is not valid' do
+          message = RouteTransferOwnerMessage.new(symbolized_body)
+
+          expect(message).not_to be_valid
+          expect(message.errors.count).to eq(1)
+          expect(message.errors[:data]).to include('32.77 must be a string')
+        end
       end
-    end
 
-    context 'when guid is not present' do
-      let(:params) { { guid: '' } }
+      context 'when guid is not present' do
+        let(:symbolized_body) do
+          {
+            data: { guid: '' }
+          }
+        end
 
-      it 'is not valid' do
-        message = RouteTransferOwnerMessage.new(params)
+        it 'is not valid' do
+          message = RouteTransferOwnerMessage.new(symbolized_body)
 
-        expect(message).not_to be_valid
-        expect(message.errors.count).to eq(1)
-        expect(message.errors[:guid]).to include("can't be blank")
+          expect(message).not_to be_valid
+          expect(message.errors.count).to eq(1)
+          expect(message.errors[:data]).to include("guid can't be blank")
+        end
       end
     end
   end