Permalink
Browse files

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.
  • Loading branch information...
1 parent e66c148 commit 94924dc32baf78f13e289172534c2e71c9c8cade @jonleighton jonleighton committed Jun 28, 2013
@@ -122,11 +122,7 @@ def klass
# 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 @@ class CollectionProxy < Relation
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 @@ def reflection_scope
def build_scope
scope = klass.unscoped
- scope.default_scoped = true
values = reflection_scope.values
preload_values = preload_scope.values
@@ -113,7 +112,7 @@ def build_scope
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 @@ def target_scope
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 @@ class Relation
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 @@ def update_all(updates)
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 @@ def delete_all(conditions = nil)
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 @@ def to_sql
# 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 @@ def pretty_print(q)
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 @@ def inspect
private
def exec_queries
- default_scoped = with_default_scope
-
- if default_scoped.equal?(self)
- @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
+ @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 @@ def sum(*args)
#
# 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 @@ def exists?(conditions = :none)
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 @@ def find_first
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 @@ class Merger # :nodoc:
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 @@ def reverse_order! # :nodoc:
# 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 @@ def only(*onlies)
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 @@ def all
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 @@ def test_unscope_with_limit_in_query
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

0 comments on commit 94924dc

Please sign in to comment.