From 05b0ec35f6fcecd61e3989964cac101e697d90ab Mon Sep 17 00:00:00 2001 From: Jeff Keen Date: Wed, 26 May 2021 10:18:07 -0500 Subject: [PATCH 1/2] Provide access to underlying model during build/update/save process, closer mirroring rails controller patterns. Changes: - (for creates): @resource = MyResource.build(params); @resource.data #=> unsaved model with attributes applied - (for updates): @resource = MyResource.find(params); @resource.assign_attributes @resource.data #=> unsaved model with attributes applied --- lib/graphiti/request_validators/validator.rb | 1 - lib/graphiti/resource.rb | 6 +++ lib/graphiti/resource/interface.rb | 11 ++++- lib/graphiti/resource/persistence.rb | 36 +++++++++------ lib/graphiti/resource_proxy.rb | 19 +++++++- lib/graphiti/runner.rb | 2 +- lib/graphiti/serializer.rb | 1 + lib/graphiti/util/persistence.rb | 12 ++++- spec/integration/rails/callbacks_spec.rb | 47 ++++++++++++++++++++ spec/persistence_spec.rb | 28 +++++++++++- 10 files changed, 143 insertions(+), 20 deletions(-) diff --git a/lib/graphiti/request_validators/validator.rb b/lib/graphiti/request_validators/validator.rb index 632de350..83a3e47a 100644 --- a/lib/graphiti/request_validators/validator.rb +++ b/lib/graphiti/request_validators/validator.rb @@ -17,7 +17,6 @@ def validate return true unless @params.has_key?(:data) resource = @root_resource - if @params[:data].has_key?(:type) if (meta_type = deserialized_payload.meta[:type].try(:to_sym)) if @root_resource.type != meta_type && @root_resource.polymorphic? diff --git a/lib/graphiti/resource.rb b/lib/graphiti/resource.rb index 8526ef4a..e58cec4c 100644 --- a/lib/graphiti/resource.rb +++ b/lib/graphiti/resource.rb @@ -97,6 +97,12 @@ def disassociate(parent, child, association_name, type) adapter.disassociate(parent, child, association_name, type) end + def assign_with_relationships(meta, attributes, relationships, caller_model = nil, foreign_key = nil) + persistence = Graphiti::Util::Persistence \ + .new(self, meta, attributes, relationships, caller_model, foreign_key) + persistence.assign + end + def persist_with_relationships(meta, attributes, relationships, caller_model = nil, foreign_key = nil) persistence = Graphiti::Util::Persistence \ .new(self, meta, attributes, relationships, caller_model, foreign_key) diff --git a/lib/graphiti/resource/interface.rb b/lib/graphiti/resource/interface.rb index 0f702cf3..ffa8f365 100644 --- a/lib/graphiti/resource/interface.rb +++ b/lib/graphiti/resource/interface.rb @@ -49,7 +49,16 @@ def _find(params = {}, base_scope = nil) def build(params, base_scope = nil) validate_request!(params) runner = Runner.new(self, params) - runner.proxy(base_scope, single: true, raise_on_missing: true) + runner.proxy(base_scope, single: true, raise_on_missing: true).tap do |instance| + instance.assign_attributes(params) # assign the params to the underlying model + end + end + + def load(models, base_scope = nil) + runner = Runner.new(self, {}, base_scope, :find) + runner.proxy(nil, bypass_required_filters: true).tap do |r| + r.data = models + end end private diff --git a/lib/graphiti/resource/persistence.rb b/lib/graphiti/resource/persistence.rb index d3a21377..164c8f87 100644 --- a/lib/graphiti/resource/persistence.rb +++ b/lib/graphiti/resource/persistence.rb @@ -4,11 +4,11 @@ module Persistence extend ActiveSupport::Concern class_methods do - def before_attributes(method = nil, only: [:create, :update], &blk) + def before_attributes(method = nil, only: [:create, :update, :assign], &blk) add_callback(:attributes, :before, method, only, &blk) end - def after_attributes(method = nil, only: [:create, :update], &blk) + def after_attributes(method = nil, only: [:create, :update, :assign], &blk) add_callback(:attributes, :after, method, only, &blk) end @@ -69,15 +69,29 @@ def add_callback(kind, lifecycle, method, only, &blk) end end + def assign(assign_params, meta = nil, action_name = nil) + id = assign_params[:id] + assign_params = assign_params.except(:id) + model_instance = nil + + run_callbacks :attributes, action_name, assign_params, meta do |params| + model_instance = if action_name != :create && id + self.class._find(id: id).data + else + call_with_meta(:build, model, meta) + end + call_with_meta(:assign_attributes, model_instance, params, meta) + model_instance + end + + model_instance + end + def create(create_params, meta = nil) model_instance = nil run_callbacks :persistence, :create, create_params, meta do - run_callbacks :attributes, :create, create_params, meta do |params| - model_instance = call_with_meta(:build, model, meta) - call_with_meta(:assign_attributes, model_instance, params, meta) - model_instance - end + model_instance = assign(create_params, meta, :create) run_callbacks :save, :create, model_instance, meta do model_instance = call_with_meta(:save, model_instance, meta) @@ -89,15 +103,9 @@ def create(create_params, meta = nil) def update(update_params, meta = nil) model_instance = nil - id = update_params[:id] - update_params = update_params.except(:id) run_callbacks :persistence, :update, update_params, meta do - run_callbacks :attributes, :update, update_params, meta do |params| - model_instance = self.class._find(id: id).data - call_with_meta(:assign_attributes, model_instance, params, meta) - model_instance - end + model_instance = assign(update_params, meta, :update) run_callbacks :save, :update, model_instance, meta do model_instance = call_with_meta(:save, model_instance, meta) diff --git a/lib/graphiti/resource_proxy.rb b/lib/graphiti/resource_proxy.rb index a698f8d9..2116079a 100644 --- a/lib/graphiti/resource_proxy.rb +++ b/lib/graphiti/resource_proxy.rb @@ -8,6 +8,7 @@ def initialize(resource, scope, query, payload: nil, single: false, raise_on_missing: false, + data: nil, cache: nil, cache_expires_in: nil) @@ -74,6 +75,11 @@ def as_graphql(options = {}) Renderer.new(self, options).as_graphql end + def data=(models) + @data = data + [@data].flatten.compact.each { |r| @resource.decorate_record(r) } + end + def data @data ||= begin records = @scope.resolve @@ -84,6 +90,7 @@ def data records end end + alias_method :to_a, :data alias_method :resolve_data, :data @@ -117,6 +124,16 @@ def pagination @pagination ||= Delegates::Pagination.new(self) end + def assign_attributes(params = nil) + # deserialize params again? + + @data = @resource.assign_with_relationships( + @payload.meta, + @payload.attributes, + @payload.relationships + ) + end + def save(action: :create) # TODO: remove this. Only used for persisting many-to-many with AR # (see activerecord adapter) @@ -170,7 +187,7 @@ def update save(action: :update) end - alias update_attributes update # standard:disable Style/Alias + alias_method :update_attributes, :update def include_hash @include_hash ||= begin diff --git a/lib/graphiti/runner.rb b/lib/graphiti/runner.rb index d2b9d070..fc9a2ea7 100644 --- a/lib/graphiti/runner.rb +++ b/lib/graphiti/runner.rb @@ -8,7 +8,6 @@ def initialize(resource_class, params, query = nil, action = nil) @params = params @query = query @action = action - validator = RequestValidator.new(jsonapi_resource, params, action) validator.validate! @@ -72,6 +71,7 @@ def proxy(base = nil, opts = {}) payload: deserialized_payload, single: opts[:single], raise_on_missing: opts[:raise_on_missing], + data: opts[:data], cache: opts[:cache], cache_expires_in: opts[:cache_expires_in] end diff --git a/lib/graphiti/serializer.rb b/lib/graphiti/serializer.rb index 47ebd7bd..b1d8cbe7 100644 --- a/lib/graphiti/serializer.rb +++ b/lib/graphiti/serializer.rb @@ -100,6 +100,7 @@ def strip_relationships!(hash) def strip_relationships? return false unless Graphiti.config.links_on_demand params = Graphiti.context[:object]&.params || {} + [false, nil, "false"].include?(params[:links]) end end diff --git a/lib/graphiti/util/persistence.rb b/lib/graphiti/util/persistence.rb index 05e63723..202e6b74 100644 --- a/lib/graphiti/util/persistence.rb +++ b/lib/graphiti/util/persistence.rb @@ -24,6 +24,14 @@ def initialize(resource, meta, attributes, relationships, caller_model, foreign_ end end + def assign + attributes = @adapter.persistence_attributes(self, @attributes) + assigned = @resource.assign(attributes, @meta, :assign) + @resource.decorate_record(assigned) + + assigned + end + # Perform the actual save logic. # # belongs_to must be processed before/separately from has_many - @@ -49,8 +57,8 @@ def run parents = @adapter.process_belongs_to(self, attributes) persisted = persist_object(@meta[:method], attributes) @resource.decorate_record(persisted) - assign_temp_id(persisted, @meta[:temp_id]) + assign_temp_id(persisted, @meta[:temp_id]) associate_parents(persisted, parents) children = @adapter.process_has_many(self, persisted) @@ -129,6 +137,8 @@ def associate_children(object, children) def persist_object(method, attributes) case method + when :assign + call_resource_method(:assign, attributes, @caller_model) when :destroy call_resource_method(:destroy, attributes[:id], @caller_model) when :update, nil, :disassociate diff --git a/spec/integration/rails/callbacks_spec.rb b/spec/integration/rails/callbacks_spec.rb index f08a74b7..893b9853 100644 --- a/spec/integration/rails/callbacks_spec.rb +++ b/spec/integration/rails/callbacks_spec.rb @@ -8,6 +8,7 @@ routes.draw do post "create" => "anonymous#create" + put "update" => "anonymous#update" delete "destroy" => "anonymous#destroy" end @@ -34,6 +35,7 @@ class ApplicationResource < Graphiti::Resource class EmployeeResource < ApplicationResource self.model = Employee + self.type = "employees" before_attributes :one before_attributes :two @@ -168,6 +170,18 @@ def create end end + def update + employee = IntegrationCallbacks::EmployeeResource._find(params) + Thread.current[:proxy] = employee + employee.assign_attributes + + if employee.update_attributes + render jsonapi: employee + else + raise "whoops" + end + end + def destroy employee = IntegrationCallbacks::EmployeeResource._find(params) Thread.current[:proxy] = employee @@ -227,6 +241,39 @@ def params end end + describe "update callbacks" do + let!(:employee) { Employee.create!(first_name: "asdf") } + let(:payload) { + {id: employee.id, + data: { + id: employee.id, + type: "employees", + attributes: {first_name: "Jane"} + }} + } + + it "fires hooks in order" do + expect { + put :update, params: payload + }.to change { Employee.find(employee.id).first_name } + employee = proxy.data + expect(employee.first_name) + .to eq("Jane5a6a7a12347b6b5b_12a_13a_14a89_10_11_14b_13b_12b") + end + + context "when an error is raised" do + before do + $raise = true + end + + it "rolls back the transaction" do + expect { + expect { put :update, params: payload }.to raise_error("test") + }.to_not(change { Employee.count }) + end + end + end + describe "destroy callbacks" do let!(:employee) { Employee.create!(first_name: "Jane") } diff --git a/spec/persistence_spec.rb b/spec/persistence_spec.rb index 0911d0d6..01945fae 100644 --- a/spec/persistence_spec.rb +++ b/spec/persistence_spec.rb @@ -37,6 +37,22 @@ def expect_errors(object, expected) expect(employee.data.first_name).to eq("Jane") end + it "can access the unsaved model after build" do + employee = klass.build(payload) + expect(employee.data).to_not be_nil + expect(employee.data.first_name).to eq("Jane") + expect(employee.data.id).to be_nil + end + + xit "can modify attributes directly on the unsaved model before save" do + employee = klass.build(payload) + expect(employee.data).to_not be_nil + employee.data.first_name = "June" + + expect(employee.save).to eq(true) + expect(employee.data.first_name).to eq("June") + end + describe "updating" do let!(:employee) { PORO::Employee.create(first_name: "asdf") } @@ -52,6 +68,16 @@ def expect_errors(object, expected) }.to raise_error(Graphiti::Errors::RecordNotFound) end end + + it "can apply attributes and access model" do + employee = klass.find(payload) + expect(employee.data.first_name).to eq("asdf") + employee.assign_attributes + expect(employee.data.first_name).to eq("Jane") + + employee = klass.find(payload) + expect(employee.data.first_name).to eq("asdf") + end end describe "destroying" do @@ -1825,8 +1851,8 @@ def delete(model, meta) context "and it is a create operation" do it "works" do - instance = klass.build(payload) expect { + instance = klass.build(payload) instance.save }.to raise_error(Graphiti::Errors::InvalidRequest, /data.attributes.id/) end From 8997e49571b6fa7cb1bae82389851cd3340ae3cc Mon Sep 17 00:00:00 2001 From: Jeff Keen Date: Tue, 27 Feb 2024 17:50:12 -0600 Subject: [PATCH 2/2] Remove default callback for assignment to keep callback order in tact for possible compatibility issues --- lib/graphiti/resource/persistence.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/graphiti/resource/persistence.rb b/lib/graphiti/resource/persistence.rb index 164c8f87..9840f413 100644 --- a/lib/graphiti/resource/persistence.rb +++ b/lib/graphiti/resource/persistence.rb @@ -4,11 +4,11 @@ module Persistence extend ActiveSupport::Concern class_methods do - def before_attributes(method = nil, only: [:create, :update, :assign], &blk) + def before_attributes(method = nil, only: [:create, :update], &blk) add_callback(:attributes, :before, method, only, &blk) end - def after_attributes(method = nil, only: [:create, :update, :assign], &blk) + def after_attributes(method = nil, only: [:create, :update], &blk) add_callback(:attributes, :after, method, only, &blk) end