Skip statement cache on through association reader

If the through class has default scopes we should skip the statement
cache.

Closes #20745.

11 files changed, 120 insertions, 21 deletions

diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index 7c729676a7..c7b396f3d4 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -251,6 +251,14 @@ module ActiveRecord
             initialize_attributes(record)
           end
         end
+
+        # Returns true if statement cache should be skipped on the association reader.
+        def skip_statement_cache?
+          reflection.scope_chain.any?(&:any?) ||
+            scope.eager_loading? ||
+            klass.scope_attributes? ||
+            reflection.source_reflection.active_record.default_scopes.any?
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 0fc2b83b71..256df3ca11 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -442,12 +442,7 @@ module ActiveRecord
 
       private
       def get_records
-        if reflection.scope_chain.any?(&:any?) ||
-          scope.eager_loading? ||
-          klass.scope_attributes?
-
-          return scope.to_a
-        end
+        return scope.to_a if skip_statement_cache?
 
         conn = klass.connection
         sc = reflection.association_scope_cache(conn, owner) do
diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb
index 30c5d72482..03cb8cb8c3 100644
--- a/activerecord/lib/active_record/associations/singular_association.rb
+++ b/activerecord/lib/active_record/associations/singular_association.rb
@@ -47,12 +47,7 @@ module ActiveRecord
         end
 
         def get_records
-          if reflection.scope_chain.any?(&:any?) ||
-              scope.eager_loading? ||
-              klass.scope_attributes?
-
-            return scope.limit(1).to_a
-          end
+          return scope.limit(1).to_a if skip_statement_cache?
 
           conn = klass.connection
           sc = reflection.association_scope_cache(conn, owner) do
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index ce2557339e..b22ce42696 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -1171,4 +1171,32 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
 
     assert_deprecated { post.people(true) }
   end
+
+  def test_has_many_through_do_not_cache_association_reader_if_the_though_method_has_default_scopes
+    member = Member.create!
+    club = Club.create!
+    TenantMembership.create!(
+      member: member,
+      club: club
+    )
+
+    TenantMembership.current_member = member
+
+    tenant_clubs = member.tenant_clubs
+    assert_equal [club], tenant_clubs
+
+    TenantMembership.current_member = nil
+
+    other_member = Member.create!
+    other_club = Club.create!
+    TenantMembership.create!(
+      member: other_member,
+      club: other_club
+    )
+
+    tenant_clubs = other_member.tenant_clubs
+    assert_equal [other_club], tenant_clubs
+  ensure
+    TenantMembership.current_member = nil
+  end
 end
diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb
index 2c8feb9152..b2b46812b9 100644
--- a/activerecord/test/cases/associations/has_one_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb
@@ -16,6 +16,10 @@ require 'models/owner'
 require 'models/post'
 require 'models/comment'
 require 'models/categorization'
+require 'models/customer'
+require 'models/carrier'
+require 'models/shop_account'
+require 'models/customer_carrier'
 
 class HasOneThroughAssociationsTest < ActiveRecord::TestCase
   fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans,
@@ -347,16 +351,33 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
     end
   end
 
-  def test_association_force_reload_with_only_true_is_deprecated
-    member = Member.find(1)
+  def test_has_one_through_do_not_cache_association_reader_if_the_though_method_has_default_scopes
+    customer = Customer.create!
+    carrier = Carrier.create!
+    customer_carrier = CustomerCarrier.create!(
+      customer: customer,
+      carrier: carrier,
+    )
+    account = ShopAccount.create!(customer_carrier: customer_carrier)
 
-    assert_deprecated { member.club(true) }
-  end
+    CustomerCarrier.current_customer = customer
 
-  def test_has_one_through_associations_are_mutable_unless_through_belongs_to
-    member_detail = MemberDetail.new(member: @member)
-    assert_raise(ActiveRecord::HasOneThroughCantAssociateThroughHasOneOrManyReflection) do
-      member_detail.membership = Membership.new
-    end
+    account_carrier = account.carrier
+    assert_equal carrier, account_carrier
+
+    CustomerCarrier.current_customer = nil
+
+    other_carrier = Carrier.create!
+    other_customer = Customer.create!
+    other_customer_carrier = CustomerCarrier.create!(
+      customer: other_customer,
+      carrier: other_carrier,
+    )
+    other_account = ShopAccount.create!(customer_carrier: other_customer_carrier)
+
+    account_carrier = other_account.carrier
+    assert_equal other_carrier, account_carrier
+  ensure
+    CustomerCarrier.current_customer = nil
   end
 end
diff --git a/activerecord/test/models/carrier.rb b/activerecord/test/models/carrier.rb
new file mode 100644
index 0000000000..230be118c3
--- /dev/null
+++ b/activerecord/test/models/carrier.rb
@@ -0,0 +1,2 @@
+class Carrier < ActiveRecord::Base
+end
diff --git a/activerecord/test/models/customer_carrier.rb b/activerecord/test/models/customer_carrier.rb
new file mode 100644
index 0000000000..37186903ff
--- /dev/null
+++ b/activerecord/test/models/customer_carrier.rb
@@ -0,0 +1,14 @@
+class CustomerCarrier < ActiveRecord::Base
+  cattr_accessor :current_customer
+
+  belongs_to :customer
+  belongs_to :carrier
+
+  default_scope -> {
+    if current_customer
+      where(customer: current_customer)
+    else
+      all
+    end
+  }
+end
diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb
index dc0566d8a7..7693c6e515 100644
--- a/activerecord/test/models/member.rb
+++ b/activerecord/test/models/member.rb
@@ -26,6 +26,9 @@ class Member < ActiveRecord::Base
   has_many :current_memberships, -> { where :favourite => true }
   has_many :clubs, :through => :current_memberships
 
+  has_many :tenant_memberships
+  has_many :tenant_clubs, through: :tenant_memberships, class_name: 'Club', source: :club
+
   has_one :club_through_many, :through => :current_memberships, :source => :club
 
   belongs_to :admittable, polymorphic: true
diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb
index df7167ee93..e181ba1f11 100644
--- a/activerecord/test/models/membership.rb
+++ b/activerecord/test/models/membership.rb
@@ -18,3 +18,18 @@ class SelectedMembership < Membership
     select("'1' as foo")
   end
 end
+
+class TenantMembership < Membership
+  cattr_accessor :current_member
+
+  belongs_to :member
+  belongs_to :club
+
+  default_scope -> {
+    if current_member
+      where(member: current_member)
+    else
+      all
+    end
+  }
+end
diff --git a/activerecord/test/models/shop_account.rb b/activerecord/test/models/shop_account.rb
new file mode 100644
index 0000000000..1580e8b20c
--- /dev/null
+++ b/activerecord/test/models/shop_account.rb
@@ -0,0 +1,6 @@
+class ShopAccount < ActiveRecord::Base
+  belongs_to :customer
+  belongs_to :customer_carrier
+
+  has_one :carrier, through: :customer_carrier
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 1fa824bf1d..08ac7749f9 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -131,6 +131,8 @@ ActiveRecord::Schema.define do
     t.timestamps null: false
   end
 
+  create_table :carriers, force: true
+
   create_table :categories, force: true do |t|
     t.string :name, null: false
     t.string :type
@@ -237,6 +239,11 @@ ActiveRecord::Schema.define do
     t.string  :gps_location
   end
 
+  create_table :customer_carriers, force: true do |t|
+    t.references :customer
+    t.references :carrier
+  end
+
   create_table :dashboards, force: true, id: false do |t|
     t.string :dashboard_id
     t.string :name
@@ -683,6 +690,11 @@ ActiveRecord::Schema.define do
     t.belongs_to :ship
   end
 
+  create_table :shop_accounts, force: true do |t|
+    t.references :customer
+    t.references :customer_carrier
+  end
+
   create_table :speedometers, force: true, id: false do |t|
     t.string :speedometer_id
     t.string :name