From d7b6e48c70076a7760e22c381e48834ecddfa6e3 Mon Sep 17 00:00:00 2001
From: Frederick Cheung <frederick.cheung@gmail.com>
Date: Thu, 25 Dec 2008 12:10:28 +0000
Subject: Fix randomly failing cookie store tests
Marshal.dump(Marshal.load(marshaled_hash)) is not guarenteed to be equal to marshaled_hash
because of the lack of ordering of hash
---
actionpack/test/controller/session/cookie_store_test.rb | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
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'
--
cgit v1.2.3
From dce0da77e7ef602f7420f43c0d1aba5a99a00bdb Mon Sep 17 00:00:00 2001
From: Frederick Cheung <frederick.cheung@gmail.com>
Date: Thu, 25 Dec 2008 11:11:00 +0000
Subject: Fix assert_select_rjs not checking id for inserts [#540
state:resolved]
---
.../lib/action_controller/assertions/selector_assertions.rb | 1 +
actionpack/test/controller/assert_select_test.rb | 8 ++++++++
2 files changed, 9 insertions(+)
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.
#
--
cgit v1.2.3
From c9d4335418823500548ad8fbc86af7c910b7644b Mon Sep 17 00:00:00 2001
From: trans <transfire@gmail.com>
Date: Thu, 25 Dec 2008 00:24:05 +0000
Subject: MaKe Hash#slice! return removed values, akin to Array [#971
state:resolved]
Signed-off-by: Frederick Cheung <frederick.cheung@gmail.com>
---
.../lib/active_support/core_ext/hash/slice.rb | 9 ++++++++-
activesupport/test/core_ext/hash_ext_test.rb | 20 +++++++++++++++++---
2 files changed, 25 insertions(+), 4 deletions(-)
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
--
cgit v1.2.3
From eb457ceee13779ade67e1bdebd2919d476148277 Mon Sep 17 00:00:00 2001
From: Pivotal Labs <opensource@pivotallabs.com>
Date: Wed, 24 Dec 2008 14:57:09 -0800
Subject: Association preloading no longer stops if it hits a nil object [#1630
state:resolved]
Signed-off-by: Frederick Cheung <frederick.cheung@gmail.com>
---
activerecord/lib/active_record/association_preload.rb | 4 ++--
.../test/cases/associations/cascaded_eager_loading_test.rb | 8 ++++++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index 7b1b2d9ad9..0bcf50c167 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -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
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'
--
cgit v1.2.3
From 5cebe69e74d411c3c9e5f6ab9d4b2b16ee36177c Mon Sep 17 00:00:00 2001
From: Frederick Cheung <frederick.cheung@gmail.com>
Date: Fri, 19 Dec 2008 01:02:21 +0000
Subject: Preload uses exclusive scope [#643 state:resolved]
With self referential associations, the scope for the the top level should not affect fetching of associations, for example
when doing
Person.male.find :all, :include => :friends
we should load all of the friends for each male, not just the male friends.
---
.../lib/active_record/association_preload.rb | 31 +++++++++++++---------
activerecord/test/cases/associations/eager_test.rb | 15 +++++++++++
activerecord/test/fixtures/people.yml | 11 +++++++-
activerecord/test/models/person.rb | 6 +++++
activerecord/test/schema/schema.rb | 6 +++--
5 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index 0bcf50c167..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
@@ -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/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|
--
cgit v1.2.3