From 3328cd1d69129c7e3a14661d8a0f4b76009ec9b0 Mon Sep 17 00:00:00 2001
From: eileencodes <eileencodes@gmail.com>
Date: Tue, 12 Jan 2021 11:03:44 -0500
Subject: [PATCH] Ensure `reload` re-applies preload values for strict loading

If you have an application that has strict_loading set and then call
`reload` that would cause the preload values to get lost and
applications would start throwing a stict loading violation error.

In order to fix this we capture the association cache and re-apply that
to the reloaded records to avoid the strict loading error. Of course if
you never preloaded your records, this will still raise a strict loading
violation.

This change also removes the `reload` definition from associations.rb
because we can get the same behavior when we reassign the association
cache.

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
---
 .../lib/active_record/associations.rb         |  5 --
 activerecord/lib/active_record/persistence.rb | 11 ++++-
 .../test/cases/strict_loading_test.rb         | 48 +++++++++++++++++++
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index da2f109caa..91676e8b72 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -326,11 +326,6 @@ module ActiveRecord
       super
     end
 
-    def reload(*) # :nodoc:
-      clear_association_cache
-      super
-    end
-
     private
       # Clears out the association cache.
       def clear_association_cache
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 5bc05b8696..a0332cce5e 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -835,6 +835,7 @@ module ActiveRecord
         self.class.unscoped { _find_record(options) }
       end
 
+      @association_cache = fresh_object.instance_variable_get(:@association_cache)
       @attributes = fresh_object.instance_variable_get(:@attributes)
       @new_record = false
       @previously_new_record = false
@@ -893,11 +894,17 @@ module ActiveRecord
     end
 
   private
+    def strict_loaded_associations
+      @association_cache.find_all do |_, assoc|
+        assoc.owner.strict_loading? && !assoc.owner.strict_loading_n_plus_one_only?
+      end.map(&:first)
+    end
+
     def _find_record(options)
       if options && options[:lock]
-        self.class.lock(options[:lock]).find(id)
+        self.class.preload(strict_loaded_associations).lock(options[:lock]).find(id)
       else
-        self.class.find(id)
+        self.class.preload(strict_loaded_associations).find(id)
       end
     end
 
diff --git a/activerecord/test/cases/strict_loading_test.rb b/activerecord/test/cases/strict_loading_test.rb
index 78ac69d15d..46c9be8c03 100644
--- a/activerecord/test/cases/strict_loading_test.rb
+++ b/activerecord/test/cases/strict_loading_test.rb
@@ -204,6 +204,54 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_strict_loading_has_one_reload
+    with_strict_loading_by_default(Developer) do
+      ship = Ship.create!(developer: Developer.first, name: "The Great Ship")
+      developer = Developer.preload(:ship).first
+
+      assert_predicate developer, :strict_loading?
+      assert_equal ship, developer.ship
+
+      developer.reload
+
+      assert_nothing_raised do
+        assert_equal ship, developer.ship
+      end
+    end
+  end
+
+  def test_strict_loading_with_has_many
+    with_strict_loading_by_default(Developer) do
+      devs = Developer.preload(:audit_logs).all
+
+      assert_nothing_raised do
+        devs.map(&:audit_logs).to_a
+      end
+
+      devs.reload
+
+      assert_nothing_raised do
+        devs.map(&:audit_logs).to_a
+      end
+    end
+  end
+
+  def test_strict_loading_with_has_many_singular_association_and_reload
+    with_strict_loading_by_default(Developer) do
+      dev = Developer.preload(:audit_logs).first
+
+      assert_nothing_raised do
+        dev.audit_logs.to_a
+      end
+
+      dev.reload
+
+      assert_nothing_raised do
+        dev.audit_logs.to_a
+      end
+    end
+  end
+
   def test_preload_audit_logs_are_strict_loading_because_parent_is_strict_loading
     developer = Developer.first