aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activerecord/CHANGELOG.md13
-rw-r--r--activerecord/lib/active_record/associations/preloader/through_association.rb21
-rw-r--r--activerecord/lib/active_record/reflection.rb2
-rw-r--r--activerecord/test/cases/reflection_test.rb16
-rw-r--r--activerecord/test/fixtures/sponsors.yml2
-rw-r--r--activerecord/test/models/cake_designer.rb3
-rw-r--r--activerecord/test/models/chef.rb3
-rw-r--r--activerecord/test/models/department.rb4
-rw-r--r--activerecord/test/models/drink_designer.rb3
-rw-r--r--activerecord/test/models/hotel.rb6
-rw-r--r--activerecord/test/schema/schema.rb16
-rw-r--r--activesupport/lib/active_support/core_ext/class/attribute.rb7
-rw-r--r--guides/source/active_record_validations.md2
13 files changed, 82 insertions, 16 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 90b5cb3b8f..0b7fbe5a48 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,16 @@
+* `scope_chain` should not be mutated for other reflections.
+
+ Currently `scope_chain` uses same array for building different
+ `scope_chain` for different associations. During processing
+ these arrays are sometimes mutated and because of in-place
+ mutation the changed `scope_chain` impacts other reflections.
+
+ Fix is to dup the value before adding to the `scope_chain`.
+
+ Fixes #3882.
+
+ *Neeraj Singh*
+
* Prevent the inversed association from being reloaded on save.
Fixes #9499.
diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb
index 3166df57eb..2a8530af62 100644
--- a/activerecord/lib/active_record/associations/preloader/through_association.rb
+++ b/activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -29,29 +29,30 @@ module ActiveRecord
source_reflection.name,
reflection_scope)
+ @preloaded_records = preloaders.flat_map(&:preloaded_records)
+
middle_to_pl = preloaders.each_with_object({}) do |pl,h|
pl.owners.each { |middle|
h[middle] = pl
}
end
+ record_offset = {}
+ @preloaded_records.each_with_index do |record,i|
+ record_offset[record] = i
+ end
+
through_records.each_with_object({}) { |(lhs,center),records_by_owner|
pl_to_middle = center.group_by { |record| middle_to_pl[record] }
records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles|
rhs_records = middles.flat_map { |r|
- r.send(source_reflection.name)
+ association = r.association source_reflection.name
+
+ association.reader
}.compact
- loaded_records = pl.preloaded_records
- i = 0
- record_index = loaded_records.each_with_object({}) { |r,indexes|
- indexes[r] = i
- i += 1
- }
- records = rhs_records.sort_by { |rhs| record_index[rhs] }
- @preloaded_records.concat rhs_records
- records
+ rhs_records.sort_by { |rhs| record_offset[rhs] }
end
}
end
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index e88c5d17cb..bce7766501 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -574,7 +574,7 @@ module ActiveRecord
# Add to it the scope from this reflection (if any)
scope_chain.first << scope if scope
- through_scope_chain = through_reflection.scope_chain
+ through_scope_chain = through_reflection.scope_chain.map(&:dup)
if options[:source_type]
through_scope_chain.first <<
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index 0e5c7df2cc..d7ad5ed29f 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -18,6 +18,11 @@ require 'models/subscription'
require 'models/tag'
require 'models/sponsor'
require 'models/edge'
+require 'models/hotel'
+require 'models/chef'
+require 'models/department'
+require 'models/cake_designer'
+require 'models/drink_designer'
class ReflectionTest < ActiveRecord::TestCase
include ActiveRecord::Reflection
@@ -227,6 +232,17 @@ class ReflectionTest < ActiveRecord::TestCase
assert_equal expected, actual
end
+ def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case
+ @hotel = Hotel.create!
+ @department = @hotel.departments.create!
+ @department.chefs.create!(employable: CakeDesigner.create!)
+ @department.chefs.create!(employable: DrinkDesigner.create!)
+
+ assert_equal 1, @hotel.cake_designers.size
+ assert_equal 1, @hotel.drink_designers.size
+ assert_equal 2, @hotel.chefs.size
+ end
+
def test_nested?
assert !Author.reflect_on_association(:comments).nested?
assert Author.reflect_on_association(:tags).nested?
diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml
index bfc6b238b1..2da541c539 100644
--- a/activerecord/test/fixtures/sponsors.yml
+++ b/activerecord/test/fixtures/sponsors.yml
@@ -8,5 +8,5 @@ boring_club_sponsor_for_groucho:
sponsorable_type: Member
crazy_club_sponsor_for_groucho:
sponsor_club: crazy_club
- sponsorable_id: 2
+ sponsorable_id: 3
sponsorable_type: Member
diff --git a/activerecord/test/models/cake_designer.rb b/activerecord/test/models/cake_designer.rb
new file mode 100644
index 0000000000..9c57ef573a
--- /dev/null
+++ b/activerecord/test/models/cake_designer.rb
@@ -0,0 +1,3 @@
+class CakeDesigner < ActiveRecord::Base
+ has_one :chef, as: :employable
+end
diff --git a/activerecord/test/models/chef.rb b/activerecord/test/models/chef.rb
new file mode 100644
index 0000000000..67a4e54f06
--- /dev/null
+++ b/activerecord/test/models/chef.rb
@@ -0,0 +1,3 @@
+class Chef < ActiveRecord::Base
+ belongs_to :employable, polymorphic: true
+end
diff --git a/activerecord/test/models/department.rb b/activerecord/test/models/department.rb
new file mode 100644
index 0000000000..08004a0ed3
--- /dev/null
+++ b/activerecord/test/models/department.rb
@@ -0,0 +1,4 @@
+class Department < ActiveRecord::Base
+ has_many :chefs
+ belongs_to :hotel
+end
diff --git a/activerecord/test/models/drink_designer.rb b/activerecord/test/models/drink_designer.rb
new file mode 100644
index 0000000000..2db968ef11
--- /dev/null
+++ b/activerecord/test/models/drink_designer.rb
@@ -0,0 +1,3 @@
+class DrinkDesigner < ActiveRecord::Base
+ has_one :chef, as: :employable
+end
diff --git a/activerecord/test/models/hotel.rb b/activerecord/test/models/hotel.rb
new file mode 100644
index 0000000000..b352cd22f3
--- /dev/null
+++ b/activerecord/test/models/hotel.rb
@@ -0,0 +1,6 @@
+class Hotel < ActiveRecord::Base
+ has_many :departments
+ 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
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 75711673a7..88a686d436 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -787,6 +787,22 @@ ActiveRecord::Schema.define do
t.string 'from'
end
+ create_table :hotels, force: true do |t|
+ end
+ create_table :departments, force: true do |t|
+ t.integer :hotel_id
+ end
+ create_table :cake_designers, force: true do |t|
+ end
+ create_table :drink_designers, force: true do |t|
+ end
+ create_table :chefs, force: true do |t|
+ t.integer :employable_id
+ t.string :employable_type
+ t.integer :department_id
+ end
+
+
except 'SQLite' do
# fk_test_has_fk should be before fk_test_has_pk
create_table :fk_test_has_fk, :force => true do |t|
diff --git a/activesupport/lib/active_support/core_ext/class/attribute.rb b/activesupport/lib/active_support/core_ext/class/attribute.rb
index 83038f9da5..f2a221c396 100644
--- a/activesupport/lib/active_support/core_ext/class/attribute.rb
+++ b/activesupport/lib/active_support/core_ext/class/attribute.rb
@@ -118,7 +118,10 @@ class Class
end
private
- def singleton_class?
- ancestors.first != self
+
+ unless respond_to?(:singleton_class?)
+ def singleton_class?
+ ancestors.first != self
+ end
end
end
diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md
index 797b996357..0df52a655f 100644
--- a/guides/source/active_record_validations.md
+++ b/guides/source/active_record_validations.md
@@ -438,8 +438,6 @@ provide a personalized message or use `presence: true` instead. When
`:in` or `:within` have a lower limit of 1, you should either provide a
personalized message or call `presence` prior to `length`.
-The `size` helper is an alias for `length`.
-
### `numericality`
This helper validates that your attributes have only numeric values. By