Merge pull request #235 from cerebris/op_response_codes

Allow alternate response code

Author: dgeb

lib/jsonapi/error_codes.rb

@@ -19,6 +19,7 @@ module JSONAPI
   INVALID_PAGE_VALUE = 118
   INVALID_FIELD_FORMAT = 119
   INVALID_FILTERS_SYNTAX = 120
+  SAVE_FAILED = 121
   FORBIDDEN = 403
   RECORD_NOT_FOUND = 404
   UNSUPPORTED_MEDIA_TYPE = 415
@@ -44,9 +45,11 @@ module JSONAPI
       INVALID_PAGE_OBJECT => 'INVALID_PAGE_OBJECT',
       INVALID_PAGE_VALUE => 'INVALID_PAGE_VALUE',
       INVALID_FIELD_FORMAT => 'INVALID_FIELD_FORMAT',
+      INVALID_FILTERS_SYNTAX => 'INVALID_FILTERS_SYNTAX',
+      SAVE_FAILED => 'SAVE_FAILED',
       FORBIDDEN => 'FORBIDDEN',
       RECORD_NOT_FOUND => 'RECORD_NOT_FOUND',
       UNSUPPORTED_MEDIA_TYPE => 'UNSUPPORTED_MEDIA_TYPE',
-      LOCKED => 'LOCKED',
-      INVALID_FILTERS_SYNTAX => 'INVALID_FILTERS_SYNTAX' }
+      LOCKED => 'LOCKED'
+    }
 end

lib/jsonapi/exceptions.rb

@@ -305,6 +305,15 @@ def errors
       end
     end
 
+    class SaveFailed < Error
+      def errors
+        [JSONAPI::Error.new(code: JSONAPI::SAVE_FAILED,
+                            status: :unprocessable_entity,
+                            title: 'Save failed or was cancelled',
+                            detail: 'Save failed or was cancelled')]
+      end
+    end
+
     class InvalidPageObject < Error
       def errors
         [JSONAPI::Error.new(code: JSONAPI::INVALID_PAGE_OBJECT,

lib/jsonapi/operation.rb

@@ -147,9 +147,9 @@ def initialize(resource_klass, options = {})
 
     def apply(context)
       resource = @resource_klass.create(context)
-      resource.replace_fields(@data)
+      result = resource.replace_fields(@data)
 
-      return JSONAPI::ResourceOperationResult.new(:created, resource)
+      return JSONAPI::ResourceOperationResult.new((result == :completed ? :created : :accepted), resource)
 
     rescue JSONAPI::Exceptions::Error => e
       return JSONAPI::ErrorsOperationResult.new(e.errors[0].code, e.errors)
@@ -165,9 +165,9 @@ def initialize(resource_klass, options = {})
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.remove
+      result = resource.remove
 
-      return JSONAPI::OperationResult.new(:no_content)
+      return JSONAPI::OperationResult.new(result == :completed ? :no_content : :accepted)
 
     rescue JSONAPI::Exceptions::Error => e
       return JSONAPI::ErrorsOperationResult.new(e.errors[0].code, e.errors)
@@ -185,9 +185,9 @@ def initialize(resource_klass, options = {})
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.replace_fields(data)
+      result = resource.replace_fields(data)
 
-      return JSONAPI::ResourceOperationResult.new(:ok, resource)
+      return JSONAPI::ResourceOperationResult.new(result == :completed ? :ok : :accepted, resource)
     end
   end
 
@@ -203,9 +203,9 @@ def initialize(resource_klass, options = {})
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.replace_has_one_link(@association_type, @key_value)
+      result = resource.replace_has_one_link(@association_type, @key_value)
 
-      return JSONAPI::OperationResult.new(:no_content)
+      return JSONAPI::OperationResult.new(result == :completed ? :no_content : :accepted)
     end
   end
 
@@ -221,9 +221,9 @@ def initialize(resource_klass, options)
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.create_has_many_links(@association_type, @data)
+      result = resource.create_has_many_links(@association_type, @data)
 
-      return JSONAPI::OperationResult.new(:no_content)
+      return JSONAPI::OperationResult.new(result == :completed ? :no_content : :accepted)
     end
   end
 
@@ -239,9 +239,9 @@ def initialize(resource_klass, options)
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.replace_has_many_links(@association_type, @data)
+      result = resource.replace_has_many_links(@association_type, @data)
 
-      return JSONAPI::OperationResult.new(:no_content)
+      return JSONAPI::OperationResult.new(result == :completed ? :no_content : :accepted)
     end
   end
 
@@ -257,9 +257,9 @@ def initialize(resource_klass, options)
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.remove_has_many_link(@association_type, @associated_key)
+      result = resource.remove_has_many_link(@association_type, @associated_key)
 
-      return JSONAPI::OperationResult.new(:no_content)
+      return JSONAPI::OperationResult.new(result == :completed ? :no_content : :accepted)
     end
   end
 
@@ -274,9 +274,9 @@ def initialize(resource_klass, options)
 
     def apply(context)
       resource = @resource_klass.find_by_key(@resource_id, context: context)
-      resource.remove_has_one_link(@association_type)
+      result = resource.remove_has_one_link(@association_type)
 
-      return JSONAPI::OperationResult.new(:no_content)
+      return JSONAPI::OperationResult.new(result == :completed ? :no_content : :accepted)
     end
   end
 end

lib/jsonapi/resource.rb

@@ -35,20 +35,26 @@ def is_new?
     end
 
     def change(callback)
+      completed = false
+
       if @changing
         run_callbacks callback do
-          yield
+          completed = (yield == :completed)
         end
       else
         run_callbacks is_new? ? :create : :update do
           @changing = true
           run_callbacks callback do
-            yield
+            completed = (yield == :completed)
           end
 
-          save if @save_needed || is_new?
+          if @save_needed || is_new?
+            completed = (save == :completed)
+          end
         end
       end
+
+      return completed ? :completed : :accepted
     end
 
     def remove
@@ -111,18 +117,42 @@ def save
       end
     end
 
+    # Override this on a resource to return a different result code. Any
+    # value other than :completed will result in operations returning
+    # `:accepted`
+    #
+    # For example to return `:accepted` if your model does not immediately
+    # save resources to the database you could override `_save` as follows:
+    #
+    # ```
+    # def _save
+    #   super
+    #   return :accepted
+    # end
+    # ```
     def _save
       unless @model.valid?
         raise JSONAPI::Exceptions::ValidationErrors.new(@model.errors.messages)
       end
 
-      saved = @model.save
+      if defined? @model.save
+        saved = @model.save
+        unless saved
+          raise JSONAPI::Exceptions::SaveFailed.new
+        end
+      else
+        saved = true
+      end
+
       @save_needed = !saved
-      saved
+
+      return :completed
     end
 
     def _remove
       @model.destroy
+
+      return :completed
     end
 
     def _create_has_many_links(association_type, association_key_values)
@@ -139,33 +169,43 @@ def _create_has_many_links(association_type, association_key_values)
           raise JSONAPI::Exceptions::HasManyRelationExists.new(association_key_value)
         end
       end
+
+      return :completed
     end
 
     def _replace_has_many_links(association_type, association_key_values)
       association = self.class._associations[association_type]
 
       send("#{association.foreign_key}=", association_key_values)
       @save_needed = true
+
+      return :completed
     end
 
     def _replace_has_one_link(association_type, association_key_value)
       association = self.class._associations[association_type]
 
       send("#{association.foreign_key}=", association_key_value)
       @save_needed = true
+
+      return :completed
     end
 
     def _remove_has_many_link(association_type, key)
       association = self.class._associations[association_type]
 
       @model.send(association.type).delete(key)
+
+      return :completed
     end
 
     def _remove_has_one_link(association_type)
       association = self.class._associations[association_type]
 
       send("#{association.foreign_key}=", nil)
       @save_needed = true
+
+      return :completed
     end
 
     def _replace_fields(field_data)
@@ -191,6 +231,8 @@ def _replace_fields(field_data)
       field_data[:has_many].each do |association_type, values|
         replace_has_many_links(association_type, values)
       end if field_data[:has_many]
+
+      return :completed
     end
 
     class << self

test/controllers/controller_test.rb

@@ -1983,7 +1983,7 @@ def test_poro_create_simple
            }
          }
 
-    assert_response :created
+    assert_response :accepted
     assert json_response['data'].is_a?(Hash)
     assert_equal 'Tabby', json_response['data']['attributes']['name']
   end
@@ -2017,7 +2017,7 @@ def test_poro_create_update
            }
          }
 
-    assert_response :created
+    assert_response :accepted
     assert json_response['data'].is_a?(Hash)
     assert_equal 'Calic', json_response['data']['attributes']['name']
 
@@ -2322,3 +2322,40 @@ def test_index_default_filter_override
     assert_equal 4, json_response['data'].size
   end
 end
+
+class Api::V1::PlanetsControllerTest < ActionController::TestCase
+  def test_save_model_callbacks
+    set_content_type_header!
+    post :create,
+         {
+           data: {
+             type: 'planets',
+             attributes: {
+               name: 'Zeus',
+               description: 'The largest planet in the solar system. Discovered in 2015.'
+             }
+           }
+         }
+
+    assert_response :created
+    assert json_response['data'].is_a?(Hash)
+    assert_equal 'Zeus', json_response['data']['attributes']['name']
+  end
+
+  def test_save_model_callbacks_fail
+    set_content_type_header!
+    post :create,
+         {
+           data: {
+             type: 'planets',
+             attributes: {
+               name: 'Pluto',
+               description: 'Yes, it is a planet.'
+             }
+           }
+         }
+
+    assert_response :unprocessable_entity
+    assert_match /Save failed or was cancelled/, json_response['errors'][0]['detail']
+  end
+end

test/fixtures/active_record.rb

@@ -222,6 +222,16 @@ class Planet < ActiveRecord::Base
   belongs_to :planet_type
 
   has_and_belongs_to_many :tags, join_table: :planets_tags
+
+  # Test model callback cancelling save
+  before_save :check_not_pluto
+
+  def check_not_pluto
+    # Pluto can't be a planet, so cancel the save
+    if name.downcase == 'pluto'
+      return false
+    end
+  end
 end
 
 class PlanetType < ActiveRecord::Base
@@ -262,10 +272,6 @@ def destroy
     $breed_data.remove(@id)
   end
 
-  def save
-    true
-  end
-
   def valid?
     @errors.clear
     if name.is_a?(String) && name.length > 0
@@ -701,6 +707,11 @@ def self.find(filters, options = {})
   def self.find_by_key(id, options = {})
     BreedResource.new($breed_data.breeds[id.to_i], options[:context])
   end
+
+  def _save
+    super
+    return :accepted
+  end
 end
 
 class PlanetResource < JSONAPI::Resource
@@ -968,7 +979,7 @@ class BadlyNamedAttributesResource < JSONAPI::Resource
                        description: 'Saturn is the sixth planet from the Sun and the second largest planet in the Solar System, after Jupiter.',
                        planet_type_id: planetoid.id)
 titan = Moon.create(name:'Titan', description: 'Best known of the Saturn moons.', planet_id: saturn.id)
-pluto = Planet.create(name: 'Pluto', description: 'Pluto is the smallest planet.', planet_type_id: planetoid.id)
+makemake = Planet.create(name: 'Makemake', description: 'A small planetoid in the Kuiperbelt.', planet_type_id: planetoid.id)
 uranus = Planet.create(name: 'Uranus', description: 'Insert adolescent jokes here.', planet_type_id: gas_giant.id)
 jupiter = Planet.create(name: 'Jupiter', description: 'A gas giant.', planet_type_id: gas_giant.id)
 betax = Planet.create(name: 'Beta X', description: 'Newly discovered Planet X', planet_type_id: unknown.id)

test/unit/operation/operations_processor_test.rb

@@ -304,8 +304,8 @@ def test_remove_resource
     op = JSONAPI::OperationsProcessor.new
 
     count = Planet.count
-    pluto = Planet.find(2)
-    assert_equal(pluto.name, 'Pluto')
+    makemake = Planet.find(2)
+    assert_equal(makemake.name, 'Makemake')
 
     operations = [
       JSONAPI::RemoveResourceOperation.new(PlanetResource, resource_id: 2),