Backport caa178 to 5-0-stable

activerecord/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Inverse association instances will now be set before `after_find` or
+    `after_initialize` callbacks are run.
+
+    Fixes #26320.
+
+    *Sean Griffin*
+
 *   Avoid loading records from database when they are already loaded using
     the `pluck` method on a collection.
 

activerecord/lib/active_record/association_relation.rb

@@ -29,7 +29,10 @@ def create!(*args, &block)
     private
 
     def exec_queries
-      super.each { |r| @association.set_inverse_instance r }
+      super do |r|
+        @association.set_inverse_instance r
+        yield r if block_given?
+      end
     end
   end
 end

activerecord/lib/active_record/associations/collection_association.rb

@@ -454,7 +454,7 @@ def null_scope?
       end
 
       private
-      def get_records
+      def get_records(&block)
         return scope.to_a if skip_statement_cache?
 
         conn = klass.connection
@@ -466,13 +466,13 @@ def get_records
         end
 
         binds = AssociationScope.get_bind_values(owner, reflection.chain)
-        sc.execute binds, klass, klass.connection
+        sc.execute(binds, klass, klass.connection, &block)
       end
 
         def find_target
-          records = get_records
-          records.each { |record| set_inverse_instance(record) }
-          records
+          get_records do |record|
+            set_inverse_instance(record)
+          end
         end
 
         # We have some records loaded from the database (persisted) and some that are

activerecord/lib/active_record/associations/join_dependency.rb

@@ -284,17 +284,19 @@ def construct(ar_parent, parent, row, rs, seen, model_cache, aliases)
       end
 
       def construct_model(record, node, row, model_cache, id, aliases)
-        model = model_cache[node][id] ||= node.instantiate(row,
-                                                           aliases.column_aliases(node))
         other = record.association(node.reflection.name)
 
+        model = model_cache[node][id] ||=
+          node.instantiate(row, aliases.column_aliases(node)) do |m|
+            other.set_inverse_instance(m)
+          end
+
         if node.reflection.collection?
           other.target.push(model)
         else
           other.target = model
         end
 
-        other.set_inverse_instance(model)
         model
       end
     end

activerecord/lib/active_record/associations/join_dependency/join_part.rb

@@ -62,8 +62,8 @@ def extract_record(row, column_names_with_alias)
           hash
         end
 
-        def instantiate(row, aliases)
-          base_klass.instantiate(extract_record(row, aliases))
+        def instantiate(row, aliases, &block)
+          base_klass.instantiate(extract_record(row, aliases), &block)
         end
       end
     end

activerecord/lib/active_record/associations/preloader/association.rb

@@ -62,7 +62,12 @@ def options
         private
 
         def associated_records_by_owner(preloader)
-          records = load_records
+          records = load_records do |record|
+            owner = owners_by_key[convert_key(record[association_key_name])]
+            association = owner.association(reflection.name)
+            association.set_inverse_instance(record)
+          end
+
           owners.each_with_object({}) do |owner, result|
             result[owner] = records[convert_key(owner[owner_key_name])] || []
           end
@@ -79,6 +84,15 @@ def owner_keys
           @owner_keys
         end
 
+        def owners_by_key
+          unless defined?(@owners_by_key)
+            @owners_by_key = owners.each_with_object({}) do |owner, h|
+              h[convert_key(owner[owner_key_name])] = owner
+            end
+          end
+          @owners_by_key
+        end
+
         def key_conversion_required?
           @key_conversion_required ||= association_key_type != owner_key_type
         end
@@ -99,13 +113,13 @@ def owner_key_type
           @model.type_for_attribute(owner_key_name.to_s).type
         end
 
-        def load_records
+        def load_records(&block)
           return {} if owner_keys.empty?
           # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000)
           # Make several smaller queries if necessary or make one query if the adapter supports it
           slices  = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size)
           @preloaded_records = slices.flat_map do |slice|
-            records_for(slice)
+            records_for(slice).load(&block)
           end
           @preloaded_records.group_by do |record| 
             convert_key(record[association_key_name])

activerecord/lib/active_record/associations/preloader/collection_association.rb

@@ -9,7 +9,6 @@ def preload(preloader)
             association = owner.association(reflection.name)
             association.loaded!
             association.target.concat(records)
-            records.each { |record| association.set_inverse_instance(record) }
           end
         end
       end

activerecord/lib/active_record/associations/preloader/singular_association.rb

@@ -11,7 +11,6 @@ def preload(preloader)
 
             association = owner.association(reflection.name)
             association.target = record
-            association.set_inverse_instance(record) if record
           end
         end
 

activerecord/lib/active_record/core.rb

@@ -346,6 +346,8 @@ def init_with(coder)
 
       self.class.define_attribute_methods
 
+      yield self if block_given?
+
       _run_find_callbacks
       _run_initialize_callbacks
 

activerecord/lib/active_record/persistence.rb

@@ -63,10 +63,10 @@ def create!(attributes = nil, &block)
       #
       # See <tt>ActiveRecord::Inheritance#discriminate_class_for_record</tt> to see
       # how this "single-table" inheritance mapping is implemented.
-      def instantiate(attributes, column_types = {})
+      def instantiate(attributes, column_types = {}, &block)
         klass = discriminate_class_for_record(attributes)
         attributes = klass.attributes_builder.build_from_database(attributes, column_types)
-        klass.allocate.init_with('attributes' => attributes, 'new_record' => false)
+        klass.allocate.init_with("attributes" => attributes, "new_record" => false, &block)
       end
 
       private

activerecord/lib/active_record/querying.rb

@@ -35,7 +35,7 @@ module Querying
     #
     #   Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date]
     #   Post.find_by_sql ["SELECT body FROM comments WHERE author = :user_id OR approved_by = :user_id", { :user_id => user_id }]
-    def find_by_sql(sql, binds = [], preparable: nil)
+    def find_by_sql(sql, binds = [], preparable: nil, &block)
       result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable)
       column_types = result_set.column_types.dup
       columns_hash.each_key { |k| column_types.delete k }
@@ -46,8 +46,8 @@ def find_by_sql(sql, binds = [], preparable: nil)
         class_name: name
       }
 
-      message_bus.instrument('instantiation.active_record', payload) do
-        result_set.map { |record| instantiate(record, column_types) }
+      message_bus.instrument("instantiation.active_record", payload) do
+        result_set.map { |record| instantiate(record, column_types, &block) }
       end
     end
 

activerecord/lib/active_record/relation.rb

@@ -576,8 +576,8 @@ def delete(id_or_array)
     # return value is the relation itself, not the records.
     #
     #   Post.where(published: true).load # => #<ActiveRecord::Relation>
-    def load
-      exec_queries unless loaded?
+    def load(&block)
+      exec_queries(&block) unless loaded?
 
       self
     end
@@ -695,8 +695,8 @@ def load_records(records)
 
     private
 
-    def exec_queries
-      @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes).freeze
+    def exec_queries(&block)
+      @records = eager_loading? ? find_with_associations.freeze : @klass.find_by_sql(arel, bound_attributes, &block).freeze
 
       preload = preload_values
       preload +=  includes_values unless eager_loading?

activerecord/lib/active_record/statement_cache.rb

@@ -101,12 +101,12 @@ def initialize(query_builder, bind_map)
       @bind_map      = bind_map
     end
 
-    def execute(params, klass, connection)
+    def execute(params, klass, connection, &block)
       bind_values = bind_map.bind params
 
       sql = query_builder.sql_for bind_values, connection
 
-      klass.find_by_sql(sql, bind_values, preparable: true)
+      klass.find_by_sql(sql, bind_values, preparable: true, &block)
     end
     alias :call :execute
   end

activerecord/test/cases/associations/inverse_associations_test.rb

@@ -495,6 +495,33 @@ def test_child_instance_should_point_to_parent_without_saving
 
     assert !man.persisted?
   end
+
+  def test_inverse_instance_should_be_set_before_find_callbacks_are_run
+    reset_callbacks(Interest, :find) do
+      Interest.after_find { raise unless association(:man).loaded? && man.present? }
+
+      assert Man.first.interests.reload.any?
+      assert Man.includes(:interests).first.interests.any?
+      assert Man.joins(:interests).includes(:interests).first.interests.any?
+    end
+  end
+
+  def test_inverse_instance_should_be_set_before_initialize_callbacks_are_run
+    reset_callbacks(Interest, :initialize) do
+      Interest.after_initialize { raise unless association(:man).loaded? && man.present? }
+
+      assert Man.first.interests.reload.any?
+      assert Man.includes(:interests).first.interests.any?
+      assert Man.joins(:interests).includes(:interests).first.interests.any?
+    end
+  end
+
+  def reset_callbacks(target, type)
+    old_callbacks = target.send(:get_callbacks, type).deep_dup
+    yield
+  ensure
+    target.send(:set_callbacks, type, old_callbacks) if old_callbacks
+  end
 end
 
 class InverseBelongsToTests < ActiveRecord::TestCase