v3(services): backfill explicit error tests for SI actions

[#172144638](https://www.pivotaltracker.com/story/show/172144638)

Author: Brian Butz

app/actions/mixins/service_instance_create.rb

@@ -2,9 +2,9 @@ module VCAP::CloudController
   module ServiceInstanceCreateMixin
     private
 
-    def validate_quotas!(e)
-      quota_errors = e.errors.on(:quota).to_a
-      plan_errors = e.errors.on(:service_plan).to_a
+    def validate_quotas!(errors)
+      quota_errors = errors.on(:quota).to_a
+      plan_errors = errors.on(:service_plan).to_a
 
       code = if quota_errors.include?(:service_instance_space_quota_exceeded)
                'ServiceInstanceSpaceQuotaExceeded'
@@ -20,16 +20,18 @@ def validate_quotas!(e)
     end
 
     def validation_error!(
-      error,
+      exception,
       name:,
       validation_error_handler:
     )
-      validate_quotas!(error)
+      errors = exception.errors
+      validate_quotas!(errors)
 
-      if error.errors.on(:name)&.include?(:unique)
+      if errors.on(:name)&.include?(:unique)
         validation_error_handler.error!("The service instance name is taken: #{name}")
       end
-      validation_error_handler.error!(error.message)
+
+      validation_error_handler.error!(exception.message)
     end
   end
 end

app/actions/service_instance_create_managed.rb

@@ -52,7 +52,11 @@ def create(message)
 
       pollable_job
     rescue Sequel::ValidationFailed => e
-      validation_error!(e, name: message.name, validation_error_handler: ValidationErrorHandler.new)
+      validation_error!(
+        e,
+        name: message.name,
+        validation_error_handler: ValidationErrorHandler.new
+      )
     end
 
     private

spec/unit/actions/service_instance_create_user_provided_spec.rb

@@ -11,6 +11,7 @@ module CloudController
       let(:message) { ServiceInstanceCreateUserProvidedMessage.new(request) }
       let(:instance) { ServiceInstance.last }
       let(:name) { 'my-service-instance' }
+      let(:tags) { %w(foo bar baz) }
 
       let(:event_repository) do
         dbl = double(Repositories::ServiceEventRepository::WithUserActor)
@@ -28,7 +29,7 @@ module CloudController
           },
           syslog_drain_url: 'https://drain.com/foo',
           route_service_url: 'https://route.com/bar',
-          tags: %w(foo bar baz),
+          tags: tags,
           relationships: {
             space: {
               data: {
@@ -102,29 +103,54 @@ module CloudController
         end
       end
 
-      context 'name is already taken' do
-        it 'raises an error' do
-          ServiceInstance.make(name: name, space: space)
-
-          expect { action.create(message) }.
-            to raise_error(
-              ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance,
-              "The service instance name is taken: #{name}"
-            )
+      describe 'validation fails' do
+        context 'name' do
+          describe 'when missing' do
+            let(:name) { '' }
+
+            it 'raises an error' do
+              expect { action.create(message) }.
+                to raise_error(ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance, 'name presence')
+            end
+          end
+
+          describe 'when too long' do
+            let(:name) { way_too_long }
+
+            it 'raises an error' do
+              expect { action.create(message) }.
+                to raise_error(ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance, 'name max_length')
+            end
+          end
+
+          describe 'when already taken' do
+            it 'raises an error' do
+              ServiceInstance.make(name: name, space: space)
+
+              expect { action.create(message) }.
+                to raise_error(
+                  ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance,
+                     "The service instance name is taken: #{name}"
+                   )
+            end
+          end
         end
-      end
 
-      context 'SQL validation fails' do
-        it 'raises an error' do
-          errors = Sequel::Model::Errors.new
-          errors.add(:blork, 'is busted')
-          expect(UserProvidedServiceInstance).to receive(:create).
-            and_raise(Sequel::ValidationFailed.new(errors))
+        context 'tags' do
+          let(:tags) { [way_too_long] }
 
-          expect { action.create(message) }.
-            to raise_error(ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance, 'blork is busted')
+          describe 'when too long' do
+            it 'raises an error' do
+              expect { action.create(message) }.
+                to raise_error(ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance, 'tags too_long')
+            end
+          end
         end
       end
+
+      def way_too_long
+        'a' * 10_000
+      end
     end
   end
 end

spec/unit/actions/service_instance_update_user_provided_spec.rb

@@ -108,33 +108,64 @@ module VCAP::CloudController
         end
       end
 
-      context 'when the name is already taken' do
-        let!(:other_service_instance) { UserProvidedServiceInstance.make(name: 'already_taken', space: service_instance.space) }
-        let(:body) do
-          { name: 'already_taken' }
-        end
+      describe 'validation fails' do
+        context 'name' do
+          describe 'when missing' do
+            let(:body) do
+              { name: '' }
+            end
 
-        it 'fails' do
-          expect {
-            action.update(service_instance, message)
-          }.to raise_error(
-            ServiceInstanceUpdateUserProvided::UnprocessableUpdate,
-            'The service instance name is taken: already_taken',
-          )
+            it 'raises an error' do
+              expect { action.update(service_instance, message) }.
+                to raise_error(ServiceInstanceUpdateUserProvided::UnprocessableUpdate, 'name presence')
+            end
+          end
+
+          describe 'when too long' do
+            let(:body) do
+              { name: way_too_long }
+            end
+
+            it 'raises an error' do
+              expect { action.update(service_instance, message) }.
+                to raise_error(ServiceInstanceUpdateUserProvided::UnprocessableUpdate, 'name max_length')
+            end
+          end
+
+          describe 'when already taken' do
+            let!(:other_service_instance) { UserProvidedServiceInstance.make(name: 'already_taken', space: service_instance.space) }
+            let(:body) do
+              { name: 'already_taken' }
+            end
+
+            it 'fails' do
+              expect {
+                action.update(service_instance, message)
+              }.to raise_error(
+                ServiceInstanceUpdateUserProvided::UnprocessableUpdate,
+                'The service instance name is taken: already_taken',
+              )
+            end
+          end
         end
-      end
 
-      context 'SQL validation fails' do
-        it 'raises an error' do
-          errors = Sequel::Model::Errors.new
-          errors.add(:blork, 'is busted')
-          expect_any_instance_of(UserProvidedServiceInstance).to receive(:update).
-            and_raise(Sequel::ValidationFailed.new(errors))
+        context 'tags' do
+          let(:body) do
+            { tags: [way_too_long] }
+          end
 
-          expect { action.update(service_instance, message) }.
-            to raise_error(ServiceInstanceUpdateUserProvided::UnprocessableUpdate, 'blork is busted')
+          describe 'when too long' do
+            it 'raises an error' do
+              expect { action.update(service_instance, message) }.
+                to raise_error(ServiceInstanceUpdateUserProvided::UnprocessableUpdate, 'tags too_long')
+            end
+          end
         end
       end
     end
+
+    def way_too_long
+      'a' * 10_000
+    end
   end
 end