diff options
| -rw-r--r-- | activerecord/CHANGELOG | 9 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_many_association.rb | 11 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 102 | ||||
| -rwxr-xr-x | activerecord/test/base_test.rb | 10 | ||||
| -rw-r--r-- | activerecord/test/conditions_scoping_test.rb | 51 | ||||
| -rwxr-xr-x | activerecord/test/readonly_test.rb | 14 |
7 files changed, 130 insertions, 69 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index d13d06585c..97261e4f66 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,14 @@ *SVN* +* Rename Base.constrain to Base.with_scope so it doesn't conflict with existing concept of database constraints. Make scoping more robust: uniform method => parameters, validated method names and supported finder parameters, raise exception on nested scopes. [Jeremy Kemper] Example: + + Comment.with_scope(:find => { :conditions => 'active=true' }, :create => { :post_id => 5 }) do + # Find where name = ? and active=true + Comment.find :all, :conditions => ['name = ?', name] + # Create comment associated with :post_id + Comment.create :body => "Hello world" + end + * Fixed that SQL Server should ignore :size declarations on anything but integer and string in the agnostic schema representation #2756 [Ryan Tomayko] * Added constrain scoping for creates using a hash of attributes bound to the :creation key [DHH]. Example: diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index 2972aa0248..a90d89a69b 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -80,7 +80,7 @@ module ActiveRecord if @target.respond_to?(method) || (!@association_class.respond_to?(method) && Class.respond_to?(method)) super else - @association_class.constrain(:conditions => @finder_sql, :joins => @join_sql, :readonly => false) do + @association_class.with_scope(:find => { :conditions => @finder_sql, :joins => @join_sql, :readonly => false }) do @association_class.send(method, *args, &block) end end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index b04cb81a5c..9ee06ffbcb 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -89,11 +89,16 @@ module ActiveRecord if @target.respond_to?(method) || (!@association_class.respond_to?(method) && Class.respond_to?(method)) super else - @association_class.constrain( + @association_class.with_scope( + :find => { :conditions => @finder_sql, :joins => @join_sql, - :readonly => false, - :creation => { @association_class_primary_key_name => @owner.id }) do + :readonly => false + }, + :create => { + @association_class_primary_key_name => @owner.id + } + ) do @association_class.send(method, *args, &block) end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 3fc55e0783..06ac4459de 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -394,11 +394,11 @@ module ActiveRecord #:nodoc: def find(*args) options = extract_options_from_args!(args) - # Inherit :readonly from scope_constraints if set. Otherwise, + # Inherit :readonly from finder scope if set. Otherwise, # if :joins is not blank then :readonly defaults to true. unless options.has_key?(:readonly) - if scope_constraints.has_key?(:readonly) - options[:readonly] = scope_constraints[:readonly] + if scoped?(:find, :readonly) + options[:readonly] = scope(:find, :readonly) elsif !options[:joins].blank? options[:readonly] = true end @@ -460,7 +460,7 @@ module ActiveRecord #:nodoc: if attributes.is_a?(Array) attributes.collect { |attr| create(attr) } else - attributes.reverse_merge!(scope_constraints[:creation]) if scope_constraints[:creation] + attributes.reverse_merge!(scope(:create)) if scoped?(:create) object = new(attributes) object.save @@ -839,23 +839,37 @@ module ActiveRecord #:nodoc: ensure logger.level = old_logger_level if logger end - - # Add constraints to all queries to the same model in the given block. - # Currently supported constraints are <tt>:conditions</tt>, <tt>:joins</tt>, - # <tt>:offset</tt>, and <tt>:limit</tt> + + # Scope parameters to method calls within the block. Takes a hash of method_name => parameters hash. + # method_name may be :find or :create. + # :find parameters may include the <tt>:conditions</tt>, <tt>:joins</tt>, + # <tt>:offset</tt>, <tt>:limit</tt>, and <tt>:readonly</tt> options. + # :create parameters are an attributes hash. # - # Article.constrain(:conditions => "blog_id = 1") do + # Article.with_scope(:find => { :conditions => "blog_id = 1" }, :create => { :blog_id => 1 }) do # Article.find(1) # => SELECT * from articles WHERE blog_id = 1 AND id = 1 + # a = Article.create(1) + # a.blog_id == 1 # end - def constrain(options = {}) - options = options.dup - options[:readonly] = true if !options[:joins].blank? && !options.has_key?(:readonly) + def with_scope(method_scoping = {}) + # Dup first and second level of hash (method and params). + method_scoping = method_scoping.inject({}) do |hash, (method, params)| + hash[method] = params.dup + hash + end + + method_scoping.assert_valid_keys [:find, :create] + if f = method_scoping[:find] + f.assert_valid_keys [:conditions, :joins, :offset, :limit, :readonly] + f[:readonly] = true if !f[:joins].blank? && !f.has_key?(:readonly) + end - self.scope_constraints = options + raise ArgumentError, "Nested scopes are not yet supported: #{scoped_methods.inspect}" unless scoped_methods.nil? - yield if block_given? + self.scoped_methods = method_scoping + yield ensure - self.scope_constraints = nil + self.scoped_methods = nil end # Overwrite the default class equality method to provide support for association proxies. @@ -918,23 +932,23 @@ module ActiveRecord #:nodoc: end def add_limit!(sql, options) - options[:limit] ||= scope_constraints[:limit] if scope_constraints[:limit] - options[:offset] ||= scope_constraints[:offset] if scope_constraints[:offset] + options[:limit] ||= scope(:find, :limit) + options[:offset] ||= scope(:find, :offset) connection.add_limit_offset!(sql, options) end - + def add_joins!(sql, options) - join = scope_constraints[:joins] || options[:joins] + join = scope(:find, :joins) || options[:joins] sql << " #{join} " if join end - + # Adds a sanitized version of +conditions+ to the +sql+ string. Note that the passed-in +sql+ string is changed. def add_conditions!(sql, conditions) - condition_segments = [scope_constraints[:conditions]] - condition_segments << sanitize_sql(conditions) unless conditions.nil? - condition_segments << type_condition unless descends_from_active_record? - condition_segments.compact! - sql << "WHERE (#{condition_segments.join(") AND (")}) " unless condition_segments.empty? + segments = [scope(:find, :conditions)] + segments << sanitize_sql(conditions) unless conditions.nil? + segments << type_condition unless descends_from_active_record? + segments.compact! + sql << "WHERE (#{segments.join(") AND (")}) " unless segments.empty? end def type_condition @@ -1050,29 +1064,37 @@ module ActiveRecord #:nodoc: @@subclasses[self] ||= [] @@subclasses[self] + extra = @@subclasses[self].inject([]) {|list, subclass| list + subclass.subclasses } end - - def scope_constraints + + # Test whether the given method and optional key are scoped. + def scoped?(method, key = nil) + scoped_methods and scoped_methods.has_key?(method) and (key.nil? or scope(method).has_key?(key)) + end + + # Retrieve the scope for the given method and optional key. + def scope(method, key = nil) + if scoped_methods and scope = scoped_methods[method] + key ? scope[key] : scope + end + end + + def scoped_methods if allow_concurrency - Thread.current[:constraints] ||= {} - Thread.current[:constraints][self] ||= {} + Thread.current[:scoped_methods] ||= {} + Thread.current[:scoped_methods][self] ||= nil else - @scope_constraints ||= {} + @scoped_methods ||= nil end end - # backwards compatibility - alias_method :scope_constrains, :scope_constraints - def scope_constraints=(value) + def scoped_methods=(value) if allow_concurrency - Thread.current[:constraints] ||= {} - Thread.current[:constraints][self] = value + Thread.current[:scoped_methods] ||= {} + Thread.current[:scoped_methods][self] = value else - @scope_constraints = value + @scoped_methods = value end end - # backwards compatibility - alias_method :scope_constrains=, :scope_constraints= - + # Returns the class type of the record using the current module as a prefix. So descendents of # MyApp::Business::Account would appear as MyApp::Business::AccountSubclass. def compute_type(type_name) @@ -1729,4 +1751,4 @@ module ActiveRecord #:nodoc: value end end -end
\ No newline at end of file +end diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index 04307267b0..f64821361f 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -1030,8 +1030,8 @@ class BasicsTest < Test::Unit::TestCase assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar} baz') } end - def test_constrain_conditions - developers = Developer.constrain(:conditions => 'salary > 90000') do + def test_scoped_find_conditions + developers = Developer.with_scope(:find => { :conditions => 'salary > 90000' }) do Developer.find(:all, :conditions => 'id < 5') end david = Developer.find(1) @@ -1039,8 +1039,8 @@ class BasicsTest < Test::Unit::TestCase assert_equal 3, developers.size end - def test_constrain_limit_offset - developers = Developer.constrain(:limit => 3, :offset => 2) do + def test_scoped_find_limit_offset + developers = Developer.with_scope(:find => { :limit => 3, :offset => 2 }) do Developer.find(:all, :order => 'id') end david = Developer.find(1) @@ -1049,7 +1049,7 @@ class BasicsTest < Test::Unit::TestCase assert !developers.include?(jamis) # Jamis has id 2 assert_equal 3, developers.size - # Now test without constraints to make sure we get the whole thing + # Test without scoped find conditions to ensure we get the whole thing developers = Developer.find(:all, :order => 'id') assert_equal 10, developers.size end diff --git a/activerecord/test/conditions_scoping_test.rb b/activerecord/test/conditions_scoping_test.rb index cf8b670c26..92dfc1c1e6 100644 --- a/activerecord/test/conditions_scoping_test.rb +++ b/activerecord/test/conditions_scoping_test.rb @@ -8,35 +8,35 @@ class ConditionsScopingTest < Test::Unit::TestCase fixtures :developers, :comments, :posts def test_set_conditions - Developer.constrain(:conditions => 'just a test...') do - assert_equal 'just a test...', Thread.current[:constraints][Developer][:conditions] + Developer.with_scope(:find => { :conditions => 'just a test...' }) do + assert_equal 'just a test...', Thread.current[:scoped_methods][Developer][:find][:conditions] end end def test_scoped_find - Developer.constrain(:conditions => "name = 'David'") do + Developer.with_scope(:find => { :conditions => "name = 'David'" }) do assert_nothing_raised { Developer.find(1) } end end def test_scoped_find_first - Developer.constrain(:conditions => "salary = 100000") do + Developer.with_scope(:find => { :conditions => "salary = 100000" }) do assert_equal Developer.find(10), Developer.find(:first, :order => 'name') end end def test_scoped_find_all - Developer.constrain(:conditions => "name = 'David'") do + Developer.with_scope(:find => { :conditions => "name = 'David'" }) do assert_equal [Developer.find(1)], Developer.find(:all) end end def test_scoped_count - Developer.constrain(:conditions => "name = 'David'") do + Developer.with_scope(:find => { :conditions => "name = 'David'" }) do assert_equal 1, Developer.count end - Developer.constrain(:conditions => 'salary = 100000') do + Developer.with_scope(:find => { :conditions => 'salary = 100000' }) do assert_equal 8, Developer.count assert_equal 1, Developer.count("name LIKE 'fixture_1%'") end @@ -45,21 +45,36 @@ class ConditionsScopingTest < Test::Unit::TestCase def test_scoped_create new_comment = nil - VerySpecialComment.constrain(:creation => { :post_id => 1 }) do - assert_equal({ :post_id => 1 }, Thread.current[:constraints][VerySpecialComment][:creation]) + VerySpecialComment.with_scope(:create => { :post_id => 1 }) do + assert_equal({ :post_id => 1 }, Thread.current[:scoped_methods][VerySpecialComment][:create]) new_comment = VerySpecialComment.create :body => "Wonderful world" end - + assert Post.find(1).comments.include?(new_comment) end - def test_immutable_constraint + def test_immutable_scope options = { :conditions => "name = 'David'" } - Developer.constrain(options) do + Developer.with_scope(:find => options) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } options[:conditions] = "name != 'David'" assert_equal %w(David), Developer.find(:all).map { |d| d.name } end + + scope = { :find => { :conditions => "name = 'David'" }} + Developer.with_scope(scope) do + assert_equal %w(David), Developer.find(:all).map { |d| d.name } + scope[:find][:conditions] = "name != 'David'" + assert_equal %w(David), Developer.find(:all).map { |d| d.name } + end + end + + def test_raise_on_nested_scope + Developer.with_scope(:find => { :conditions => '1=1' }) do + assert_raise(ArgumentError) do + Developer.with_scope(:find => { :conditions => '2=2' }) { } + end + end end end @@ -84,7 +99,12 @@ class HasManyScopingTest< Test::Unit::TestCase assert_equal 4, Comment.find_all_by_type('Comment').size assert_equal 2, @welcome.comments.find_all_by_type('Comment').size end - + + def test_raise_on_nested_scope + Comment.with_scope(:find => { :conditions => '1=1' }) do + assert_raise(ArgumentError) { @welcome.comments.what_are_you } + end + end end @@ -106,6 +126,11 @@ class HasAndBelongsToManyScopingTest< Test::Unit::TestCase assert_equal 2, @welcome.categories.find_all_by_type('Category').size end + def test_raise_on_nested_scope + Category.with_scope(:find => { :conditions => '1=1' }) do + assert_raise(ArgumentError) { @welcome.categories.what_are_you } + end + end end diff --git a/activerecord/test/readonly_test.rb b/activerecord/test/readonly_test.rb index f8acc16493..bd9b8ef1b7 100755 --- a/activerecord/test/readonly_test.rb +++ b/activerecord/test/readonly_test.rb @@ -4,7 +4,7 @@ require 'fixtures/comment' require 'fixtures/developer' require 'fixtures/project' -# Dummy class methods to test implicit association constraints. +# Dummy class methods to test implicit association scoping. def Comment.foo() find :first end def Project.foo() find :first end @@ -64,14 +64,14 @@ class ReadOnlyTest < Test::Unit::TestCase end - def test_readonly_constraint - Post.constrain(:conditions => '1=1') do + def test_readonly_scoping + Post.with_scope(:find => { :conditions => '1=1' }) do assert !Post.find(1).readonly? assert Post.find(1, :readonly => true).readonly? assert !Post.find(1, :readonly => false).readonly? end - Post.constrain(:joins => ' ') do + Post.with_scope(:find => { :joins => ' ' }) do assert !Post.find(1).readonly? assert Post.find(1, :readonly => true).readonly? assert !Post.find(1, :readonly => false).readonly? @@ -80,21 +80,21 @@ class ReadOnlyTest < Test::Unit::TestCase # Oracle barfs on this because the join includes unqualified and # conflicting column names unless current_adapter?(:OCIAdapter) - Post.constrain(:joins => ', developers') do + Post.with_scope(:find => { :joins => ', developers' }) do assert Post.find(1).readonly? assert Post.find(1, :readonly => true).readonly? assert !Post.find(1, :readonly => false).readonly? end end - Post.constrain(:readonly => true) do + Post.with_scope(:find => { :readonly => true }) do assert Post.find(1).readonly? assert Post.find(1, :readonly => true).readonly? assert !Post.find(1, :readonly => false).readonly? end end - def test_association_collection_method_missing_constraint_not_readonly + def test_association_collection_method_missing_scoping_not_readonly assert !Developer.find(1).projects.foo.readonly? assert !Post.find(1).comments.foo.readonly? end |