Evaluate default scopes at the last possible moment in order to avoid problems with default scopes getting included into other scopes and then being unable to remove the default part via unscoped.

5 files changed, 40 insertions, 16 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index e0fb94c96e..08a41e2d8b 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1110,9 +1110,7 @@ module ActiveRecord #:nodoc:
           # If another Active Record class has been passed in, get its current scope
           scope = scope.current_scope if !scope.is_a?(Relation) && scope.respond_to?(:current_scope)
 
-          # Cannot use self.current_scope, because that could trigger build_default_scope, which
-          # could in turn result in with_scope being called and hence an infinite loop.
-          previous_scope = Thread.current[:"#{self}_current_scope"]
+          previous_scope = self.current_scope
 
           if scope.is_a?(Hash)
             # Dup first and second level of hash (method and params).
@@ -1123,6 +1121,7 @@ module ActiveRecord #:nodoc:
 
             scope.assert_valid_keys([ :find, :create ])
             relation = construct_finder_arel(scope[:find] || {})
+            relation.default_scoped = true unless action == :overwrite
 
             if previous_scope && previous_scope.create_with_value && scope[:create]
               scope_for_create = if action == :merge
@@ -1171,7 +1170,7 @@ MSG
         end
 
         def current_scope #:nodoc:
-          Thread.current[:"#{self}_current_scope"] ||= build_default_scope
+          Thread.current[:"#{self}_current_scope"]
         end
 
         def current_scope=(scope) #:nodoc:
@@ -1227,15 +1226,13 @@ class Post < ActiveRecord::Base
 end
 WARN
 
-          # Reset the current scope as it may contain scopes based on a now-invalid default scope
-          self.current_scope = nil
           self.default_scopes = default_scopes.dup << scope
         end
 
         def build_default_scope #:nodoc:
           if method(:default_scope).owner != Base.singleton_class
-            # Exclusively scope to just the relation, to avoid infinite recursion where the
-            # default scope tries to use the default scope tries to use the default scope...
+            # Use relation.scoping to ensure we ignore whatever the current value of
+            # self.current_scope may be.
             relation.scoping { default_scope }
           elsif default_scopes.any?
             default_scopes.inject(relation) do |default_scope, scope|
diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index a398476dd6..68e3018e22 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -35,7 +35,13 @@ module ActiveRecord
         if options
           scoped.apply_finder_options(options)
         else
-          (current_scope || relation).clone
+          if current_scope
+            current_scope.clone
+          else
+            scope = relation.clone
+            scope.default_scoped = true
+            scope
+          end
         end
       end
 
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 645ad0fce1..4aa2516cb2 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -6,7 +6,7 @@ module ActiveRecord
     JoinOperation = Struct.new(:relation, :join_class, :on)
     ASSOCIATION_METHODS = [:includes, :eager_load, :preload]
     MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :bind]
-    SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from]
+    SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from, :reorder]
 
     include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches
 
@@ -15,14 +15,16 @@ module ActiveRecord
     delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :to => :klass
 
     attr_reader :table, :klass, :loaded
-    attr_accessor :extensions
+    attr_accessor :extensions, :default_scoped
     alias :loaded? :loaded
+    alias :default_scoped? :default_scoped
 
     def initialize(klass, table)
       @klass, @table = klass, table
 
       @implicit_readonly = nil
       @loaded            = false
+      @default_scoped    = false
 
       SINGLE_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_value", nil)}
       (ASSOCIATION_METHODS + MULTI_VALUE_METHODS).each {|v| instance_variable_set(:"@#{v}_values", [])}
@@ -372,7 +374,7 @@ module ActiveRecord
     end
 
     def where_values_hash
-      equalities = @where_values.grep(Arel::Nodes::Equality).find_all { |node|
+      equalities = with_default_scope.where_values.grep(Arel::Nodes::Equality).find_all { |node|
         node.left.relation.name == table_name
       }
 
@@ -400,6 +402,15 @@ module ActiveRecord
       to_a.inspect
     end
 
+    def with_default_scope #:nodoc:
+      if default_scoped?
+        default_scope = @klass.send(:build_default_scope)
+        default_scope ? default_scope.merge(self) : self
+      else
+        self
+      end
+    end
+
     protected
 
     def method_missing(method, *args, &block)
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 02b7056989..94aa999715 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -8,7 +8,8 @@ module ActiveRecord
     attr_accessor :includes_values, :eager_load_values, :preload_values,
                   :select_values, :group_values, :order_values, :joins_values,
                   :where_values, :having_values, :bind_values,
-                  :limit_value, :offset_value, :lock_value, :readonly_value, :create_with_value, :from_value
+                  :limit_value, :offset_value, :lock_value, :readonly_value, :create_with_value,
+                  :from_value, :reorder_value
 
     def includes(*args)
       args.reject! {|a| a.blank? }
@@ -63,7 +64,11 @@ module ActiveRecord
     end
 
     def reorder(*args)
-      except(:order).order(args)
+      return self if args.blank?
+
+      relation = clone
+      relation.reorder_value = args.flatten
+      relation
     end
 
     def joins(*args)
@@ -163,7 +168,7 @@ module ActiveRecord
     end
 
     def arel
-      @arel ||= build_arel
+      @arel ||= with_default_scope.build_arel
     end
 
     def build_arel
@@ -180,7 +185,8 @@ module ActiveRecord
 
       arel.group(*@group_values.uniq.reject{|g| g.blank?}) unless @group_values.empty?
 
-      arel.order(*@order_values.uniq.reject{|o| o.blank?}) unless @order_values.empty?
+      order = @reorder_value ? @reorder_value : @order_values
+      arel.order(*order.uniq.reject{|o| o.blank?}) unless order.empty?
 
       build_select(arel, @select_values.uniq)
 
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index 128e0fbd86..69706b5ead 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -8,6 +8,8 @@ module ActiveRecord
 
       merged_relation = clone
 
+      r = r.with_default_scope if r.default_scoped? && r.klass != klass
+
       Relation::ASSOCIATION_METHODS.each do |method|
         value = r.send(:"#{method}_values")
 
@@ -70,6 +72,7 @@ module ActiveRecord
     #
     def except(*skips)
       result = self.class.new(@klass, table)
+      result.default_scoped = default_scoped
 
       ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) - skips).each do |method|
         result.send(:"#{method}_values=", send(:"#{method}_values"))
@@ -94,6 +97,7 @@ module ActiveRecord
     #
     def only(*onlies)
       result = self.class.new(@klass, table)
+      result.default_scoped = default_scoped
 
       ((Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS) & onlies).each do |method|
         result.send(:"#{method}_values=", send(:"#{method}_values"))