mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Deprecate using class level querying methods if the receiver scope regarded as leaked
This deprecates using class level querying methods if the receiver scope regarded as leaked, since #32380 and #35186 may cause that silently leaking information when people upgrade the app. We need deprecation first before making those.
This commit is contained in:
parent
cdb8697b4a
commit
4c6171d605
6 changed files with 75 additions and 21 deletions
|
@ -1,3 +1,8 @@
|
||||||
|
* Deprecate using class level querying methods if the receiver scope
|
||||||
|
regarded as leaked. Use `klass.unscoped` to avoid the leaking scope.
|
||||||
|
|
||||||
|
*Ryuta Kamizono*
|
||||||
|
|
||||||
* Allow applications to automatically switch connections.
|
* Allow applications to automatically switch connections.
|
||||||
|
|
||||||
Adds a middleware and configuration options that can be used in your
|
Adds a middleware and configuration options that can be used in your
|
||||||
|
|
|
@ -67,6 +67,7 @@ module ActiveRecord
|
||||||
# user = users.new { |user| user.name = 'Oscar' }
|
# user = users.new { |user| user.name = 'Oscar' }
|
||||||
# user.name # => Oscar
|
# user.name # => Oscar
|
||||||
def new(attributes = nil, &block)
|
def new(attributes = nil, &block)
|
||||||
|
block = _deprecated_scope_block("new", &block)
|
||||||
scoping { klass.new(attributes, &block) }
|
scoping { klass.new(attributes, &block) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -92,8 +93,13 @@ module ActiveRecord
|
||||||
# users.create(name: nil) # validation on name
|
# users.create(name: nil) # validation on name
|
||||||
# # => #<User id: nil, name: nil, ...>
|
# # => #<User id: nil, name: nil, ...>
|
||||||
def create(attributes = nil, &block)
|
def create(attributes = nil, &block)
|
||||||
|
if attributes.is_a?(Array)
|
||||||
|
attributes.collect { |attr| create(attr, &block) }
|
||||||
|
else
|
||||||
|
block = _deprecated_scope_block("create", &block)
|
||||||
scoping { klass.create(attributes, &block) }
|
scoping { klass.create(attributes, &block) }
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
# Similar to #create, but calls
|
# Similar to #create, but calls
|
||||||
# {create!}[rdoc-ref:Persistence::ClassMethods#create!]
|
# {create!}[rdoc-ref:Persistence::ClassMethods#create!]
|
||||||
|
@ -102,8 +108,13 @@ module ActiveRecord
|
||||||
# Expects arguments in the same format as
|
# Expects arguments in the same format as
|
||||||
# {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!].
|
# {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!].
|
||||||
def create!(attributes = nil, &block)
|
def create!(attributes = nil, &block)
|
||||||
|
if attributes.is_a?(Array)
|
||||||
|
attributes.collect { |attr| create!(attr, &block) }
|
||||||
|
else
|
||||||
|
block = _deprecated_scope_block("create!", &block)
|
||||||
scoping { klass.create!(attributes, &block) }
|
scoping { klass.create!(attributes, &block) }
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def first_or_create(attributes = nil, &block) # :nodoc:
|
def first_or_create(attributes = nil, &block) # :nodoc:
|
||||||
first || create(attributes, &block)
|
first || create(attributes, &block)
|
||||||
|
@ -312,12 +323,12 @@ module ActiveRecord
|
||||||
# Please check unscoped if you want to remove all previous scopes (including
|
# Please check unscoped if you want to remove all previous scopes (including
|
||||||
# the default_scope) during the execution of a block.
|
# the default_scope) during the execution of a block.
|
||||||
def scoping
|
def scoping
|
||||||
@delegate_to_klass ? yield : _scoping(self) { yield }
|
already_in_scope? ? yield : _scoping(self) { yield }
|
||||||
end
|
end
|
||||||
|
|
||||||
def _exec_scope(*args, &block) # :nodoc:
|
def _exec_scope(name, *args, &block) # :nodoc:
|
||||||
@delegate_to_klass = true
|
@delegate_to_klass = true
|
||||||
instance_exec(*args, &block) || self
|
_scoping(_deprecated_spawn(name)) { instance_exec(*args, &block) || self }
|
||||||
ensure
|
ensure
|
||||||
@delegate_to_klass = false
|
@delegate_to_klass = false
|
||||||
end
|
end
|
||||||
|
@ -516,6 +527,7 @@ module ActiveRecord
|
||||||
|
|
||||||
def reset
|
def reset
|
||||||
@delegate_to_klass = false
|
@delegate_to_klass = false
|
||||||
|
@_deprecated_scope_source = nil
|
||||||
@to_sql = @arel = @loaded = @should_eager_load = nil
|
@to_sql = @arel = @loaded = @should_eager_load = nil
|
||||||
@records = [].freeze
|
@records = [].freeze
|
||||||
@offsets = {}
|
@offsets = {}
|
||||||
|
@ -624,7 +636,10 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
attr_reader :_deprecated_scope_source # :nodoc:
|
||||||
|
|
||||||
protected
|
protected
|
||||||
|
attr_writer :_deprecated_scope_source # :nodoc:
|
||||||
|
|
||||||
def load_records(records)
|
def load_records(records)
|
||||||
@records = records.freeze
|
@records = records.freeze
|
||||||
|
@ -632,6 +647,24 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
def already_in_scope?
|
||||||
|
@delegate_to_klass && begin
|
||||||
|
scope = klass.current_scope(true)
|
||||||
|
scope && !scope._deprecated_scope_source
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def _deprecated_spawn(name)
|
||||||
|
spawn.tap { |scope| scope._deprecated_scope_source = name }
|
||||||
|
end
|
||||||
|
|
||||||
|
def _deprecated_scope_block(name, &block)
|
||||||
|
-> record do
|
||||||
|
klass.current_scope = _deprecated_spawn(name)
|
||||||
|
yield record if block_given?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def _scoping(scope)
|
def _scoping(scope)
|
||||||
previous, klass.current_scope = klass.current_scope(true), scope
|
previous, klass.current_scope = klass.current_scope(true), scope
|
||||||
yield
|
yield
|
||||||
|
|
|
@ -8,7 +8,7 @@ module ActiveRecord
|
||||||
module SpawnMethods
|
module SpawnMethods
|
||||||
# This is overridden by Associations::CollectionProxy
|
# This is overridden by Associations::CollectionProxy
|
||||||
def spawn #:nodoc:
|
def spawn #:nodoc:
|
||||||
@delegate_to_klass ? klass.all : clone
|
already_in_scope? ? klass.all : clone
|
||||||
end
|
end
|
||||||
|
|
||||||
# Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation.
|
# Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation.
|
||||||
|
|
|
@ -27,6 +27,14 @@ module ActiveRecord
|
||||||
scope = current_scope
|
scope = current_scope
|
||||||
|
|
||||||
if scope
|
if scope
|
||||||
|
if scope._deprecated_scope_source
|
||||||
|
ActiveSupport::Deprecation.warn(<<~MSG.squish)
|
||||||
|
Class level methods will no longer inherit scoping from `#{scope._deprecated_scope_source}`
|
||||||
|
in Rails 6.1. To continue using the scoped relation, pass it into the block directly.
|
||||||
|
To instead access the full set of models, as Rails 6.1 will, use `#{name}.unscoped`.
|
||||||
|
MSG
|
||||||
|
end
|
||||||
|
|
||||||
if self == scope.klass
|
if self == scope.klass
|
||||||
scope.clone
|
scope.clone
|
||||||
else
|
else
|
||||||
|
@ -180,7 +188,7 @@ module ActiveRecord
|
||||||
|
|
||||||
if body.respond_to?(:to_proc)
|
if body.respond_to?(:to_proc)
|
||||||
singleton_class.define_method(name) do |*args|
|
singleton_class.define_method(name) do |*args|
|
||||||
scope = all._exec_scope(*args, &body)
|
scope = all._exec_scope(name, *args, &body)
|
||||||
scope = scope.extending(extension) if extension
|
scope = scope.extending(extension) if extension
|
||||||
scope
|
scope
|
||||||
end
|
end
|
||||||
|
|
|
@ -1169,10 +1169,12 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
|
|
||||||
def test_first_or_create_with_after_initialize
|
def test_first_or_create_with_after_initialize
|
||||||
Bird.create!(color: "yellow", name: "canary")
|
Bird.create!(color: "yellow", name: "canary")
|
||||||
parrot = Bird.where(color: "green").first_or_create do |bird|
|
parrot = assert_deprecated do
|
||||||
|
Bird.where(color: "green").first_or_create do |bird|
|
||||||
bird.name = "parrot"
|
bird.name = "parrot"
|
||||||
bird.enable_count = true
|
bird.enable_count = true
|
||||||
end
|
end
|
||||||
|
end
|
||||||
assert_equal 0, parrot.total_count
|
assert_equal 0, parrot.total_count
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -1180,7 +1182,7 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
Bird.create!(color: "yellow", name: "canary")
|
Bird.create!(color: "yellow", name: "canary")
|
||||||
parrot = Bird.where(color: "green").first_or_create do |bird|
|
parrot = Bird.where(color: "green").first_or_create do |bird|
|
||||||
bird.name = "parrot"
|
bird.name = "parrot"
|
||||||
assert_equal 0, Bird.count
|
assert_deprecated { assert_equal 0, Bird.count }
|
||||||
end
|
end
|
||||||
assert_kind_of Bird, parrot
|
assert_kind_of Bird, parrot
|
||||||
assert_predicate parrot, :persisted?
|
assert_predicate parrot, :persisted?
|
||||||
|
@ -1224,10 +1226,12 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
|
|
||||||
def test_first_or_create_bang_with_after_initialize
|
def test_first_or_create_bang_with_after_initialize
|
||||||
Bird.create!(color: "yellow", name: "canary")
|
Bird.create!(color: "yellow", name: "canary")
|
||||||
parrot = Bird.where(color: "green").first_or_create! do |bird|
|
parrot = assert_deprecated do
|
||||||
|
Bird.where(color: "green").first_or_create! do |bird|
|
||||||
bird.name = "parrot"
|
bird.name = "parrot"
|
||||||
bird.enable_count = true
|
bird.enable_count = true
|
||||||
end
|
end
|
||||||
|
end
|
||||||
assert_equal 0, parrot.total_count
|
assert_equal 0, parrot.total_count
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -1235,7 +1239,7 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
Bird.create!(color: "yellow", name: "canary")
|
Bird.create!(color: "yellow", name: "canary")
|
||||||
parrot = Bird.where(color: "green").first_or_create! do |bird|
|
parrot = Bird.where(color: "green").first_or_create! do |bird|
|
||||||
bird.name = "parrot"
|
bird.name = "parrot"
|
||||||
assert_equal 0, Bird.count
|
assert_deprecated { assert_equal 0, Bird.count }
|
||||||
end
|
end
|
||||||
assert_kind_of Bird, parrot
|
assert_kind_of Bird, parrot
|
||||||
assert_predicate parrot, :persisted?
|
assert_predicate parrot, :persisted?
|
||||||
|
@ -1287,10 +1291,12 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
|
|
||||||
def test_first_or_initialize_with_after_initialize
|
def test_first_or_initialize_with_after_initialize
|
||||||
Bird.create!(color: "yellow", name: "canary")
|
Bird.create!(color: "yellow", name: "canary")
|
||||||
parrot = Bird.where(color: "green").first_or_initialize do |bird|
|
parrot = assert_deprecated do
|
||||||
|
Bird.where(color: "green").first_or_initialize do |bird|
|
||||||
bird.name = "parrot"
|
bird.name = "parrot"
|
||||||
bird.enable_count = true
|
bird.enable_count = true
|
||||||
end
|
end
|
||||||
|
end
|
||||||
assert_equal 0, parrot.total_count
|
assert_equal 0, parrot.total_count
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -1298,7 +1304,7 @@ class RelationTest < ActiveRecord::TestCase
|
||||||
Bird.create!(color: "yellow", name: "canary")
|
Bird.create!(color: "yellow", name: "canary")
|
||||||
parrot = Bird.where(color: "green").first_or_initialize do |bird|
|
parrot = Bird.where(color: "green").first_or_initialize do |bird|
|
||||||
bird.name = "parrot"
|
bird.name = "parrot"
|
||||||
assert_equal 0, Bird.count
|
assert_deprecated { assert_equal 0, Bird.count }
|
||||||
end
|
end
|
||||||
assert_kind_of Bird, parrot
|
assert_kind_of Bird, parrot
|
||||||
assert_not_predicate parrot, :persisted?
|
assert_not_predicate parrot, :persisted?
|
||||||
|
|
|
@ -50,7 +50,7 @@ class NamedScopingTest < ActiveRecord::TestCase
|
||||||
|
|
||||||
def test_calling_merge_at_first_in_scope
|
def test_calling_merge_at_first_in_scope
|
||||||
Topic.class_eval do
|
Topic.class_eval do
|
||||||
scope :calling_merge_at_first_in_scope, Proc.new { merge(Topic.replied) }
|
scope :calling_merge_at_first_in_scope, Proc.new { merge(Topic.unscoped.replied) }
|
||||||
end
|
end
|
||||||
assert_equal Topic.calling_merge_at_first_in_scope.to_a, Topic.replied.to_a
|
assert_equal Topic.calling_merge_at_first_in_scope.to_a, Topic.replied.to_a
|
||||||
end
|
end
|
||||||
|
@ -448,8 +448,10 @@ class NamedScopingTest < ActiveRecord::TestCase
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_class_method_in_scope
|
def test_class_method_in_scope
|
||||||
|
assert_deprecated do
|
||||||
assert_equal [topics(:second)], topics(:first).approved_replies.ordered
|
assert_equal [topics(:second)], topics(:first).approved_replies.ordered
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def test_nested_scoping
|
def test_nested_scoping
|
||||||
expected = Reply.approved
|
expected = Reply.approved
|
||||||
|
|
Loading…
Reference in a new issue