mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Simplify/fix implementation of default scopes
The previous implementation was necessary in order to support stuff like: class Post < ActiveRecord::Base default_scope where(published: true) scope :ordered, order("created_at") end If we didn't evaluate the default scope at the last possible moment before sending the SQL to the database, it would become impossible to do: Post.unscoped.ordered This is because the default scope would already be bound up in the "ordered" scope, and therefore wouldn't be removed by the "Post.unscoped" part. In 4.0, we have deprecated all "eager" forms of scopes. So now you must write: class Post < ActiveRecord::Base default_scope { where(published: true) } scope :ordered, -> { order("created_at") } end This prevents the default scope getting bound up inside the "ordered" scope, which means we can now have a simpler/better/more natural implementation of default scoping. A knock on effect is that some things that didn't work properly now do. For example it was previously impossible to use #except to remove a part of the default scope, since the default scope was evaluated after the call to #except.
This commit is contained in:
parent
e66c148571
commit
94924dc32b
12 changed files with 31 additions and 74 deletions
|
@ -122,11 +122,7 @@ module ActiveRecord
|
|||
# Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the
|
||||
# through association's scope)
|
||||
def target_scope
|
||||
all = klass.all
|
||||
scope = AssociationRelation.new(klass, klass.arel_table, self)
|
||||
scope.merge! all
|
||||
scope.default_scoped = all.default_scoped?
|
||||
scope
|
||||
AssociationRelation.new(klass, klass.arel_table, self).merge!(klass.all)
|
||||
end
|
||||
|
||||
# Loads the \target if needed and returns it.
|
||||
|
|
|
@ -33,7 +33,6 @@ module ActiveRecord
|
|||
def initialize(klass, association) #:nodoc:
|
||||
@association = association
|
||||
super klass, klass.arel_table
|
||||
self.default_scoped = true
|
||||
merge! association.scope(nullify: false)
|
||||
end
|
||||
|
||||
|
|
|
@ -98,7 +98,6 @@ module ActiveRecord
|
|||
|
||||
def build_scope
|
||||
scope = klass.unscoped
|
||||
scope.default_scoped = true
|
||||
|
||||
values = reflection_scope.values
|
||||
preload_values = preload_scope.values
|
||||
|
@ -113,7 +112,7 @@ module ActiveRecord
|
|||
scope.where!(klass.table_name => { reflection.type => model.base_class.sti_name })
|
||||
end
|
||||
|
||||
scope
|
||||
klass.default_scoped.merge(scope)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -15,7 +15,7 @@ module ActiveRecord
|
|||
scope = super
|
||||
chain[1..-1].each do |reflection|
|
||||
scope.merge!(
|
||||
reflection.klass.all.with_default_scope.
|
||||
reflection.klass.all.
|
||||
except(:select, :create_with, :includes, :preload, :joins, :eager_load)
|
||||
)
|
||||
end
|
||||
|
|
|
@ -17,17 +17,14 @@ module ActiveRecord
|
|||
include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation
|
||||
|
||||
attr_reader :table, :klass, :loaded
|
||||
attr_accessor :default_scoped
|
||||
alias :model :klass
|
||||
alias :loaded? :loaded
|
||||
alias :default_scoped? :default_scoped
|
||||
|
||||
def initialize(klass, table, values = {})
|
||||
@klass = klass
|
||||
@table = table
|
||||
@values = values
|
||||
@loaded = false
|
||||
@default_scoped = false
|
||||
@klass = klass
|
||||
@table = table
|
||||
@values = values
|
||||
@loaded = false
|
||||
end
|
||||
|
||||
def initialize_copy(other)
|
||||
|
@ -313,7 +310,7 @@ module ActiveRecord
|
|||
stmt.table(table)
|
||||
stmt.key = table[primary_key]
|
||||
|
||||
if with_default_scope.joins_values.any?
|
||||
if joins_values.any?
|
||||
@klass.connection.join_to_update(stmt, arel)
|
||||
else
|
||||
stmt.take(arel.limit)
|
||||
|
@ -438,7 +435,7 @@ module ActiveRecord
|
|||
stmt = Arel::DeleteManager.new(arel.engine)
|
||||
stmt.from(table)
|
||||
|
||||
if with_default_scope.joins_values.any?
|
||||
if joins_values.any?
|
||||
@klass.connection.join_to_delete(stmt, arel, table[primary_key])
|
||||
else
|
||||
stmt.wheres = arel.constraints
|
||||
|
@ -512,12 +509,11 @@ module ActiveRecord
|
|||
# User.where(name: 'Oscar').where_values_hash
|
||||
# # => {name: "Oscar"}
|
||||
def where_values_hash
|
||||
scope = with_default_scope
|
||||
equalities = scope.where_values.grep(Arel::Nodes::Equality).find_all { |node|
|
||||
equalities = where_values.grep(Arel::Nodes::Equality).find_all { |node|
|
||||
node.left.relation.name == table_name
|
||||
}
|
||||
|
||||
binds = Hash[scope.bind_values.find_all(&:first).map { |column, v| [column.name, v] }]
|
||||
binds = Hash[bind_values.find_all(&:first).map { |column, v| [column.name, v] }]
|
||||
binds.merge!(Hash[bind_values.find_all(&:first).map { |column, v| [column.name, v] }])
|
||||
|
||||
Hash[equalities.map { |where|
|
||||
|
@ -565,16 +561,6 @@ module ActiveRecord
|
|||
q.pp(self.to_a)
|
||||
end
|
||||
|
||||
def with_default_scope #:nodoc:
|
||||
if default_scoped? && default_scope = klass.send(:build_default_scope)
|
||||
default_scope = default_scope.merge(self)
|
||||
default_scope.default_scoped = false
|
||||
default_scope
|
||||
else
|
||||
self
|
||||
end
|
||||
end
|
||||
|
||||
# Returns true if relation is blank.
|
||||
def blank?
|
||||
to_a.blank?
|
||||
|
@ -594,22 +580,16 @@ module ActiveRecord
|
|||
private
|
||||
|
||||
def exec_queries
|
||||
default_scoped = with_default_scope
|
||||
@records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
|
||||
|
||||
if default_scoped.equal?(self)
|
||||
@records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
|
||||
|
||||
preload = preload_values
|
||||
preload += includes_values unless eager_loading?
|
||||
preload.each do |associations|
|
||||
ActiveRecord::Associations::Preloader.new(@records, associations).run
|
||||
end
|
||||
|
||||
@records.each { |record| record.readonly! } if readonly_value
|
||||
else
|
||||
@records = default_scoped.to_a
|
||||
preload = preload_values
|
||||
preload += includes_values unless eager_loading?
|
||||
preload.each do |associations|
|
||||
ActiveRecord::Associations::Preloader.new(@records, associations).run
|
||||
end
|
||||
|
||||
@records.each { |record| record.readonly! } if readonly_value
|
||||
|
||||
@loaded = true
|
||||
@records
|
||||
end
|
||||
|
|
|
@ -91,20 +91,14 @@ module ActiveRecord
|
|||
#
|
||||
# Person.sum("2 * age")
|
||||
def calculate(operation, column_name, options = {})
|
||||
relation = with_default_scope
|
||||
|
||||
if column_name.is_a?(Symbol) && attribute_aliases.key?(column_name.to_s)
|
||||
column_name = attribute_aliases[column_name.to_s].to_sym
|
||||
end
|
||||
|
||||
if relation.equal?(self)
|
||||
if has_include?(column_name)
|
||||
construct_relation_for_association_calculations.calculate(operation, column_name, options)
|
||||
else
|
||||
perform_calculation(operation, column_name, options)
|
||||
end
|
||||
if has_include?(column_name)
|
||||
construct_relation_for_association_calculations.calculate(operation, column_name, options)
|
||||
else
|
||||
relation.calculate(operation, column_name, options)
|
||||
perform_calculation(operation, column_name, options)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -211,7 +211,6 @@ module ActiveRecord
|
|||
relation = relation.where(table[primary_key].eq(conditions)) if conditions != :none
|
||||
end
|
||||
|
||||
relation = relation.with_default_scope
|
||||
connection.select_value(relation.arel, "#{name} Exists", relation.bind_values)
|
||||
end
|
||||
|
||||
|
@ -354,7 +353,7 @@ module ActiveRecord
|
|||
if loaded?
|
||||
@records.first
|
||||
else
|
||||
@first ||= find_first_with_limit(with_default_scope.order_values, 1).first
|
||||
@first ||= find_first_with_limit(order_values, 1).first
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -42,10 +42,6 @@ module ActiveRecord
|
|||
attr_reader :relation, :values, :other
|
||||
|
||||
def initialize(relation, other)
|
||||
if other.default_scoped? && other.klass != relation.klass
|
||||
other = other.with_default_scope
|
||||
end
|
||||
|
||||
@relation = relation
|
||||
@values = other.values
|
||||
@other = other
|
||||
|
|
|
@ -795,7 +795,7 @@ module ActiveRecord
|
|||
|
||||
# Returns the Arel object associated with the relation.
|
||||
def arel
|
||||
@arel ||= with_default_scope.build_arel
|
||||
@arel ||= build_arel
|
||||
end
|
||||
|
||||
# Like #arel, but ignores the default scope of the model.
|
||||
|
|
|
@ -65,7 +65,6 @@ module ActiveRecord
|
|||
|
||||
def relation_with(values) # :nodoc:
|
||||
result = Relation.new(klass, table, values)
|
||||
result.default_scoped = default_scoped
|
||||
result.extend(*extending_values) if extending_values.any?
|
||||
result
|
||||
end
|
||||
|
|
|
@ -25,22 +25,18 @@ module ActiveRecord
|
|||
if current_scope
|
||||
current_scope.clone
|
||||
else
|
||||
scope = relation
|
||||
scope.default_scoped = true
|
||||
scope
|
||||
default_scoped
|
||||
end
|
||||
end
|
||||
|
||||
def default_scoped # :nodoc:
|
||||
relation.merge(build_default_scope)
|
||||
end
|
||||
|
||||
# Collects attributes from scopes that should be applied when creating
|
||||
# an AR instance for the particular class this is called on.
|
||||
def scope_attributes # :nodoc:
|
||||
if current_scope
|
||||
current_scope.scope_for_create
|
||||
else
|
||||
scope = relation
|
||||
scope.default_scoped = true
|
||||
scope.scope_for_create
|
||||
end
|
||||
all.scope_for_create
|
||||
end
|
||||
|
||||
# Are there default attributes associated with this scope?
|
||||
|
|
|
@ -153,9 +153,8 @@ class DefaultScopingTest < ActiveRecord::TestCase
|
|||
end
|
||||
|
||||
def test_order_to_unscope_reordering
|
||||
expected = DeveloperOrderedBySalary.all.collect { |dev| [dev.name, dev.id] }
|
||||
received = DeveloperOrderedBySalary.order('salary DESC, name ASC').reverse_order.unscope(:order).collect { |dev| [dev.name, dev.id] }
|
||||
assert_equal expected, received
|
||||
scope = DeveloperOrderedBySalary.order('salary DESC, name ASC').reverse_order.unscope(:order)
|
||||
assert !(scope.to_sql =~ /order/i)
|
||||
end
|
||||
|
||||
def test_unscope_reverse_order
|
||||
|
|
Loading…
Reference in a new issue