Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix polymorphic eager load with foreign_key as String.

The foreign_key could be `String` and just doing `owners_map[owner_key]`
could return `nil`.
To prevent this bug, we should `to_s` both keys if their types are
different.

Fixes #14734.
  • Loading branch information...
commit 4debc86bd329d31360272ed15459cde3b9af3a10 1 parent b17f6e6
@laurocaetano laurocaetano authored
View
6 activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+* Fixed polymorphic eager loading when using a String as foreign key.
+
+ Fixes #14734.
+
+ *Lauro Caetano*
+
* Changed scoped blocks to be executed with `instance_eval`
Named scopes (i.e. using STI) were previously cached according to
View
19 activerecord/lib/active_record/associations/preloader/association.rb
@@ -58,7 +58,11 @@ def owner_key_name
def owners_by_key
@owners_by_key ||= owners.group_by do |owner|
- owner[owner_key_name]
+ if owner_key_type == association_key_type
+ owner[owner_key_name]
+ else
+ owner[owner_key_name].to_s
+ end
end
end
@@ -93,13 +97,24 @@ def associated_records_by_owner(preloader)
records_by_owner
end
+ def association_key_type
+ @klass.column_types[association_key_name.to_s].type
+ end
+
+ def owner_key_type
+ @model.column_types[owner_key_name.to_s].type
+ end
+
def load_slices(slices)
@preloaded_records = slices.flat_map { |slice|
records_for(slice)
}
@preloaded_records.map { |record|
- [record, record[association_key_name]]
+ key = record[association_key_name]
+ key = key.to_s unless owner_key_type == association_key_type
+
+ [record, key]
}
end
View
13 activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -570,6 +570,19 @@ def test_cant_save_readonly_association
assert companies(:first_client).readonly_firm.readonly?
end
+ def test_test_polymorphic_assignment_foreign_key_type_string
+ comment = Comment.first
+ comment.author = Author.first
+ comment.resource = Member.first
+ comment.save
+
+ assert_equal Comment.all.to_a,
+ Comment.includes(:author).to_a
+
+ assert_equal Comment.all.to_a,
+ Comment.includes(:resource).to_a
+ end
+
def test_polymorphic_assignment_foreign_type_field_updating
# should update when assigning a saved record
sponsor = Sponsor.new
View
3  activerecord/test/models/comment.rb
@@ -7,6 +7,9 @@ class Comment < ActiveRecord::Base
scope :created, -> { all }
belongs_to :post, :counter_cache => true
+ belongs_to :author, polymorphic: true
+ belongs_to :resource, polymorphic: true
+
has_many :ratings
belongs_to :first_post, :foreign_key => :post_id
View
3  activerecord/test/schema/schema.rb
@@ -187,6 +187,9 @@ def except(adapter_names_to_exclude)
t.integer :taggings_count, default: 0
t.integer :children_count, default: 0
t.integer :parent_id
+ t.references :author, polymorphic: true
+ t.string :resource_id
+ t.string :resource_type
end
create_table :companies, force: true do |t|
Please sign in to comment.
Something went wrong with that request. Please try again.