Add before_commit hooks (#72)

These hooks run after validating the whole graph, but before closing the
transaction. Helpful for things like "contact this service after saving,
but rollback if the service is down".

Moves the existing sideload hooks to the same place (the previous
behavior was to fire before validations).

Implemented by a Hook accumulator that uses Thread.current. I've tried
this a few different ways but recursive functions that return a mash of
objects seem to add a lot of complexity to the code for no real reason.
The premise of registering hooks during a complex process, then calling
those hooks later, is simpler.

This does add a small amount of duplication between the create/update
actions and the destroy action. This is because we're currently not
supporting DELETE requests with a body (nested deletes) so the
processing logic is different. Think this can be assimliated in a
separate PR.

Author: richmolj

lib/jsonapi_compliable.rb

@@ -24,6 +24,7 @@
 require "jsonapi_compliable/util/persistence"
 require "jsonapi_compliable/util/validation_response"
 require "jsonapi_compliable/util/sideload"
+require "jsonapi_compliable/util/hooks"
 
 # require correct jsonapi-rb before extensions
 if defined?(Rails)

lib/jsonapi_compliable/base.rb

@@ -253,9 +253,20 @@ def jsonapi_update
       end
     end
 
+    # Delete the model
+    # Any error, including validation errors, will roll back the transaction.
+    #
+    # Note: +before_commit+ hooks still run unless excluded
+    #
+    # @return [Util::ValidationResponse]
     def jsonapi_destroy
-      _persist do
-        jsonapi_resource.destroy(params[:id])
+      jsonapi_resource.transaction do
+        model = jsonapi_resource.destroy(params[:id])
+        validator = ::JsonapiCompliable::Util::ValidationResponse.new \
+          model, deserialized_params
+        validator.validate!
+        jsonapi_resource.before_commit(model, :destroy)
+        validator
       end
     end
 
@@ -321,6 +332,18 @@ def default_jsonapi_render_options
 
     private
 
+    def _persist
+      jsonapi_resource.transaction do
+        ::JsonapiCompliable::Util::Hooks.record do
+          model = yield
+          validator = ::JsonapiCompliable::Util::ValidationResponse.new \
+            model, deserialized_params
+          validator.validate!
+          validator
+        end
+      end
+    end
+
     def force_includes?
       not deserialized_params.data.nil?
     end
@@ -330,16 +353,5 @@ def perform_render_jsonapi(opts)
       JSONAPI::Serializable::Renderer.new
         .render(opts.delete(:jsonapi), opts).to_json
     end
-
-    def _persist
-      validation_response = nil
-      jsonapi_resource.transaction do
-        object = yield
-        validation_response = Util::ValidationResponse.new \
-          object, deserialized_params
-        raise Errors::ValidationError unless validation_response.to_a[1]
-      end
-      validation_response
-    end
   end
 end

lib/jsonapi_compliable/errors.rb

@@ -1,7 +1,14 @@
 module JsonapiCompliable
   module Errors
     class BadFilter < StandardError; end
-    class ValidationError < StandardError; end
+
+    class ValidationError < StandardError
+      attr_reader :validation_response
+
+      def initialize(validation_response)
+        @validation_response = validation_response
+      end
+    end
 
     class MissingSerializer < StandardError
       def initialize(class_name, serializer_name)

lib/jsonapi_compliable/resource.rb

@@ -257,6 +257,29 @@ def self.model(klass)
       config[:model] = klass
     end
 
+    # Register a hook that fires AFTER all validation logic has run -
+    # including validation of nested objects - but BEFORE the transaction
+    # has closed.
+    #
+    # Helpful for things like "contact this external service after persisting
+    # data, but roll everything back if there's an error making the service call"
+    #
+    # @param [Hash] +only: [:create, :update, :destroy]+
+    def self.before_commit(only: [:create, :update, :destroy], &blk)
+      Array(only).each do |verb|
+        config[:before_commit][verb] = blk
+      end
+    end
+
+    # Actually fire the before commit hooks
+    #
+    # @see .before_commit
+    # @api private
+    def before_commit(model, method)
+      hook = self.class.config[:before_commit][method]
+      hook.call(model) if hook
+    end
+
     # Define custom sorting logic
     #
     # @example Sort on alternate table
@@ -391,6 +414,7 @@ def self.config
           sorting: nil,
           pagination: nil,
           model: nil,
+          before_commit: {},
           adapter: Adapters::Abstract.new
         }
       end
@@ -705,7 +729,8 @@ def transaction
         adapter.transaction(model) do
           response = yield
         end
-      rescue Errors::ValidationError
+      rescue Errors::ValidationError => e
+        response = e.validation_response
       end
       response
     end

lib/jsonapi_compliable/util/hooks.rb

@@ -0,0 +1,33 @@
+module JsonapiCompliable
+  module Util
+    class Hooks
+      def self.record
+        self.hooks = []
+        begin
+          yield.tap { run }
+        ensure
+          self.hooks = []
+        end
+      end
+
+      def self._hooks
+        Thread.current[:_compliable_hooks] ||= []
+      end
+      private_class_method :_hooks
+
+      def self.hooks=(val)
+        Thread.current[:_compliable_hooks] = val
+      end
+
+      # Because hooks will be added from the outer edges of
+      # the graph, working inwards
+      def self.add(prc)
+        _hooks.unshift(prc)
+      end
+
+      def self.run
+        _hooks.each { |h| h.call }
+      end
+    end
+  end
+end

lib/jsonapi_compliable/util/persistence.rb

@@ -27,10 +27,11 @@ def initialize(resource, meta, attributes, relationships, caller_model)
   # * associate parent objects with current object
   # * process children
   # * associate children
+  # * record hooks for later playback
   # * run post-process sideload hooks
   # * return current object
   #
-  # @return the persisted model instance
+  # @return a model instance
   def run
     parents = process_belongs_to(@relationships)
     update_foreign_key_for_parents(parents)
@@ -45,13 +46,20 @@ def run
     end
 
     associate_children(persisted, children) unless @meta[:method] == :destroy
+
     post_process(persisted, parents)
     post_process(persisted, children)
+    before_commit = -> { @resource.before_commit(persisted, @meta[:method]) }
+    add_hook(before_commit)
     persisted
   end
 
   private
 
+  def add_hook(prc)
+    ::JsonapiCompliable::Util::Hooks.add(prc)
+  end
+
   # The child's attributes should be modified to nil-out the
   # foreign_key when the parent is being destroyed or disassociated
   #
@@ -147,7 +155,8 @@ def post_process(caller_model, processed)
     groups.each_pair do |method, group|
       group.group_by { |g| g[:sideload] }.each_pair do |sideload, members|
         objects = members.map { |x| x[:object] }
-        sideload.fire_hooks!(caller_model, objects, method)
+        hook = -> { sideload.fire_hooks!(caller_model, objects, method) }
+        add_hook(hook)
       end
     end
   end

lib/jsonapi_compliable/util/validation_response.rb

@@ -30,6 +30,13 @@ def to_a
     [object, success?]
   end
 
+  def validate!
+    unless success?
+      raise ::JsonapiCompliable::Errors::ValidationError.new(self)
+    end
+    self
+  end
+
   private
 
   def valid_object?(object)

spec/fixtures/employee_directory.rb

@@ -48,6 +48,21 @@
 
 class ApplicationRecord < ActiveRecord::Base
   self.abstract_class = true
+  attr_accessor :force_validation_error
+
+  before_save do
+    add_validation_error if force_validation_error
+
+    if Rails::VERSION::MAJOR >= 5
+      throw(:abort) if errors.present?
+    else
+      errors.blank?
+    end
+  end
+
+  def add_validation_error
+    errors.add(:base, 'Forced validation error')
+  end
 end
 
 class Classification < ApplicationRecord
@@ -74,8 +89,6 @@ class HomeOffice < ApplicationRecord
 end
 
 class Employee < ApplicationRecord
-  attr_accessor :force_validation_error
-
   belongs_to :workspace, polymorphic: true
   belongs_to :classification
   has_many :positions
@@ -98,10 +111,6 @@ class Employee < ApplicationRecord
       errors.blank?
     end
   end
-
-  def add_validation_error
-    errors.add(:base, 'Forced validation error')
-  end
 end
 
 class Position < ApplicationRecord