Skip to content

Commit 63fc110

Browse files
committed
Merge pull request #31895 from kamipo/do_not_attempt_to_find_inverse_of_polymorphic
Make `reflection.klass` raise if `polymorphic?` not to be misused
1 parent 7123c9b commit 63fc110

File tree

7 files changed

+31
-10
lines changed

7 files changed

+31
-10
lines changed

activerecord/CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
* Make `reflection.klass` raise if `polymorphic?` not to be misused.
2+
3+
Fixes #31876.
4+
5+
*Ryuta Kamizono*
6+
7+
18
## Rails 5.2.0.rc1 (January 30, 2018) ##
29

310
* Deprecate `expand_hash_conditions_for_aggregates` without replacement.

activerecord/lib/active_record/reflection.rb

+7-10
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,9 @@ def mapping
416416
# Active Record class.
417417
class AssociationReflection < MacroReflection #:nodoc:
418418
def compute_class(name)
419+
if polymorphic?
420+
raise ArgumentError, "Polymorphic association does not support to compute class."
421+
end
419422
active_record.send(:compute_type, name)
420423
end
421424

@@ -608,13 +611,7 @@ def automatic_inverse_of
608611
if can_find_inverse_of_automatically?(self)
609612
inverse_name = ActiveSupport::Inflector.underscore(options[:as] || active_record.name.demodulize).to_sym
610613

611-
begin
612-
reflection = klass._reflect_on_association(inverse_name)
613-
rescue NameError
614-
# Give up: we couldn't compute the klass type so we won't be able
615-
# to find any associations either.
616-
reflection = false
617-
end
614+
reflection = klass._reflect_on_association(inverse_name)
618615

619616
if valid_inverse_reflection?(reflection)
620617
return inverse_name
@@ -626,9 +623,6 @@ def automatic_inverse_of
626623
# +automatic_inverse_of+ method is a valid reflection. We must
627624
# make sure that the reflection's active_record name matches up
628625
# with the current reflection's klass name.
629-
#
630-
# Note: klass will always be valid because when there's a NameError
631-
# from calling +klass+, +reflection+ will already be set to false.
632626
def valid_inverse_reflection?(reflection)
633627
reflection &&
634628
klass <= reflection.active_record &&
@@ -732,6 +726,9 @@ def join_foreign_key
732726
end
733727

734728
private
729+
def can_find_inverse_of_automatically?(_)
730+
!polymorphic? && super
731+
end
735732

736733
def calculate_constructable(macro, options)
737734
!polymorphic?

activerecord/test/cases/associations/inverse_associations_test.rb

+10
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,16 @@ def test_associations_with_no_inverse_of_should_return_nil
190190
assert_nil belongs_to_ref.inverse_of
191191
end
192192

193+
def test_polymorphic_associations_dont_attempt_to_find_inverse_of
194+
belongs_to_ref = Sponsor.reflect_on_association(:sponsor)
195+
assert_raise(ArgumentError) { belongs_to_ref.klass }
196+
assert_nil belongs_to_ref.inverse_of
197+
198+
belongs_to_ref = Face.reflect_on_association(:human)
199+
assert_raise(ArgumentError) { belongs_to_ref.klass }
200+
assert_nil belongs_to_ref.inverse_of
201+
end
202+
193203
def test_this_inverse_stuff
194204
firm = Firm.create!(name: "Adequate Holdings")
195205
Project.create!(name: "Project 1", firm: firm)

activerecord/test/models/face.rb

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
class Face < ActiveRecord::Base
44
belongs_to :man, inverse_of: :face
5+
belongs_to :human, polymorphic: true
56
belongs_to :polymorphic_man, polymorphic: true, inverse_of: :polymorphic_face
67
# Oracle identifier length is limited to 30 bytes or less, `polymorphic` renamed `poly`
78
belongs_to :poly_man_without_inverse, polymorphic: true

activerecord/test/models/man.rb

+3
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,6 @@ class Man < ActiveRecord::Base
1111
has_many :secret_interests, class_name: "Interest", inverse_of: :secret_man
1212
has_one :mixed_case_monkey
1313
end
14+
15+
class Human < Man
16+
end

activerecord/test/models/sponsor.rb

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
class Sponsor < ActiveRecord::Base
44
belongs_to :sponsor_club, class_name: "Club", foreign_key: "club_id"
55
belongs_to :sponsorable, polymorphic: true
6+
belongs_to :sponsor, polymorphic: true
67
belongs_to :thing, polymorphic: true, foreign_type: :sponsorable_type, foreign_key: :sponsorable_id
78
belongs_to :sponsorable_with_conditions, -> { where name: "Ernie" }, polymorphic: true,
89
foreign_type: "sponsorable_type", foreign_key: "sponsorable_id"

activerecord/test/schema/schema.rb

+2
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,7 @@
814814
create_table :sponsors, force: true do |t|
815815
t.integer :club_id
816816
t.references :sponsorable, polymorphic: true, index: false
817+
t.references :sponsor, polymorphic: true, index: false
817818
end
818819

819820
create_table :string_key_objects, id: false, force: true do |t|
@@ -951,6 +952,7 @@
951952
t.string :poly_man_without_inverse_type
952953
t.integer :horrible_polymorphic_man_id
953954
t.string :horrible_polymorphic_man_type
955+
t.references :human, polymorphic: true, index: false
954956
end
955957

956958
create_table :interests, force: true do |t|

0 commit comments

Comments
 (0)