Implement suggestion from #58, with test coverage

app/controllers/scimitar/active_record_backed_resources_controller.rb

@@ -150,6 +150,16 @@ def storage_scope
         raise NotImplementedError
       end
 
+      # Return an Array of exceptions that #save! can rescue and handle with a
+      # SCIM error automatically.
+      #
+      def scimitar_rescuable_exceptions
+        [
+          ActiveRecord::RecordInvalid,
+          ActiveRecord::RecordNotSaved
+        ]
+      end
+
       # Find a record by ID. Subclasses can override this if they need special
       # lookup behaviour.
       #
@@ -189,7 +199,7 @@ def save!(record, &block)
         else
           record.save!
         end
-      rescue ActiveRecord::RecordInvalid => exception
+      rescue *scimitar_rescuable_exceptions => exception
         handle_invalid_record(exception.record)
       end
 

spec/requests/active_record_backed_resources_controller_spec.rb

@@ -315,7 +315,12 @@
           attributes = { userName: '4' } # Minimum required by schema
           attributes = spec_helper_hupcase(attributes) if force_upper_case
 
+          # Prove that certain known pathways are called; can then unit test
+          # those if need be and be sure that this covers #create actions.
+          #
           expect_any_instance_of(MockUsersController).to receive(:create).once.and_call_original
+          expect_any_instance_of(MockUsersController).to receive(:save! ).once.and_call_original
+
           expect {
             post "/Users", params: attributes.merge(format: :scim)
           }.to change { MockUser.count }.by(1)
@@ -521,7 +526,11 @@
         attributes = { userName: '4' }  # Minimum required by schema
         attributes = spec_helper_hupcase(attributes) if force_upper_case
 
+        # Prove that certain known pathways are called; can then unit test
+        # those if need be and be sure that this covers #replace actions.
+        #
         expect_any_instance_of(MockUsersController).to receive(:replace).once.and_call_original
+        expect_any_instance_of(MockUsersController).to receive(:save! ).once.and_call_original
         expect {
           put "/Users/#{@u2.primary_key}", params: attributes.merge(format: :scim)
         }.to_not change { MockUser.count }
@@ -690,7 +699,12 @@
 
         payload = spec_helper_hupcase(payload) if force_upper_case
 
+        # Prove that certain known pathways are called; can then unit test
+        # those if need be and be sure that this covers #update actions.
+        #
         expect_any_instance_of(MockUsersController).to receive(:update).once.and_call_original
+        expect_any_instance_of(MockUsersController).to receive(:save! ).once.and_call_original
+
         expect {
           patch "/Users/#{@u2.primary_key}", params: payload.merge(format: :scim)
         }.to_not change { MockUser.count }
@@ -1092,6 +1106,10 @@
     end # "context 'with a block' do"
   end # "context '#update' do"
 
+  # ===========================================================================
+  # In-passing parts of tests above show that #create, #replace and #update all
+  # route through #save!, so now add some unit tests for that and for exception
+  # handling overrides invoked via #save!.
   # ===========================================================================
 
   context 'overriding #save!' do
@@ -1114,6 +1132,67 @@
     end
   end # "context 'overriding #save!' do
 
+  context 'custom on-save exceptions' do
+    MockUsersController.new.send(:scimitar_rescuable_exceptions).each do | exception_class |
+      it "handles out-of-box exception #{exception_class}" do
+        expect_any_instance_of(MockUsersController).to receive(:create).once.and_call_original
+        expect_any_instance_of(MockUsersController).to receive(:save! ).once.and_call_original
+
+        expect_any_instance_of(MockUser).to receive(:save!).once { raise exception_class }
+
+        expect {
+          post "/Users", params: { format: :scim, userName: SecureRandom.uuid }
+        }.to_not change { MockUser.count }
+
+        expect(response.status                 ).to eql(400)
+        expect(response.headers['Content-Type']).to eql('application/scim+json; charset=utf-8')
+
+        result = JSON.parse(response.body)
+
+        expect(result['detail']).to eql('Invalid request') # Check basic SCIM error rendering - good enough given other tests elsewhere
+      end
+    end
+
+    it 'handles custom exceptions' do
+      exception_class = RuntimeError # (for testing only; usually, this would provoke a 500 response)
+
+      expect_any_instance_of(MockUsersController).to receive(:create).once.and_call_original
+      expect_any_instance_of(MockUsersController).to receive(:save! ).once.and_call_original
+
+      expect_any_instance_of(MockUsersController).to receive(:scimitar_rescuable_exceptions).once { [ exception_class ] }
+      expect_any_instance_of(MockUser           ).to receive(:save!                        ).once { raise exception_class }
+
+      expect {
+        post "/Users", params: { format: :scim, userName: SecureRandom.uuid }
+      }.to_not change { MockUser.count }
+
+      expect(response.status                 ).to eql(400)
+      expect(response.headers['Content-Type']).to eql('application/scim+json; charset=utf-8')
+
+      result = JSON.parse(response.body)
+
+      expect(result['detail']).to eql('Invalid request')
+    end
+
+    it 'reports other exceptions as 500s' do
+      expect_any_instance_of(MockUsersController).to receive(:create).once.and_call_original
+      expect_any_instance_of(MockUsersController).to receive(:save! ).once.and_call_original
+
+      expect_any_instance_of(MockUser).to receive(:save!).once { raise RuntimeError }
+
+      expect {
+        post "/Users", params: { format: :scim, userName: SecureRandom.uuid }
+      }.to_not change { MockUser.count }
+
+      expect(response.status                 ).to eql(500)
+      expect(response.headers['Content-Type']).to eql('application/scim+json; charset=utf-8')
+
+      result = JSON.parse(response.body)
+
+      expect(result['detail']).to eql('RuntimeError')
+    end
+  end
+
   # ===========================================================================
 
   context '#destroy' do