API: Disallow options=nil, fix and add tests

Author: hoffmaen

app/actions/route_create.rb

@@ -16,7 +16,7 @@ def create(message:, space:, domain:, manifest_triggered: false)
         port: port(message, domain),
         space: space,
         domain: domain,
-        options: message.options
+        options: message.options.nil? ? {} : message.options.compact
       )
 
       Route.db.transaction do

app/messages/validators.rb

@@ -242,12 +242,6 @@ def validate(record)
 
   class OptionsValidator < ActiveModel::Validator
     def validate(record)
-      if record.options.blank?
-        # Route Creation with explicit 'null' options is not allowed
-        record.errors.add(:options, message: 'is not a valid object') if record.is_a?(VCAP::CloudController::RouteCreateMessage) && record.options.nil?
-        return
-      end
-
       unless record.options.is_a?(Hash)
         record.errors.add(:options, message: "'options' is not a valid object")
         return

spec/unit/messages/route_create_message_spec.rb

@@ -392,6 +392,24 @@ module VCAP::CloudController
           end
         end
 
+        context 'when nil' do
+          let(:params) do
+            {
+              host: 'some-host',
+              relationships: {
+                space: { data: { guid: 'space-guid' } },
+                domain: { data: { guid: 'domain-guid' } }
+              },
+              options: nil
+            }
+          end
+
+          it 'is not valid' do
+            expect(subject).not_to be_valid
+            expect(subject.errors[:options]).to include("'options' is not a valid object")
+          end
+        end
+
         context 'when options has invalid key' do
           let(:params) do
             {
@@ -443,6 +461,23 @@ module VCAP::CloudController
             end
           end
 
+          context 'when lb_algo is nil' do
+            let(:params) do
+              {
+                host: 'some-host',
+                relationships: {
+                  space: { data: { guid: 'space-guid' } },
+                  domain: { data: { guid: 'domain-guid' } }
+                },
+                options: { lb_algo: nil }
+              }
+            end
+
+            it 'is valid' do
+              expect(subject).to be_valid
+            end
+          end
+
           context 'when lb_algo has invalid value' do
             let(:params) do
               {

spec/unit/messages/route_update_message_spec.rb

@@ -18,6 +18,11 @@ module VCAP::CloudController
         expect(message).to be_valid
       end
 
+      it 'accepts options: {}' do
+        message = RouteUpdateMessage.new(params.merge(options: {}))
+        expect(message).to be_valid
+      end
+
       it 'accepts options params with round-robin load-balancing algorithm' do
         message = RouteUpdateMessage.new(params.merge(options: { lb_algo: 'round-robin' }))
         expect(message).to be_valid
@@ -28,9 +33,10 @@ module VCAP::CloudController
         expect(message).to be_valid
       end
 
-      it 'accepts options: nil to unset options' do
+      it 'does not accept options: nil' do
         message = RouteUpdateMessage.new(params.merge(options: nil))
-        expect(message).to be_valid
+        expect(message).not_to be_valid
+        expect(message.errors.full_messages[0]).to include("Options 'options' is not a valid object")
       end
 
       it 'accepts lb_algo: nil to unset load-balancing algorithm' do

spec/unit/messages/validators_spec.rb

@@ -555,6 +555,17 @@ def options_message
         expect(message).to be_valid
       end
 
+      it 'adds invalid options message when options is null' do
+        message = OptionsMessage.new({ options: nil })
+        expect(message).not_to be_valid
+        expect(message.errors_on(:options)).to include("'options' is not a valid object")
+      end
+
+      it 'successfully validates empty load balancer' do
+        message = OptionsMessage.new({ options: { lb_algo: nil } })
+        expect(message).to be_valid
+      end
+
       it 'adds invalid object error message when options is not an object' do
         message = OptionsMessage.new({ options: 'cheesecake' })
         expect(message).not_to be_valid

spec/unit/presenters/v3/route_presenter_spec.rb

@@ -138,7 +138,7 @@ module VCAP::CloudController::Presenters::V3
           )
         end
 
-        it 'does not output options' do
+        it 'outputs empty options hash' do
           expect(subject[:options]).to eq({})
         end
       end