Skip to content
This repository
Browse code

Fix infinite recursion where a lazy default scope references a scope.…

… Fixes #1264.
  • Loading branch information...
commit c69111ba5fde8445237f682c88b927bcde1588d4 1 parent d21fef3
Jon Leighton authored May 25, 2011
15  activerecord/lib/active_record/base.rb
@@ -428,6 +428,10 @@ class Base
428 428
     class_attribute :default_scopes, :instance_writer => false
429 429
     self.default_scopes = []
430 430
 
  431
+    # Boolean flag to prevent infinite recursion when evaluating default scopes
  432
+    class_attribute :apply_default_scope, :instance_writer => false
  433
+    self.apply_default_scope = true
  434
+
431 435
     # Returns a hash of all the attributes that have been specified for serialization as
432 436
     # keys and their class restriction as values.
433 437
     class_attribute :serialized_attributes
@@ -1261,11 +1265,14 @@ def default_scope(scope = {})
1261 1265
           self.default_scopes = default_scopes + [scope]
1262 1266
         end
1263 1267
 
  1268
+        # The apply_default_scope flag is used to prevent an infinite recursion situation where
  1269
+        # a default scope references a scope which has a default scope which references a scope...
1264 1270
         def build_default_scope #:nodoc:
  1271
+          return unless apply_default_scope
  1272
+          self.apply_default_scope = false
  1273
+
1265 1274
           if method(:default_scope).owner != Base.singleton_class
1266  
-            # Use relation.scoping to ensure we ignore whatever the current value of
1267  
-            # self.current_scope may be.
1268  
-            relation.scoping { default_scope }
  1275
+            default_scope
1269 1276
           elsif default_scopes.any?
1270 1277
             default_scopes.inject(relation) do |default_scope, scope|
1271 1278
               if scope.is_a?(Hash)
@@ -1277,6 +1284,8 @@ def build_default_scope #:nodoc:
1277 1284
               end
1278 1285
             end
1279 1286
           end
  1287
+        ensure
  1288
+          self.apply_default_scope = true
1280 1289
         end
1281 1290
 
1282 1291
         # Returns the class type of the record using the current module as a prefix. So descendants of
2  activerecord/lib/active_record/relation.rb
@@ -424,7 +424,7 @@ def inspect
424 424
     end
425 425
 
426 426
     def with_default_scope #:nodoc:
427  
-      if default_scoped? && default_scope = @klass.send(:build_default_scope)
  427
+      if default_scoped? && default_scope = klass.send(:build_default_scope)
428 428
         default_scope = default_scope.merge(self)
429 429
         default_scope.default_scoped = false
430 430
         default_scope
8  activerecord/test/cases/relation_scoping_test.rb
@@ -312,6 +312,14 @@ def test_default_scope_as_class_method
312 312
     assert_equal [developers(:david).becomes(ClassMethodDeveloperCalledDavid)], ClassMethodDeveloperCalledDavid.all
313 313
   end
314 314
 
  315
+  def test_default_scope_as_class_method_referencing_scope
  316
+    assert_equal [developers(:david).becomes(ClassMethodReferencingScopeDeveloperCalledDavid)], ClassMethodReferencingScopeDeveloperCalledDavid.all
  317
+  end
  318
+
  319
+  def test_default_scope_as_block_referencing_scope
  320
+    assert_equal [developers(:david).becomes(LazyBlockReferencingScopeDeveloperCalledDavid)], LazyBlockReferencingScopeDeveloperCalledDavid.all
  321
+  end
  322
+
315 323
   def test_default_scope_with_lambda
316 324
     assert_equal [developers(:david).becomes(LazyLambdaDeveloperCalledDavid)], LazyLambdaDeveloperCalledDavid.all
317 325
   end
15  activerecord/test/models/developer.rb
@@ -127,6 +127,21 @@ def self.default_scope
127 127
   end
128 128
 end
129 129
 
  130
+class ClassMethodReferencingScopeDeveloperCalledDavid < ActiveRecord::Base
  131
+  self.table_name = 'developers'
  132
+  scope :david, where(:name => 'David')
  133
+
  134
+  def self.default_scope
  135
+    david
  136
+  end
  137
+end
  138
+
  139
+class LazyBlockReferencingScopeDeveloperCalledDavid < ActiveRecord::Base
  140
+  self.table_name = 'developers'
  141
+  scope :david, where(:name => 'David')
  142
+  default_scope { david }
  143
+end
  144
+
130 145
 class DeveloperCalledJamis < ActiveRecord::Base
131 146
   self.table_name = 'developers'
132 147
 

0 notes on commit c69111b

José Valim

@jonleighton, maybe you would like to use @variables instead? setting a class_attribute is a bit expensive (as it defines a new method) and @variable is enough because it does not seem like you need inheritance here.

Jon Leighton

Ha. Yes. I think I just defined this as a class_attribute without thinking, because default_scopes is a class attribute. I will change it. Come to think of it, default_scopes probably doesn't need to be a class attribute either.

Andrew White

What about STI where a subclass adds a default scope? Sounds like you'd want inheritance then.

Please sign in to comment.
Something went wrong with that request. Please try again.