diff options
-rw-r--r-- | activerecord/CHANGELOG | 3 | ||||
-rw-r--r-- | activerecord/lib/active_record/associations/association_collection.rb | 2 | ||||
-rwxr-xr-x | activerecord/lib/active_record/base.rb | 25 | ||||
-rw-r--r-- | activerecord/test/conditions_scoping_test.rb | 9 | ||||
-rwxr-xr-x | activerecord/test/readonly_test.rb | 66 |
5 files changed, 90 insertions, 15 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index b0686e9e53..a4508c9086 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,8 @@ *SVN* +* Constraints are cloned so they can't be inadvertently modified while they're +in effect. Added :readonly finder constraint. Calling an association collection's class method (Part.foobar via item.parts.foobar) constrains :readonly => false since the collection's :joins constraint would otherwise force it to true. [Jeremy Kemper <rails@bitsweat.net>] + * Added :offset and :limit to the kinds of options that Base.constrain can use #2466 [duane.johnson@gmail.com] * Fixed handling of nil number columns on Oracle and cleaned up tests for Oracle in general #2555 [schoenm@earthlink.net] diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 164d11b2d1..cbfbdf1541 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -128,7 +128,7 @@ module ActiveRecord if @target.respond_to?(method) or (not @association_class.respond_to?(method) and Class.respond_to?(method)) super else - @association_class.constrain(:conditions => @finder_sql, :joins => @join_sql) { @association_class.send(method, *args, &block) } + @association_class.constrain(:conditions => @finder_sql, :joins => @join_sql, :readonly => false) { @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 febb35dd5b..fa7ed9482a 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -384,9 +384,14 @@ module ActiveRecord #:nodoc: def find(*args) options = extract_options_from_args!(args) - # :joins implies :readonly => true if unset. - if options[:joins] and !options.has_key?(:readonly) - options[:readonly] = true + # Inherit :readonly from scope_constraints 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] + elsif !options[:joins].blank? + options[:readonly] = true + end end case args.first @@ -815,13 +820,15 @@ module ActiveRecord #:nodoc: # Article.constrain(:conditions => "blog_id = 1") do # Article.find(1) # => SELECT * from articles WHERE blog_id = 1 AND id = 1 # end - def constrain(options = {}, &block) - begin - self.scope_constraints = options - block.call if block_given? - ensure - self.scope_constraints = nil + def constrain(options = {}) + options = options.dup + if !options[:joins].blank? and !options.has_key?(:readonly) + options[:readonly] = true end + self.scope_constraints = options + yield if block_given? + ensure + self.scope_constraints = nil end # Overwrite the default class equality method to provide support for association proxies. diff --git a/activerecord/test/conditions_scoping_test.rb b/activerecord/test/conditions_scoping_test.rb index b945593b57..a997ee9e85 100644 --- a/activerecord/test/conditions_scoping_test.rb +++ b/activerecord/test/conditions_scoping_test.rb @@ -41,6 +41,15 @@ class ConditionsScopingTest < Test::Unit::TestCase assert_equal 1, Developer.count("name LIKE 'fixture_1%'") end end + + def test_immutable_constraint + options = { :conditions => "name = 'David'" } + Developer.constrain(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 + end end class HasManyScopingTest< Test::Unit::TestCase diff --git a/activerecord/test/readonly_test.rb b/activerecord/test/readonly_test.rb index 458cd6886f..d621432596 100755 --- a/activerecord/test/readonly_test.rb +++ b/activerecord/test/readonly_test.rb @@ -1,12 +1,19 @@ require 'abstract_unit' +require 'fixtures/post' +require 'fixtures/comment' require 'fixtures/developer' require 'fixtures/project' +# Dummy class methods to test implicit association constraints. +def Comment.foo() find :first end +def Project.foo() find :first end + + class ReadOnlyTest < Test::Unit::TestCase - fixtures :developers, :projects, :developers_projects + fixtures :posts, :comments, :developers, :projects, :developers_projects def test_cant_save_readonly_record - dev = Developer.find(:first) + dev = Developer.find(1) assert !dev.readonly? dev.readonly! @@ -21,21 +28,70 @@ class ReadOnlyTest < Test::Unit::TestCase assert_raise(ActiveRecord::ReadOnlyRecord) { dev.save! } end + def test_find_with_readonly_option Developer.find(:all).each { |d| assert !d.readonly? } Developer.find(:all, :readonly => false).each { |d| assert !d.readonly? } Developer.find(:all, :readonly => true).each { |d| assert d.readonly? } end + def test_find_with_joins_option_implies_readonly - Developer.find(:all, :joins => '').each { |d| assert d.readonly? } - Developer.find(:all, :joins => '', :readonly => false).each { |d| assert !d.readonly? } + # Blank joins don't count. + Developer.find(:all, :joins => ' ').each { |d| assert !d.readonly? } + Developer.find(:all, :joins => ' ', :readonly => false).each { |d| assert !d.readonly? } + + # Others do. + Developer.find(:all, :joins => ', projects').each { |d| assert d.readonly? } + Developer.find(:all, :joins => ', projects', :readonly => false).each { |d| assert !d.readonly? } end + def test_habtm_find_readonly - dev = Developer.find(:first) + dev = Developer.find(1) + assert !dev.projects.empty? dev.projects.each { |p| assert !p.readonly? } dev.projects.find(:all) { |p| assert !p.readonly? } dev.projects.find(:all, :readonly => true) { |p| assert p.readonly? } end + + def test_has_many_find_readonly + post = Post.find(1) + assert !post.comments.empty? + post.comments.each { |r| assert !r.readonly? } + post.comments.find(:all) { |r| assert !r.readonly? } + post.comments.find(:all, :readonly => true) { |r| assert r.readonly? } + end + + + def test_readonly_constraint + Post.constrain(: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 + assert !Post.find(1).readonly? + assert Post.find(1, :readonly => true).readonly? + assert !Post.find(1, :readonly => false).readonly? + end + + Post.constrain(:joins => ', developers') do + assert Post.find(1).readonly? + assert Post.find(1, :readonly => true).readonly? + assert !Post.find(1, :readonly => false).readonly? + end + + Post.constrain(: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 + assert !Developer.find(1).projects.foo.readonly? + assert !Post.find(1).comments.foo.readonly? + end end |