diff options
11 files changed, 101 insertions, 27 deletions
diff --git a/actionpack/lib/action_controller/assertions/selector_assertions.rb b/actionpack/lib/action_controller/assertions/selector_assertions.rb index 248ca85994..7f8fe9ab19 100644 --- a/actionpack/lib/action_controller/assertions/selector_assertions.rb +++ b/actionpack/lib/action_controller/assertions/selector_assertions.rb @@ -402,6 +402,7 @@ module ActionController if rjs_type if rjs_type == :insert position = args.shift + id = args.shift insertion = "insert_#{position}".to_sym raise ArgumentError, "Unknown RJS insertion type #{position}" unless RJS_STATEMENTS[insertion] statement = "(#{RJS_STATEMENTS[insertion]})" diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index ed8c4427c9..99c57c0c91 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -248,6 +248,14 @@ class AssertSelectTest < ActionController::TestCase end end + def test_assert_select_rjs_for_positioned_insert_should_fail_when_mixing_arguments + render_rjs do |page| + page.insert_html :top, "test1", "<div id=\"1\">foo</div>" + page.insert_html :bottom, "test2", "<div id=\"2\">foo</div>" + end + assert_raises(Assertion) {assert_select_rjs :insert, :top, "test2"} + end + # # Test css_select. # diff --git a/actionpack/test/controller/session/cookie_store_test.rb b/actionpack/test/controller/session/cookie_store_test.rb index 69aec59dc0..d349c18d1d 100644 --- a/actionpack/test/controller/session/cookie_store_test.rb +++ b/actionpack/test/controller/session/cookie_store_test.rb @@ -25,7 +25,7 @@ class CookieStoreTest < ActionController::IntegrationTest def set_session_value session[:foo] = "bar" - render :text => Marshal.dump(session.to_hash) + render :text => Verifier.generate(session.to_hash) end def get_session_value @@ -94,8 +94,7 @@ class CookieStoreTest < ActionController::IntegrationTest with_test_route_set do get '/set_session_value' assert_response :success - session_payload = Verifier.generate(Marshal.load(response.body)) - assert_equal ["_myapp_session=#{session_payload}; path=/"], + assert_equal ["_myapp_session=#{response.body}; path=/"], headers['Set-Cookie'] end end @@ -148,8 +147,8 @@ class CookieStoreTest < ActionController::IntegrationTest with_test_route_set do get '/set_session_value' assert_response :success - session_payload = Verifier.generate(Marshal.load(response.body)) - assert_equal ["_myapp_session=#{session_payload}; path=/"], + session_payload = response.body + assert_equal ["_myapp_session=#{response.body}; path=/"], headers['Set-Cookie'] get '/call_reset_session' diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 7b1b2d9ad9..9d0bf3a308 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -43,7 +43,7 @@ module ActiveRecord # loading in a more high-level (application developer-friendly) manner. module ClassMethods protected - + # Eager loads the named associations for the given ActiveRecord record(s). # # In this description, 'association name' shall refer to the name passed @@ -94,8 +94,8 @@ module ActiveRecord raise "parent must be an association name" unless parent.is_a?(String) || parent.is_a?(Symbol) preload_associations(records, parent, preload_options) reflection = reflections[parent] - parents = records.map {|record| record.send(reflection.name)}.flatten - unless parents.empty? || parents.first.nil? + parents = records.map {|record| record.send(reflection.name)}.flatten.compact + unless parents.empty? parents.first.class.preload_associations(parents, child) end end @@ -113,7 +113,7 @@ module ActiveRecord # unnecessarily records.group_by {|record| class_to_reflection[record.class] ||= record.class.reflections[association]}.each do |reflection, records| raise ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" unless reflection - + # 'reflection.macro' can return 'belongs_to', 'has_many', etc. Thus, # the following could call 'preload_belongs_to_association', # 'preload_has_many_association', etc. @@ -128,7 +128,7 @@ module ActiveRecord association_proxy.target.push(*[associated_record].flatten) end end - + def add_preloaded_record_to_collection(parent_records, reflection_name, associated_record) parent_records.each do |parent_record| parent_record.send("set_#{reflection_name}_target", associated_record) @@ -183,18 +183,19 @@ module ActiveRecord conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}" conditions << append_conditions(reflection, preload_options) - associated_records = reflection.klass.find(:all, :conditions => [conditions, ids], - :include => options[:include], - :joins => "INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}", - :select => "#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id", - :order => options[:order]) - + associated_records = reflection.klass.with_exclusive_scope do + reflection.klass.find(:all, :conditions => [conditions, ids], + :include => options[:include], + :joins => "INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}", + :select => "#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id", + :order => options[:order]) + end set_association_collection_records(id_to_record_map, reflection.name, associated_records, 'the_parent_record_id') end def preload_has_one_association(records, reflection, preload_options={}) return if records.first.send("loaded_#{reflection.name}?") - id_to_record_map, ids = construct_id_map(records) + id_to_record_map, ids = construct_id_map(records) options = reflection.options records.each {|record| record.send("set_#{reflection.name}_target", nil)} if options[:through] @@ -248,7 +249,7 @@ module ActiveRecord reflection.primary_key_name) end end - + def preload_through_records(records, reflection, through_association) through_reflection = reflections[through_association] through_primary_key = through_reflection.primary_key_name @@ -333,11 +334,13 @@ module ActiveRecord end conditions = "#{table_name}.#{connection.quote_column_name(primary_key)} #{in_or_equals_for_ids(ids)}" conditions << append_conditions(reflection, preload_options) - associated_records = klass.find(:all, :conditions => [conditions, ids], + associated_records = klass.with_exclusive_scope do + klass.find(:all, :conditions => [conditions, ids], :include => options[:include], :select => options[:select], :joins => options[:joins], :order => options[:order]) + end set_association_single_records(id_map, reflection.name, associated_records, primary_key) end end @@ -355,13 +358,15 @@ module ActiveRecord conditions << append_conditions(reflection, preload_options) - reflection.klass.find(:all, + reflection.klass.with_exclusive_scope do + reflection.klass.find(:all, :select => (preload_options[:select] || options[:select] || "#{table_name}.*"), :include => preload_options[:include] || options[:include], :conditions => [conditions, ids], :joins => options[:joins], :group => preload_options[:group] || options[:group], :order => preload_options[:order] || options[:order]) + end end diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 8c9ae8a031..45e74ea024 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -104,6 +104,14 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase authors.first.posts.first.special_comments.first.post.very_special_comment end end + + def test_eager_association_loading_where_first_level_returns_nil + authors = Author.find(:all, :include => {:post_about_thinking => :comments}, :order => 'authors.id DESC') + assert_equal [authors(:mary), authors(:david)], authors + assert_no_queries do + authors[1].post_about_thinking.comments.first + end + end end require 'models/vertex' diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index a2d0efab92..afbd9fddf9 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -771,4 +771,19 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal author_addresses(:david_address), authors[0].author_address end + def test_preload_belongs_to_uses_exclusive_scope + people = Person.males.find(:all, :include => :primary_contact) + assert_not_equal people.length, 0 + people.each do |person| + assert_no_queries {assert_not_nil person.primary_contact} + assert_equal Person.find(person.id).primary_contact, person.primary_contact + end + end + + def test_preload_has_many_uses_exclusive_scope + people = Person.males.find :all, :include => :agents + people.each do |person| + assert_equal Person.find(person.id).agents, person.agents + end + end end diff --git a/activerecord/test/fixtures/people.yml b/activerecord/test/fixtures/people.yml index d5a69e561d..3babb1fe59 100644 --- a/activerecord/test/fixtures/people.yml +++ b/activerecord/test/fixtures/people.yml @@ -1,6 +1,15 @@ michael: id: 1 first_name: Michael + primary_contact_id: 2 + gender: M david: id: 2 - first_name: David
\ No newline at end of file + first_name: David + primary_contact_id: 3 + gender: M +susan: + id: 3 + first_name: Susan + primary_contact_id: 2 + gender: F
\ No newline at end of file diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 430d0b38f7..ec2f684a6e 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -7,4 +7,10 @@ class Person < ActiveRecord::Base has_many :jobs, :through => :references has_one :favourite_reference, :class_name => 'Reference', :conditions => ['favourite=?', true] has_many :posts_with_comments_sorted_by_comment_id, :through => :readers, :source => :post, :include => :comments, :order => 'comments.id' + + belongs_to :primary_contact, :class_name => 'Person' + has_many :agents, :class_name => 'Person', :foreign_key => 'primary_contact_id' + + named_scope :males, :conditions => { :gender => 'M' } + named_scope :females, :conditions => { :gender => 'F' } end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index fbacc692b4..8199cb8fc7 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -298,8 +298,10 @@ ActiveRecord::Schema.define do end create_table :people, :force => true do |t| - t.string :first_name, :null => false - t.integer :lock_version, :null => false, :default => 0 + t.string :first_name, :null => false + t.references :primary_contact + t.string :gender, :limit => 1 + t.integer :lock_version, :null => false, :default => 0 end create_table :pets, :primary_key => :pet_id ,:force => true do |t| diff --git a/activesupport/lib/active_support/core_ext/hash/slice.rb b/activesupport/lib/active_support/core_ext/hash/slice.rb index 88df49a69f..d845a6d8ca 100644 --- a/activesupport/lib/active_support/core_ext/hash/slice.rb +++ b/activesupport/lib/active_support/core_ext/hash/slice.rb @@ -24,10 +24,17 @@ module ActiveSupport #:nodoc: end # Replaces the hash with only the given keys. + # Returns a hash contained the removed key/value pairs + # {:a => 1, :b => 2, :c => 3, :d => 4}.slice!(:a, :b) # => {:c => 3, :d =>4} def slice!(*keys) - replace(slice(*keys)) + keys = keys.map! { |key| convert_key(key) } if respond_to?(:convert_key) + omit = slice(*self.keys - keys) + hash = slice(*keys) + replace(hash) + omit end end end end end + diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index 63ccb5a7da..b63ab30965 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -287,10 +287,14 @@ class HashExtTest < Test::Unit::TestCase # Should return a new hash with only the given keys. assert_equal expected, original.slice(:a, :b) assert_not_equal expected, original + end + + def test_slice_inplace + original = { :a => 'x', :b => 'y', :c => 10 } + expected = { :c => 10 } # Should replace the hash with only the given keys. assert_equal expected, original.slice!(:a, :b) - assert_equal expected, original end def test_slice_with_an_array_key @@ -300,10 +304,14 @@ class HashExtTest < Test::Unit::TestCase # Should return a new hash with only the given keys when given an array key. assert_equal expected, original.slice([:a, :b], :c) assert_not_equal expected, original + end + + def test_slice_inplace_with_an_array_key + original = { :a => 'x', :b => 'y', :c => 10, [:a, :b] => "an array key" } + expected = { :a => 'x', :b => 'y' } # Should replace the hash with only the given keys when given an array key. assert_equal expected, original.slice!([:a, :b], :c) - assert_equal expected, original end def test_slice_with_splatted_keys @@ -322,11 +330,17 @@ class HashExtTest < Test::Unit::TestCase # Should return a new hash with only the given keys. assert_equal expected, original.slice(*keys), keys.inspect assert_not_equal expected, original + end + end + def test_indifferent_slice_inplace + original = { :a => 'x', :b => 'y', :c => 10 }.with_indifferent_access + expected = { :c => 10 }.with_indifferent_access + + [['a', 'b'], [:a, :b]].each do |keys| # Should replace the hash with only the given keys. copy = original.dup assert_equal expected, copy.slice!(*keys) - assert_equal expected, copy end end |