Fix typos and validation logic

Author: hoffmaen

app/messages/manifest_routes_update_message.rb

@@ -110,25 +110,24 @@ def validate_route_hash_options(route)
       hash_header = options[:hash_header]
       hash_balance = options[:hash_balance]
 
-      return if validate_route_hash_header(route, hash_header)
-      return if validate_route_hash_balance(route, hash_balance)
+      validate_route_hash_header(route, hash_header)
+      validate_route_hash_balance(route, hash_balance)
 
       validate_route_hash_options_with_loadbalancing(route, loadbalancing, hash_header, hash_balance)
     end
 
     def validate_route_hash_header(route, hash_header)
-      return false unless hash_header.present? && (hash_header.to_s.length > 128)
+      return unless hash_header.present? && (hash_header.to_s.length > 128)
 
       errors.add(:base, message: "Route '#{route[:route]}': Hash header must be at most 128 characters")
-      true
     end
 
     def validate_route_hash_balance(route, hash_balance)
-      return false if hash_balance.blank?
+      return if hash_balance.blank?
 
       if hash_balance.to_s.length > 16
         errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be at most 16 characters")
-        return true
+        return
       end
 
       validate_route_hash_balance_numeric(route, hash_balance)
@@ -137,11 +136,9 @@ def validate_route_hash_balance(route, hash_balance)
     def validate_route_hash_balance_numeric(route, hash_balance)
       balance_float = Float(hash_balance)
       # Must be either 0 or >= 1.1 and <= 10.0
-      errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be either 0 or between to 1.1 and 10.0") unless balance_float == 0 || balance_float.between?(1.1, 10)
-      false
+      errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be either 0 or between 1.1 and 10.0") unless balance_float == 0 || balance_float.between?(1.1, 10)
     rescue ArgumentError, TypeError
       errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be a numeric value")
-      false
     end
 
     def validate_route_hash_options_with_loadbalancing(route, loadbalancing, hash_header, hash_balance)

spec/unit/messages/manifest_routes_update_message_spec.rb

@@ -644,7 +644,7 @@ module VCAP::CloudController
               msg = ManifestRoutesUpdateMessage.new(body)
 
               expect(msg.valid?).to be(false)
-              expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be either 0 or between to 1.1 and 10.0")
+              expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be either 0 or between 1.1 and 10.0")
             end
           end
 
@@ -685,7 +685,7 @@ module VCAP::CloudController
               msg = ManifestRoutesUpdateMessage.new(body)
 
               expect(msg.valid?).to be(false)
-              expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be either 0 or between to 1.1 and 10.0")
+              expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be either 0 or between 1.1 and 10.0")
             end
           end
 
@@ -790,6 +790,29 @@ module VCAP::CloudController
             end
           end
 
+          context 'when a route contains multiple issues with hash options' do
+            let(:body) do
+              { 'routes' =>
+                [
+                  { 'route' => 'existing.example.com',
+                    'options' => {
+                      'loadbalancing' => 'hash',
+                      'hash_header' => 'X' * 129,
+                      'hash_balance' => '1' * 17
+                    } }
+                ] }
+            end
+
+            it 'returns false and prints all errors' do
+              msg = ManifestRoutesUpdateMessage.new(body)
+
+              expect(msg.valid?).to be(false)
+              expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash header must be at most 128 characters")
+              expect(msg.errors.full_messages).to include("Route 'existing.example.com': Hash balance must be at most 16 characters")
+
+            end
+          end
+
           context 'when multiple routes have mixed valid and invalid hash options' do
             let(:body) do
               { 'routes' =>