Merge pull request #153 from A-Boudi/122-add-before-validation-hook

Add before_validation hooks to resources

Author: richmolj

CHANGELOG.md

@@ -46,6 +46,11 @@ Fixes:
 
 ### master (unreleased)
 
+Features:
+
+- [#153](https://github.com/graphiti-api/graphiti/pull/153) Add after_graph_persist hook.
+  This hook fires after the graph of resources is persisted and before validation. (@A-Boudi)
+
 <!-- ### [version (YYYY-MM-DD)](diff_link) -->
 <!-- Breaking changes:-->
 <!-- Features:-->

lib/graphiti/resource.rb

@@ -112,6 +112,13 @@ def resolve(scope)
       adapter.resolve(scope)
     end
 
+    def after_graph_persist(model, metadata)
+      hooks = self.class.config[:after_graph_persist][metadata[:method]] || []
+      hooks.each do |hook|
+        instance_exec(model, metadata, &hook)
+      end
+    end
+
     def before_commit(model, metadata)
       hooks = self.class.config[:before_commit][metadata[:method]] || []
       hooks.each do |hook|

lib/graphiti/resource/configuration.rb

@@ -190,6 +190,7 @@ def config
               sort_all: nil,
               sorts: {},
               pagination: nil,
+              after_graph_persist: {},
               before_commit: {},
               after_commit: {},
               attributes: {},

lib/graphiti/resource/dsl.rb

@@ -82,6 +82,13 @@ def default_filter(name = nil, &blk)
           }
         end
 
+        def after_graph_persist(only: [:create, :update, :destroy], &blk)
+          Array(only).each do |verb|
+            config[:after_graph_persist][verb] ||= []
+            config[:after_graph_persist][verb] << blk
+          end
+        end
+
         def before_commit(only: [:create, :update, :destroy], &blk)
           Array(only).each do |verb|
             config[:before_commit][verb] ||= []

lib/graphiti/resource_proxy.rb

@@ -121,6 +121,7 @@ def destroy
         metadata = {method: :destroy}
         model = @resource.destroy(@query.filters[:id], metadata)
         model.instance_variable_set(:@__serializer_klass, @resource.serializer)
+        @resource.after_graph_persist(model, metadata)
         validator = ::Graphiti::Util::ValidationResponse.new \
           model, @payload
         validator.validate!
@@ -161,6 +162,7 @@ def persist
       transaction_response = @resource.transaction do
         ::Graphiti::Util::TransactionHooksRecorder.record do
           model = yield
+          ::Graphiti::Util::TransactionHooksRecorder.run_graph_persist_hooks
           validator = ::Graphiti::Util::ValidationResponse.new \
             model, @payload
           validator.validate!

lib/graphiti/util/persistence.rb

@@ -60,6 +60,8 @@ def run
 
     post_process(persisted, parents)
     post_process(persisted, children)
+    after_graph_persist = -> { @resource.after_graph_persist(persisted, metadata) }
+    add_hook(after_graph_persist, :after_graph_persist)
     before_commit = -> { @resource.before_commit(persisted, metadata) }
     add_hook(before_commit, :before_commit)
     after_commit = -> { @resource.after_commit(persisted, metadata) }

lib/graphiti/util/transaction_hooks_recorder.rb

@@ -41,6 +41,10 @@ def record
           end
         end
 
+        def run_graph_persist_hooks
+          run(:after_graph_persist)
+        end
+
         # Because hooks will be added from the outer edges of
         # the graph, working inwards
         def add(prc, lifecycle_event)
@@ -59,6 +63,7 @@ def _hooks
 
         def reset_hooks
           Thread.current[:_graphiti_hooks] = {
+            after_graph_persist: [],
             before_commit: [],
             after_commit: [],
           }

spec/integration/rails/persistence_spec.rb

@@ -276,6 +276,94 @@
       end
     end
 
+    describe "after graph persisted validation" do
+      subject(:make_request) { do_update(payload) }
+
+      let(:klass) do
+        Class.new(EmployeeResource) do
+          self.validate_endpoints = false
+
+          after_graph_persist do |model|
+            model.valid?(:after_graph_persisted)
+          end
+        end
+      end
+
+      let(:polyvalentEmployee) do
+        Class.new(Employee) do
+          def self.model_name
+            ActiveModel::Name.new(self, nil, 'PolyvalentEmployee')
+          end
+          validates :positions, length: { minimum: 2, too_short: 'too short, minimum is 2' }, on: :after_graph_persisted
+        end
+      end
+
+      let(:employee) { Employee.create!(first_name: "Jane") }
+
+      let(:payload) do
+        {
+          data: {
+            type: "employees",
+            id: employee.id.to_s,
+            relationships: {
+              positions: {
+                data: [
+                  { 'temp-id': "pos1", type: "positions", method: "create" },
+                  { 'temp-id': "pos2", type: "positions", method: "create" },
+                ],
+              },
+            },
+          },
+          included: [
+            {
+              'temp-id': "pos1",
+              type: "positions",
+              attributes: {title: "foo"},
+            },
+            {
+              'temp-id': "pos2",
+              type: "positions",
+              attributes: {title: "bar"},
+            },
+          ],
+        }
+      end
+
+      before do
+        klass.model = polyvalentEmployee
+        allow(controller).to receive(:resource) { klass }
+      end
+
+      context "when valid" do
+        it "responds with the persisted data" do
+          make_request
+          expect(jsonapi_included.count).to eq(2)
+          expect(jsonapi_included.map { |inc| inc.attributes["title"] }).to eq(["foo", "bar"])
+        end
+      end
+
+      context "when validation error" do
+        before do
+          payload[:data][:relationships][:positions][:data].pop
+        end
+
+        it "returns validation error response" do
+          make_request
+          expect(json["errors"].first).to match(
+            "code" => "unprocessable_entity",
+            "status" => "422",
+            "source" => {"pointer" => "/data/relationships/positions"},
+            "detail" => "Positions too short, minimum is 2",
+            "title" => "Validation Error",
+            "meta" => hash_including(
+              "attribute" => "positions",
+              "message" => "too short, minimum is 2"
+            )
+          )
+        end
+      end
+    end
+
     describe "non-writable foreign keys" do
       subject(:make_request) { do_update(payload) }
 

spec/integration/rails/transaction_hooks_spec.rb

@@ -1,7 +1,7 @@
 # rubocop: disable Style/GlobalVars
 
 if ENV["APPRAISAL_INITIALIZED"]
-  RSpec.describe "before_ & after_commit hooks", type: :controller do
+  RSpec.describe "after_graph_persist, before_ & after_commit hooks", type: :controller do
     class Callbacks
       class << self
         attr_accessor :fired, :in_transaction_during, :entities
@@ -27,6 +27,7 @@ def in_transaction?
       Callbacks.fired = {}
       Callbacks.in_transaction_during = {}
       Callbacks.entities = []
+      $raise_on_after_graph_persist = {employee: false}
       $raise_on_before_commit = {employee: true}
     end
 
@@ -63,7 +64,7 @@ class PositionResource < ApplicationResource
         before_commit do |position|
           Callbacks.add(:before_position, position)
           if $raise_on_before_commit[:position]
-            raise "rollitback_book"
+            raise "rollitback_position"
           end
         end
 
@@ -75,6 +76,13 @@ class EmployeeResource < ApplicationResource
 
         attribute :first_name, :string
 
+        after_graph_persist do |employee|
+          Callbacks.add(:after_graph_persist, employee)
+          if $raise_on_after_graph_persist[:employee]
+            raise "rollitback"
+          end
+        end
+
         before_commit only: [:create] do |employee|
           Callbacks.add(:before_create, employee)
           if $raise_on_before_commit[:employee]
@@ -191,7 +199,7 @@ def json
           post :create, params: payload
         }.to raise_error("rollitback")
         expect(Employee.count).to be_zero
-        expect(Callbacks.entities.length).to eq(1)
+        expect(Callbacks.entities.length).to eq(2)
         expect(Callbacks.fired[:before_create]).to be_a(Employee)
       end
 
@@ -218,7 +226,7 @@ def json
           expect {
             post :create, params: payload
           }.to raise_error("whoops")
-          expect(Callbacks.entities).to be_empty
+          expect(Callbacks.entities).to include(:after_graph_persist)
         end
       end
 
@@ -230,13 +238,31 @@ def json
         it "fires all before and after_commit hooks" do
           post :create, params: payload
           expect(Callbacks.entities).to eq([
+            :after_graph_persist,
             :before_create,
             :stacked_before_create,
             :employee_after_create,
             :employee_after_create_eval_test,
           ])
         end
       end
+
+      context "when an error is raised after_graph_persist" do
+        before do
+          $raise_on_after_graph_persist = {employee: true}
+        end
+
+        it "does not run before_commit callbacks" do
+          expect_any_instance_of(Graphiti::Util::ValidationResponse)
+            .to_not receive(:validate!)
+          expect {
+            post :create, params: payload
+          }.to raise_error("rollitback")
+          expect(Employee.count).to be_zero
+          expect(Callbacks.entities.length).to eq(1)
+          expect(Callbacks.fired[:after_graph_persist]).to be_a(Employee)
+        end
+      end
     end
 
     context "nested" do
@@ -281,6 +307,7 @@ def json
         post :create, params: payload
 
         expect(Callbacks.entities).to eq([
+          :after_graph_persist,
           :before_create,
           :stacked_before_create,
           :before_position,
@@ -309,6 +336,12 @@ def json
         expect(Callbacks.in_transaction_during[:employee_after_create]).to eq false
         expect(Callbacks.fired[:employee_after_create_eval_test]).to be_a(IntegrationHooks::EmployeeResource)
       end
+
+      it "can access children resources from after_graph_persist" do
+        post :create, params: payload
+        expect(Callbacks.fired[:employee_after_create].positions.length).to eq(1)
+        expect(Callbacks.fired[:employee_after_create].positions[0]).to be_a(Position)
+      end
     end
 
     context "when yielding meta" do