From fb86ecd604a362355827bbf05776b847b0ded9a5 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 5 Feb 2018 21:40:44 +0900 Subject: Make `reflection.klass` raise if `polymorphic?` not to be misused This is an alternative of #31877 to fix #31876 caused by #28808. This issue was caused by a combination of several loose implementation. * finding automatic inverse association of polymorphic without context (caused by #28808) * returning `klass` even if `polymorphic?` (exists before #28808) * loose verification by `valid_inverse_reflection?` (exists before #28808) This makes `klass` raise if `polymorphic?` not to be misused. This issue will not happen unless polymorphic `klass` is misused. Fixes #31876. Closes #31877. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/reflection.rb | 5 ++++- .../test/cases/associations/inverse_associations_test.rb | 10 ++++++++++ activerecord/test/models/face.rb | 1 + activerecord/test/models/man.rb | 3 +++ activerecord/test/models/sponsor.rb | 1 + activerecord/test/schema/schema.rb | 2 ++ 7 files changed, 27 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 325c4abfc8..36bb5e1daf 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 6.0.0.alpha (Unreleased) ## +* Make `reflection.klass` raise if `polymorphic?` not to be misused. + + Fixes #31876. + + *Ryuta Kamizono* + * Rails 6 requires Ruby 2.4.1 or newer. *Jeremy Daer* diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index c66c7778f4..101ac2d33a 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -416,6 +416,9 @@ module ActiveRecord # Active Record class. class AssociationReflection < MacroReflection #:nodoc: def compute_class(name) + if polymorphic? + raise ArgumentError, "Polymorphic association does not support to compute class." + end active_record.send(:compute_type, name) end @@ -610,7 +613,7 @@ module ActiveRecord begin reflection = klass._reflect_on_association(inverse_name) - rescue NameError + rescue ArgumentError # Give up: we couldn't compute the klass type so we won't be able # to find any associations either. reflection = false diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index bad1bcdb67..896bf574f4 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -190,6 +190,16 @@ class InverseAssociationTests < ActiveRecord::TestCase assert_nil belongs_to_ref.inverse_of end + def test_polymorphic_associations_dont_attempt_to_find_inverse_of + belongs_to_ref = Sponsor.reflect_on_association(:sponsor) + assert_raise(ArgumentError) { belongs_to_ref.klass } + assert_nil belongs_to_ref.inverse_of + + belongs_to_ref = Face.reflect_on_association(:human) + assert_raise(ArgumentError) { belongs_to_ref.klass } + assert_nil belongs_to_ref.inverse_of + end + def test_this_inverse_stuff firm = Firm.create!(name: "Adequate Holdings") Project.create!(name: "Project 1", firm: firm) diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb index 948435136d..e900fd40fb 100644 --- a/activerecord/test/models/face.rb +++ b/activerecord/test/models/face.rb @@ -2,6 +2,7 @@ class Face < ActiveRecord::Base belongs_to :man, inverse_of: :face + belongs_to :human, polymorphic: true belongs_to :polymorphic_man, polymorphic: true, inverse_of: :polymorphic_face # Oracle identifier length is limited to 30 bytes or less, `polymorphic` renamed `poly` belongs_to :poly_man_without_inverse, polymorphic: true diff --git a/activerecord/test/models/man.rb b/activerecord/test/models/man.rb index 3acd89a48e..e26920e951 100644 --- a/activerecord/test/models/man.rb +++ b/activerecord/test/models/man.rb @@ -11,3 +11,6 @@ class Man < ActiveRecord::Base has_many :secret_interests, class_name: "Interest", inverse_of: :secret_man has_one :mixed_case_monkey end + +class Human < Man +end diff --git a/activerecord/test/models/sponsor.rb b/activerecord/test/models/sponsor.rb index f190860fd1..18ff103ffe 100644 --- a/activerecord/test/models/sponsor.rb +++ b/activerecord/test/models/sponsor.rb @@ -3,6 +3,7 @@ class Sponsor < ActiveRecord::Base belongs_to :sponsor_club, class_name: "Club", foreign_key: "club_id" belongs_to :sponsorable, polymorphic: true + belongs_to :sponsor, polymorphic: true belongs_to :thing, polymorphic: true, foreign_type: :sponsorable_type, foreign_key: :sponsorable_id belongs_to :sponsorable_with_conditions, -> { where name: "Ernie" }, polymorphic: true, foreign_type: "sponsorable_type", foreign_key: "sponsorable_id" diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 7d008eecd5..3f4fe16678 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -814,6 +814,7 @@ ActiveRecord::Schema.define do create_table :sponsors, force: true do |t| t.integer :club_id t.references :sponsorable, polymorphic: true, index: false + t.references :sponsor, polymorphic: true, index: false end create_table :string_key_objects, id: false, force: true do |t| @@ -951,6 +952,7 @@ ActiveRecord::Schema.define do t.string :poly_man_without_inverse_type t.integer :horrible_polymorphic_man_id t.string :horrible_polymorphic_man_type + t.references :human, polymorphic: true, index: false end create_table :interests, force: true do |t| -- cgit v1.2.3