Merge pull request #33938 from faucct/bugfix/preload_through_no_records

ActiveRecord::Associations::Preloader should not fail to preload through missing records

2 files changed, 29 insertions, 28 deletions

diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb
index a8f94b574d..9253f4ccb4 100644
--- a/activerecord/lib/active_record/associations/preloader.rb
+++ b/activerecord/lib/active_record/associations/preloader.rb
@@ -98,12 +98,14 @@ module ActiveRecord
 
         # Loads all the given data into +records+ for the +association+.
         def preloaders_on(association, records, scope, polymorphic_parent = false)
-          if association.respond_to?(:to_hash)
+          case association
+          when Hash
             preloaders_for_hash(association, records, scope, polymorphic_parent)
-          elsif association.is_a?(Symbol)
-            preloaders_for_one(association, records, scope, polymorphic_parent)
-          elsif association.respond_to?(:to_str)
-            preloaders_for_one(association.to_sym, records, scope, polymorphic_parent)
+          when Symbol, String
+            grouped_records(association.to_sym, records, polymorphic_parent)
+              .flat_map do |reflection, reflection_records|
+                preloaders_for_reflection reflection, reflection_records, scope
+              end
           else
             raise ArgumentError, "#{association.inspect} was not recognized for preload"
           end
@@ -111,17 +113,15 @@ module ActiveRecord
 
         def preloaders_for_hash(association, records, scope, polymorphic_parent)
           association.flat_map { |parent, child|
-            loaders = preloaders_for_one parent, records, scope, polymorphic_parent
-
-            recs = loaders.flat_map(&:preloaded_records).uniq
-
-            reflection = records.first.class._reflect_on_association(parent)
-            polymorphic_parent = reflection && reflection.options[:polymorphic]
-
-            loaders.concat Array.wrap(child).flat_map { |assoc|
-              preloaders_on assoc, recs, scope, polymorphic_parent
-            }
-            loaders
+            grouped_records(parent, records, polymorphic_parent).flat_map do |reflection, reflection_records|
+              loaders = preloaders_for_reflection(reflection, reflection_records, scope)
+              recs = loaders.flat_map(&:preloaded_records)
+              child_polymorphic_parent = reflection && reflection.options[:polymorphic]
+              loaders.concat Array.wrap(child).flat_map { |assoc|
+                preloaders_on assoc, recs, scope, child_polymorphic_parent
+              }
+              loaders
+            end
           }
         end
 
@@ -137,13 +137,11 @@ module ActiveRecord
         # Additionally, polymorphic belongs_to associations can have multiple associated
         # classes, depending on the polymorphic_type field. So we group by the classes as
         # well.
-        def preloaders_for_one(association, records, scope, polymorphic_parent)
-          grouped_records(association, records, polymorphic_parent).flat_map do |reflection, klasses|
-            klasses.map do |rhs_klass, rs|
-              loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope)
-              loader.run self
-              loader
-            end
+        def preloaders_for_reflection(reflection, records, scope)
+          records.group_by { |record| record.association(reflection.name).klass }.map do |rhs_klass, rs|
+            loader = preloader_for(reflection, rs).new(rhs_klass, rs, reflection, scope)
+            loader.run self
+            loader
           end
         end
 
@@ -151,11 +149,9 @@ module ActiveRecord
           h = {}
           records.each do |record|
             next unless record
-            next if polymorphic_parent && !record.class._reflect_on_association(association)
-            assoc = record.association(association)
-            next unless assoc.klass
-            klasses = h[assoc.reflection] ||= {}
-            (klasses[assoc.klass] ||= []) << record
+            reflection = record.class._reflect_on_association(association)
+            next if polymorphic_parent && !reflection || !record.association(association).klass
+            (h[reflection] ||= []) << record
           end
           h
         end
diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index ba2104eb26..4cbc65b6d8 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -160,6 +160,11 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_preload_through_missing_records
+    post = Post.where.not(author_id: Author.select(:id)).preload(author: { comments: :post }).first!
+    assert_no_queries { assert_nil post.author }
+  end
+
   def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through
     source = Vertex.all.merge!(includes: { sinks: { sinks: { sinks: :sinks } } }, order: "vertices.id").first
     assert_equal vertices(:vertex_4), assert_no_queries { source.sinks.first.sinks.first.sinks.first }