From 8423bb6a684fd68461faab87149a5bdfea61f43a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 17 Apr 2009 19:35:03 -0500 Subject: Improve rewindable input test coverage so tests fail when you remove the middleware Signed-off-by: Joshua Peek --- actionpack/test/dispatch/request/multipart_params_parsing_test.rb | 3 +-- actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 2f409f020d..33c909aad9 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -206,8 +206,7 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest end def call(env) - req = Rack::Request.new(env) - req.params # Parse params + env['rack.input'].read @app.call(env) end end diff --git a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb index 51a660f614..44d13797f0 100644 --- a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb @@ -150,8 +150,7 @@ class UrlEncodedParamsParsingTest < ActionController::IntegrationTest end def call(env) - req = Rack::Request.new(env) - req.params # Parse params + env['rack.input'].read @app.call(env) end end -- cgit v1.2.3 From 7ce0778a1516110cf8015e59e2e8fac15032379c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 17 Apr 2009 21:53:44 -0500 Subject: Always buffer rack.input if it is not rewindable Signed-off-by: Joshua Peek --- .../action_dispatch/middleware/rewindable_input.rb | 25 +++++++--------------- actionpack/test/controller/dispatcher_test.rb | 2 +- .../request/multipart_params_parsing_test.rb | 1 + .../request/url_encoded_params_parsing_test.rb | 1 + 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/rewindable_input.rb b/actionpack/lib/action_dispatch/middleware/rewindable_input.rb index 725414efc4..c818f28cce 100644 --- a/actionpack/lib/action_dispatch/middleware/rewindable_input.rb +++ b/actionpack/lib/action_dispatch/middleware/rewindable_input.rb @@ -1,27 +1,18 @@ module ActionDispatch class RewindableInput - class RewindableIO < ActiveSupport::BasicObject - def initialize(io) - @io = io - @rewindable = io.is_a?(::StringIO) - end - - def method_missing(method, *args, &block) - unless @rewindable - @io = ::StringIO.new(@io.read) - @rewindable = true - end - - @io.__send__(method, *args, &block) - end - end - def initialize(app) @app = app end def call(env) - env['rack.input'] = RewindableIO.new(env['rack.input']) + begin + env['rack.input'].rewind + rescue NoMethodError, Errno::ESPIPE + # Handles exceptions raised by input streams that cannot be rewound + # such as when using plain CGI under Apache + env['rack.input'] = StringIO.new(env['rack.input'].read) + end + @app.call(env) end end diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index 721bcf6136..569d698a03 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -94,7 +94,7 @@ class DispatcherTest < Test::Unit::TestCase def dispatch(cache_classes = true) ActionController::Routing::RouteSet.any_instance.stubs(:call).returns([200, {}, 'response']) Dispatcher.define_dispatcher_callbacks(cache_classes) - Dispatcher.new.call({}) + Dispatcher.new.call({'rack.input' => StringIO.new('')}) end def assert_subclasses(howmany, klass, message = klass.subclasses.inspect) diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 33c909aad9..88b81dc493 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -207,6 +207,7 @@ class MultipartParamsParsingTest < ActionController::IntegrationTest def call(env) env['rack.input'].read + env['rack.input'].rewind @app.call(env) end end diff --git a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb index 44d13797f0..8de4a83d76 100644 --- a/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/url_encoded_params_parsing_test.rb @@ -151,6 +151,7 @@ class UrlEncodedParamsParsingTest < ActionController::IntegrationTest def call(env) env['rack.input'].read + env['rack.input'].rewind @app.call(env) end end -- cgit v1.2.3 From 489abfd3b23f3c4b3de86aeb3bde3970771c001b Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Mon, 20 Apr 2009 13:51:11 +0100 Subject: Ensure JoinAssociation uses aliased table name when multiple associations have hash conditions on the same table --- activerecord/lib/active_record/associations.rb | 2 +- activerecord/lib/active_record/associations/association_proxy.rb | 4 ++-- activerecord/lib/active_record/base.rb | 4 ++-- activerecord/test/cases/associations/inner_join_association_test.rb | 5 +++++ activerecord/test/models/author.rb | 2 ++ 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 53a710537f..fa18822c24 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -2123,7 +2123,7 @@ module ActiveRecord klass.send(:type_condition, aliased_table_name)] unless klass.descends_from_active_record? [through_reflection, reflection].each do |ref| - join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions]))} " if ref && ref.options[:conditions] + join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions], aliased_table_name))} " if ref && ref.options[:conditions] end join diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index 676c4ace61..241b9bfee0 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -169,8 +169,8 @@ module ActiveRecord end # Forwards the call to the reflection class. - def sanitize_sql(sql) - @reflection.klass.send(:sanitize_sql, sql) + def sanitize_sql(sql, table_name = @reflection.klass.quoted_table_name) + @reflection.klass.send(:sanitize_sql, sql, table_name) end # Assigns the ID of the owner to the corresponding foreign key in +record+. diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9943a7014a..251d5000f5 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2231,12 +2231,12 @@ module ActiveRecord #:nodoc: # ["name='%s' and group_id='%s'", "foo'bar", 4] returns "name='foo''bar' and group_id='4'" # { :name => "foo'bar", :group_id => 4 } returns "name='foo''bar' and group_id='4'" # "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'" - def sanitize_sql_for_conditions(condition) + def sanitize_sql_for_conditions(condition, table_name = quoted_table_name) return nil if condition.blank? case condition when Array; sanitize_sql_array(condition) - when Hash; sanitize_sql_hash_for_conditions(condition) + when Hash; sanitize_sql_hash_for_conditions(condition, table_name) else condition end end diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index f87c914125..7141531740 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -29,6 +29,11 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase assert_match /INNER JOIN .?categories.? ON.*AND.*.?General.?.*TERMINATING_MARKER/, sql end + def test_construct_finder_sql_applies_aliases_tables_on_association_conditions + result = Author.find(:all, :joins => [:thinking_posts, :welcome_posts]) + assert_equal authors(:david), result.first + end + def test_construct_finder_sql_unpacks_nested_joins sql = Author.send(:construct_finder_sql, :joins => {:posts => [[:comments]]}) assert_no_match /inner join.*inner join.*inner join/i, sql, "only two join clauses should be present" diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 4ffac4fe32..669c664bf4 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -25,6 +25,8 @@ class Author < ActiveRecord::Base has_many :comments_with_order_and_conditions, :through => :posts, :source => :comments, :order => 'comments.body', :conditions => "comments.body like 'Thank%'" has_many :comments_with_include, :through => :posts, :source => :comments, :include => :post + has_many :thinking_posts, :class_name => 'Post', :conditions => { :title => 'So I was thinking' } + has_many :welcome_posts, :class_name => 'Post', :conditions => { :title => 'Welcome to the weblog' } has_many :comments_desc, :through => :posts, :source => :comments, :order => 'comments.id DESC' has_many :limited_comments, :through => :posts, :source => :comments, :limit => 1 -- cgit v1.2.3 From de0ea3866370ec61581f910cf393a3cc97eba32f Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Mon, 20 Apr 2009 18:12:40 +0100 Subject: Ensure :dependent => :delete_all works for association with hash conditions --- activerecord/lib/active_record/associations.rb | 2 +- .../test/cases/associations/has_many_associations_test.rb | 6 ++++++ activerecord/test/models/author.rb | 2 +- activerecord/test/models/company.rb | 13 ++++++------- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index fa18822c24..2115878e32 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1375,7 +1375,7 @@ module ActiveRecord dependent_conditions = [] dependent_conditions << "#{reflection.primary_key_name} = \#{record.quoted_id}" dependent_conditions << "#{reflection.options[:as]}_type = '#{base_class.name}'" if reflection.options[:as] - dependent_conditions << sanitize_sql(reflection.options[:conditions]) if reflection.options[:conditions] + dependent_conditions << sanitize_sql(reflection.options[:conditions], reflection.quoted_table_name) if reflection.options[:conditions] dependent_conditions << extra_conditions if extra_conditions dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ") dependent_conditions = dependent_conditions.gsub('@', '\@') diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 30edf79a26..5df74fcdcd 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -719,6 +719,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert Client.find(:all, :conditions => "firm_id=#{firm.id}").empty? end + def test_dependence_for_associations_with_hash_condition + david = authors(:david) + post = posts(:thinking).id + assert_difference('Post.count', -1) { assert david.destroy } + end + def test_destroy_dependent_when_deleted_from_association firm = Firm.find(:first) assert_equal 2, firm.clients.size diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 669c664bf4..0d9ee36b20 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -25,7 +25,7 @@ class Author < ActiveRecord::Base has_many :comments_with_order_and_conditions, :through => :posts, :source => :comments, :order => 'comments.body', :conditions => "comments.body like 'Thank%'" has_many :comments_with_include, :through => :posts, :source => :comments, :include => :post - has_many :thinking_posts, :class_name => 'Post', :conditions => { :title => 'So I was thinking' } + has_many :thinking_posts, :class_name => 'Post', :conditions => { :title => 'So I was thinking' }, :dependent => :delete_all has_many :welcome_posts, :class_name => 'Post', :conditions => { :title => 'Welcome to the weblog' } has_many :comments_desc, :through => :posts, :source => :comments, :order => 'comments.id DESC' diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 02a775f9ef..eb68153bbe 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -78,13 +78,6 @@ class DependentFirm < Company has_many :companies, :foreign_key => 'client_of', :order => "id", :dependent => :nullify end -class ExclusivelyDependentFirm < Company - has_one :account, :foreign_key => "firm_id", :dependent => :delete - has_many :dependent_sanitized_conditional_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all, :conditions => "name = 'BigShot Inc.'" - has_many :dependent_conditional_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all, :conditions => ["name = ?", 'BigShot Inc.'] - has_many :dependent_hash_conditional_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all, :conditions => {:name => 'BigShot Inc.'} -end - class Client < Company belongs_to :firm, :foreign_key => "client_of" belongs_to :firm_with_basic_id, :class_name => "Firm", :foreign_key => "firm_id" @@ -125,6 +118,12 @@ class Client < Company end end +class ExclusivelyDependentFirm < Company + has_one :account, :foreign_key => "firm_id", :dependent => :delete + has_many :dependent_sanitized_conditional_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all, :conditions => "name = 'BigShot Inc.'" + has_many :dependent_conditional_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all, :conditions => ["name = ?", 'BigShot Inc.'] + has_many :dependent_hash_conditional_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all, :conditions => {:name => 'BigShot Inc.'} +end class SpecialClient < Client end -- cgit v1.2.3