From 4d92877c741441e0a150e4f11eb75e4e235be345 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Sun, 21 Jan 2024 20:02:27 -0600 Subject: [PATCH 1/7] test: failing request posting sti with polymorphic has one --- test/fixtures/active_record.rb | 11 ++++-- test/integration/requests/request_test.rb | 41 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index c08aa896..92281e2a 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -275,6 +275,7 @@ t.string :drive_layout t.string :serial_number t.integer :person_id + t.references :imageable, polymorphic: true, index: true t.timestamps null: false end @@ -734,6 +735,9 @@ class Picture < ActiveRecord::Base class Vehicle < ActiveRecord::Base belongs_to :person + belongs_to :imageable, polymorphic: true + # belongs_to :document, -> { where( pictures: { imageable_type: 'Document' } ) }, foreign_key: 'imageable_id' + # belongs_to :product, -> { where( pictures: { imageable_type: 'Product' } ) }, foreign_key: 'imageable_id' end class Car < Vehicle @@ -743,13 +747,13 @@ class Boat < Vehicle end class Document < ActiveRecord::Base - has_many :pictures, as: :imageable + has_many :pictures, as: :imageable # polymorphic belongs_to :author, class_name: 'Person', foreign_key: 'author_id' has_one :file_properties, as: :fileable end class Product < ActiveRecord::Base - has_many :pictures, as: :imageable + has_many :pictures, as: :imageable # polymorphic belongs_to :designer, class_name: 'Person', foreign_key: 'designer_id' has_one :file_properties, as: :fileable end @@ -1336,6 +1340,7 @@ class VehicleResource < JSONAPI::Resource immutable has_one :person + has_one :imageable, polymorphic: true attributes :make, :model, :serial_number end @@ -1915,6 +1920,8 @@ class PreferencesResource < PreferencesResource; end class SectionResource < SectionResource; end class TagResource < TagResource; end class CommentResource < CommentResource; end + class DocumentResource < DocumentResource; end + class ProductResource < ProductResource; end class VehicleResource < VehicleResource; end class CarResource < CarResource; end class BoatResource < BoatResource; end diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index defe3619..e82da269 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -359,6 +359,47 @@ def test_post_polymorphic_with_has_many_relationship assert_equal Car, person.vehicles.fourth.class end + def test_post_sti_polymorphic_with_has_one_relationship + post '/cars', params: + { + 'data' => { + 'type' => 'cars', + 'attributes' => { + 'make' => 'Mazda', + 'model' => 'Miata MX5', + 'drive_layout' => 'Front Engine RWD', + 'serial_number' => '32432adfsfdysua', + }, + 'relationships' => { + 'person' => { + 'data' => { + 'type' => 'people', 'id' => '1001', + } + }, + 'imageable' => { + 'data' => { + 'type' => 'products', 'id' => '1', + } + }, + } + } + }.to_json, + headers: { + 'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE, + 'Accept' => JSONAPI::MEDIA_TYPE + } + + assert_jsonapi_response 201 + + body = JSON.parse(response.body) + car = Vehicle.find(body.dig("data", "id")) + + assert_equal "Car", car.type + assert_equal "Mazda", car.make + assert_equal Product, car.imageable.class + assert_equal Person, car.person.class + end + def test_post_polymorphic_invalid_with_wrong_type post '/people', params: { From 4e655020790392bfe48a5a36212e83a0fe358541 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 22 Jan 2024 09:25:26 -0600 Subject: [PATCH 2/7] fix: polymorphic resource assignment --- lib/jsonapi/resource_common.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index 85c81361..d4e7dfe1 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -1200,12 +1200,16 @@ def define_relationship_methods(relationship_name, relationship_klass, options) def define_foreign_key_setter(relationship) if relationship.polymorphic? define_on_resource "#{relationship.foreign_key}=" do |v| - _model.method("#{relationship.foreign_key}=").call(v[:id]) - _model.public_send("#{relationship.polymorphic_type}=", v[:type]) + _model.assign_attributes( + relationship.foreign_key => v[:id], + relationship.polymorphic_type => v[:type]&.to_s&.classify + ) end else define_on_resource "#{relationship.foreign_key}=" do |value| - _model.method("#{relationship.foreign_key}=").call(value) + _model.assign_attributes( + relationship.foreign_key => value + ) end end relationship.foreign_key From 85c067c2036db2f9a1533f785bb317d860fc914e Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 22 Jan 2024 09:33:43 -0600 Subject: [PATCH 3/7] chore: prefer not introducing assign_attributes from ActiveModel --- lib/jsonapi/resource_common.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index d4e7dfe1..de375007 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -1200,16 +1200,12 @@ def define_relationship_methods(relationship_name, relationship_klass, options) def define_foreign_key_setter(relationship) if relationship.polymorphic? define_on_resource "#{relationship.foreign_key}=" do |v| - _model.assign_attributes( - relationship.foreign_key => v[:id], - relationship.polymorphic_type => v[:type]&.to_s&.classify - ) + _model.public_send("#{relationship.foreign_key}=", v[:id]) + _model.public_send("#{relationship.polymorphic_type}=", v[:type]&.to_s&.classify) end else define_on_resource "#{relationship.foreign_key}=" do |value| - _model.assign_attributes( - relationship.foreign_key => value - ) + _model.public_send("#{relationship.foreign_key}=", value) end end relationship.foreign_key From 2e54a15d60cddb8616439e5c5f6b3280e072d754 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 22 Jan 2024 09:38:05 -0600 Subject: [PATCH 4/7] chore: extract format_model_polymorphic_type --- lib/jsonapi/resource_common.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index de375007..e45b4866 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -1199,9 +1199,12 @@ def define_relationship_methods(relationship_name, relationship_klass, options) def define_foreign_key_setter(relationship) if relationship.polymorphic? + define_on_resource "format_model_polymorphic_type" do |resource_object_type| + resource_object_type&.to_s&.classify + end define_on_resource "#{relationship.foreign_key}=" do |v| _model.public_send("#{relationship.foreign_key}=", v[:id]) - _model.public_send("#{relationship.polymorphic_type}=", v[:type]&.to_s&.classify) + _model.public_send("#{relationship.polymorphic_type}=", format_model_polymorphic_type(v[:type])) end else define_on_resource "#{relationship.foreign_key}=" do |value| From 050d4a517dad8561446b5f2f1a617792b183d5cc Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Mon, 22 Jan 2024 16:50:12 -0500 Subject: [PATCH 5/7] Add polymorphic_type_for method --- lib/jsonapi/resource_common.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index e45b4866..82cb0960 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -578,6 +578,10 @@ def resource_type_for(model) end end + def polymorphic_type_for(model_name) + model_name&.to_s&.classify + end + attr_accessor :_attributes, :_relationships, :_type, @@ -1199,12 +1203,9 @@ def define_relationship_methods(relationship_name, relationship_klass, options) def define_foreign_key_setter(relationship) if relationship.polymorphic? - define_on_resource "format_model_polymorphic_type" do |resource_object_type| - resource_object_type&.to_s&.classify - end define_on_resource "#{relationship.foreign_key}=" do |v| _model.public_send("#{relationship.foreign_key}=", v[:id]) - _model.public_send("#{relationship.polymorphic_type}=", format_model_polymorphic_type(v[:type])) + _model.public_send("#{relationship.polymorphic_type}=", self.class.polymorphic_type_for(v[:type])) end else define_on_resource "#{relationship.foreign_key}=" do |value| From c0ca4384eb19270df4a7fbd52feeff28821f19b4 Mon Sep 17 00:00:00 2001 From: lgebhardt Date: Mon, 22 Jan 2024 16:53:04 -0500 Subject: [PATCH 6/7] Favor classify over singularize.camelize --- lib/jsonapi/acts_as_resource_controller.rb | 2 +- lib/jsonapi/link_builder.rb | 2 +- lib/jsonapi/relationship.rb | 4 ++-- lib/jsonapi/resource_common.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/jsonapi/acts_as_resource_controller.rb b/lib/jsonapi/acts_as_resource_controller.rb index 86b39e6c..105d9110 100644 --- a/lib/jsonapi/acts_as_resource_controller.rb +++ b/lib/jsonapi/acts_as_resource_controller.rb @@ -164,7 +164,7 @@ def base_url end def resource_klass_name - @resource_klass_name ||= "#{self.class.name.underscore.sub(/_controller$/, '').singularize}_resource".camelize + @resource_klass_name ||= "#{self.class.name.underscore.sub(/_controller$/, '').classify}Resource" end def verify_content_type_header diff --git a/lib/jsonapi/link_builder.rb b/lib/jsonapi/link_builder.rb index 23b94a4d..63b160fc 100644 --- a/lib/jsonapi/link_builder.rb +++ b/lib/jsonapi/link_builder.rb @@ -92,7 +92,7 @@ def build_engine begin unless scopes.empty? - "#{ scopes.first.to_s.camelize }::Engine".safe_constantize + "#{ scopes.first.to_s.classify }::Engine".safe_constantize end # :nocov: diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb index da706523..77720b63 100644 --- a/lib/jsonapi/relationship.rb +++ b/lib/jsonapi/relationship.rb @@ -155,7 +155,7 @@ class ToOne < Relationship def initialize(name, options = {}) super - @class_name = options.fetch(:class_name, name.to_s.camelize) + @class_name = options.fetch(:class_name, name.to_s.classify) @foreign_key ||= "#{name}_id".to_sym @foreign_key_on = options.fetch(:foreign_key_on, :self) # if parent_resource @@ -231,7 +231,7 @@ class ToMany < Relationship def initialize(name, options = {}) super - @class_name = options.fetch(:class_name, name.to_s.camelize.singularize) + @class_name = options.fetch(:class_name, name.to_s.classify) @foreign_key ||= "#{name.to_s.singularize}_ids".to_sym @reflect = options.fetch(:reflect, true) == true # if parent_resource diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index 82cb0960..8f1af047 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -561,7 +561,7 @@ def resource_klass_for_model(model) end def _resource_name_from_type(type) - "#{type.to_s.underscore.singularize}_resource".camelize + "#{type.to_s.classify}Resource" end def resource_type_for(model) From 9e98c0ba0b0fe7d9e52db0e6d716658e35283240 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Mon, 22 Jan 2024 17:48:00 -0600 Subject: [PATCH 7/7] chore: we can rely on JSON.parsed_body now --- test/integration/requests/request_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index fa4467ca..d6bdf7f0 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -391,7 +391,7 @@ def test_post_sti_polymorphic_with_has_one_relationship assert_jsonapi_response 201 - body = JSON.parse(response.body) + body = response.parsed_body car = Vehicle.find(body.dig("data", "id")) assert_equal "Car", car.type