Merge pull request #32380 from kamipo/fix_leaking_scope

Chaining named scope is no longer leaking to class level querying methods

7 files changed, 32 insertions, 8 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 654caafc92..c412646cc9 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Chaining named scope is no longer leaking to class level querying methods.
+
+    Fixes #14003.
+
+    *Ryuta Kamizono*
+
 *   Allow applications to automatically switch connections.
 
     Adds a middleware and configuration options that can be used in your
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index a863227276..bab00ef065 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -312,7 +312,7 @@ module ActiveRecord
     # Please check unscoped if you want to remove all previous scopes (including
     # the default_scope) during the execution of a block.
     def scoping
-      @delegate_to_klass ? yield : klass._scoping(self) { yield }
+      @delegate_to_klass && klass.current_scope ? yield : klass._scoping(self) { yield }
     end
 
     def _exec_scope(*args, &block) # :nodoc:
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index 7874c4c35a..d758e289ca 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -8,7 +8,7 @@ module ActiveRecord
   module SpawnMethods
     # This is overridden by Associations::CollectionProxy
     def spawn #:nodoc:
-      @delegate_to_klass ? klass.all : clone
+      @delegate_to_klass && klass.current_scope ? klass.all : clone
     end
 
     # Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation.
diff --git a/activerecord/lib/active_record/scoping.rb b/activerecord/lib/active_record/scoping.rb
index 9eba1254a4..c3a56b2174 100644
--- a/activerecord/lib/active_record/scoping.rb
+++ b/activerecord/lib/active_record/scoping.rb
@@ -23,11 +23,11 @@ module ActiveRecord
         current_scope
       end
 
-      private
-        def current_scope(skip_inherited_scope = false)
-          ScopeRegistry.value_for(:current_scope, self, skip_inherited_scope)
-        end
+      def current_scope(skip_inherited_scope = false)
+        ScopeRegistry.value_for(:current_scope, self, skip_inherited_scope)
+      end
 
+      private
         def current_scope=(scope)
           ScopeRegistry.set_value_for(:current_scope, self, scope)
         end
@@ -84,7 +84,7 @@ module ActiveRecord
         base = model.base_class
         while klass <= base
           value = @registry[scope_type][klass.name]
-          return value if value
+          return value || nil unless value.nil?
           klass = klass.superclass
         end
       end
diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb
index 5278eb29a9..987e6bd63f 100644
--- a/activerecord/lib/active_record/scoping/named.rb
+++ b/activerecord/lib/active_record/scoping/named.rb
@@ -180,7 +180,8 @@ module ActiveRecord
 
           if body.respond_to?(:to_proc)
             singleton_class.define_method(name) do |*args|
-              scope = all._exec_scope(*args, &body)
+              scope = all
+              scope = _scoping(false) { scope._exec_scope(*args, &body) }
               scope = scope.extending(extension) if extension
               scope
             end
diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb
index 418a2ae04e..27f9df295f 100644
--- a/activerecord/test/cases/scoping/named_scoping_test.rb
+++ b/activerecord/test/cases/scoping/named_scoping_test.rb
@@ -447,6 +447,16 @@ class NamedScopingTest < ActiveRecord::TestCase
     assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).to_a.uniq
   end
 
+  def test_chaining_doesnt_leak_conditions_to_another_scopes
+    expected = Topic.where(approved: false).where(id: Topic.children.select(:parent_id))
+    assert_equal expected.to_a, Topic.rejected.has_children.to_a
+  end
+
+  def test_nested_scoping
+    expected = Reply.approved
+    assert_equal expected.to_a, Topic.rejected.nested_scoping(expected)
+  end
+
   def test_scopes_batch_finders
     assert_equal 4, Topic.approved.count
 
diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb
index a6a47687a2..fdb461ed7f 100644
--- a/activerecord/test/models/topic.rb
+++ b/activerecord/test/models/topic.rb
@@ -10,6 +10,9 @@ class Topic < ActiveRecord::Base
   scope :approved, -> { where(approved: true) }
   scope :rejected, -> { where(approved: false) }
 
+  scope :children, -> { where.not(parent_id: nil) }
+  scope :has_children, -> { where(id: Topic.children.select(:parent_id)) }
+
   scope :scope_with_lambda, lambda { all }
 
   scope :by_lifo, -> { where(author_name: "lifo") }
@@ -88,6 +91,10 @@ class Topic < ActiveRecord::Base
     write_attribute(:approved, val)
   end
 
+  def self.nested_scoping(scope)
+    scope.base
+  end
+
   private
 
     def default_written_on