Permalink
Browse files

Deprecated support for passing hashes and relations to default_scope,…

… in favour of defining a 'default_scope' class method in the model. See the CHANGELOG for more details.
  • Loading branch information...
1 parent fc9a04b commit 5740d4ec0c16d68b82f440e74fd8b74ae3fe48e6 @jonleighton jonleighton committed with tenderlove Apr 3, 2011
View
@@ -1,5 +1,26 @@
*Rails 3.1.0 (unreleased)*
+* Deprecated support for passing hashes and relations to 'default_scope'. Please create a class
+ method for your scope instead. For example, change this:
+
+ class Post < ActiveRecord::Base
+ default_scope where(:published => true)
+ end
+
+ To this:
+
+ class Post < ActiveRecord::Base
+ def self.default_scope
+ where(:published => true)
+ end
+ end
+
+ Rationale: It will make the implementation simpler because we can simply use inheritance to
+ handle inheritance scenarios, rather than trying to make up our own rules about what should
+ happen when you call default_scope multiple times or in subclasses.
+
+ [Jon Leighton]
+
* PostgreSQL adapter only supports PostgreSQL version 8.2 and higher.
* ConnectionManagement middleware is changed to clean up the connection pool
@@ -425,8 +425,8 @@ class Base
self.store_full_sti_class = true
# Stores the default scope for the class
- class_attribute :default_scoping, :instance_writer => false
- self.default_scoping = []
+ class_attribute :default_scopes, :instance_writer => false
+ self.default_scopes = []
# Returns a hash of all the attributes that have been specified for serialization as
# keys and their class restriction as values.
@@ -870,7 +870,9 @@ def arel_engine
# Returns a scope for this class without taking into account the default_scope.
#
# class Post < ActiveRecord::Base
- # default_scope :published => true
+ # def self.default_scope
+ # where :published => true
+ # end
# end
#
# Post.all # Fires "SELECT * FROM posts WHERE published = true"
@@ -892,13 +894,8 @@ def unscoped #:nodoc:
block_given? ? relation.scoping { yield } : relation
end
- def scoped_methods #:nodoc:
- key = :"#{self}_scoped_methods"
- Thread.current[key] = Thread.current[key].presence || self.default_scoping.dup
- end
-
def before_remove_const #:nodoc:
- reset_scoped_methods
+ self.current_scope = nil
end
# Specifies how the record is loaded by +Marshal+.
@@ -1020,7 +1017,7 @@ def method_missing(method_id, *arguments, &block)
super unless all_attributes_exists?(attribute_names)
if match.finder?
options = arguments.extract_options!
- relation = options.any? ? construct_finder_arel(options, current_scoped_methods) : scoped
+ relation = options.any? ? scoped(options) : scoped
relation.send :find_by_attributes, match, attribute_names, *arguments
elsif match.instantiator?
scoped.send :find_or_instantiator_by_attributes, match, attribute_names, *arguments, &block
@@ -1109,43 +1106,48 @@ def all_attributes_exists?(attribute_names)
# end
#
# *Note*: the +:find+ scope also has effect on update and deletion methods, like +update_all+ and +delete_all+.
- def with_scope(method_scoping = {}, action = :merge, &block)
- method_scoping = method_scoping.method_scoping if method_scoping.respond_to?(:method_scoping)
+ def with_scope(scope = {}, action = :merge, &block)
+ # 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"]
- if method_scoping.is_a?(Hash)
+ if scope.is_a?(Hash)
# Dup first and second level of hash (method and params).
- method_scoping = method_scoping.dup
- method_scoping.each do |method, params|
- method_scoping[method] = params.dup unless params == true
+ scope = scope.dup
+ scope.each do |method, params|
+ scope[method] = params.dup unless params == true
end
- method_scoping.assert_valid_keys([ :find, :create ])
- relation = construct_finder_arel(method_scoping[:find] || {})
+ scope.assert_valid_keys([ :find, :create ])
+ relation = construct_finder_arel(scope[:find] || {})
- if current_scoped_methods && current_scoped_methods.create_with_value && method_scoping[:create]
+ if previous_scope && previous_scope.create_with_value && scope[:create]
scope_for_create = if action == :merge
- current_scoped_methods.create_with_value.merge(method_scoping[:create])
+ previous_scope.create_with_value.merge(scope[:create])
else
- method_scoping[:create]
+ scope[:create]
end
relation = relation.create_with(scope_for_create)
else
- scope_for_create = method_scoping[:create]
- scope_for_create ||= current_scoped_methods.create_with_value if current_scoped_methods
+ scope_for_create = scope[:create]
+ scope_for_create ||= previous_scope.create_with_value if previous_scope
relation = relation.create_with(scope_for_create) if scope_for_create
end
- method_scoping = relation
+ scope = relation
end
- method_scoping = current_scoped_methods.merge(method_scoping) if current_scoped_methods && action == :merge
+ scope = previous_scope.merge(scope) if previous_scope && action == :merge
- self.scoped_methods << method_scoping
+ self.current_scope = scope
begin
yield
ensure
- self.scoped_methods.pop
+ self.current_scope = previous_scope
end
end
@@ -1168,39 +1170,82 @@ def with_exclusive_scope(method_scoping = {}, &block)
with_scope(method_scoping, :overwrite, &block)
end
- # Sets the default options for the model. The format of the
- # <tt>options</tt> argument is the same as in find.
+ def current_scope #:nodoc:
+ Thread.current[:"#{self}_current_scope"] ||= build_default_scope
+ end
+
+ def current_scope=(scope) #:nodoc:
+ Thread.current[:"#{self}_current_scope"] = scope
+ end
+
+ # Implement this method in your model to set a default scope for all operations on
+ # the model.
#
# class Person < ActiveRecord::Base
- # default_scope order('last_name, first_name')
+ # def self.default_scope
+ # order('last_name, first_name')
+ # end
# end
#
- # <tt>default_scope</tt> is also applied while creating/building a record. It is not
+ # Person.all # => SELECT * FROM people ORDER BY last_name, first_name
+ #
+ # The <tt>default_scope</tt> is also applied while creating/building a record. It is not
# applied while updating a record.
#
# class Article < ActiveRecord::Base
- # default_scope where(:published => true)
+ # def self.default_scope
+ # where(:published => true)
+ # end
# end
#
# Article.new.published # => true
# Article.create.published # => true
- def default_scope(options = {})
- reset_scoped_methods
- default_scoping = self.default_scoping.dup
- self.default_scoping = default_scoping << construct_finder_arel(options, default_scoping.pop)
- end
+ #
+ # === Deprecation warning
+ #
+ # There is an alternative syntax as follows:
+ #
+ # class Person < ActiveRecord::Base
+ # default_scope order('last_name, first_name')
+ # end
+ #
+ # This is now deprecated and will be removed in Rails 3.2.
+ def default_scope(scope = {})
+ ActiveSupport::Deprecation.warn <<-WARN
+Passing a hash or scope to default_scope is deprecated and will be removed in Rails 3.2. You should create a class method for your scope instead. For example, change this:
- def current_scoped_methods #:nodoc:
- method = scoped_methods.last
- if method.respond_to?(:call)
- relation.scoping { method.call }
- else
- method
- end
+class Post < ActiveRecord::Base
+ default_scope where(:published => true)
+end
+
+To this:
+
+class Post < ActiveRecord::Base
+ def self.default_scope
+ where(:published => true)
+ end
+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 reset_scoped_methods #:nodoc:
- Thread.current[:"#{self}_scoped_methods"] = nil
+ 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...
+ relation.scoping { default_scope }
+ elsif default_scopes.any?
+ default_scopes.inject(relation) do |default_scope, scope|
+ if scope.is_a?(Hash)
+ default_scope.apply_finder_options(scope)
+ else
+ default_scope.merge(scope)
+ end
+ end
+ end
end
# Returns the class type of the record using the current module as a prefix. So descendants of
@@ -1916,11 +1961,8 @@ def convert_number_column_value(value)
end
def populate_with_current_scope_attributes
- if scope = self.class.send(:current_scoped_methods)
- create_with = scope.scope_for_create
- create_with.each { |att,value|
- respond_to?("#{att}=") && send("#{att}=", value)
- }
+ self.class.scoped.scope_for_create.each do |att,value|
+ respond_to?("#{att}=") && send("#{att}=", value)
end
end
@@ -35,7 +35,7 @@ def scoped(options = nil)
if options
scoped.apply_finder_options(options)
else
- current_scoped_methods ? relation.merge(current_scoped_methods) : relation.clone
+ (current_scope || relation).clone
end
end
@@ -154,12 +154,7 @@ def many?
# Please check unscoped if you want to remove all previous scopes (including
# the default_scope) during the execution of a block.
def scoping
- @klass.scoped_methods << self
- begin
- yield
- ensure
- @klass.scoped_methods.pop
- end
+ @klass.send(:with_scope, self, :overwrite) { yield }
end
# Updates all records with details given if they match a set of conditions supplied, limits and order can
@@ -70,16 +70,16 @@ def test_create_from_association_should_respect_default_scope
# would be convenient), because this would cause that scope to be applied to any callbacks etc.
def test_build_and_create_should_not_happen_within_scope
car = cars(:honda)
- original_scoped_methods = Bulb.scoped_methods
+ scoped_count = car.foo_bulbs.scoped.where_values.count
- bulb = car.bulbs.build
- assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ bulb = car.foo_bulbs.build
+ assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count
- bulb = car.bulbs.create
- assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ bulb = car.foo_bulbs.create
+ assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count
- bulb = car.bulbs.create!
- assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ bulb = car.foo_bulbs.create!
+ assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count
end
def test_no_sql_should_be_fired_if_association_already_loaded
@@ -165,16 +165,16 @@ def test_successful_build_association
def test_build_and_create_should_not_happen_within_scope
pirate = pirates(:blackbeard)
- original_scoped_methods = Bulb.scoped_methods.dup
+ scoped_count = pirate.association(:foo_bulb).scoped.where_values.count
- bulb = pirate.build_bulb
- assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ bulb = pirate.build_foo_bulb
+ assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count
- bulb = pirate.create_bulb
- assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ bulb = pirate.create_foo_bulb
+ assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count
- bulb = pirate.create_bulb!
- assert_equal original_scoped_methods, bulb.scoped_methods_after_initialize
+ bulb = pirate.create_foo_bulb!
+ assert_not_equal scoped_count, bulb.scope_after_initialize.where_values.count
end
def test_create_association
@@ -1630,14 +1630,18 @@ def test_default_scope_is_reset
Object.const_set :UnloadablePost, Class.new(ActiveRecord::Base)
UnloadablePost.table_name = 'posts'
UnloadablePost.class_eval do
- default_scope order('posts.comments_count ASC')
+ class << self
+ def default_scope
+ order('posts.comments_count ASC')
+ end
+ end
end
- UnloadablePost.scoped_methods # make Thread.current[:UnloadablePost_scoped_methods] not nil
+ UnloadablePost.scoped # make Thread.current[:UnloadablePost_scoped_methods] not nil
UnloadablePost.unloadable
- assert_not_nil Thread.current[:UnloadablePost_scoped_methods]
+ assert_not_nil Thread.current[:UnloadablePost_current_scope]
ActiveSupport::Dependencies.remove_unloadable_constants!
- assert_nil Thread.current[:UnloadablePost_scoped_methods]
+ assert_nil Thread.current[:UnloadablePost_current_scope]
ensure
Object.class_eval{ remove_const :UnloadablePost } if defined?(UnloadablePost)
end
@@ -249,22 +249,21 @@ def test_immutable_scope
end
def test_scoped_with_duck_typing
- scoping = Struct.new(:method_scoping).new(:find => { :conditions => ["name = ?", 'David'] })
+ scoping = Struct.new(:current_scope).new(:find => { :conditions => ["name = ?", 'David'] })
Developer.send(:with_scope, scoping) do
assert_equal %w(David), Developer.find(:all).map { |d| d.name }
end
end
def test_ensure_that_method_scoping_is_correctly_restored
- scoped_methods = Developer.instance_eval('current_scoped_methods')
-
begin
Developer.send(:with_scope, :find => { :conditions => "name = 'Jamis'" }) do
raise "an exception"
end
rescue
end
- assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods')
+
+ assert !Developer.scoped.where_values.include?("name = 'Jamis'")
end
end
@@ -509,14 +508,15 @@ def test_immutable_merged_scope
def test_ensure_that_method_scoping_is_correctly_restored
Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do
- scoped_methods = Developer.instance_eval('current_scoped_methods')
begin
Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do
raise "an exception"
end
rescue
end
- assert_equal scoped_methods, Developer.instance_eval('current_scoped_methods')
+
+ assert Developer.scoped.where_values.include?("name = 'David'")
+ assert !Developer.scoped.where_values.include?("name = 'Maiha'")
end
end
Oops, something went wrong.

0 comments on commit 5740d4e

Please sign in to comment.