Revert "Revert "Merge pull request rails#28828 from kamipo/fix_extend…

…ing_modules_on_association""

This reverts commit d6679af.

We already released this commit so we can't just revert anymore.

activerecord/lib/active_record/associations/association.rb

@@ -83,7 +83,7 @@ def target=(target)
       end
 
       def scope
-        target_scope.merge(association_scope)
+        target_scope.merge!(association_scope)
       end
 
       # The scope for this association.

activerecord/lib/active_record/associations/collection_association.rb

@@ -39,13 +39,7 @@ def reader(force_reload = false)
           reload
         end
 
-        if null_scope?
-          # Cache the proxy separately before the owner has an id
-          # or else a post-save proxy will still lack the id
-          @null_proxy ||= CollectionProxy.create(klass, self)
-        else
-          @proxy ||= CollectionProxy.create(klass, self)
-        end
+        CollectionProxy.create(klass, self)
       end
 
       # Implements the writer method, e.g. foo.items= for Foo.has_many :items
@@ -427,9 +421,9 @@ def add_to_target(record, skip_callbacks = false, &block)
         replace_on_target(record, index, skip_callbacks, &block)
       end
 
-      def scope(opts = {})
-        scope = super()
-        scope.none! if opts.fetch(:nullify, true) && null_scope?
+      def scope
+        scope = super
+        scope.none! if null_scope?
         scope
       end
 

activerecord/lib/active_record/associations/collection_proxy.rb

@@ -28,12 +28,9 @@ module Associations
     # is computed directly through SQL and does not trigger by itself the
     # instantiation of the actual post records.
     class CollectionProxy < Relation
-      delegate :exists?, :update_all, :arel, to: :scope
-
       def initialize(klass, association) #:nodoc:
         @association = association
         super klass, klass.arel_table, klass.predicate_builder
-        merge! association.scope(nullify: false)
       end
 
       def target
@@ -902,19 +899,10 @@ def proxy_association
         @association
       end
 
-      # We don't want this object to be put on the scoping stack, because
-      # that could create an infinite loop where we call an @association
-      # method, which gets the current scope, which is this object, which
-      # delegates to @association, and so on.
-      def scoping
-        @association.scope.scoping { yield }
-      end
-
       # Returns a <tt>Relation</tt> object for the records in this association
       def scope
-        @association.scope
+        @scope ||= @association.scope
       end
-      alias spawn scope
 
       # Equivalent to <tt>Array#==</tt>. Returns +true+ if the two arrays
       # contain the same number of elements and if each element is equal
@@ -1046,6 +1034,7 @@ def clear
       #   person.pets(true)  # fetches pets from the database
       #   # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>]
       def reload
+        @scope = nil
         proxy_association.reload
         self
       end
@@ -1067,20 +1056,51 @@ def reload
       #   person.pets  # fetches pets from the database
       #   # => [#<Pet id: 1, name: "Snoop", group: "dogs", person_id: 1>]
       def reset
+        @scope = nil
         proxy_association.reset
         proxy_association.reset_scope
         self
       end
 
+      delegate_methods = [
+        QueryMethods,
+        SpawnMethods,
+      ].flat_map { |klass|
+        klass.public_instance_methods(false)
+      } - self.public_instance_methods(false) + [:scoping]
+
+      delegate(*delegate_methods, to: :scope)
+
+      module DelegateExtending # :nodoc:
+        private
+          def method_missing(method, *args, &block)
+            extending_values = association_scope.extending_values
+            if extending_values.any? && (extending_values - self.class.included_modules).any?
+              self.class.include(*extending_values)
+              public_send(method, *args, &block)
+            else
+              super
+            end
+          end
+      end
+
       private
 
         def null_scope?
           @association.null_scope?
         end
 
+        def association_scope
+          @association.association_scope
+        end
+
         def exec_queries
           load_target
         end
+
+        def respond_to_missing?(method, _)
+          association_scope.respond_to?(method) || super
+        end
     end
   end
 end

activerecord/lib/active_record/relation/delegation.rb

@@ -24,6 +24,8 @@ def initialize_relation_delegate_cache
 
       def inherited(child_class)
         child_class.initialize_relation_delegate_cache
+        delegate = child_class.relation_delegate_class(ActiveRecord::Associations::CollectionProxy)
+        delegate.include ActiveRecord::Associations::CollectionProxy::DelegateExtending
         super
       end
     end

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -587,13 +587,10 @@ def test_update_all_on_association_accessed_before_save
   end
 
   def test_update_all_on_association_accessed_before_save_with_explicit_foreign_key
-    # We can use the same cached proxy object because the id is available for the scope
-    firm = Firm.new(name: 'Firm', id: 100)
-    clients_proxy_id = firm.clients.object_id
+    firm = Firm.new(name: "Firm", id: 100)
     firm.clients << Client.first
     firm.save!
-    assert_equal firm.clients.count, firm.clients.update_all(description: 'Great!')
-    assert_equal clients_proxy_id, firm.clients.object_id
+    assert_equal firm.clients.count, firm.clients.update_all(description: "Great!")
   end
 
   def test_belongs_to_sanity

activerecord/test/cases/associations_test.rb

@@ -237,11 +237,6 @@ def test_scoped_allows_conditions
     assert_equal david.projects, david.projects.scope
   end
 
-  test "proxy object is cached" do
-    david = developers(:david)
-    assert david.projects.equal?(david.projects)
-  end
-
   test "inverses get set of subsets of the association" do
     man = Man.create
     man.interests.create

activerecord/test/models/comment.rb

@@ -19,6 +19,11 @@ class Comment < ActiveRecord::Base
   has_many :children, :class_name => 'Comment', :foreign_key => :parent_id
   belongs_to :parent, :class_name => 'Comment', :counter_cache => :children_count
 
+  # Should not be called if extending modules that having the method exists on an association.
+  def self.greeting
+    raise
+  end
+
   def self.what_are_you
     'a comment...'
   end