diff options
author | eileencodes <eileencodes@gmail.com> | 2015-03-15 10:33:29 -0400 |
---|---|---|
committer | eileencodes <eileencodes@gmail.com> | 2015-03-15 10:39:42 -0400 |
commit | 51660f0191a7c71ce298c71e7366ebab55be729c (patch) | |
tree | 326e7e3c22269759b1bb0c194e1384f2b396e90b /activerecord/test | |
parent | b6c038b600487b1adcf8f5e69ba70a992b07e195 (diff) | |
download | rails-51660f0191a7c71ce298c71e7366ebab55be729c.tar.gz rails-51660f0191a7c71ce298c71e7366ebab55be729c.tar.bz2 rails-51660f0191a7c71ce298c71e7366ebab55be729c.zip |
Fix leaky chain on polymorphic association
If there was a polymorphic hm:t association with a scope AND second
non-scoped hm:t association on a model the polymorphic scope would leak
through into the call for the non-polymorhic hm:t association.
This would only break if `hotel.drink_designers` was called before
`hotel.recipes`. If `hotel.recipes` was called first there would be
no problem with the SQL.
Before (employable_type should not be here):
```
SELECT COUNT(*) FROM "drink_designers" INNER JOIN "chefs" ON
"drink_designers"."id" = "chefs"."employable_id" INNER JOIN
"departments" ON "chefs"."department_id" = "departments"."id" WHERE
"departments"."hotel_id" = ? AND "chefs"."employable_type" = ?
[["hotel_id", 1], ["employable_type", "DrinkDesigner"]]
```
After:
```
SELECT COUNT(*) FROM "recipes" INNER JOIN "chefs" ON "recipes"."chef_id"
= "chefs"."id" INNER JOIN "departments" ON "chefs"."department_id" =
"departments"."id" WHERE "departments"."hotel_id" = ? [["hotel_id", 1]]
```
From the SQL you can see that `employable_type` was leaking through when
calling recipes. The solution is to dup the chain of the polymorphic
association so it doesn't get cached. Additionally, this follows
`scope_chain` which dup's the `source_reflection`'s `scope_chain`.
This required another model/table/relationship because the leak only
happens on a hm:t polymorphic that's called before another hm:t on the
same model.
I am specifically testing the SQL here instead of the number of records
becasue the test could pass if there was 1 drink designer recipe for the
drink designer chef even though the `employable_type` was leaking through.
This needs to specifically check that `employable_type` is not in the SQL
statement.
Diffstat (limited to 'activerecord/test')
-rw-r--r-- | activerecord/test/cases/reflection_test.rb | 11 | ||||
-rw-r--r-- | activerecord/test/models/chef.rb | 1 | ||||
-rw-r--r-- | activerecord/test/models/hotel.rb | 1 | ||||
-rw-r--r-- | activerecord/test/models/recipe.rb | 3 | ||||
-rw-r--r-- | activerecord/test/schema/schema.rb | 4 |
5 files changed, 20 insertions, 0 deletions
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 67e9bef808..9893fbd6b9 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -23,6 +23,7 @@ require 'models/chef' require 'models/department' require 'models/cake_designer' require 'models/drink_designer' +require 'models/recipe' class ReflectionTest < ActiveRecord::TestCase include ActiveRecord::Reflection @@ -277,6 +278,16 @@ class ReflectionTest < ActiveRecord::TestCase assert_equal 2, @hotel.chefs.size end + def test_scope_chain_of_polymorphic_association_does_not_leak_into_other_hmt_associations + hotel = Hotel.create! + department = hotel.departments.create! + drink = department.chefs.create!(employable: DrinkDesigner.create!) + recipe = Recipe.create!(chef_id: drink.id, hotel_id: hotel.id) + + hotel.drink_designers.to_a + assert_sql(/^(?!.*employable_type).*$/) { hotel.recipes.to_a } + end + def test_nested? assert !Author.reflect_on_association(:comments).nested? assert Author.reflect_on_association(:tags).nested? diff --git a/activerecord/test/models/chef.rb b/activerecord/test/models/chef.rb index 67a4e54f06..698a52e045 100644 --- a/activerecord/test/models/chef.rb +++ b/activerecord/test/models/chef.rb @@ -1,3 +1,4 @@ class Chef < ActiveRecord::Base belongs_to :employable, polymorphic: true + has_many :recipes end diff --git a/activerecord/test/models/hotel.rb b/activerecord/test/models/hotel.rb index b352cd22f3..491f8dfde3 100644 --- a/activerecord/test/models/hotel.rb +++ b/activerecord/test/models/hotel.rb @@ -3,4 +3,5 @@ class Hotel < ActiveRecord::Base has_many :chefs, through: :departments has_many :cake_designers, source_type: 'CakeDesigner', source: :employable, through: :chefs has_many :drink_designers, source_type: 'DrinkDesigner', source: :employable, through: :chefs + has_many :recipes, through: :chefs end diff --git a/activerecord/test/models/recipe.rb b/activerecord/test/models/recipe.rb new file mode 100644 index 0000000000..c387230603 --- /dev/null +++ b/activerecord/test/models/recipe.rb @@ -0,0 +1,3 @@ +class Recipe < ActiveRecord::Base + belongs_to :chef +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 5e5f7a798e..7b42f8a4a5 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -886,6 +886,10 @@ ActiveRecord::Schema.define do t.string :employable_type t.integer :department_id end + create_table :recipes, force: true do |t| + t.integer :chef_id + t.integer :hotel_id + end create_table :records, force: true do |t| end |