From 96be539b0b18c19941d8089dac35cc2f6eb52097 Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Mon, 16 Mar 2009 22:06:07 -0400 Subject: [PATCH] Better Default Nested Scope, including Block --- activerecord/lib/active_record/base.rb | 135 ++++++++++++++---------- activerecord/test/cases/base_test.rb | 6 +- activerecord/test/cases/finder_test.rb | 18 ++-- activerecord/test/cases/fixtures_test.rb | 2 +- activerecord/test/cases/method_scoping_test.rb | 49 +++++++-- activerecord/test/fixtures/developers.yml | 8 ++- activerecord/test/models/developer.rb | 11 ++ 7 files changed, 150 insertions(+), 79 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2a53851..86a391a 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1522,13 +1522,9 @@ module ActiveRecord #:nodoc: if scoped?(:find, :order) scope = scope(:find) original_scoped_order = scope[:order] - scope[:order] = reverse_sql_order(original_scoped_order) - end - - begin + with_scope(:find => { :order => reverse_sql_order(scope[:order]) }) { find_initial(options) } + else find_initial(options.merge({ :order => order })) - ensure - scope[:order] = original_scoped_order if original_scoped_order end end @@ -2094,66 +2090,96 @@ module ActiveRecord #:nodoc: def with_scope(method_scoping = {}, action = :merge, &block) method_scoping = method_scoping.method_scoping if method_scoping.respond_to?(:method_scoping) - # Dup first and second level of hash (method and params). - method_scoping = method_scoping.inject({}) do |hash, (method, params)| - hash[method] = (params == true) ? params : params.dup - hash - end - method_scoping.assert_valid_keys([ :find, :create ]) if f = method_scoping[:find] f.assert_valid_keys(VALID_FIND_OPTIONS) set_readonly_option! f end + + method_scoping = merge_scopings(method_scoping, current_scoped_methods || {}, action) + + self.scoped_methods << method_scoping + begin + yield + ensure + self.scoped_methods.pop + end + end + + def without_default_scope + backup_default_scoping = self.default_scoping.dup + self.default_scoping = [] + begin + yield + ensure + self.default_scoping = backup_default_scoping + end + end - # Merge scopings - if [:merge, :reverse_merge].include?(action) && current_scoped_methods - method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)| - case hash[method] - when Hash - if method == :find - (hash[method].keys + params.keys).uniq.each do |key| - merge = hash[method][key] && params[key] # merge if both scopes have the same key - if key == :conditions && merge - if params[key].is_a?(Hash) && hash[method][key].is_a?(Hash) - hash[method][key] = merge_conditions(hash[method][key].deep_merge(params[key])) - else - hash[method][key] = merge_conditions(params[key], hash[method][key]) - end - elsif key == :include && merge - hash[method][key] = merge_includes(hash[method][key], params[key]).uniq - elsif key == :joins && merge - hash[method][key] = merge_joins(params[key], hash[method][key]) + # Works like with_scope, but discards any nested properties. + def with_exclusive_scope(method_scoping = {}, &block) + without_default_scope { with_scope(method_scoping, :overwrite, &block) } + end + + def merge_scopings(new_scopings, existing_scopings, action = :merge) + # Dup first and second level of hash (method and params). + new_scopings = new_scopings.inject({}) do |hash, (method, params)| + hash[method] = (params == true) ? params : params.dup + hash + end + + return new_scopings unless [:merge, :reverse_merge].include?(action) + + existing_scopings.inject(new_scopings) do |hash, (method, params)| + case hash[method] + when Hash + if method == :find + (hash[method].keys + params.keys).uniq.each do |key| + merge = hash[method][key] && params[key] # merge if both scopes have the same key + if key == :conditions && merge + if params[key].is_a?(Hash) && hash[method][key].is_a?(Hash) + hash[method][key] = merge_conditions(hash[method][key].deep_merge(params[key])) else - hash[method][key] = hash[method][key] || params[key] + hash[method][key] = merge_conditions(params[key], hash[method][key]) end - end - else - if action == :reverse_merge - hash[method] = hash[method].merge(params) + elsif key == :include && merge + hash[method][key] = merge_includes(hash[method][key], params[key]).uniq + elsif key == :joins && merge + hash[method][key] = merge_joins(params[key], hash[method][key]) else - hash[method] = params.merge(hash[method]) + hash[method][key] = hash[method][key] || params[key] end end else - hash[method] = params - end - hash + if action == :reverse_merge + hash[method] = hash[method].merge(params) + else + hash[method] = params.merge(hash[method]) + end + end + else + hash[method] = params end + hash end - - self.scoped_methods << method_scoping - begin - yield - ensure - self.scoped_methods.pop + end + + def eval_default_scoping + default_scoping.inject({}) do |hash, ds| + scope_options = ds.call + + clean_scope_options = { :find => scope_options } + clean_scope_options[:create] = + (scope_options.is_a?(Hash) && scope_options.has_key?(:conditions) && scope_options[:conditions].respond_to?(:merge)) ? + scope_options[:conditions] : {} + + merge_scopings(hash, clean_scope_options, :merge) end end - - # Works like with_scope, but discards any nested properties. - def with_exclusive_scope(method_scoping = {}, &block) - with_scope(method_scoping, :overwrite, &block) + + def current_merged_scopings + merge_scopings(current_scoped_methods || {}, eval_default_scoping, :merge) end def subclasses #:nodoc: @@ -2167,26 +2193,27 @@ module ActiveRecord #:nodoc: # class Person < ActiveRecord::Base # default_scope :order => 'last_name, first_name' # end - def default_scope(options = {}) - self.default_scoping << { :find => options, :create => (options.is_a?(Hash) && options.has_key?(:conditions)) ? options[:conditions] : {} } + def default_scope(options = {}, &block) + raise ArgumentError.new("Cannot take options and block at the same time") if !options.empty? && block_given? + self.default_scoping << lambda{ block ? block.call : options } end # Test whether the given method and optional key are scoped. def scoped?(method, key = nil) #:nodoc: - if current_scoped_methods && (scope = current_scoped_methods[method]) + if scope = current_merged_scopings[method] !key || !scope[key].nil? end end # Retrieve the scope for the given method and optional key. def scope(method, key = nil) #:nodoc: - if current_scoped_methods && (scope = current_scoped_methods[method]) + if scope = current_merged_scopings[method] key ? scope[key] : scope end end def scoped_methods #:nodoc: - Thread.current[:"#{self}_scoped_methods"] ||= self.default_scoping.dup + Thread.current[:"#{self}_scoped_methods"] ||= [] end def current_scoped_methods #:nodoc: diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 99d7796..1ab8827 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1719,8 +1719,8 @@ class BasicsTest < ActiveRecord::TestCase scoped_developers = Developer.with_scope(:find => { :limit => 1, :order => 'salary DESC' }) do Developer.find(:all) end - assert_equal 'Jamis', scoped_developers.first.name - assert scoped_developers.include?(developers(:jamis)) + assert_equal 'Future', scoped_developers.first.name + assert scoped_developers.include?(developers(:future)) # Test scope without order and order in find scoped_developers = Developer.with_scope(:find => { :limit => 1 }) do Developer.find(:all, :order => 'salary DESC') @@ -1759,7 +1759,7 @@ class BasicsTest < ActiveRecord::TestCase developers = Developer.with_scope(:find => { :group => 'salary', :having => "SUM(salary) > 10000", :select => "SUM(salary) as salary" }) do Developer.find(:all) end - assert_equal 3, developers.size + assert_equal 4, developers.size end def test_find_last diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index d877895..4f4f9ac 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -133,11 +133,11 @@ class FinderTest < ActiveRecord::TestCase assert_equal 2, Entrant.find([1,3,2], :limit => 2).size assert_equal 1, Entrant.find([1,3,2], :limit => 3, :offset => 2).size - # Also test an edge case: If you have 11 results, and you set a - # limit of 3 and offset of 9, then you should find that there + # Also test an edge case: If you have 12 results, and you set a + # limit of 3 and offset of 10, then you should find that there # will be only 2 results, regardless of the limit. devs = Developer.find :all - last_devs = Developer.find devs.map(&:id), :limit => 3, :offset => 9 + last_devs = Developer.find devs.map(&:id), :limit => 3, :offset => 10 assert_equal 2, last_devs.size end @@ -180,21 +180,21 @@ class FinderTest < ActiveRecord::TestCase def test_find_with_group developers = Developer.find(:all, :group => "salary", :select => "salary") - assert_equal 4, developers.size - assert_equal 4, developers.map(&:salary).uniq.size + assert_equal 5, developers.size + assert_equal 5, developers.map(&:salary).uniq.size end def test_find_with_group_and_having developers = Developer.find(:all, :group => "salary", :having => "sum(salary) > 10000", :select => "salary") - assert_equal 3, developers.size - assert_equal 3, developers.map(&:salary).uniq.size + assert_equal 4, developers.size + assert_equal 4, developers.map(&:salary).uniq.size assert developers.all? { |developer| developer.salary > 10000 } end def test_find_with_group_and_sanitized_having developers = Developer.find(:all, :group => "salary", :having => ["sum(salary) > ?", 10000], :select => "salary") - assert_equal 3, developers.size - assert_equal 3, developers.map(&:salary).uniq.size + assert_equal 4, developers.size + assert_equal 4, developers.map(&:salary).uniq.size assert developers.all? { |developer| developer.salary > 10000 } end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 252bf4f..4f17dcf 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -138,7 +138,7 @@ class FixturesTest < ActiveRecord::TestCase end def test_erb_in_fixtures - assert_equal 11, @developers.size + assert_equal 12, @developers.size assert_equal "fixture_5", @dev_5.name end diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 3c34cde..712b9bb 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -590,28 +590,40 @@ class DefaultScopingTest < ActiveRecord::TestCase received = DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } assert_equal expected, received end + + def test_default_scope_with_block + assert_equal 12, Developer.count + assert_equal 11, DeveloperCurrent.count + assert_instance_of Time, DeveloperCurrent.send(:eval_default_scoping)[:find][:conditions][1] + end def test_default_scoping_with_threads - scope = [{ :create => {}, :find => { :order => 'salary DESC' } }] + scope = { :create => {}, :find => { :order => 'salary DESC' } } 2.times do - Thread.new { assert_equal scope, DeveloperOrderedBySalary.send(:scoped_methods) }.join + Thread.new { assert_equal scope, DeveloperOrderedBySalary.send(:eval_default_scoping) }.join end end def test_default_scoping_with_inheritance - scope = [{ :create => {}, :find => { :order => 'salary DESC' } }] - + scope = { :create => {}, :find => { :order => 'salary DESC' } } + # Inherit a class having a default scope and define a new default scope klass = Class.new(DeveloperOrderedBySalary) - klass.send :default_scope, {} - + klass.send :default_scope, { :conditions => { :salary => 0 } } + # Scopes added on children should append to parent scope - expected_klass_scope = [{ :create => {}, :find => { :order => 'salary DESC' }}, { :create => {}, :find => {} }] - assert_equal expected_klass_scope, klass.send(:scoped_methods) - + expected_klass_scope = { :create => { :salary => 0 }, + :find => { :conditions => { :salary => 0 }, :order => 'salary DESC' } } + assert_equal expected_klass_scope, klass.send(:eval_default_scoping) + # Parent should still have the original scope - assert_equal scope, DeveloperOrderedBySalary.send(:scoped_methods) + assert_equal scope, DeveloperOrderedBySalary.send(:eval_default_scoping) + end + + def test_default_scoping_with_condition_string_and_inheritance + scope = { :create => {}, :find => { :conditions => "salary > 0", :order => 'salary DESC' } } + assert_equal scope, DeveloperSalaryHiding.send(:eval_default_scoping) end def test_method_scope @@ -629,7 +641,7 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_named_scope - expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } + expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.salary } received = DeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.salary } assert_equal expected, received end @@ -647,6 +659,21 @@ class DefaultScopingTest < ActiveRecord::TestCase received = DeveloperOrderedBySalary.find(:all, :order => 'salary').collect { |dev| dev.salary } assert_equal expected, received end + + def test_without_default_scope + count = Developer.count + assert_equal (count - 1), DeveloperCurrent.count + assert_equal count, DeveloperCurrent.send(:without_default_scope){ DeveloperCurrent.count } + end + + def test_without_default_scope_inside_with_scope + expected = Developer.find(:all, :order => 'name').map(&:name) + received = DeveloperCurrent.send(:with_scope, :find => { :order => "name" }) do + DeveloperCurrent.send(:without_default_scope){ DeveloperCurrent.find(:all) }.map(&:name) + end + assert_equal expected, received + end + end =begin diff --git a/activerecord/test/fixtures/developers.yml b/activerecord/test/fixtures/developers.yml index 308bf75..7e6b05b 100644 --- a/activerecord/test/fixtures/developers.yml +++ b/activerecord/test/fixtures/developers.yml @@ -18,4 +18,10 @@ dev_<%= digit %>: poor_jamis: id: 11 name: Jamis - salary: 9000 \ No newline at end of file + salary: 9000 + +future: + id: 12 + name: Future + salary: 1000000 + created_at: <%= Time.now.tomorrow.to_s(:db) %> \ No newline at end of file diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 92039a4..eedda6d 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -89,3 +89,14 @@ class DeveloperOrderedBySalary < ActiveRecord::Base end end end + +class DeveloperSalaryHiding < DeveloperOrderedBySalary + default_scope :conditions => "salary > 0" +end + +class DeveloperCurrent < ActiveRecord::Base + self.table_name = 'developers' + default_scope do + { :conditions => ['created_at <= ?', Time.now] } + end +end \ No newline at end of file -- 1.5.5