aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2018-03-29 06:52:04 +0900
committerRyuta Kamizono <kamipo@gmail.com>2019-02-06 00:37:08 +0900
commit2935d07569337aeaa06d03a56180d235a7fbbebf (patch)
tree6546991716de14af2b40aaa60cb1ecf6bcbd9853 /activerecord
parent44232b485485634d681c60868c619323f882e59f (diff)
downloadrails-2935d07569337aeaa06d03a56180d235a7fbbebf.tar.gz
rails-2935d07569337aeaa06d03a56180d235a7fbbebf.tar.bz2
rails-2935d07569337aeaa06d03a56180d235a7fbbebf.zip
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.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md6
-rw-r--r--activerecord/lib/active_record/relation.rb2
-rw-r--r--activerecord/lib/active_record/relation/spawn_methods.rb2
-rw-r--r--activerecord/lib/active_record/scoping.rb10
-rw-r--r--activerecord/lib/active_record/scoping/named.rb3
-rw-r--r--activerecord/test/cases/scoping/named_scoping_test.rb10
-rw-r--r--activerecord/test/models/topic.rb7
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