From a9484d9162e425a5fb720b13107259490f846b94 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Sun, 31 Dec 2023 17:07:07 +0530 Subject: [PATCH 1/3] refactor: lookup polymorphic types only once --- lib/jsonapi/resource_common.rb | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/lib/jsonapi/resource_common.rb b/lib/jsonapi/resource_common.rb index c4731df1..001de721 100644 --- a/lib/jsonapi/resource_common.rb +++ b/lib/jsonapi/resource_common.rb @@ -1020,17 +1020,7 @@ def polymorphic(polymorphic = true) end def _polymorphic_types - @poly_hash ||= {}.tap do |hash| - ObjectSpace.each_object do |klass| - next unless Module === klass - if klass < ActiveRecord::Base - klass.reflect_on_all_associations(:has_many).select{|r| r.options[:as] }.each do |reflection| - (hash[reflection.options[:as]] ||= []) << klass.name.underscore - end - end - end - end - @poly_hash[_polymorphic_name.to_sym] + JSONAPI::Relationship.polymorphic_types(_polymorphic_name.to_sym) end def _polymorphic_resource_klasses From 6deebf3e6cb9ea170612cc847754f3df6a9afd9f Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Sun, 31 Dec 2023 17:14:23 +0530 Subject: [PATCH 2/3] fix: more flexible polymorphic types lookup --- lib/jsonapi/relationship.rb | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb index 31a053b2..94df45dc 100644 --- a/lib/jsonapi/relationship.rb +++ b/lib/jsonapi/relationship.rb @@ -90,14 +90,37 @@ def self.polymorphic_types(name) @poly_hash ||= {}.tap do |hash| ObjectSpace.each_object do |klass| next unless Module === klass - if ActiveRecord::Base > klass + is_active_record_inspectable = ActiveRecord::Base > klass + is_active_record_inspectable &&= klass.respond_to?(:reflect_on_all_associations, true) + is_active_record_inspectable &&= format_polymorphic_klass_type(klass).present? + if is_active_record_inspectable klass.reflect_on_all_associations(:has_many).select { |r| r.options[:as] }.each do |reflection| - (hash[reflection.options[:as]] ||= []) << klass.name.underscore + (hash[reflection.options[:as]] ||= []) << format_polymorphic_klass_type(klass).underscore end end end end - @poly_hash[name.to_sym] + @poly_hash.fetch(name.to_sym) do + klass = name.classify.safe_constantize + if klass.nil? + warn "[POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for #{name}" + else + polymorphic_type = format_polymorphic_klass_type(klass) + warn "[POLYMORPHIC TYPE] Found polymorphic types through reflection for #{name}: #{polymorphic_type}" + @poly_hash[name.to_sym] = [polymorphic_type] + end + end + end + + def self.format_polymorphic_klass_type(klass) + klass.name || + begin + klass.model_name.name + rescue ArgumentError => ex + # klass.base_class may be nil + warn "[POLYMORPHIC TYPE] #{__callee__} #{klass} #{ex.inspect}" + nil + end end def resource_types @@ -203,7 +226,7 @@ def polymorphic_type def setup_implicit_relationships_for_polymorphic_types(exclude_linkage_data: true) types = self.class.polymorphic_types(_relation_name) unless types.present? - warn "No polymorphic types found for #{parent_resource.name} #{_relation_name}" + warn "[POLYMORPHIC TYPE] No polymorphic types found for #{parent_resource.name} #{_relation_name}" return end From cc46f11014371b04a0b0a42849e62378cd1c4c9d Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 17 Jan 2024 01:45:46 -0600 Subject: [PATCH 3/3] test: add polymorphic lookup tests they pass on v-11-dev I'm going to look into the existing lookup warnings now ``` [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for fileable [POLYMORPHIC TYPE] No polymorphic types found for FilePropertiesResource fileable [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for respondent [POLYMORPHIC TYPE] No polymorphic types found for QuestionResource respondent [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for respondent [POLYMORPHIC TYPE] No polymorphic types found for AnswerResource respondent [POLYMORPHIC TYPE NOT FOUND] No polymorphic types found for keepable [POLYMORPHIC TYPE] No polymorphic types found for KeeperResource keepable ``` --- test/fixtures/active_record.rb | 60 ++++++++++++++ test/integration/requests/polymorphic_test.rb | 83 +++++++++++++++++++ test/test_helper.rb | 3 + 3 files changed, 146 insertions(+) create mode 100644 test/integration/requests/polymorphic_test.rb diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index c08aa896..fe5068d5 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -429,6 +429,18 @@ t.integer :version t.timestamps null: false end + create_table :contact_media do |t| + t.string :name + t.references :party, polymorphic: true, index: true + end + + create_table :individuals do |t| + t.string :name + end + + create_table :organizations do |t| + t.string :name + end end ### MODELS @@ -624,6 +636,24 @@ class Fact < ActiveRecord::Base class Like < ActiveRecord::Base end +class TestApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end + +class ContactMedium < TestApplicationRecord + belongs_to :party, polymorphic: true, inverse_of: :contact_media + belongs_to :individual, -> { where( contact_media: { party_type: 'Individual' } ).eager_load( :contact_media ) }, foreign_key: 'party_id' + belongs_to :organization, -> { where( contact_media: { party_type: 'Organization' } ).eager_load( :contact_media ) }, foreign_key: 'party_id' +end + +class Individual < TestApplicationRecord + has_many :contact_media, as: :party +end + +class Organization < TestApplicationRecord + has_many :contact_media, as: :party +end + class Breed include ActiveModel::Model @@ -1227,6 +1257,18 @@ class IndicatorsController < JSONAPI::ResourceController class RobotsController < JSONAPI::ResourceController end +class IndividualsController < BaseController +end + +class OrganizationsController < BaseController +end + +class ContactMediaController < BaseController +end + +class PartiesController < BaseController +end + ### RESOURCES class BaseResource < JSONAPI::Resource abstract @@ -2685,6 +2727,24 @@ class RobotResource < ::JSONAPI::Resource end end +class ContactMediumResource < JSONAPI::Resource + attribute :name + has_one :party, polymorphic: true +end + +class IndividualResource < JSONAPI::Resource + attribute :name + has_many :contact_media +end + +class OrganizationResource < JSONAPI::Resource + attribute :name + has_many :contact_media +end + +class PartyResource < JSONAPI::Resource +end + ### PORO Data - don't do this in a production app $breed_data = BreedData.new $breed_data.add(Breed.new(0, 'persian')) diff --git a/test/integration/requests/polymorphic_test.rb b/test/integration/requests/polymorphic_test.rb new file mode 100644 index 00000000..c259e030 --- /dev/null +++ b/test/integration/requests/polymorphic_test.rb @@ -0,0 +1,83 @@ +require File.expand_path('../../../test_helper', __FILE__) + +# copied from https://github.com/cerebris/jsonapi-resources/blob/e60dc7dd2c7fdc85834163a7e706a10a8940a62b/test/integration/requests/polymorphic_test.rb +# https://github.com/cerebris/jsonapi-resources/compare/bf4_fix_polymorphic_relations_lookup?expand=1 +class PolymorphicTest < ActionDispatch::IntegrationTest + + def json_api_headers + {'Accept' => JSONAPI::MEDIA_TYPE, 'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE} + end + + def test_find_party_via_contact_medium + individual = Individual.create(name: 'test') + contact_medium = ContactMedium.create(party: individual, name: 'test contact medium') + fetched_party = contact_medium.party + assert_same individual, fetched_party, "Expect an individual to have been found via contact medium model's relationship 'party'" + end + + def test_get_individual + individual = Individual.create(name: 'test') + ContactMedium.create(party: individual, name: 'test contact medium') + get "/individuals/#{individual.id}" + assert_jsonapi_response 200 + end + + def test_get_party_via_contact_medium + individual = Individual.create(name: 'test') + contact_medium = ContactMedium.create(party: individual, name: 'test contact medium') + get "/contact_media/#{contact_medium.id}/party" + assert_jsonapi_response 200, "Expect an individual to have been found via contact medium resource's relationship 'party'" + end +end + +# copied from https://github.com/cerebris/jsonapi-resources/pull/1349/files +# require File.expand_path('../test_helper', __FILE__) +# +# Replace this with the code necessary to make your test fail. +# class BugTest < Minitest::Test +# include Rack::Test::Methods +# +# def json_api_headers +# {'Accept' => JSONAPI::MEDIA_TYPE, 'CONTENT_TYPE' => JSONAPI::MEDIA_TYPE} +# end +# +# def teardown +# Individual.delete_all +# ContactMedium.delete_all +# end +# +# def test_find_party_via_contact_medium +# individual = Individual.create(name: 'test') +# contact_medium = ContactMedium.create(party: individual, name: 'test contact medium') +# fetched_party = contact_medium.party +# assert_same individual, fetched_party, "Expect an individual to have been found via contact medium model's relationship 'party'" +# end +# +# def test_get_individual +# individual = Individual.create(name: 'test') +# ContactMedium.create(party: individual, name: 'test contact medium') +# get "/individuals/#{individual.id}" +# assert_last_response_status 200 +# end +# +# def test_get_party_via_contact_medium +# individual = Individual.create(name: 'test') +# contact_medium = ContactMedium.create(party: individual, name: 'test contact medium') +# get "/contact_media/#{contact_medium.id}/party" +# assert_last_response_status 200, "Expect an individual to have been found via contact medium resource's relationship 'party'" +# end +# +# private +# +# def assert_last_response_status(status, failure_reason=nil) +# if last_response.status != status +# json_response = JSON.parse(last_response.body) rescue last_response.body +# pp json_response +# end +# assert_equal status, last_response.status, failure_reason +# end +# +# def app +# Rails.application +# end +# end diff --git a/test/test_helper.rb b/test/test_helper.rb index b92d8428..5487b699 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -414,6 +414,9 @@ class CatResource < JSONAPI::Resource jsonapi_resources :widgets, only: [:index] jsonapi_resources :indicators, only: [:index] jsonapi_resources :robots, only: [:index] + jsonapi_resources :contact_media + jsonapi_resources :individuals + jsonapi_resources :organizations mount MyEngine::Engine => "/boomshaka", as: :my_engine mount ApiV2Engine::Engine => "/api_v2", as: :api_v2_engine