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.

9 files changed, 65 insertions, 1 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/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/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|