From 4a86c4f1444d4cde66ed710ebb47b1ced93142f7 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Thu, 7 Jan 2021 18:10:07 -0500 Subject: [PATCH 1/2] Run the bug --- lib/jsonapi/relationship.rb | 45 ++++++++++++++++++++++---- test/bug_1305_test.rb | 44 +++++++++++++++++++++++++ test/fixtures/active_record.rb | 59 ++++++++++++++++++++++++++++++++++ test/test_helper.rb | 14 ++++++++ 4 files changed, 155 insertions(+), 7 deletions(-) create mode 100644 test/bug_1305_test.rb diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb index 77e700b78..f23e32347 100644 --- a/lib/jsonapi/relationship.rb +++ b/lib/jsonapi/relationship.rb @@ -58,17 +58,48 @@ def table_name end def self.polymorphic_types(name) + polymorphic_types_lookup[name.to_sym] + end + + def self.polymorphic_types_lookup(warn_on_no_name: false) @poly_hash ||= {}.tap do |hash| - ObjectSpace.each_object do |klass| - next unless Module === klass - if ActiveRecord::Base > klass - klass.reflect_on_all_associations(:has_many).select{|r| r.options[:as] }.each do |reflection| - (hash[reflection.options[:as]] ||= []) << klass.name.downcase - end + build_active_record_polymorphic_types_lookup(hash, warn_on_no_name: warn_on_no_name) + end + end + + def self.build_active_record_polymorphic_types_lookup(hash, warn_on_no_name: false) + candidate_active_record_polymorphic_classes.each do |klass| + if klass.name.nil? + if warn_on_no_name + model_name = + if klass.respond_to?(:model_name) + begin + klass.model_name.name + rescue ArgumentError => e + "Responds to ActiveModel::Naming but #{e.message}" + end + else + "Does not extend ActiveModel::Naming" + end + warn "No class name found for #{klass} (#{model_name})" end + next + end + klass.reflect_on_all_associations(:has_many).select{|r| r.options[:as] }.each do |reflection| + (hash[reflection.options[:as]] ||= []) << klass.name.downcase + end + end + end + + def self.candidate_active_record_polymorphic_classes + candidate_polymorphic_classes = [] + ObjectSpace.each_object do |klass| + next unless Module === klass + if ActiveRecord::Base > klass + candidate_polymorphic_classes << klass end end - @poly_hash[name.to_sym] + candidate_polymorphic_classes end def resource_types diff --git a/test/bug_1305_test.rb b/test/bug_1305_test.rb new file mode 100644 index 000000000..05e3172dd --- /dev/null +++ b/test/bug_1305_test.rb @@ -0,0 +1,44 @@ +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.ok? + 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" + # pp [:last_response, last_response] + # ["{\"errors\":[{\"title\":\"Internal Server Error\",\"detail\":\"Internal Server Error\",\"code\":\"500\",\"status\":\"500\",\"meta\":{\"exception\":\"Can't join 'ContactMedium' to association named 'organization'; perhaps you misspelled it?\" + assert last_response.ok?, "Expect an individual to have been found via contact medium resource's relationship 'party'" + end + + private + + def app + Rails.application + end +end diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index bdb718bbf..706845bbb 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -429,6 +429,19 @@ 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 @@ -643,6 +656,22 @@ 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 +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 @@ -1246,6 +1275,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 @@ -2688,6 +2729,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/test_helper.rb b/test/test_helper.rb index 97e51fe7d..b0966241e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -46,6 +46,10 @@ ActiveSupport::Deprecation.silenced = true puts "Testing With RAILS VERSION #{Rails.version}" +Minitest.after_run do + puts "Found JSONAPI::Relationship.polymorphic_types_lookup" + puts "\t #{JSONAPI::Relationship.polymorphic_types_lookup(warn_on_no_name: true).inspect}\n" +end class TestApp < Rails::Application config.eager_load = false @@ -450,6 +454,16 @@ class CatResource < JSONAPI::Resource mount MyEngine::Engine => "/boomshaka", as: :my_engine mount ApiV2Engine::Engine => "/api_v2", as: :api_v2_engine + + jsonapi_resources :contact_media do + jsonapi_relationships + end + jsonapi_resources :individuals do + jsonapi_relationships + end + jsonapi_resources :organizations do + jsonapi_relationships + end end MyEngine::Engine.routes.draw do From ef72900917413bc110af373e66303c9deb8da389 Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 12 Jan 2021 09:12:07 -0600 Subject: [PATCH 2/2] Add some debug info to the assertions --- test/bug_1305_test.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/bug_1305_test.rb b/test/bug_1305_test.rb index 05e3172dd..f1a8c6463 100644 --- a/test/bug_1305_test.rb +++ b/test/bug_1305_test.rb @@ -24,20 +24,26 @@ def test_get_individual individual = Individual.create(name: 'test') ContactMedium.create(party: individual, name: 'test contact medium') get "/individuals/#{individual.id}" - assert last_response.ok? + 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" - # pp [:last_response, last_response] - # ["{\"errors\":[{\"title\":\"Internal Server Error\",\"detail\":\"Internal Server Error\",\"code\":\"500\",\"status\":\"500\",\"meta\":{\"exception\":\"Can't join 'ContactMedium' to association named 'organization'; perhaps you misspelled it?\" - assert last_response.ok?, "Expect an individual to have been found via contact medium resource's relationship '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) + pp json_response + end + assert_equal status, last_response.status, failure_reason + end + def app Rails.application end