Skip to content
Permalink
Browse files

Deprecate using class level querying methods if the receiver scope re…

…garded 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.
  • Loading branch information...
kamipo committed Feb 15, 2019
1 parent cdb8697 commit 8a9e02c0153bc79f07fbd66e2be6f59d2bd32f19
@@ -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.

Adds a middleware and configuration options that can be used in your
@@ -67,6 +67,7 @@ def bind_attribute(name, value) # :nodoc:
# user = users.new { |user| user.name = 'Oscar' }
# user.name # => Oscar
def new(attributes = nil, &block)
block = _deprecated_scope_block("new", &block)
scoping { klass.new(attributes, &block) }
end

@@ -92,7 +93,12 @@ def new(attributes = nil, &block)
# users.create(name: nil) # validation on name
# # => #<User id: nil, name: nil, ...>
def create(attributes = nil, &block)
scoping { klass.create(attributes, &block) }
if attributes.is_a?(Array)
attributes.collect { |attr| create(attr, &block) }
else
block = _deprecated_scope_block("create", &block)
scoping { klass.create(attributes, &block) }
end
end

# Similar to #create, but calls
@@ -102,7 +108,12 @@ def create(attributes = nil, &block)
# Expects arguments in the same format as
# {ActiveRecord::Base.create!}[rdoc-ref:Persistence::ClassMethods#create!].
def create!(attributes = nil, &block)
scoping { klass.create!(attributes, &block) }
if attributes.is_a?(Array)
attributes.collect { |attr| create!(attr, &block) }
else
block = _deprecated_scope_block("create!", &block)
scoping { klass.create!(attributes, &block) }
end
end

def first_or_create(attributes = nil, &block) # :nodoc:
@@ -312,12 +323,12 @@ def cache_key(timestamp_column = :updated_at)
# 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 : _scoping(self) { yield }
already_in_scope? ? yield : _scoping(self) { yield }
end

def _exec_scope(*args, &block) # :nodoc:
def _exec_scope(name, *args, &block) # :nodoc:
@delegate_to_klass = true
instance_exec(*args, &block) || self
_scoping(_deprecated_spawn(name)) { instance_exec(*args, &block) || self }
ensure
@delegate_to_klass = false
end
@@ -516,6 +527,7 @@ def reload

def reset
@delegate_to_klass = false
@_deprecated_scope_source = nil
@to_sql = @arel = @loaded = @should_eager_load = nil
@records = [].freeze
@offsets = {}
@@ -624,14 +636,35 @@ def preload_associations(records) # :nodoc:
end
end

attr_reader :_deprecated_scope_source # :nodoc:

protected
attr_writer :_deprecated_scope_source

def load_records(records)
@records = records.freeze
@loaded = true
end

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)
previous, klass.current_scope = klass.current_scope(true), scope
yield
@@ -8,7 +8,7 @@ module ActiveRecord
module SpawnMethods
# This is overridden by Associations::CollectionProxy
def spawn #:nodoc:
@delegate_to_klass ? klass.all : clone
already_in_scope? ? klass.all : clone
end

# Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation.
@@ -27,6 +27,14 @@ def all
scope = current_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
scope.clone
else
@@ -180,7 +188,7 @@ def scope(name, body, &block)

if body.respond_to?(:to_proc)
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
end
@@ -1169,9 +1169,11 @@ def test_first_or_create_with_no_parameters

def test_first_or_create_with_after_initialize
Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_create do |bird|
bird.name = "parrot"
bird.enable_count = true
parrot = assert_deprecated do
Bird.where(color: "green").first_or_create do |bird|
bird.name = "parrot"
bird.enable_count = true
end
end
assert_equal 0, parrot.total_count
end
@@ -1180,7 +1182,7 @@ def test_first_or_create_with_block
Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_create do |bird|
bird.name = "parrot"
assert_equal 0, Bird.count
assert_deprecated { assert_equal 0, Bird.count }
end
assert_kind_of Bird, parrot
assert_predicate parrot, :persisted?
@@ -1224,9 +1226,11 @@ def test_first_or_create_bang_with_no_parameters

def test_first_or_create_bang_with_after_initialize
Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_create! do |bird|
bird.name = "parrot"
bird.enable_count = true
parrot = assert_deprecated do
Bird.where(color: "green").first_or_create! do |bird|
bird.name = "parrot"
bird.enable_count = true
end
end
assert_equal 0, parrot.total_count
end
@@ -1235,7 +1239,7 @@ def test_first_or_create_bang_with_valid_block
Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_create! do |bird|
bird.name = "parrot"
assert_equal 0, Bird.count
assert_deprecated { assert_equal 0, Bird.count }
end
assert_kind_of Bird, parrot
assert_predicate parrot, :persisted?
@@ -1287,9 +1291,11 @@ def test_first_or_initialize_with_no_parameters

def test_first_or_initialize_with_after_initialize
Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_initialize do |bird|
bird.name = "parrot"
bird.enable_count = true
parrot = assert_deprecated do
Bird.where(color: "green").first_or_initialize do |bird|
bird.name = "parrot"
bird.enable_count = true
end
end
assert_equal 0, parrot.total_count
end
@@ -1298,7 +1304,7 @@ def test_first_or_initialize_with_block
Bird.create!(color: "yellow", name: "canary")
parrot = Bird.where(color: "green").first_or_initialize do |bird|
bird.name = "parrot"
assert_equal 0, Bird.count
assert_deprecated { assert_equal 0, Bird.count }
end
assert_kind_of Bird, parrot
assert_not_predicate parrot, :persisted?
@@ -50,7 +50,7 @@ def test_delegates_finds_and_calculations_to_the_base_class

def test_calling_merge_at_first_in_scope
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
assert_equal Topic.calling_merge_at_first_in_scope.to_a, Topic.replied.to_a
end
@@ -448,7 +448,9 @@ def test_chaining_combines_conditions_when_searching
end

def test_class_method_in_scope
assert_equal [topics(:second)], topics(:first).approved_replies.ordered
assert_deprecated do
assert_equal [topics(:second)], topics(:first).approved_replies.ordered
end
end

def test_nested_scoping

0 comments on commit 8a9e02c

Please sign in to comment.
You can’t perform that action at this time.