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

Active Record uses `scoping` to delegate to named scopes from relations
for propagating the chaining source scope. It was needed to restore the
source scope in named scopes, but it was caused undesired behavior that
pollute all class level querying methods.

Example:

```ruby
class Topic < ActiveRecord::Base
  scope :toplevel, -> { where(parent_id: nil) }
  scope :children, -> { where.not(parent_id: nil) }
  scope :has_children, -> { where(id: Topic.children.select(:parent_id)) }
end

# Works as expected.
Topic.toplevel.where(id: Topic.children.select(:parent_id))

# Doesn't work due to leaking `toplevel` to `Topic.children`.
Topic.toplevel.has_children
```

Since #29301, the receiver in named scopes has changed from the model
class to the chaining source scope, so the polluting class level
querying methods is no longer required for that purpose.

Fixes #14003.
This commit is contained in:
Ryuta Kamizono 2018-03-29 06:52:04 +09:00
parent 44232b4854
commit 2935d07569
7 changed files with 32 additions and 8 deletions

View File

@ -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

View File

@ -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:

View File

@ -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.

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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