From d5994ee48af14d67f0eec7d23863d4b19211b078 Mon Sep 17 00:00:00 2001 From: John Mileham Date: Thu, 3 Mar 2011 23:26:45 -0500 Subject: [PATCH 01/54] Change behavior of count(:limit => x, :offset => y) to limit/offset before counting. --- .../active_record/relation/calculations.rb | 40 +++++++++++------ activerecord/test/cases/calculations_test.rb | 45 ++++++++++++------- activerecord/test/cases/relations_test.rb | 28 ++++++++++++ 3 files changed, 84 insertions(+), 29 deletions(-) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index c1842b1a96..7bfeb20a2b 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -183,10 +183,13 @@ module ActiveRecord end end - def aggregate_column(column_name) + def aggregate_column(column_name, subquery_alias = nil) if @klass.column_names.include?(column_name.to_s) - Arel::Attribute.new(@klass.unscoped.table, column_name) + Arel::Attribute.new(subquery_alias || @klass.unscoped.table, column_name) else + if subquery_alias && (split_name = column_name.to_s.split(".")).length > 1 + column_name = split_name.last + end Arel.sql(column_name == :all ? "*" : column_name.to_s) end end @@ -196,24 +199,22 @@ module ActiveRecord end def execute_simple_calculation(operation, column_name, distinct) #:nodoc: - column = aggregate_column(column_name) - # Postgresql doesn't like ORDER BY when there are no GROUP BY relation = except(:order) - select_value = operation_over_aggregate_column(column, operation, distinct) - relation.select_values = [select_value] + if operation == "count" && (relation.limit_value || relation.offset_value) + # Shortcut when limit is zero. + return 0 if relation.limit_value == 0 - query_builder = relation.arel + query_builder = build_count_subquery(relation, column_name, distinct) + else + column = aggregate_column(column_name) - if operation == "count" - limit = relation.limit_value - offset = relation.offset_value + select_value = operation_over_aggregate_column(column, operation, distinct) - unless limit && offset - query_builder.limit = nil - query_builder.offset = nil - end + relation.select_values = [select_value] + + query_builder = relation.arel end type_cast_calculated_value(@klass.connection.select_value(query_builder.to_sql), column_for(column_name), operation) @@ -312,5 +313,16 @@ module ActiveRecord select if select !~ /(,|\*)/ end end + + def build_count_subquery(relation, column_name, distinct) + # Arel doesn't do subqueries + subquery_alias = arel_table.alias("subquery_for_count") + aliased_column = aggregate_column(column_name, subquery_alias) + select_value = operation_over_aggregate_column(aliased_column, 'count', distinct) + + relation.select_values = [(column_name == :all ? 1 : aggregate_column(column_name))] + subquery_sql = "(#{relation.arel.to_sql}) #{subquery_alias.name}" + subquery_alias.relation.select_manager.project(select_value).from(subquery_sql) + end end end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index caf07a7357..1e8ce4fda7 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -65,7 +65,7 @@ class CalculationsTest < ActiveRecord::TestCase c = Account.sum(:credit_limit, :group => :firm_id) [1,6,2].each { |firm_id| assert c.keys.include?(firm_id) } end - + def test_should_group_by_multiple_fields c = Account.count(:all, :group => ['firm_id', :credit_limit]) [ [nil, 50], [1, 50], [6, 50], [6, 55], [9, 53], [2, 60] ].each { |firm_and_limit| assert c.keys.include?(firm_and_limit) } @@ -109,6 +109,35 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [2, 6], c.keys.compact end + def test_limit_should_apply_before_count + accounts = Account.limit(3).where('firm_id IS NOT NULL') + + assert_equal 3, accounts.count(:firm_id) + assert_equal 3, accounts.select(:firm_id).count + end + + def test_count_should_shortcut_with_limit_zero + accounts = Account.limit(0) + + assert_no_queries { assert_equal 0, accounts.count } + end + + def test_limit_is_kept + return if current_adapter?(:OracleAdapter) + + queries = assert_sql { Account.limit(1).count } + assert_equal 1, queries.length + assert_match(/LIMIT/, queries.first) + end + + def test_offset_is_kept + return if current_adapter?(:OracleAdapter) + + queries = assert_sql { Account.offset(1).count } + assert_equal 1, queries.length + assert_match(/OFFSET/, queries.first) + end + def test_limit_with_offset_is_kept return if current_adapter?(:OracleAdapter) @@ -118,20 +147,6 @@ class CalculationsTest < ActiveRecord::TestCase assert_match(/OFFSET/, queries.first) end - def test_offset_without_limit_removes_offset - queries = assert_sql { Account.offset(1).count } - assert_equal 1, queries.length - assert_no_match(/LIMIT/, queries.first) - assert_no_match(/OFFSET/, queries.first) - end - - def test_limit_without_offset_removes_limit - queries = assert_sql { Account.limit(1).count } - assert_equal 1, queries.length - assert_no_match(/LIMIT/, queries.first) - assert_no_match(/OFFSET/, queries.first) - end - def test_no_limit_no_offset queries = assert_sql { Account.count } assert_equal 1, queries.length diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 37bbb17e74..e9c006d623 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -666,6 +666,34 @@ class RelationTest < ActiveRecord::TestCase assert_no_queries { assert_equal 5, best_posts.size } end + def test_size_with_limit + posts = Post.limit(6) + + assert_queries(1) { assert_equal 6, posts.size } + assert ! posts.loaded? + + best_posts = posts.where(:comments_count => 0) + best_posts.to_a # force load + assert_no_queries { assert_equal 5, best_posts.size } + end + + def test_size_with_zero_limit + posts = Post.limit(0) + + assert_no_queries { assert_equal 0, posts.size } + assert ! posts.loaded? + + posts.to_a # force load + assert_no_queries { assert_equal 0, posts.size } + end + + def test_empty_with_zero_limit + posts = Post.limit(0) + + assert_no_queries { assert_equal true, posts.empty? } + assert ! posts.loaded? + end + def test_count_complex_chained_relations posts = Post.select('comments_count').where('id is not null').group("author_id").where("comments_count > 0") From 28c73f012328c8386acfc608f0dfb1a459dbf170 Mon Sep 17 00:00:00 2001 From: John Mileham Date: Thu, 24 Mar 2011 15:09:24 -0400 Subject: [PATCH 02/54] Use Arel to build subquery. Adapt tests to changed fixtures. --- .../active_record/relation/calculations.rb | 23 +++++++++---------- activerecord/test/cases/relations_test.rb | 6 ++--- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 7bfeb20a2b..869eebfa34 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -183,13 +183,10 @@ module ActiveRecord end end - def aggregate_column(column_name, subquery_alias = nil) + def aggregate_column(column_name) if @klass.column_names.include?(column_name.to_s) - Arel::Attribute.new(subquery_alias || @klass.unscoped.table, column_name) + Arel::Attribute.new(@klass.unscoped.table, column_name) else - if subquery_alias && (split_name = column_name.to_s.split(".")).length > 1 - column_name = split_name.last - end Arel.sql(column_name == :all ? "*" : column_name.to_s) end end @@ -315,14 +312,16 @@ module ActiveRecord end def build_count_subquery(relation, column_name, distinct) - # Arel doesn't do subqueries - subquery_alias = arel_table.alias("subquery_for_count") - aliased_column = aggregate_column(column_name, subquery_alias) - select_value = operation_over_aggregate_column(aliased_column, 'count', distinct) + column_alias = Arel.sql('count_column') + subquery_alias = Arel.sql('subquery_for_count') - relation.select_values = [(column_name == :all ? 1 : aggregate_column(column_name))] - subquery_sql = "(#{relation.arel.to_sql}) #{subquery_alias.name}" - subquery_alias.relation.select_manager.project(select_value).from(subquery_sql) + aliased_column = aggregate_column(column_name == :all ? 1 : column_name).as(column_alias) + relation.select_values = [aliased_column] + subquery = relation.arel.as(subquery_alias) + + sm = Arel::SelectManager.new relation.engine + select_value = operation_over_aggregate_column(column_alias, 'count', distinct) + sm.project(select_value).from(subquery) end end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index dad6665990..dfb5735b5d 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -667,14 +667,14 @@ class RelationTest < ActiveRecord::TestCase end def test_size_with_limit - posts = Post.limit(6) + posts = Post.limit(10) - assert_queries(1) { assert_equal 6, posts.size } + assert_queries(1) { assert_equal 10, posts.size } assert ! posts.loaded? best_posts = posts.where(:comments_count => 0) best_posts.to_a # force load - assert_no_queries { assert_equal 5, best_posts.size } + assert_no_queries { assert_equal 9, best_posts.size } end def test_size_with_zero_limit From 0e1fed537a7699a7ded02f77b71d222d7207c5ad Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Sun, 27 Mar 2011 18:52:21 -0300 Subject: [PATCH 03/54] Revert "Removed #update_attribute method. New #update_column method." This reverts commit 45c233ef819dc7b67e259dd73f24721fec28b8c8. Signed-off-by: Santiago Pastorino --- .../associations/has_one_association.rb | 3 +- activerecord/lib/active_record/persistence.rb | 18 ++-- ...s_and_belongs_to_many_associations_test.rb | 2 +- .../has_many_associations_test.rb | 6 +- .../has_many_through_associations_test.rb | 4 +- .../has_one_through_associations_test.rb | 4 +- .../cases/associations/join_model_test.rb | 12 +-- activerecord/test/cases/associations_test.rb | 4 +- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/calculations_test.rb | 4 +- activerecord/test/cases/dirty_test.rb | 5 +- activerecord/test/cases/persistence_test.rb | 86 ++++++++++--------- activerecord/test/cases/timestamp_test.rb | 6 +- 13 files changed, 79 insertions(+), 77 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index dfcb116392..1d2e8667e4 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -34,8 +34,7 @@ module ActiveRecord when :destroy target.destroy when :nullify - target.send("#{reflection.foreign_key}=", nil) - target.save(:validations => false) + target.update_attribute(reflection.foreign_key, nil) end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 3377a5934b..17a64b6e86 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -104,17 +104,19 @@ module ActiveRecord became end - # Updates a single attribute of an object, without calling save. + # Updates a single attribute and saves the record. + # This is especially useful for boolean flags on existing records. Also note that # # * Validation is skipped. - # * Callbacks are skipped. - # * updated_at/updated_on column is not updated in any case. + # * Callbacks are invoked. + # * updated_at/updated_on column is updated if that column is available. + # * Updates all the attributes that are dirty in this object. # - def update_column(name, value) + def update_attribute(name, value) name = name.to_s raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name) send("#{name}=", value) - self.class.update_all({ name => value }, self.class.primary_key => id) + save(:validate => false) end # Updates the attributes of the model from the passed-in hash and saves the @@ -154,7 +156,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def increment!(attribute, by = 1) - increment(attribute, by).update_column(attribute, self[attribute]) + increment(attribute, by).update_attribute(attribute, self[attribute]) end # Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1). @@ -171,7 +173,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def decrement!(attribute, by = 1) - decrement(attribute, by).update_column(attribute, self[attribute]) + decrement(attribute, by).update_attribute(attribute, self[attribute]) end # Assigns to +attribute+ the boolean opposite of attribute?. So @@ -188,7 +190,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def toggle!(attribute) - toggle(attribute).update_column(attribute, self[attribute]) + toggle(attribute).update_attribute(attribute, self[attribute]) end # Reloads the attributes of this object from the database. diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index f4d14853d3..73d02c9676 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -604,7 +604,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase project = SpecialProject.create("name" => "Special Project") assert developer.save developer.projects << project - developer.update_column("name", "Bruza") + developer.update_attribute("name", "Bruza") assert_equal 1, Developer.connection.select_value(<<-end_sql).to_i SELECT count(*) FROM developers_projects WHERE project_id = #{project.id} diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 16d4877fe8..ad774eb9ce 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -639,7 +639,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_delete_all post = posts(:welcome) - post.update_column(:taggings_with_delete_all_count, post.taggings_count) + post.update_attribute(:taggings_with_delete_all_count, post.taggings_count) assert_difference "post.reload.taggings_with_delete_all_count", -1 do post.taggings_with_delete_all.delete(post.taggings_with_delete_all.first) @@ -648,7 +648,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_destroy post = posts(:welcome) - post.update_column(:taggings_with_destroy_count, post.taggings_count) + post.update_attribute(:taggings_with_destroy_count, post.taggings_count) assert_difference "post.reload.taggings_with_destroy_count", -1 do post.taggings_with_destroy.delete(post.taggings_with_destroy.first) @@ -787,7 +787,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm = Firm.find(:first) # break the vanilla firm_id foreign key assert_equal 2, firm.clients.count - firm.clients.first.update_column(:firm_id, nil) + firm.clients.first.update_attribute(:firm_id, nil) assert_equal 1, firm.clients(true).count assert_equal 1, firm.clients_using_primary_key_with_delete_all.count old_record = firm.clients_using_primary_key_with_delete_all.first diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 1efe3420a0..9adaebe924 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -286,7 +286,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_update_counter_caches_on_delete_with_dependent_destroy post = posts(:welcome) tag = post.tags.create!(:name => 'doomed') - post.update_column(:tags_with_destroy_count, post.tags.count) + post.update_attribute(:tags_with_destroy_count, post.tags.count) assert_difference ['post.reload.taggings_count', 'post.reload.tags_with_destroy_count'], -1 do posts(:welcome).tags_with_destroy.delete(tag) @@ -296,7 +296,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_update_counter_caches_on_delete_with_dependent_nullify post = posts(:welcome) tag = post.tags.create!(:name => 'doomed') - post.update_column(:tags_with_nullify_count, post.tags.count) + post.update_attribute(:tags_with_nullify_count, post.tags.count) assert_no_difference 'post.reload.taggings_count' do assert_difference 'post.reload.tags_with_nullify_count', -1 do diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 968025ade8..9ba5549277 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -90,12 +90,12 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_with_conditions_eager_loading # conditions on the through table assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :favourite_club).favourite_club - memberships(:membership_of_favourite_club).update_column(:favourite, false) + memberships(:membership_of_favourite_club).update_attribute(:favourite, false) assert_equal nil, Member.find(@member.id, :include => :favourite_club).reload.favourite_club # conditions on the source table assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :hairy_club).hairy_club - clubs(:moustache_club).update_column(:name, "Association of Clean-Shaven Persons") + clubs(:moustache_club).update_attribute(:name, "Association of Clean-Shaven Persons") assert_equal nil, Member.find(@member.id, :include => :hairy_club).reload.hairy_club end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 5a7b139030..1f95b31497 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -161,7 +161,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_delete_all assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDeleteAll' + posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDeleteAll' post = find_post_with_dependency(1, :has_many, :taggings, :delete_all) old_count = Tagging.count @@ -172,7 +172,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_destroy assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDestroy' + posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDestroy' post = find_post_with_dependency(1, :has_many, :taggings, :destroy) old_count = Tagging.count @@ -183,7 +183,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_nullify assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyNullify' + posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyNullify' post = find_post_with_dependency(1, :has_many, :taggings, :nullify) old_count = Tagging.count @@ -194,7 +194,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_one_with_destroy assert posts(:welcome).tagging - posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneDestroy' + posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneDestroy' post = find_post_with_dependency(1, :has_one, :tagging, :destroy) old_count = Tagging.count @@ -205,7 +205,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_one_with_nullify assert posts(:welcome).tagging - posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneNullify' + posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneNullify' post = find_post_with_dependency(1, :has_one, :tagging, :nullify) old_count = Tagging.count @@ -707,7 +707,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase # create dynamic Post models to allow different dependency options def find_post_with_dependency(post_id, association, association_name, dependency) class_name = "PostWith#{association.to_s.classify}#{dependency.to_s.classify}" - Post.find(post_id).update_column :type, class_name + Post.find(post_id).update_attribute :type, class_name klass = Object.const_set(class_name, Class.new(ActiveRecord::Base)) klass.set_table_name 'posts' klass.send(association, association_name, :as => :taggable, :dependent => dependency) diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 04f628a398..47b8e48582 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -66,7 +66,7 @@ class AssociationsTest < ActiveRecord::TestCase ship = Ship.create!(:name => "The good ship Dollypop") part = ship.parts.create!(:name => "Mast") part.mark_for_destruction - ShipPart.find(part.id).update_column(:name, 'Deck') + ShipPart.find(part.id).update_attribute(:name, 'Deck') ship.parts.send(:load_target) assert_equal 'Deck', ship.parts[0].name end @@ -170,7 +170,7 @@ class AssociationProxyTest < ActiveRecord::TestCase david = developers(:david) assert !david.projects.loaded? - david.update_column(:created_at, Time.now) + david.update_attribute(:created_at, Time.now) assert !david.projects.loaded? end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index aeb0b28bab..fba7af741d 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -484,7 +484,7 @@ class BasicsTest < ActiveRecord::TestCase weird.reload assert_equal 'value', weird.send('a$b') - weird.update_column('a$b', 'value2') + weird.update_attribute('a$b', 'value2') weird.reload assert_equal 'value2', weird.send('a$b') end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index c97f1ae634..caf07a7357 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -311,8 +311,8 @@ class CalculationsTest < ActiveRecord::TestCase def test_should_count_scoped_select_with_options Account.update_all("credit_limit = NULL") - Account.last.update_column('credit_limit', 49) - Account.first.update_column('credit_limit', 51) + Account.last.update_attribute('credit_limit', 49) + Account.first.update_attribute('credit_limit', 51) assert_equal 1, Account.scoped(:select => "credit_limit").count(:conditions => ['credit_limit >= 50']) end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index b75482a956..a6738fb654 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -413,7 +413,7 @@ class DirtyTest < ActiveRecord::TestCase with_partial_updates(Topic) do Topic.create!(:author_name => 'Bill', :content => {:a => "a"}) topic = Topic.select('id, author_name').first - topic.update_column :author_name, 'John' + topic.update_attribute :author_name, 'John' topic = Topic.first assert_not_nil topic.content end @@ -487,8 +487,7 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.previous_changes.key?('created_on') pirate = Pirate.find_by_catchphrase("Ahoy!") - pirate.catchphrase = "Ninjas suck!" - pirate.save(:validations => false) + pirate.update_attribute(:catchphrase, "Ninjas suck!") assert_equal 2, pirate.previous_changes.size assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes['catchphrase'] diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 68d861c9c2..8ca9d626d1 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -327,62 +327,66 @@ class PersistencesTest < ActiveRecord::TestCase assert_raise(ActiveSupport::FrozenObjectError) { client.name = "something else" } end - def test_update_column - topic = Topic.find(1) - topic.update_column("approved", true) - assert topic.approved? - topic.reload - assert topic.approved? + def test_update_attribute + assert !Topic.find(1).approved? + Topic.find(1).update_attribute("approved", true) + assert Topic.find(1).approved? - topic.update_column(:approved, false) - assert !topic.approved? - topic.reload - assert !topic.approved? + Topic.find(1).update_attribute(:approved, false) + assert !Topic.find(1).approved? end - def test_update_column_with_model_having_primary_key_other_than_id + def test_update_attribute_for_readonly_attribute minivan = Minivan.find('m1') - new_name = 'sebavan' - - minivan.update_column(:name, new_name) - assert_equal new_name, minivan.name + assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } end - def test_update_column_for_readonly_attribute - minivan = Minivan.find('m1') - prev_color = minivan.color - assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_column(:color, 'black') } - assert_equal prev_color, minivan.color + # This test is correct, but it is hard to fix it since + # update_attribute trigger simply call save! that triggers + # all callbacks. + # def test_update_attribute_with_one_changed_and_one_updated + # t = Topic.order('id').limit(1).first + # title, author_name = t.title, t.author_name + # t.author_name = 'John' + # t.update_attribute(:title, 'super_title') + # assert_equal 'John', t.author_name + # assert_equal 'super_title', t.title + # assert t.changed?, "topic should have changed" + # assert t.author_name_changed?, "author_name should have changed" + # assert !t.title_changed?, "title should not have changed" + # assert_nil t.title_change, 'title change should be nil' + # assert_equal ['author_name'], t.changed + # + # t.reload + # assert_equal 'David', t.author_name + # assert_equal 'super_title', t.title + # end + + def test_update_attribute_with_one_updated + t = Topic.first + title = t.title + t.update_attribute(:title, 'super_title') + assert_equal 'super_title', t.title + assert !t.changed?, "topic should not have changed" + assert !t.title_changed?, "title should not have changed" + assert_nil t.title_change, 'title change should be nil' + + t.reload + assert_equal 'super_title', t.title end - def test_update_column_should_not_modify_updated_at + def test_update_attribute_for_updated_at_on developer = Developer.find(1) prev_month = Time.now.prev_month - developer.update_column(:updated_at, prev_month) + developer.update_attribute(:updated_at, prev_month) assert_equal prev_month, developer.updated_at - developer.update_column(:salary, 80001) - assert_equal prev_month, developer.updated_at + developer.update_attribute(:salary, 80001) + assert_not_equal prev_month, developer.updated_at developer.reload - assert_equal prev_month, developer.updated_at - end - - def test_update_column_with_one_changed_and_one_updated - t = Topic.order('id').limit(1).first - title, author_name = t.title, t.author_name - t.author_name = 'John' - t.update_column(:title, 'super_title') - assert_equal 'John', t.author_name - assert_equal 'super_title', t.title - assert t.changed?, "topic should have changed" - assert t.author_name_changed?, "author_name should have changed" - assert t.title_changed?, "title should have changed" - - t.reload - assert_equal author_name, t.author_name - assert_equal 'super_title', t.title + assert_not_equal prev_month, developer.updated_at end def test_update_attributes diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 08bbd14d38..1c21f0f3b6 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -113,8 +113,7 @@ class TimestampTest < ActiveRecord::TestCase pet = Pet.first owner = pet.owner - owner.happy_at = 3.days.ago - owner.save + owner.update_attribute(:happy_at, 3.days.ago) previously_owner_updated_at = owner.updated_at pet.name = "I'm a parrot" @@ -132,9 +131,8 @@ class TimestampTest < ActiveRecord::TestCase toy = Toy.first pet = toy.pet owner = pet.owner - time = 3.days.ago - owner.update_column(:updated_at, time) + owner.update_attribute(:updated_at, (time = 3.days.ago)) toy.touch owner.reload From b023bcf71b685c0088b2b1f79bc52e8797ce3538 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 27 Mar 2011 19:37:18 -0300 Subject: [PATCH 04/54] Make edge and dev options use edge mysql2 --- railties/lib/rails/generators/app_base.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index ab7ed4eb9e..7c9494e46d 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -112,7 +112,15 @@ module Rails end def database_gemfile_entry - options[:skip_active_record] ? "" : "gem '#{gem_for_database}'" + entry = options[:skip_active_record] ? "" : "gem '#{gem_for_database}'" + if options[:database] == 'mysql' + if options.dev? || options.edge? + entry += ", :git => 'git://github.com/brianmario/mysql2.git'" + else + entry += "\n# gem 'mysql2', :git => 'git://github.com/brianmario/mysql2.git'" + end + end + entry end def rails_gemfile_entry @@ -120,22 +128,22 @@ module Rails <<-GEMFILE gem 'rails', :path => '#{Rails::Generators::RAILS_DEV_PATH}' gem 'arel', :git => 'git://github.com/rails/arel.git' -gem "rack", :git => "git://github.com/rack/rack.git" +gem 'rack', :git => 'git://github.com/rack/rack.git' GEMFILE elsif options.edge? <<-GEMFILE gem 'rails', :git => 'git://github.com/rails/rails.git' gem 'arel', :git => 'git://github.com/rails/arel.git' -gem "rack", :git => "git://github.com/rack/rack.git" +gem 'rack', :git => 'git://github.com/rack/rack.git' GEMFILE else <<-GEMFILE gem 'rails', '#{Rails::VERSION::STRING}' # Bundle edge Rails instead: -# gem 'rails', :git => 'git://github.com/rails/rails.git' -# gem 'arel', :git => 'git://github.com/rails/arel.git' -# gem "rack", :git => "git://github.com/rack/rack.git" +# gem 'rails', :git => 'git://github.com/rails/rails.git' +# gem 'arel', :git => 'git://github.com/rails/arel.git' +# gem 'rack', :git => 'git://github.com/rack/rack.git' GEMFILE end end From e6a8a3adaf08cc9d551b4a125aaa276812984be3 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 28 Mar 2011 01:42:30 +0200 Subject: [PATCH 05/54] let these heredocs flow with the code --- railties/lib/rails/generators/app_base.rb | 29 ++++++++++++----------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 7c9494e46d..a2eaf7a6fb 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -1,5 +1,6 @@ require 'digest/md5' require 'active_support/secure_random' +require 'active_support/core_ext/string/strip' require 'rails/version' unless defined?(Rails::VERSION) require 'rbconfig' require 'open-uri' @@ -125,25 +126,25 @@ module Rails def rails_gemfile_entry if options.dev? - <<-GEMFILE -gem 'rails', :path => '#{Rails::Generators::RAILS_DEV_PATH}' -gem 'arel', :git => 'git://github.com/rails/arel.git' -gem 'rack', :git => 'git://github.com/rack/rack.git' + <<-GEMFILE.strip_heredoc + gem 'rails', :path => '#{Rails::Generators::RAILS_DEV_PATH}' + gem 'arel', :git => 'git://github.com/rails/arel.git' + gem 'rack', :git => 'git://github.com/rack/rack.git' GEMFILE elsif options.edge? - <<-GEMFILE -gem 'rails', :git => 'git://github.com/rails/rails.git' -gem 'arel', :git => 'git://github.com/rails/arel.git' -gem 'rack', :git => 'git://github.com/rack/rack.git' + <<-GEMFILE.strip_heredoc + gem 'rails', :git => 'git://github.com/rails/rails.git' + gem 'arel', :git => 'git://github.com/rails/arel.git' + gem 'rack', :git => 'git://github.com/rack/rack.git' GEMFILE else - <<-GEMFILE -gem 'rails', '#{Rails::VERSION::STRING}' + <<-GEMFILE.strip_heredoc + gem 'rails', '#{Rails::VERSION::STRING}' -# Bundle edge Rails instead: -# gem 'rails', :git => 'git://github.com/rails/rails.git' -# gem 'arel', :git => 'git://github.com/rails/arel.git' -# gem 'rack', :git => 'git://github.com/rack/rack.git' + # Bundle edge Rails instead: + # gem 'rails', :git => 'git://github.com/rails/rails.git' + # gem 'arel', :git => 'git://github.com/rails/arel.git' + # gem 'rack', :git => 'git://github.com/rack/rack.git' GEMFILE end end From 245542ea2994961731be105db6c076256a22a7a9 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Sun, 27 Mar 2011 19:12:28 -0300 Subject: [PATCH 06/54] Added new #update_column method. Signed-off-by: Santiago Pastorino --- activerecord/CHANGELOG | 9 ++ .../active_record/attribute_methods/write.rb | 1 + activerecord/lib/active_record/persistence.rb | 14 +++ ...s_and_belongs_to_many_associations_test.rb | 2 +- .../has_many_associations_test.rb | 6 +- .../has_many_through_associations_test.rb | 4 +- .../has_one_through_associations_test.rb | 4 +- .../cases/associations/join_model_test.rb | 12 +-- activerecord/test/cases/associations_test.rb | 4 +- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/calculations_test.rb | 4 +- activerecord/test/cases/dirty_test.rb | 2 +- activerecord/test/cases/persistence_test.rb | 86 +++++++++++++++++++ activerecord/test/cases/timestamp_test.rb | 3 +- 14 files changed, 132 insertions(+), 21 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 6be46349fb..3b6a7d7208 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,14 @@ *Rails 3.1.0 (unreleased)* +* Added an update_column method on ActiveRecord. This new method updates a given attribute on an object, skipping validations and callbacks. + It is recommended to use #update_attribute unless you are sure you do not want to execute any callback, including the modification of + the updated_at column. It should not be called on new records. + Example: + + User.first.update_column(:name, "sebastian") # => true + + [Sebastian Martinez] + * Associations with a :through option can now use *any* association as the through or source association, including other associations which have a :through option and has_and_belongs_to_many associations diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 3c4dab304e..7661676f8c 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -32,6 +32,7 @@ module ActiveRecord @attributes[attr_name] = value end end + alias_method :raw_write_attribute, :write_attribute private # Handle *= for method_missing. diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 17a64b6e86..a916c88348 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -119,6 +119,20 @@ module ActiveRecord save(:validate => false) end + # Updates a single attribute of an object, without calling save. + # + # * Validation is skipped. + # * Callbacks are skipped. + # * updated_at/updated_on column is not updated if that column is available. + # + def update_column(name, value) + name = name.to_s + raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name) + raise ActiveRecordError, "can not update on a new record object" unless persisted? + raw_write_attribute(name, value) + self.class.update_all({ name => value }, self.class.primary_key => id) == 1 + end + # Updates the attributes of the model from the passed-in hash and saves the # record, all wrapped in a transaction. If the object is invalid, the saving # will fail and false will be returned. diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 73d02c9676..f4d14853d3 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -604,7 +604,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase project = SpecialProject.create("name" => "Special Project") assert developer.save developer.projects << project - developer.update_attribute("name", "Bruza") + developer.update_column("name", "Bruza") assert_equal 1, Developer.connection.select_value(<<-end_sql).to_i SELECT count(*) FROM developers_projects WHERE project_id = #{project.id} diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ad774eb9ce..16d4877fe8 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -639,7 +639,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_delete_all post = posts(:welcome) - post.update_attribute(:taggings_with_delete_all_count, post.taggings_count) + post.update_column(:taggings_with_delete_all_count, post.taggings_count) assert_difference "post.reload.taggings_with_delete_all_count", -1 do post.taggings_with_delete_all.delete(post.taggings_with_delete_all.first) @@ -648,7 +648,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_updates_counter_cache_with_dependent_destroy post = posts(:welcome) - post.update_attribute(:taggings_with_destroy_count, post.taggings_count) + post.update_column(:taggings_with_destroy_count, post.taggings_count) assert_difference "post.reload.taggings_with_destroy_count", -1 do post.taggings_with_destroy.delete(post.taggings_with_destroy.first) @@ -787,7 +787,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase firm = Firm.find(:first) # break the vanilla firm_id foreign key assert_equal 2, firm.clients.count - firm.clients.first.update_attribute(:firm_id, nil) + firm.clients.first.update_column(:firm_id, nil) assert_equal 1, firm.clients(true).count assert_equal 1, firm.clients_using_primary_key_with_delete_all.count old_record = firm.clients_using_primary_key_with_delete_all.first diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 9adaebe924..1efe3420a0 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -286,7 +286,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_update_counter_caches_on_delete_with_dependent_destroy post = posts(:welcome) tag = post.tags.create!(:name => 'doomed') - post.update_attribute(:tags_with_destroy_count, post.tags.count) + post.update_column(:tags_with_destroy_count, post.tags.count) assert_difference ['post.reload.taggings_count', 'post.reload.tags_with_destroy_count'], -1 do posts(:welcome).tags_with_destroy.delete(tag) @@ -296,7 +296,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def test_update_counter_caches_on_delete_with_dependent_nullify post = posts(:welcome) tag = post.tags.create!(:name => 'doomed') - post.update_attribute(:tags_with_nullify_count, post.tags.count) + post.update_column(:tags_with_nullify_count, post.tags.count) assert_no_difference 'post.reload.taggings_count' do assert_difference 'post.reload.tags_with_nullify_count', -1 do diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 9ba5549277..968025ade8 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -90,12 +90,12 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_with_conditions_eager_loading # conditions on the through table assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :favourite_club).favourite_club - memberships(:membership_of_favourite_club).update_attribute(:favourite, false) + memberships(:membership_of_favourite_club).update_column(:favourite, false) assert_equal nil, Member.find(@member.id, :include => :favourite_club).reload.favourite_club # conditions on the source table assert_equal clubs(:moustache_club), Member.find(@member.id, :include => :hairy_club).hairy_club - clubs(:moustache_club).update_attribute(:name, "Association of Clean-Shaven Persons") + clubs(:moustache_club).update_column(:name, "Association of Clean-Shaven Persons") assert_equal nil, Member.find(@member.id, :include => :hairy_club).reload.hairy_club end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 1f95b31497..5a7b139030 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -161,7 +161,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_delete_all assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDeleteAll' + posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDeleteAll' post = find_post_with_dependency(1, :has_many, :taggings, :delete_all) old_count = Tagging.count @@ -172,7 +172,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_destroy assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyDestroy' + posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyDestroy' post = find_post_with_dependency(1, :has_many, :taggings, :destroy) old_count = Tagging.count @@ -183,7 +183,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_many_with_nullify assert_equal 1, posts(:welcome).taggings.count - posts(:welcome).taggings.first.update_attribute :taggable_type, 'PostWithHasManyNullify' + posts(:welcome).taggings.first.update_column :taggable_type, 'PostWithHasManyNullify' post = find_post_with_dependency(1, :has_many, :taggings, :nullify) old_count = Tagging.count @@ -194,7 +194,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_one_with_destroy assert posts(:welcome).tagging - posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneDestroy' + posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneDestroy' post = find_post_with_dependency(1, :has_one, :tagging, :destroy) old_count = Tagging.count @@ -205,7 +205,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_delete_polymorphic_has_one_with_nullify assert posts(:welcome).tagging - posts(:welcome).tagging.update_attribute :taggable_type, 'PostWithHasOneNullify' + posts(:welcome).tagging.update_column :taggable_type, 'PostWithHasOneNullify' post = find_post_with_dependency(1, :has_one, :tagging, :nullify) old_count = Tagging.count @@ -707,7 +707,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase # create dynamic Post models to allow different dependency options def find_post_with_dependency(post_id, association, association_name, dependency) class_name = "PostWith#{association.to_s.classify}#{dependency.to_s.classify}" - Post.find(post_id).update_attribute :type, class_name + Post.find(post_id).update_column :type, class_name klass = Object.const_set(class_name, Class.new(ActiveRecord::Base)) klass.set_table_name 'posts' klass.send(association, association_name, :as => :taggable, :dependent => dependency) diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 47b8e48582..04f628a398 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -66,7 +66,7 @@ class AssociationsTest < ActiveRecord::TestCase ship = Ship.create!(:name => "The good ship Dollypop") part = ship.parts.create!(:name => "Mast") part.mark_for_destruction - ShipPart.find(part.id).update_attribute(:name, 'Deck') + ShipPart.find(part.id).update_column(:name, 'Deck') ship.parts.send(:load_target) assert_equal 'Deck', ship.parts[0].name end @@ -170,7 +170,7 @@ class AssociationProxyTest < ActiveRecord::TestCase david = developers(:david) assert !david.projects.loaded? - david.update_attribute(:created_at, Time.now) + david.update_column(:created_at, Time.now) assert !david.projects.loaded? end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index fba7af741d..aeb0b28bab 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -484,7 +484,7 @@ class BasicsTest < ActiveRecord::TestCase weird.reload assert_equal 'value', weird.send('a$b') - weird.update_attribute('a$b', 'value2') + weird.update_column('a$b', 'value2') weird.reload assert_equal 'value2', weird.send('a$b') end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index caf07a7357..c97f1ae634 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -311,8 +311,8 @@ class CalculationsTest < ActiveRecord::TestCase def test_should_count_scoped_select_with_options Account.update_all("credit_limit = NULL") - Account.last.update_attribute('credit_limit', 49) - Account.first.update_attribute('credit_limit', 51) + Account.last.update_column('credit_limit', 49) + Account.first.update_column('credit_limit', 51) assert_equal 1, Account.scoped(:select => "credit_limit").count(:conditions => ['credit_limit >= 50']) end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index a6738fb654..b1ce846218 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -413,7 +413,7 @@ class DirtyTest < ActiveRecord::TestCase with_partial_updates(Topic) do Topic.create!(:author_name => 'Bill', :content => {:a => "a"}) topic = Topic.select('id, author_name').first - topic.update_attribute :author_name, 'John' + topic.update_column :author_name, 'John' topic = Topic.first assert_not_nil topic.content end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 8ca9d626d1..8e6141eb81 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -389,6 +389,92 @@ class PersistencesTest < ActiveRecord::TestCase assert_not_equal prev_month, developer.updated_at end + def test_update_column + topic = Topic.find(1) + topic.update_column("approved", true) + assert topic.approved? + topic.reload + assert topic.approved? + + topic.update_column(:approved, false) + assert !topic.approved? + topic.reload + assert !topic.approved? + end + + def test_update_column_should_not_use_setter_method + dev = Developer.find(1) + dev.instance_eval { def salary=(value); write_attribute(:salary, value * 2); end } + + dev.update_column(:salary, 80000) + assert_equal 80000, dev.salary + + dev.reload + assert_equal 80000, dev.salary + end + + def test_update_column_should_raise_exception_if_new_record + topic = Topic.new + assert_raises(ActiveRecord::ActiveRecordError) { topic.update_column("approved", false) } + end + + def test_update_column_should_not_leave_the_object_dirty + topic = Topic.find(1) + topic.update_attribute("content", "Have a nice day") + + topic.reload + topic.update_column(:content, "You too") + assert_equal [], topic.changed + + topic.reload + topic.update_column("content", "Have a nice day") + assert_equal [], topic.changed + end + + def test_update_column_with_model_having_primary_key_other_than_id + minivan = Minivan.find('m1') + new_name = 'sebavan' + + minivan.update_column(:name, new_name) + assert_equal new_name, minivan.name + end + + def test_update_column_for_readonly_attribute + minivan = Minivan.find('m1') + prev_color = minivan.color + assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_column(:color, 'black') } + assert_equal prev_color, minivan.color + end + + def test_update_column_should_not_modify_updated_at + developer = Developer.find(1) + prev_month = Time.now.prev_month + + developer.update_column(:updated_at, prev_month) + assert_equal prev_month, developer.updated_at + + developer.update_column(:salary, 80001) + assert_equal prev_month, developer.updated_at + + developer.reload + assert_equal prev_month, developer.updated_at + end + + def test_update_column_with_one_changed_and_one_updated + t = Topic.order('id').limit(1).first + title, author_name = t.title, t.author_name + t.author_name = 'John' + t.update_column(:title, 'super_title') + assert_equal 'John', t.author_name + assert_equal 'super_title', t.title + assert t.changed?, "topic should have changed" + assert t.author_name_changed?, "author_name should have changed" + + t.reload + assert_equal author_name, t.author_name + assert_equal 'super_title', t.title + end + def test_update_attributes topic = Topic.find(1) assert !topic.approved? diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 1c21f0f3b6..ceb1452afd 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -131,8 +131,9 @@ class TimestampTest < ActiveRecord::TestCase toy = Toy.first pet = toy.pet owner = pet.owner + time = 3.days.ago - owner.update_attribute(:updated_at, (time = 3.days.ago)) + owner.update_column(:updated_at, time) toy.touch owner.reload From 0a28073acc8e81e75a71be7098a4753325ce64f0 Mon Sep 17 00:00:00 2001 From: Rolf Timmermans Date: Mon, 28 Mar 2011 15:09:29 +0800 Subject: [PATCH 07/54] Engines that use isolate_namespace with nested modules should set correct module prefix for their routes. --- railties/lib/rails/engine.rb | 4 +-- railties/test/railties/engine_test.rb | 39 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 4fc23fe277..ee265366ff 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -360,11 +360,11 @@ module Rails def isolate_namespace(mod) engine_name(generate_railtie_name(mod)) - name = engine_name - self.routes.default_scope = {:module => name} + self.routes.default_scope = { :module => ActiveSupport::Inflector.underscore(mod.name) } self.isolated = true unless mod.respond_to?(:_railtie) + name = engine_name _railtie = self mod.singleton_class.instance_eval do define_method(:_railtie) do diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 0ce00db3c4..20797a2b0c 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -559,6 +559,45 @@ module RailtiesTest assert_match /name="post\[title\]"/, last_response.body end + test "isolated engine should set correct route module prefix for nested namespace" do + @plugin.write "lib/bukkits.rb", <<-RUBY + module Bukkits + module Awesome + class Engine < ::Rails::Engine + isolate_namespace Bukkits::Awesome + end + end + end + RUBY + + app_file "config/routes.rb", <<-RUBY + AppTemplate::Application.routes.draw do + mount Bukkits::Awesome::Engine => "/bukkits", :as => "bukkits" + end + RUBY + + @plugin.write "config/routes.rb", <<-RUBY + Bukkits::Awesome::Engine.routes.draw do + match "/foo" => "foo#index" + end + RUBY + + @plugin.write "app/controllers/bukkits/awesome/foo_controller.rb", <<-RUBY + class Bukkits::Awesome::FooController < ActionController::Base + def index + render :text => "ok" + end + end + RUBY + + add_to_config("config.action_dispatch.show_exceptions = false") + + boot_rails + + get("/bukkits/foo") + assert_equal "ok", last_response.body + end + test "loading seed data" do @plugin.write "db/seeds.rb", <<-RUBY Bukkits::Engine.config.bukkits_seeds_loaded = true From fb215110401c70cfc7013c6e2ad5753fa4e374e9 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Mon, 28 Mar 2011 10:19:41 -0300 Subject: [PATCH 08/54] Bring #reorder back Signed-off-by: Santiago Pastorino --- activerecord/lib/active_record/base.rb | 2 +- activerecord/lib/active_record/relation/query_methods.rb | 4 ++++ activerecord/test/cases/relation_scoping_test.rb | 4 ++-- activerecord/test/cases/relations_test.rb | 6 ++++++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b778b0c0f0..616b5cc3b4 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -439,7 +439,7 @@ module ActiveRecord #:nodoc: class << self # Class methods delegate :find, :first, :last, :all, :destroy, :destroy_all, :exists?, :delete, :delete_all, :update, :update_all, :to => :scoped delegate :find_each, :find_in_batches, :to => :scoped - delegate :select, :group, :order, :except, :limit, :offset, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :create_with, :to => :scoped + delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :create_with, :to => :scoped delegate :count, :average, :minimum, :maximum, :sum, :calculate, :to => :scoped # Executes a custom SQL query against your database and returns all the results. The results will diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 9470e7c6c5..02b7056989 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -62,6 +62,10 @@ module ActiveRecord relation end + def reorder(*args) + except(:order).order(args) + end + def joins(*args) return self if args.compact.blank? diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index 7369aaea1d..30a783d5a2 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -429,9 +429,9 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal expected, received end - def test_except_and_order_overrides_default_scope_order + def test_reorder_overrides_default_scope_order expected = Developer.order('name DESC').collect { |dev| dev.name } - received = DeveloperOrderedBySalary.except(:order).order('name DESC').collect { |dev| dev.name } + received = DeveloperOrderedBySalary.reorder('name DESC').collect { |dev| dev.name } assert_equal expected, received end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 4c5c871251..2304c20a5f 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -151,6 +151,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal topics(:fourth).title, topics.first.title end + def test_finding_with_reorder + topics = Topic.order('author_name').order('title').reorder('id').all + topics_titles = topics.map{ |t| t.title } + assert_equal ['The First Topic', 'The Second Topic of the day', 'The Third Topic of the day', 'The Fourth Topic of the day'], topics_titles + end + def test_finding_with_order_and_take entrants = Entrant.order("id ASC").limit(2).to_a From e1a5007207b8bd9e91711428acc3104f4c8e8fb8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 15:27:37 -0700 Subject: [PATCH 09/54] sql logger ignores schema statements --- .../connection_adapters/sqlite_adapter.rb | 4 +-- .../lib/active_record/log_subscriber.rb | 3 +++ .../test/cases/log_subscriber_test.rb | 27 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index ae61d6ce94..32229a8410 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -222,7 +222,7 @@ module ActiveRecord # SCHEMA STATEMENTS ======================================== - def tables(name = nil) #:nodoc: + def tables(name = 'SCHEMA') #:nodoc: sql = <<-SQL SELECT name FROM sqlite_master @@ -350,7 +350,7 @@ module ActiveRecord end def table_structure(table_name) - structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})").to_hash + structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", 'SCHEMA').to_hash raise(ActiveRecord::StatementInvalid, "Could not find table '#{table_name}'") if structure.empty? structure end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index afadbf03ef..d31e321440 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -23,6 +23,9 @@ module ActiveRecord return unless logger.debug? payload = event.payload + + return if 'SCHEMA' == payload[:name] + name = '%s (%.1fms)' % [payload[:name], event.duration] sql = payload[:sql].squeeze(' ') binds = nil diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index cbaaca764b..8ebde933b4 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -22,6 +22,33 @@ class LogSubscriberTest < ActiveRecord::TestCase ActiveRecord::Base.logger = logger end + def test_schema_statements_are_ignored + event = Struct.new(:duration, :payload) + + logger = Class.new(ActiveRecord::LogSubscriber) { + attr_accessor :debugs + + def initialize + @debugs = [] + super + end + + def debug message + @debugs << message + end + }.new + assert_equal 0, logger.debugs.length + + logger.sql(event.new(0, { :sql => 'hi mom!' })) + assert_equal 1, logger.debugs.length + + logger.sql(event.new(0, { :sql => 'hi mom!', :name => 'foo' })) + assert_equal 2, logger.debugs.length + + logger.sql(event.new(0, { :sql => 'hi mom!', :name => 'SCHEMA' })) + assert_equal 2, logger.debugs.length + end + def test_basic_query_logging Developer.all wait From 7b4866e0aef1ed8ec08fd9eac104df42bce3c1c9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 16:08:42 -0700 Subject: [PATCH 10/54] namespacing connection management tests. :heart: --- .../test/cases/connection_management_test.rb | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index c535119972..c5511673ac 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -1,25 +1,29 @@ require "cases/helper" -class ConnectionManagementTest < ActiveRecord::TestCase - def setup - @env = {} - @app = stub('App') - @management = ActiveRecord::ConnectionAdapters::ConnectionManagement.new(@app) +module ActiveRecord + module ConnectionAdapters + class ConnectionManagementTest < ActiveRecord::TestCase + def setup + @env = {} + @app = stub('App') + @management = ConnectionManagement.new(@app) - @connections_cleared = false - ActiveRecord::Base.stubs(:clear_active_connections!).with { @connections_cleared = true } - end + @connections_cleared = false + ActiveRecord::Base.stubs(:clear_active_connections!).with { @connections_cleared = true } + end - test "clears active connections after each call" do - @app.expects(:call).with(@env) - @management.call(@env) - assert @connections_cleared - end + test "clears active connections after each call" do + @app.expects(:call).with(@env) + @management.call(@env) + assert @connections_cleared + end - test "doesn't clear active connections when running in a test case" do - @env['rack.test'] = true - @app.expects(:call).with(@env) - @management.call(@env) - assert !@connections_cleared + test "doesn't clear active connections when running in a test case" do + @env['rack.test'] = true + @app.expects(:call).with(@env) + @management.call(@env) + assert !@connections_cleared + end + end end end From ec0cacc2938cb7159ae6085fc653b2be906a748f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 16:22:37 -0700 Subject: [PATCH 11/54] testing app delegation from the ConnectionManagement middleware --- .../test/cases/connection_management_test.rb | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index c5511673ac..82ea46b41f 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -3,24 +3,41 @@ require "cases/helper" module ActiveRecord module ConnectionAdapters class ConnectionManagementTest < ActiveRecord::TestCase + class App + attr_reader :calls + def initialize + @calls = [] + end + + def call(env) + @calls << env + [200, {}, [['hi mom']]] + end + end + def setup @env = {} - @app = stub('App') + @app = App.new @management = ConnectionManagement.new(@app) @connections_cleared = false ActiveRecord::Base.stubs(:clear_active_connections!).with { @connections_cleared = true } end + def test_app_delegation + manager = ConnectionManagement.new(@app) + + manager.call @env + assert_equal [@env], @app.calls + end + test "clears active connections after each call" do - @app.expects(:call).with(@env) @management.call(@env) assert @connections_cleared end test "doesn't clear active connections when running in a test case" do @env['rack.test'] = true - @app.expects(:call).with(@env) @management.call(@env) assert !@connections_cleared end From 4211866b7a1d0abf0c9150fd61ea67a8043b831d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 16:43:34 -0700 Subject: [PATCH 12/54] adding active_connection? to the connection pool --- .../connection_adapters/abstract/connection_pool.rb | 6 ++++++ activerecord/test/cases/connection_pool_test.rb | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 4297c26413..61e44d09bf 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -151,6 +151,12 @@ module ActiveRecord @reserved_connections[current_connection_id] ||= checkout end + # Check to see if there is an active connection in this connection + # pool. + def active_connection? + @reserved_connections.key? current_connection_id + end + # Signal that the thread is finished with the current connection. # #release_connection releases the connection-thread association # and returns the connection to the pool. diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index 7ac14fa8d6..f92f4e62c5 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -18,6 +18,14 @@ module ActiveRecord end end + def test_active_connection? + assert !@pool.active_connection? + assert @pool.connection + assert @pool.active_connection? + @pool.release_connection + assert !@pool.active_connection? + end + def test_pool_caches_columns columns = @pool.columns['posts'] assert_equal columns, @pool.columns['posts'] From 25f94971abf71fe51089f53b72cc08b636c230b3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 17:29:37 -0700 Subject: [PATCH 13/54] adding active_connections? to the connection pool for finding open connections --- .../abstract/connection_pool.rb | 6 ++++ .../connection_handler_test.rb | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 activerecord/test/cases/connection_adapters/connection_handler_test.rb diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 61e44d09bf..7a900055a9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -352,6 +352,12 @@ module ActiveRecord @connection_pools[name] = ConnectionAdapters::ConnectionPool.new(spec) end + # Returns true if there are any active connections among the connection + # pools that the ConnectionHandler is managing. + def active_connections? + connection_pools.values.any? { |pool| pool.active_connection? } + end + # Returns any connections in use by the current thread back to the pool, # and also returns connections to the pool cached by threads that are no # longer alive. diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb new file mode 100644 index 0000000000..abf317768f --- /dev/null +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -0,0 +1,33 @@ +require "cases/helper" + +module ActiveRecord + module ConnectionAdapters + class ConnectionHandlerTest < ActiveRecord::TestCase + def setup + @handler = ConnectionHandler.new + @handler.establish_connection 'america', Base.connection_pool.spec + @klass = Struct.new(:name).new('america') + end + + def test_retrieve_connection + assert @handler.retrieve_connection(@klass) + end + + def test_active_connections? + assert !@handler.active_connections? + assert @handler.retrieve_connection(@klass) + assert @handler.active_connections? + @handler.clear_active_connections! + assert !@handler.active_connections? + end + + def test_retrieve_connection_pool_with_ar_base + assert_nil @handler.retrieve_connection_pool(ActiveRecord::Base) + end + + def test_retrieve_connection_pool + assert_not_nil @handler.retrieve_connection_pool(@klass) + end + end + end +end From aea1477362b640ebe52cf991b915ad32e7bf2571 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 17:47:46 -0700 Subject: [PATCH 14/54] make sure we have an active database connection before running each connection management test --- .../abstract/connection_specification.rb | 6 +++++- activerecord/test/cases/connection_management_test.rb | 9 +++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index d88720c8bf..bcd3abc08d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -116,7 +116,11 @@ module ActiveRecord connection_handler.remove_connection(klass) end - delegate :clear_active_connections!, :clear_reloadable_connections!, + def clear_active_connections! + connection_handler.clear_active_connections! + end + + delegate :clear_reloadable_connections!, :clear_all_connections!,:verify_active_connections!, :to => :connection_handler end end diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index 82ea46b41f..1313e28bb1 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -20,8 +20,9 @@ module ActiveRecord @app = App.new @management = ConnectionManagement.new(@app) - @connections_cleared = false - ActiveRecord::Base.stubs(:clear_active_connections!).with { @connections_cleared = true } + # make sure we have an active connection + assert ActiveRecord::Base.connection + assert ActiveRecord::Base.connection_handler.active_connections? end def test_app_delegation @@ -33,13 +34,13 @@ module ActiveRecord test "clears active connections after each call" do @management.call(@env) - assert @connections_cleared + assert !ActiveRecord::Base.connection_handler.active_connections? end test "doesn't clear active connections when running in a test case" do @env['rack.test'] = true @management.call(@env) - assert !@connections_cleared + assert ActiveRecord::Base.connection_handler.active_connections? end end end From e2b07ee000439d0bd41f725ff9f7ad53e52a7e9b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 28 Mar 2011 18:09:50 -0700 Subject: [PATCH 15/54] Added Base.http_basic_authenticate_with to do simple http basic authentication with a single class method call [DHH] --- actionpack/CHANGELOG | 37 +++++++++++++++++++ .../metal/http_authentication.rb | 30 ++++++++------- .../http_basic_authentication_test.rb | 16 ++++++++ 3 files changed, 69 insertions(+), 14 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index a90a7b37f7..7f1e7d1c1d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,42 @@ *Rails 3.1.0 (unreleased)* +* Added Base.http_basic_authenticate_with to do simple http basic authentication with a single class method call [DHH] + + class PostsController < ApplicationController + USER_NAME, PASSWORD = "dhh", "secret" + + before_filter :authenticate, :except => [ :index ] + + def index + render :text => "Everyone can see me!" + end + + def edit + render :text => "I'm only accessible if you know the password" + end + + private + def authenticate + authenticate_or_request_with_http_basic do |user_name, password| + user_name == USER_NAME && password == PASSWORD + end + end + end + + ..can now be written as + + class PostsController < ApplicationController + http_basic_authenticate_with :name => "dhh", "secret", :except => :index + + def index + render :text => "Everyone can see me!" + end + + def edit + render :text => "I'm only accessible if you know the password" + end + end + * Allow you to add `force_ssl` into controller to force browser to transfer data via HTTPS protocol on that particular controller. You can also specify `:only` or `:except` to specific it to particular action. [DHH and Prem Sichanugrist] * Allow FormHelper#form_for to specify the :method as a direct option instead of through the :html hash [DHH] diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 39c804d707..e28709d8cf 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -8,9 +8,7 @@ module ActionController # === Simple \Basic example # # class PostsController < ApplicationController - # USER_NAME, PASSWORD = "dhh", "secret" - # - # before_filter :authenticate, :except => [ :index ] + # http_basic_authenticate_with :name => "dhh", "secret", :except => :index # # def index # render :text => "Everyone can see me!" @@ -19,15 +17,7 @@ module ActionController # def edit # render :text => "I'm only accessible if you know the password" # end - # - # private - # def authenticate - # authenticate_or_request_with_http_basic do |user_name, password| - # user_name == USER_NAME && password == PASSWORD - # end - # end - # end - # + # end # # === Advanced \Basic example # @@ -115,6 +105,20 @@ module ActionController extend self module ControllerMethods + extend ActiveSupport::Concern + + module ClassMethods + def http_basic_authenticate_with(options = {}) + before_filter(options.except(:name, :password, :realm)) do + authenticate_or_request_with_http_basic(options[:realm] || "Application") do + authenticate_or_request_with_http_basic do |name, password| + name == options[:name] && password == options[:password] + end + end + end + end + end + def authenticate_or_request_with_http_basic(realm = "Application", &login_procedure) authenticate_with_http_basic(&login_procedure) || request_http_basic_authentication(realm) end @@ -378,7 +382,6 @@ module ActionController # # RewriteRule ^(.*)$ dispatch.fcgi [E=X-HTTP_AUTHORIZATION:%{HTTP:Authorization},QSA,L] module Token - extend self module ControllerMethods @@ -458,6 +461,5 @@ module ActionController controller.__send__ :render, :text => "HTTP Token: Access denied.\n", :status => :unauthorized end end - end end diff --git a/actionpack/test/controller/http_basic_authentication_test.rb b/actionpack/test/controller/http_basic_authentication_test.rb index 01c650a494..bd3e13e6fa 100644 --- a/actionpack/test/controller/http_basic_authentication_test.rb +++ b/actionpack/test/controller/http_basic_authentication_test.rb @@ -6,6 +6,8 @@ class HttpBasicAuthenticationTest < ActionController::TestCase before_filter :authenticate_with_request, :only => :display before_filter :authenticate_long_credentials, :only => :show + http_basic_authenticate_with :name => "David", :password => "Goliath", :only => :search + def index render :text => "Hello Secret" end @@ -17,6 +19,10 @@ class HttpBasicAuthenticationTest < ActionController::TestCase def show render :text => 'Only for loooooong credentials' end + + def search + render :text => 'All inline' + end private @@ -104,6 +110,16 @@ class HttpBasicAuthenticationTest < ActionController::TestCase assert assigns(:logged_in) assert_equal 'Definitely Maybe', @response.body end + + test "authenticate with class method" do + @request.env['HTTP_AUTHORIZATION'] = encode_credentials('David', 'Goliath') + get :search + assert_response :success + + @request.env['HTTP_AUTHORIZATION'] = encode_credentials('David', 'WRONG!') + get :search + assert_response :unauthorized + end private From 3d1e7c2645af6c187d5ab6d2a02bd1e7b9ad7af3 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Mon, 28 Mar 2011 18:15:41 -0700 Subject: [PATCH 16/54] Fix examples --- actionpack/CHANGELOG | 2 +- actionpack/lib/action_controller/metal/http_authentication.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 7f1e7d1c1d..6032c6b7da 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -26,7 +26,7 @@ ..can now be written as class PostsController < ApplicationController - http_basic_authenticate_with :name => "dhh", "secret", :except => :index + http_basic_authenticate_with :name => "dhh", :password => "secret", :except => :index def index render :text => "Everyone can see me!" diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index e28709d8cf..87c3239486 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -8,7 +8,7 @@ module ActionController # === Simple \Basic example # # class PostsController < ApplicationController - # http_basic_authenticate_with :name => "dhh", "secret", :except => :index + # http_basic_authenticate_with :name => "dhh", :password => "secret", :except => :index # # def index # render :text => "Everyone can see me!" From 51551a0a5bd6f7e4116bc3759a4d7c15634643dc Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 29 Mar 2011 02:59:24 +0700 Subject: [PATCH 17/54] Update the wildcard route to be non-greedy by default, therefore be able to match the (.:format) segment [#6605 state:resolved] After some discussion with Andrew White, it seems like this is a better approach for handling a wildcard route. However, user can still bring back the old behavior by supplying `:format => false` to the route. Signed-off-by: Andrew White --- actionpack/CHANGELOG | 8 +++++ actionpack/actionpack.gemspec | 2 +- .../lib/action_dispatch/routing/mapper.rb | 8 ++++- .../action_dispatch/routing/mapper_test.rb | 32 ++++++++++++++++++- railties/guides/source/routing.textile | 12 +++++++ 5 files changed, 59 insertions(+), 3 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6032c6b7da..3eba2281c4 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,13 @@ *Rails 3.1.0 (unreleased)* +* Wildcard route will always matching the optional format segment by default. For example if you have this route: + + map '*pages' => 'pages#show' + + by requesting '/foo/bar.json', your `params[:pages]` will be equals to "foo/bar" with the request format of JSON. If you want the old 3.0.x behavior back, you could supply `:format => false` like this: + + map '*pages' => 'pages#show', :format => false + * Added Base.http_basic_authenticate_with to do simple http basic authentication with a single class method call [DHH] class PostsController < ApplicationController diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index f6bc5e0d37..651f3b164a 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -26,7 +26,7 @@ Gem::Specification.new do |s| s.add_dependency('i18n', '~> 0.5.0') s.add_dependency('rack', '~> 1.2.1') s.add_dependency('rack-test', '~> 0.5.7') - s.add_dependency('rack-mount', '~> 0.6.13') + s.add_dependency('rack-mount', '~> 0.7.1') s.add_dependency('tzinfo', '~> 0.3.23') s.add_dependency('erubis', '~> 2.6.6') end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 14c424f24b..35be0b3a27 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -104,10 +104,16 @@ module ActionDispatch @options.reverse_merge!(:controller => /.+?/) end + # Add a constraint for wildcard route to make it non-greedy and match the + # optional format part of the route by default + if path.match(/\*([^\/]+)$/) && @options[:format] != false + @options.reverse_merge!(:"#{$1}" => /.+?/) + end + if @options[:format] == false @options.delete(:format) path - elsif path.include?(":format") || path.end_with?('/') || path.match(/^\/?\*/) + elsif path.include?(":format") || path.end_with?('/') path else "#{path}(.:format)" diff --git a/actionpack/test/action_dispatch/routing/mapper_test.rb b/actionpack/test/action_dispatch/routing/mapper_test.rb index e21b271907..f90de735b6 100644 --- a/actionpack/test/action_dispatch/routing/mapper_test.rb +++ b/actionpack/test/action_dispatch/routing/mapper_test.rb @@ -25,6 +25,10 @@ module ActionDispatch def conditions routes.map { |x| x[1] } end + + def requirements + routes.map { |x| x[2] } + end end def test_initialize @@ -50,8 +54,34 @@ module ActionDispatch def test_map_wildcard fakeset = FakeSet.new mapper = Mapper.new fakeset - mapper.match '/*path', :to => 'pages#show', :as => :page + mapper.match '/*path', :to => 'pages#show' + assert_equal '/*path(.:format)', fakeset.conditions.first[:path_info] + assert_equal /.+?/, fakeset.requirements.first[:path] + end + + def test_map_wildcard_with_other_element + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.match '/*path/foo/:bar', :to => 'pages#show' + assert_equal '/*path/foo/:bar(.:format)', fakeset.conditions.first[:path_info] + assert_nil fakeset.requirements.first[:path] + end + + def test_map_wildcard_with_multiple_wildcard + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.match '/*foo/*bar', :to => 'pages#show' + assert_equal '/*foo/*bar(.:format)', fakeset.conditions.first[:path_info] + assert_nil fakeset.requirements.first[:foo] + assert_equal /.+?/, fakeset.requirements.first[:bar] + end + + def test_map_wildcard_with_format_false + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.match '/*path', :to => 'pages#show', :format => false assert_equal '/*path', fakeset.conditions.first[:path_info] + assert_nil fakeset.requirements.first[:path] end end end diff --git a/railties/guides/source/routing.textile b/railties/guides/source/routing.textile index c447fd911a..58b75b9a1d 100644 --- a/railties/guides/source/routing.textile +++ b/railties/guides/source/routing.textile @@ -557,6 +557,18 @@ match '*a/foo/*b' => 'test#index' would match +zoo/woo/foo/bar/baz+ with +params[:a]+ equals +"zoo/woo"+, and +params[:b]+ equals +"bar/baz"+. +NOTE: Starting from Rails 3.1, wildcard route will always matching the optional format segment by default. For example if you have this route: + + +map '*pages' => 'pages#show' + + +NOTE: By requesting +"/foo/bar.json"+, your +params[:pages]+ will be equals to +"foo/bar"+ with the request format of JSON. If you want the old 3.0.x behavior back, you could supply +:format => false+ like this: + + +map '*pages' => 'pages#show', :format => false + + h4. Redirection You can redirect any path to another path using the +redirect+ helper in your router: From 84a4ef63a4b890dc29cd62543f52c863fb0f0dd5 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 29 Mar 2011 03:01:46 +0700 Subject: [PATCH 18/54] Move mapper_test to the appropriate location It seems like in 89c5b9aee7d7db95cec9e5a934c3761872ab107e Aaron actually put the test in action_dispatch folder. However, there's already a `test/dispatch` directory which I think it's more appropriate. Signed-off-by: Andrew White --- .../test/{action_dispatch/routing => dispatch}/mapper_test.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename actionpack/test/{action_dispatch/routing => dispatch}/mapper_test.rb (100%) diff --git a/actionpack/test/action_dispatch/routing/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb similarity index 100% rename from actionpack/test/action_dispatch/routing/mapper_test.rb rename to actionpack/test/dispatch/mapper_test.rb From b155fdadf334cff32a7e648c86c3c97f2f51257f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 29 Mar 2011 12:51:58 +0100 Subject: [PATCH 19/54] Change exists? so that it doesn't instantiate records [#6127 state:resolved] --- .../lib/active_record/relation/finder_methods.rb | 14 ++++++++++---- activerecord/test/cases/finder_test.rb | 5 +++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 563843f3cc..8fa315bdf3 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -183,7 +183,9 @@ module ActiveRecord def exists?(id = nil) id = id.id if ActiveRecord::Base === id - relation = select("1").limit(1) + join_dependency = construct_join_dependency_for_association_find + relation = construct_relation_for_association_find(join_dependency) + relation = relation.except(:select).select("1").limit(1) case id when Array, Hash @@ -192,14 +194,13 @@ module ActiveRecord relation = relation.where(table[primary_key].eq(id)) if id end - relation.first ? true : false + connection.select_value(relation.to_sql) ? true : false end protected def find_with_associations - including = (@eager_load_values + @includes_values).uniq - join_dependency = ActiveRecord::Associations::JoinDependency.new(@klass, including, []) + join_dependency = construct_join_dependency_for_association_find relation = construct_relation_for_association_find(join_dependency) rows = connection.select_all(relation.to_sql, 'SQL', relation.bind_values) join_dependency.instantiate(rows) @@ -207,6 +208,11 @@ module ActiveRecord [] end + def construct_join_dependency_for_association_find + including = (@eager_load_values + @includes_values).uniq + ActiveRecord::Associations::JoinDependency.new(@klass, including, []) + end + def construct_relation_for_association_calculations including = (@eager_load_values + @includes_values).uniq join_dependency = ActiveRecord::Associations::JoinDependency.new(@klass, including, arel.froms.first) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 543981b4a0..316113634b 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -74,6 +74,11 @@ class FinderTest < ActiveRecord::TestCase end end + def test_exists_does_not_instantiate_records + Developer.expects(:instantiate).never + Developer.exists? + end + def test_find_by_array_of_one_id assert_kind_of(Array, Topic.find([ 1 ])) assert_equal(1, Topic.find([ 1 ]).length) From 52351bcfeb833b00366f91e965c057f431b5a8aa Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Tue, 29 Mar 2011 09:40:42 -0300 Subject: [PATCH 20/54] Remove 'warning: ambiguous first argument' when running ActionPack tests Signed-off-by: Santiago Pastorino --- actionpack/test/dispatch/mapper_test.rb | 4 ++-- actionpack/test/template/date_helper_test.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index f90de735b6..b6c08ffc33 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -56,7 +56,7 @@ module ActionDispatch mapper = Mapper.new fakeset mapper.match '/*path', :to => 'pages#show' assert_equal '/*path(.:format)', fakeset.conditions.first[:path_info] - assert_equal /.+?/, fakeset.requirements.first[:path] + assert_equal(/.+?/, fakeset.requirements.first[:path]) end def test_map_wildcard_with_other_element @@ -73,7 +73,7 @@ module ActionDispatch mapper.match '/*foo/*bar', :to => 'pages#show' assert_equal '/*foo/*bar(.:format)', fakeset.conditions.first[:path_info] assert_nil fakeset.requirements.first[:foo] - assert_equal /.+?/, fakeset.requirements.first[:bar] + assert_equal(/.+?/, fakeset.requirements.first[:bar]) end def test_map_wildcard_with_format_false diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb index aca2fef170..fd1f824a39 100644 --- a/actionpack/test/template/date_helper_test.rb +++ b/actionpack/test/template/date_helper_test.rb @@ -2720,11 +2720,11 @@ class DateHelperTest < ActionView::TestCase end def test_time_tag_pubdate_option - assert_match /.*<\/time>/, time_tag(Time.now, :pubdate => true) + assert_match(/.*<\/time>/, time_tag(Time.now, :pubdate => true)) end def test_time_tag_with_given_text - assert_match /Right now<\/time>/, time_tag(Time.now, 'Right now') + assert_match(/Right now<\/time>/, time_tag(Time.now, 'Right now')) end def test_time_tag_with_different_format From e8d20b858d004e26c3b8c25aae099fce2eca6857 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 29 Mar 2011 07:29:10 -0700 Subject: [PATCH 21/54] Dont call authenticate_or_request_with_http_basic twice --- .../lib/action_controller/metal/http_authentication.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 87c3239486..b98429792d 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -110,10 +110,8 @@ module ActionController module ClassMethods def http_basic_authenticate_with(options = {}) before_filter(options.except(:name, :password, :realm)) do - authenticate_or_request_with_http_basic(options[:realm] || "Application") do - authenticate_or_request_with_http_basic do |name, password| - name == options[:name] && password == options[:password] - end + authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password| + name == options[:name] && password == options[:password] end end end From a9dafbb28de3e34c31ebf184fbc4e2042c7ff207 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 29 Mar 2011 15:08:40 +0100 Subject: [PATCH 22/54] Delegate first!, last!, any? and many? to scoped --- activerecord/lib/active_record/base.rb | 3 ++- activerecord/test/cases/finder_test.rb | 16 +++++++++++++++ activerecord/test/cases/named_scope_test.rb | 17 +++++++++++++++- .../source/active_record_querying.textile | 20 +++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 616b5cc3b4..fe81c7dc2f 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -437,7 +437,8 @@ module ActiveRecord #:nodoc: self._attr_readonly = [] class << self # Class methods - delegate :find, :first, :last, :all, :destroy, :destroy_all, :exists?, :delete, :delete_all, :update, :update_all, :to => :scoped + delegate :find, :first, :first!, :last, :last!, :all, :exists?, :any?, :many?, :to => :scoped + delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, :to => :scoped delegate :find_each, :find_in_batches, :to => :scoped delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :create_with, :to => :scoped delegate :count, :average, :minimum, :maximum, :sum, :calculate, :to => :scoped diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 316113634b..1ec00b15b2 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -208,6 +208,14 @@ class FinderTest < ActiveRecord::TestCase end end + def test_model_class_responds_to_first_bang + assert_equal topics(:first), Topic.first! + assert_raises ActiveRecord::RecordNotFound do + Topic.delete_all + Topic.first! + end + end + def test_last_bang_present assert_nothing_raised do assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").last! @@ -220,6 +228,14 @@ class FinderTest < ActiveRecord::TestCase end end + def test_model_class_responds_to_last_bang + assert_equal topics(:fourth), Topic.last! + assert_raises ActiveRecord::RecordNotFound do + Topic.delete_all + Topic.last! + end + end + def test_unexisting_record_exception_handling assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1).parent diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index fb050c3e52..9b20ea08de 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -64,7 +64,7 @@ class NamedScopeTest < ActiveRecord::TestCase assert Reply.scopes.include?(:base) assert_equal Reply.find(:all), Reply.base end - + def test_classes_dont_inherit_subclasses_scopes assert !ActiveRecord::Base.scopes.include?(:base) end @@ -246,6 +246,12 @@ class NamedScopeTest < ActiveRecord::TestCase assert_no_queries { assert topics.any? } end + def test_model_class_should_respond_to_any + assert Topic.any? + Topic.delete_all + assert !Topic.any? + end + def test_many_should_not_load_results topics = Topic.base assert_queries(2) do @@ -280,6 +286,15 @@ class NamedScopeTest < ActiveRecord::TestCase assert Topic.base.many? end + def test_model_class_should_respond_to_many + Topic.delete_all + assert !Topic.many? + Topic.create! + assert !Topic.many? + Topic.create! + assert Topic.many? + end + def test_should_build_on_top_of_scope topic = Topic.approved.build({}) assert topic.approved diff --git a/railties/guides/source/active_record_querying.textile b/railties/guides/source/active_record_querying.textile index 2c5d9e67e3..f3a10b8b92 100644 --- a/railties/guides/source/active_record_querying.textile +++ b/railties/guides/source/active_record_querying.textile @@ -962,6 +962,26 @@ Client.exists? The above returns +false+ if the +clients+ table is empty and +true+ otherwise. +You can also use +any?+ and +many?+ to check for existence on a model or relation. + + +# via a model +Post.any? +Post.many? + +# via a named scope +Post.recent.any? +Post.recent.many? + +# via a relation +Post.where(:published => true).any? +Post.where(:published => true).many? + +# via an association +Post.first.categories.any? +Post.first.categories.many? + + h3. Calculations This section uses count as an example method in this preamble, but the options described apply to all sub-sections. From 555d0163897601010ab1305f41ed393ec517b61e Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sun, 27 Mar 2011 09:54:38 +0100 Subject: [PATCH 23/54] Quote find_in_batches ORDER BY clause [#6620 state:resolved] --- .../active_record/attribute_methods/primary_key.rb | 13 ++++++++++++- activerecord/lib/active_record/relation.rb | 2 +- activerecord/lib/active_record/relation/batches.rb | 2 +- activerecord/test/cases/batches_test.rb | 10 ++++++++++ activerecord/test/cases/primary_keys_test.rb | 9 +++++++++ 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index fcdd31ddea..5f06452247 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -17,6 +17,11 @@ module ActiveRecord @primary_key ||= reset_primary_key end + # Returns a quoted version of the primary key name, used to construct SQL statements. + def quoted_primary_key + @quoted_primary_key ||= connection.quote_column_name(primary_key) + end + def reset_primary_key #:nodoc: key = self == base_class ? get_primary_key(base_class.name) : base_class.primary_key @@ -43,7 +48,12 @@ module ActiveRecord end attr_accessor :original_primary_key - attr_writer :primary_key + + # Attribute writer for the primary key column + def primary_key=(value) + @quoted_primary_key = nil + @primary_key = value + end # Sets the name of the primary key column to use to the given value, # or (if the value is nil or false) to the value returned by the given @@ -53,6 +63,7 @@ module ActiveRecord # set_primary_key "sysid" # end def set_primary_key(value = nil, &block) + @quoted_primary_key = nil @primary_key ||= '' self.original_primary_key = @primary_key value &&= value.to_s diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 8e545f9cad..896daf516e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -12,7 +12,7 @@ module ActiveRecord # These are explicitly delegated to improve performance (avoids method_missing) delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to => :to_a - delegate :table_name, :primary_key, :to => :klass + delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :to => :klass attr_reader :table, :klass, :loaded attr_accessor :extensions diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index bf5a60f458..d52b84179f 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -83,7 +83,7 @@ module ActiveRecord private def batch_order - "#{table_name}.#{primary_key} ASC" + "#{quoted_table_name}.#{quoted_primary_key} ASC" end end end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index dc0e0da4c5..6620464d6a 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -83,4 +83,14 @@ class EachTest < ActiveRecord::TestCase Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch } end end + + def test_find_in_batches_should_quote_batch_order + c = Post.connection + assert_sql(/ORDER BY #{c.quote_table_name('posts')}.#{c.quote_column_name('id')}/) do + Post.find_in_batches(:batch_size => 1) do |batch| + assert_kind_of Array, batch + assert_kind_of Post, batch.first + end + end + end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 63d8c7d1c1..05a41d8a0a 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -136,4 +136,13 @@ class PrimaryKeysTest < ActiveRecord::TestCase assert_nil ActiveRecord::Base.connection.primary_key('developers_projects') end end + + def test_quoted_primary_key_after_set_primary_key + k = Class.new( ActiveRecord::Base ) + assert_equal k.connection.quote_column_name("id"), k.quoted_primary_key + k.primary_key = "foo" + assert_equal k.connection.quote_column_name("foo"), k.quoted_primary_key + k.set_primary_key "bar" + assert_equal k.connection.quote_column_name("bar"), k.quoted_primary_key + end end From 94907035b6a4a8e415cf19471a7ae77fac937209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 29 Mar 2011 19:30:59 +0200 Subject: [PATCH 24/54] Pass the proper method_name instead of hardcoding to action_name. Conflicts: actionpack/lib/action_controller/metal/implicit_render.rb --- .../lib/abstract_controller/callbacks.rb | 2 +- .../action_controller/metal/implicit_render.rb | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 1943ca4436..95992c2698 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -14,7 +14,7 @@ module AbstractController # Override AbstractController::Base's process_action to run the # process_action callbacks around the normal behavior. def process_action(method_name, *args) - run_callbacks(:process_action, action_name) do + run_callbacks(:process_action, method_name) do super end end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index cfa7004048..4b301c0d90 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -1,9 +1,13 @@ module ActionController module ImplicitRender - def send_action(*) - ret = super - default_render unless response_body - ret + def send_action(method, *args) + if respond_to?(method, true) + ret = super + default_render unless response_body + ret + else + default_render + end end def default_render @@ -11,10 +15,8 @@ module ActionController end def method_for_action(action_name) - super || begin - if template_exists?(action_name.to_s, _prefixes) - "default_render" - end + super || if template_exists?(action_name.to_s, _prefixes) + action_name.to_s end end end From e5246092d1ce30961af4f7d9b5ad86071298cf1c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 15:37:07 -0700 Subject: [PATCH 25/54] proxy body responses so we close database connections after body is flushed --- activerecord/CHANGELOG | 3 ++ .../abstract/connection_pool.rb | 31 ++++++++++++++----- .../test/cases/connection_management_test.rb | 24 ++++++++++++-- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 3b6a7d7208..e536d2b408 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,8 @@ *Rails 3.1.0 (unreleased)* +* ConnectionManagement middleware is changed to clean up the connection pool + after the rack body has been flushed. + * Added an update_column method on ActiveRecord. This new method updates a given attribute on an object, skipping validations and callbacks. It is recommended to use #update_attribute unless you are sure you do not want to execute any callback, including the modification of the updated_at column. It should not be called on new records. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 7a900055a9..5e12f80263 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -417,18 +417,35 @@ module ActiveRecord end class ConnectionManagement + class Proxy # :nodoc: + attr_reader :body, :testing + + def initialize(body, testing = false) + @body = body + @testing = testing + end + + def each(&block) + body.each(&block) + end + + def close + body.close if body.respond_to?(:close) + + # Don't return connection (and perform implicit rollback) if + # this request is a part of integration test + ActiveRecord::Base.clear_active_connections! unless testing + end + end + def initialize(app) @app = app end def call(env) - @app.call(env) - ensure - # Don't return connection (and perform implicit rollback) if - # this request is a part of integration test - unless env.key?("rack.test") - ActiveRecord::Base.clear_active_connections! - end + status, headers, body = @app.call(env) + + [status, headers, Proxy.new(body, env.key?('rack.test'))] end end end diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index 1313e28bb1..3734f8e5f0 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -11,7 +11,7 @@ module ActiveRecord def call(env) @calls << env - [200, {}, [['hi mom']]] + [200, {}, ['hi mom']] end end @@ -32,11 +32,31 @@ module ActiveRecord assert_equal [@env], @app.calls end - test "clears active connections after each call" do + def test_connections_are_active_after_call @management.call(@env) + assert ActiveRecord::Base.connection_handler.active_connections? + end + + def test_body_responds_to_each + _, _, body = @management.call(@env) + bits = [] + body.each { |bit| bits << bit } + assert_equal ['hi mom'], bits + end + + def test_connections_are_cleared_after_body_close + _, _, body = @management.call(@env) + body.close assert !ActiveRecord::Base.connection_handler.active_connections? end + def test_active_connections_are_not_cleared_on_body_close_during_test + @env['rack.test'] = true + _, _, body = @management.call(@env) + body.close + assert ActiveRecord::Base.connection_handler.active_connections? + end + test "doesn't clear active connections when running in a test case" do @env['rack.test'] = true @management.call(@env) From 3b2a032677a2261499aa5d2de019f31f1173a858 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 15:42:32 -0700 Subject: [PATCH 26/54] clearing active connections in the ConnectionManagement middleware if an exception happens --- .../connection_adapters/abstract/connection_pool.rb | 3 +++ activerecord/test/cases/connection_management_test.rb | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 5e12f80263..45900d27dc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -446,6 +446,9 @@ module ActiveRecord status, headers, body = @app.call(env) [status, headers, Proxy.new(body, env.key?('rack.test'))] + rescue + ActiveRecord::Base.clear_active_connections! + raise end end end diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index 3734f8e5f0..0d4a9a287e 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -57,6 +57,13 @@ module ActiveRecord assert ActiveRecord::Base.connection_handler.active_connections? end + def test_connections_closed_if_exception + app = Class.new(App) { def call(env); raise; end }.new + explosive = ConnectionManagement.new(app) + assert_raises(RuntimeError) { explosive.call(@env) } + assert !ActiveRecord::Base.connection_handler.active_connections? + end + test "doesn't clear active connections when running in a test case" do @env['rack.test'] = true @management.call(@env) From c7b7c6ad1c773102753f1a11b857d0e37ceb6a21 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 15:47:16 -0700 Subject: [PATCH 27/54] make sure that active connections are not cleared during test when an exception happens --- .../connection_adapters/abstract/connection_pool.rb | 6 ++++-- activerecord/test/cases/connection_management_test.rb | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 45900d27dc..b4db1eed18 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -443,11 +443,13 @@ module ActiveRecord end def call(env) + testing = env.key?('rack.test') + status, headers, body = @app.call(env) - [status, headers, Proxy.new(body, env.key?('rack.test'))] + [status, headers, Proxy.new(body, testing)] rescue - ActiveRecord::Base.clear_active_connections! + ActiveRecord::Base.clear_active_connections! unless testing raise end end diff --git a/activerecord/test/cases/connection_management_test.rb b/activerecord/test/cases/connection_management_test.rb index 0d4a9a287e..85871aebdf 100644 --- a/activerecord/test/cases/connection_management_test.rb +++ b/activerecord/test/cases/connection_management_test.rb @@ -64,6 +64,14 @@ module ActiveRecord assert !ActiveRecord::Base.connection_handler.active_connections? end + def test_connections_not_closed_if_exception_and_test + @env['rack.test'] = true + app = Class.new(App) { def call(env); raise; end }.new + explosive = ConnectionManagement.new(app) + assert_raises(RuntimeError) { explosive.call(@env) } + assert ActiveRecord::Base.connection_handler.active_connections? + end + test "doesn't clear active connections when running in a test case" do @env['rack.test'] = true @management.call(@env) From 6067d29a1fd78a60c44f18d4c0815002ff2f8c84 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 17:09:22 -0700 Subject: [PATCH 28/54] oracle stores this with microseconds, so convert to seconds before comparing --- activerecord/test/cases/persistence_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 8e6141eb81..9aa13f04cd 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -457,7 +457,7 @@ class PersistencesTest < ActiveRecord::TestCase assert_equal prev_month, developer.updated_at developer.reload - assert_equal prev_month, developer.updated_at + assert_equal prev_month.to_i, developer.updated_at.to_i end def test_update_column_with_one_changed_and_one_updated From 58becf116580c37c63b89f4a660ebe293f6e7c4e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 17:27:32 -0700 Subject: [PATCH 29/54] order is not guaranteed by this select, so add an order and call first! --- activerecord/test/cases/finder_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 1ec00b15b2..014588b6d9 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -209,7 +209,7 @@ class FinderTest < ActiveRecord::TestCase end def test_model_class_responds_to_first_bang - assert_equal topics(:first), Topic.first! + assert_equal topics(:first), Topic.order(:id).first! assert_raises ActiveRecord::RecordNotFound do Topic.delete_all Topic.first! From 2be383b94619595a2b4886e35f599757eab6c878 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 17:50:39 -0700 Subject: [PATCH 30/54] test against AR class rather than the relation (thanks Andrew White!) --- activerecord/test/cases/finder_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 014588b6d9..3c242667eb 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -209,9 +209,9 @@ class FinderTest < ActiveRecord::TestCase end def test_model_class_responds_to_first_bang - assert_equal topics(:first), Topic.order(:id).first! + assert Topic.first! + Topic.delete_all assert_raises ActiveRecord::RecordNotFound do - Topic.delete_all Topic.first! end end From ba51aa0b1ba7488011388ed4cf33f72917da0729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 30 Mar 2011 17:22:05 +0200 Subject: [PATCH 31/54] Make action_method? public and change implicit rendering to override it instead. --- actionpack/lib/abstract_controller/base.rb | 33 ++++++++++--------- .../metal/implicit_render.rb | 6 ++-- .../new_base/render_implicit_action_test.rb | 5 +++ 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index c384fd0978..07ff5ad9f3 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -128,20 +128,23 @@ module AbstractController self.class.action_methods end - private + # Returns true if the name can be considered an action. This can + # be overridden in subclasses to modify the semantics of what + # can be considered an action. + # + # For instance, this is overriden by ActionController to add + # the implicit rendering feature. + # + # ==== Parameters + # * name - The name of an action to be tested + # + # ==== Returns + # * TrueClass, FalseClass + def action_method?(name) + self.class.action_methods.include?(name) + end - # Returns true if the name can be considered an action. This can - # be overridden in subclasses to modify the semantics of what - # can be considered an action. - # - # ==== Parameters - # * name - The name of an action to be tested - # - # ==== Returns - # * TrueClass, FalseClass - def action_method?(name) - self.class.action_methods.include?(name) - end + private # Call the action. Override this in a subclass to modify the # behavior around processing an action. This, and not #process, @@ -160,8 +163,8 @@ module AbstractController # If the action name was not found, but a method called "action_missing" # was found, #method_for_action will return "_handle_action_missing". # This method calls #action_missing with the current action name. - def _handle_action_missing - action_missing(@_action_name) + def _handle_action_missing(*args) + action_missing(@_action_name, *args) end # Takes an action name and returns the name of the method that will diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 4b301c0d90..678f4ca763 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -14,10 +14,8 @@ module ActionController render end - def method_for_action(action_name) - super || if template_exists?(action_name.to_s, _prefixes) - action_name.to_s - end + def action_method?(action_name) + super || template_exists?(action_name.to_s, _prefixes) end end end diff --git a/actionpack/test/controller/new_base/render_implicit_action_test.rb b/actionpack/test/controller/new_base/render_implicit_action_test.rb index 9f69d20329..667a9021be 100644 --- a/actionpack/test/controller/new_base/render_implicit_action_test.rb +++ b/actionpack/test/controller/new_base/render_implicit_action_test.rb @@ -24,5 +24,10 @@ module RenderImplicitAction assert_body "Hello hyphen-ated!" assert_status 200 end + + test "action_method? returns true for implicit actions" do + assert SimpleController.new.action_method?(:hello_world) + assert SimpleController.new.action_method?(:"hyphen-ated") + end end end From cfb6f77ac0574d3cd613c99fcc32135016b4284c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 30 Mar 2011 09:54:46 -0700 Subject: [PATCH 32/54] TableAlias leg ordering has changed, so change accordingly --- .../lib/active_record/associations/join_dependency/join_part.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/join_dependency/join_part.rb b/activerecord/lib/active_record/associations/join_dependency/join_part.rb index 3279e56e7d..2b1d888a9a 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_part.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_part.rb @@ -22,7 +22,7 @@ module ActiveRecord end def aliased_table - Arel::Nodes::TableAlias.new aliased_table_name, table + Arel::Nodes::TableAlias.new table, aliased_table_name end def ==(other) From 4f90b28e06589a51f4655e38e94c948aaf02d05f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 30 Mar 2011 16:29:34 -0300 Subject: [PATCH 33/54] Bring back AMo#i18n_key method --- activemodel/lib/active_model/errors.rb | 4 ++-- activemodel/lib/active_model/naming.rb | 15 ++++++++++----- activemodel/lib/active_model/translation.rb | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index c2f0228785..5e3cf510b0 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -295,8 +295,8 @@ module ActiveModel type = options.delete(:message) if options[:message].is_a?(Symbol) defaults = @base.class.lookup_ancestors.map do |klass| - [ :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.underscore}.attributes.#{attribute}.#{type}", - :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.underscore}.#{type}" ] + [ :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.i18n_key}.attributes.#{attribute}.#{type}", + :"#{@base.class.i18n_scope}.errors.models.#{klass.model_name.i18n_key}.#{type}" ] end defaults << options.delete(:message) diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index eb9b847509..315ccd40e9 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -4,7 +4,7 @@ require 'active_support/core_ext/module/introspection' module ActiveModel class Name < String - attr_reader :singular, :plural, :element, :collection, :partial_path, :route_key, :param_key + attr_reader :singular, :plural, :element, :collection, :partial_path, :route_key, :param_key, :i18n_key alias_method :cache_key, :collection def initialize(klass, namespace = nil) @@ -20,6 +20,7 @@ module ActiveModel @partial_path = "#{@collection}/#{@element}".freeze @param_key = (namespace ? _singularize(@unnamespaced) : @singular).freeze @route_key = (namespace ? ActiveSupport::Inflector.pluralize(@param_key) : @plural).freeze + @i18n_key = self.underscore.to_sym end # Transform the model name into a more humane format, using I18n. By default, @@ -33,7 +34,7 @@ module ActiveModel @klass.respond_to?(:i18n_scope) defaults = @klass.lookup_ancestors.map do |klass| - klass.model_name.underscore.to_sym + klass.model_name.i18n_key end defaults << options[:default] if options[:default] @@ -44,9 +45,10 @@ module ActiveModel end private - def _singularize(str) - ActiveSupport::Inflector.underscore(str).tr('/', '_') - end + + def _singularize(string, replacement='_') + ActiveSupport::Inflector.underscore(string).tr('/', replacement) + end end # == Active Model Naming @@ -62,6 +64,9 @@ module ActiveModel # BookCover.model_name # => "BookCover" # BookCover.model_name.human # => "Book cover" # + # BookCover.model_name.i18n_key # => "book_cover" + # BookModule::BookCover.model_name.i18n_key # => "book_module.book_cover" + # # Providing the functionality that ActiveModel::Naming provides in your object # is required to pass the Active Model Lint test. So either extending the provided # method below, or rolling your own is required. diff --git a/activemodel/lib/active_model/translation.rb b/activemodel/lib/active_model/translation.rb index dbb76244e4..920a133159 100644 --- a/activemodel/lib/active_model/translation.rb +++ b/activemodel/lib/active_model/translation.rb @@ -44,7 +44,7 @@ module ActiveModel # Specify +options+ with additional translating options. def human_attribute_name(attribute, options = {}) defaults = lookup_ancestors.map do |klass| - :"#{self.i18n_scope}.attributes.#{klass.model_name.underscore}.#{attribute}" + :"#{self.i18n_scope}.attributes.#{klass.model_name.i18n_key}.#{attribute}" end defaults << :"attributes.#{attribute}" From 58c3ec1b7b7ee073edf9c245de5d06426be60a25 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 30 Mar 2011 15:29:52 -0700 Subject: [PATCH 34/54] use assert_equal so we get normal error messages along with our custom failure message --- .../lib/action_dispatch/testing/assertions/routing.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 11e8c63fa0..b760db42e2 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -46,7 +46,7 @@ module ActionDispatch expected_options.stringify_keys! msg = build_message(message, "The recognized options did not match , difference: ", request.path_parameters, expected_options, expected_options.diff(request.path_parameters)) - assert_block(msg) { request.path_parameters == expected_options } + assert_equal(expected_options, request.path_parameters, msg) end # Asserts that the provided options can be used to generate the provided path. This is the inverse of +assert_recognizes+. @@ -84,11 +84,11 @@ module ActionDispatch found_extras = options.reject {|k, v| ! extra_keys.include? k} msg = build_message(message, "found extras , not ", found_extras, extras) - assert_block(msg) { found_extras == extras } + assert_equal(extras, found_extras, msg) msg = build_message(message, "The generated path did not match ", generated_path, expected_path) - assert_block(msg) { expected_path == generated_path } + assert_equal(expected_path, generated_path, msg) end # Asserts that path and options match both ways; in other words, it verifies that path generates From 286709336577c767498785bc7be486eefe3faa4b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 30 Mar 2011 21:16:29 -0700 Subject: [PATCH 35/54] Delegate pending to skip if Minitest is available --- .../lib/active_support/testing/pending.rb | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/activesupport/lib/active_support/testing/pending.rb b/activesupport/lib/active_support/testing/pending.rb index 3d119e2fba..feac7bc347 100644 --- a/activesupport/lib/active_support/testing/pending.rb +++ b/activesupport/lib/active_support/testing/pending.rb @@ -11,32 +11,36 @@ module ActiveSupport @@at_exit = false def pending(description = "", &block) - if description.is_a?(Symbol) - is_pending = $tags[description] - return block.call unless is_pending - end - - if block_given? - failed = false - - begin - block.call - rescue Exception - failed = true + if defined?(::MiniTest) + skip(description.blank? ? nil : description) + else + if description.is_a?(Symbol) + is_pending = $tags[description] + return block.call unless is_pending end - flunk("<#{description}> did not fail.") unless failed - end + if block_given? + failed = false - caller[0] =~ (/(.*):(.*):in `(.*)'/) - @@pending_cases << "#{$3} at #{$1}, line #{$2}" - print "P" + begin + block.call + rescue Exception + failed = true + end - @@at_exit ||= begin - at_exit do - puts "\nPending Cases:" - @@pending_cases.each do |test_case| - puts test_case + flunk("<#{description}> did not fail.") unless failed + end + + caller[0] =~ (/(.*):(.*):in `(.*)'/) + @@pending_cases << "#{$3} at #{$1}, line #{$2}" + print "P" + + @@at_exit ||= begin + at_exit do + puts "\nPending Cases:" + @@pending_cases.each do |test_case| + puts test_case + end end end end From a64abdda2505895fec6f0243db5928316c4df90a Mon Sep 17 00:00:00 2001 From: Amaia Castro Date: Thu, 31 Mar 2011 13:19:19 +0200 Subject: [PATCH 36/54] Explain in the method doc that you need to call respond_to at the class level in order to use respond_with. --- actionpack/lib/action_controller/metal/mime_responds.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index a2e06fe0a6..998bef6556 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -222,6 +222,9 @@ module ActionController #:nodoc: # is quite simple (it just needs to respond to call), you can even give # a proc to it. # + # In order to use respond_with, first you need to declare the formats your + # controller responds to in the class level with a call to respond_to. + # def respond_with(*resources, &block) raise "In order to use respond_with, first you need to declare the formats your " << "controller responds to in the class level" if self.class.mimes_for_respond_to.empty? From 9766997f4ce26fe0d97d7b9eebf885ddb517c80c Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Wed, 30 Mar 2011 20:53:42 +0200 Subject: [PATCH 37/54] when using respond_with with an invalid resource and custom options, the default response status and error messages should be returned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../lib/action_controller/metal/responder.rb | 3 ++- .../test/controller/mime_responds_test.rb | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 4b45413cf8..82d11c8481 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -156,7 +156,8 @@ module ActionController #:nodoc: if get? display resource elsif has_errors? - display resource.errors, :status => :unprocessable_entity + # bypass the options merging of display + controller.render format => resource.errors, :status => :unprocessable_entity elsif post? display resource, :status => :created, :location => api_location elsif has_empty_resource_definition? diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 5debf96232..eead857927 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -953,6 +953,23 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal 201, @response.status end + def test_using_resource_with_status_and_location_with_invalid_resource + errors = { :name => :invalid } + Customer.any_instance.stubs(:errors).returns(errors) + + @request.accept = "text/xml" + + post :using_resource_with_status_and_location + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + + put :using_resource_with_status_and_location + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + end + def test_using_resource_with_responder get :using_resource_with_responder assert_equal "Resource name is david", @response.body From 48404a751d7cab1556c390a5915c90947d56b46e Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 31 Mar 2011 18:18:11 +0200 Subject: [PATCH 38/54] only try to display an api template in responders if the request is a get or there are no errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../lib/action_controller/metal/responder.rb | 13 ++++++++---- .../test/controller/mime_responds_test.rb | 21 +++++++++++++++++++ ...ing_invalid_resource_with_template.xml.erb | 1 + 3 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 82d11c8481..3862a68b44 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -131,7 +131,11 @@ module ActionController #:nodoc: # responds to :to_format and display it. # def to_format - default_render + if get? || !has_errors? + default_render + else + display_errors + end rescue ActionView::MissingTemplate => e api_behavior(e) end @@ -155,9 +159,6 @@ module ActionController #:nodoc: if get? display resource - elsif has_errors? - # bypass the options merging of display - controller.render format => resource.errors, :status => :unprocessable_entity elsif post? display resource, :status => :created, :location => api_location elsif has_empty_resource_definition? @@ -210,6 +211,10 @@ module ActionController #:nodoc: controller.render given_options.merge!(options).merge!(format => resource) end + def display_errors + controller.render format => resource.errors, :status => :unprocessable_entity + end + # Check whether the resource has errors. # def has_errors? diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index eead857927..4fde08b3f5 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -558,6 +558,10 @@ class RespondWithController < ActionController::Base respond_with(resource, :location => "http://test.host/", :status => :created) end + def using_invalid_resource_with_template + respond_with(resource) + end + def using_resource_with_responder responder = proc { |c, r, o| c.render :text => "Resource name is #{r.first.name}" } respond_with(resource, :responder => responder) @@ -970,6 +974,23 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal nil, @response.location end + def test_using_invalid_resource_with_template + errors = { :name => :invalid } + Customer.any_instance.stubs(:errors).returns(errors) + + @request.accept = "text/xml" + + post :using_invalid_resource_with_template + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + + put :using_invalid_resource_with_template + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + end + def test_using_resource_with_responder get :using_resource_with_responder assert_equal "Resource name is david", @response.body diff --git a/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb b/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb new file mode 100644 index 0000000000..bf5869ed22 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb @@ -0,0 +1 @@ +I should not be displayed \ No newline at end of file From b45302d7676a5e38d82662f9068ee6d832ff2e3c Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 31 Mar 2011 18:25:29 +0200 Subject: [PATCH 39/54] pass respond_with options to controller render when using a template for api navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../metal/implicit_render.rb | 4 ++-- .../action_controller/metal/mime_responds.rb | 4 ++-- .../lib/action_controller/metal/responder.rb | 2 +- .../test/controller/mime_responds_test.rb | 19 +++++++++++++++++++ .../using_options_with_template.xml.erb | 1 + 5 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 678f4ca763..3ec0c4c6a4 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -10,8 +10,8 @@ module ActionController end end - def default_render - render + def default_render(*args) + render(*args) end def action_method?(action_name) diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index a2e06fe0a6..7a8fa7bd86 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -189,7 +189,7 @@ module ActionController #:nodoc: raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given? if response = retrieve_response_from_mimes(mimes, &block) - response.call + response.call(nil) end end @@ -259,7 +259,7 @@ module ActionController #:nodoc: # def retrieve_response_from_mimes(mimes=nil, &block) mimes ||= collect_mimes_from_class_level - collector = Collector.new(mimes) { default_render } + collector = Collector.new(mimes) { |options| default_render(options || {}) } block.call(collector) if block_given? if format = request.negotiate_mime(collector.order) diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 3862a68b44..3d009e3de3 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -187,7 +187,7 @@ module ActionController #:nodoc: # controller. # def default_render - @default_response.call + @default_response.call(options) end # Display is just a shortcut to render a resource with the current format. diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 4fde08b3f5..41f80d0784 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -562,6 +562,11 @@ class RespondWithController < ActionController::Base respond_with(resource) end + def using_options_with_template + @customer = resource + respond_with(@customer, :status => 123, :location => "http://test.host/") + end + def using_resource_with_responder responder = proc { |c, r, o| c.render :text => "Resource name is #{r.first.name}" } respond_with(resource, :responder => responder) @@ -991,6 +996,20 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal nil, @response.location end + def test_using_options_with_template + @request.accept = "text/xml" + + post :using_options_with_template + assert_equal "david", @response.body + assert_equal 123, @response.status + assert_equal "http://test.host/", @response.location + + put :using_options_with_template + assert_equal "david", @response.body + assert_equal 123, @response.status + assert_equal "http://test.host/", @response.location + end + def test_using_resource_with_responder get :using_resource_with_responder assert_equal "Resource name is david", @response.body diff --git a/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb b/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb new file mode 100644 index 0000000000..b313017913 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb @@ -0,0 +1 @@ +<%= @customer.name %> \ No newline at end of file From 7a9dafd96cd4735b9134081f1d8103a6c62a6809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 31 Mar 2011 19:00:05 +0200 Subject: [PATCH 40/54] Improve docs. --- .../lib/action_controller/metal/responder.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 3d009e3de3..59a3621f72 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -77,6 +77,37 @@ module ActionController #:nodoc: # # respond_with(@project, :manager, @task) # + # === Custom options + # + # respond_with also allow you to pass options that are forwarded + # to the underlying render call. Those options are only applied success + # scenarios. For instance, you can do the following in the create method above: + # + # def create + # @project = Project.find(params[:project_id]) + # @task = @project.comments.build(params[:task]) + # flash[:notice] = 'Task was successfully created.' if @task.save + # respond_with(@project, @task, :status => 201) + # end + # + # This will return status 201 if the task was saved with success. If not, + # it will simply ignore the given options and return status 422 and the + # resource errors. To customize the failure scenario, you can pass a + # a block to respond_with: + # + # def create + # @project = Project.find(params[:project_id]) + # @task = @project.comments.build(params[:task]) + # respond_with(@project, @task, :status => 201) do |format| + # if @task.save + # flash[:notice] = 'Task was successfully created.' + # else + # format.html { render "some_special_template" } + # end + # end + # end + # + # Using respond_with with a block follows the same syntax as respond_to. class Responder attr_reader :controller, :request, :format, :resource, :resources, :options From b27057731cba8678265026f0fdfe75f5ae411d4d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 31 Mar 2011 12:11:03 -0700 Subject: [PATCH 41/54] Use Turn to format all Rails tests and enable the natural language case names --- activesupport/lib/active_support/test_case.rb | 5 +++++ rails.gemspec | 1 + 2 files changed, 6 insertions(+) diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index fb52fc7083..a48656192c 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -15,6 +15,11 @@ rescue LoadError Mocha.const_set :ExpectationError, Class.new(StandardError) end +# Added by Turn to support natural case names in the output formatting +if defined?(MiniTest) && MiniTest::Unit.respond_to?(:use_natural_language_case_names=) + MiniTest::Unit.use_natural_language_case_names = true +end + module ActiveSupport class TestCase < ::Test::Unit::TestCase if defined? MiniTest diff --git a/rails.gemspec b/rails.gemspec index 98b5f46554..2711d95f34 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -25,5 +25,6 @@ Gem::Specification.new do |s| s.add_dependency('activeresource', version) s.add_dependency('actionmailer', version) s.add_dependency('railties', version) + s.add_dependency('turn', '~> 0.8.2') s.add_dependency('bundler', '~> 1.0') end From edf7c9a6a3331bfc0beabc9dc9c8beac22677e53 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 31 Mar 2011 12:17:28 -0700 Subject: [PATCH 42/54] require turn only for minitest --- activesupport/lib/active_support/test_case.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index a48656192c..d25b092fb4 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -16,8 +16,12 @@ rescue LoadError end # Added by Turn to support natural case names in the output formatting -if defined?(MiniTest) && MiniTest::Unit.respond_to?(:use_natural_language_case_names=) - MiniTest::Unit.use_natural_language_case_names = true +if defined?(MiniTest) + require 'turn' + + if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) + MiniTest::Unit.use_natural_language_case_names = true + end end module ActiveSupport From 6eff04499e28865890e1ae3915fe80e4903a997b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 31 Mar 2011 12:25:04 -0700 Subject: [PATCH 43/54] Add using Turn with natural language test case names if the library is available (which it will be in Rails 3.1) [DHH] --- activesupport/CHANGELOG | 2 ++ activesupport/lib/active_support/test_case.rb | 19 ++--------- .../lib/active_support/testing/mochaing.rb | 7 ++++ .../active_support/testing/turn_formatting.rb | 33 +++++++++++++++++++ 4 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 activesupport/lib/active_support/testing/mochaing.rb create mode 100644 activesupport/lib/active_support/testing/turn_formatting.rb diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 373236ce9a..1be5f39ac4 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* Add using Turn with natural language test case names if the library is available (which it will be in Rails 3.1) [DHH] + * LocalCache strategy is now a real middleware class, not an anonymous class posing for pictures. diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index d25b092fb4..7ac9ad46e9 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -5,25 +5,10 @@ require 'active_support/testing/deprecation' require 'active_support/testing/declarative' require 'active_support/testing/pending' require 'active_support/testing/isolation' +require 'active_support/testing/turn_formatting' +require 'active_support/testing/mochaing' require 'active_support/core_ext/kernel/reporting' -begin - silence_warnings { require 'mocha' } -rescue LoadError - # Fake Mocha::ExpectationError so we can rescue it in #run. Bleh. - Object.const_set :Mocha, Module.new - Mocha.const_set :ExpectationError, Class.new(StandardError) -end - -# Added by Turn to support natural case names in the output formatting -if defined?(MiniTest) - require 'turn' - - if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) - MiniTest::Unit.use_natural_language_case_names = true - end -end - module ActiveSupport class TestCase < ::Test::Unit::TestCase if defined? MiniTest diff --git a/activesupport/lib/active_support/testing/mochaing.rb b/activesupport/lib/active_support/testing/mochaing.rb new file mode 100644 index 0000000000..4ad75a6681 --- /dev/null +++ b/activesupport/lib/active_support/testing/mochaing.rb @@ -0,0 +1,7 @@ +begin + silence_warnings { require 'mocha' } +rescue LoadError + # Fake Mocha::ExpectationError so we can rescue it in #run. Bleh. + Object.const_set :Mocha, Module.new + Mocha.const_set :ExpectationError, Class.new(StandardError) +end \ No newline at end of file diff --git a/activesupport/lib/active_support/testing/turn_formatting.rb b/activesupport/lib/active_support/testing/turn_formatting.rb new file mode 100644 index 0000000000..ed9381e298 --- /dev/null +++ b/activesupport/lib/active_support/testing/turn_formatting.rb @@ -0,0 +1,33 @@ +# Turn gives you prettier formatting for MiniTest and inline failure reporting. +# It also allows us to report test cases in natural language rather than with underscores. Example: +# +# CommentsControllerTest: +# PASS the truth (0.03s) +# +# APITest +# test_api_without_subdomain PASS +# test_create_milestone_using_typed_xml FAIL +# /test/integration/api_test.rb:50:in `test_create_milestone_using_typed_xml' +# <2006-05-01> expected but was +# . +# test_create_milestone_using_untyped_xml FAIL +# /test/integration/api_test.rb:38:in `test_create_milestone_using_untyped_xml' +# <2006-05-01> expected but was +# . + +# +# vs: +# +# .FF + +if defined?(MiniTest) + begin + silence_warnings { require 'turn' } + + if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) + MiniTest::Unit.use_natural_language_case_names = true + end + rescue LoadError + # If there's no turn, that's fine, it's just formatting + end +end \ No newline at end of file From caf0a72c85ddd30f901c33a2d72cf2a7453ed469 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 31 Mar 2011 14:33:24 -0700 Subject: [PATCH 44/54] Direct logging of Active Record to STDOUT so it's shown inline with the results in the console [DHH] --- railties/CHANGELOG | 2 ++ railties/lib/rails/commands/console.rb | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/railties/CHANGELOG b/railties/CHANGELOG index c1e0a214d2..07e7e461de 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* Direct logging of Active Record to STDOUT so it's shown inline with the results in the console [DHH] + * Added `config.force_ssl` configuration which loads Rack::SSL middleware and force all requests to be under HTTPS protocol [DHH, Prem Sichanugrist, and Josh Peek] * Added `rails plugin new` command which generates rails plugin with gemspec, tests and dummy application for testing [Piotr Sarnacki] diff --git a/railties/lib/rails/commands/console.rb b/railties/lib/rails/commands/console.rb index de2f190ad5..2a6cdca440 100644 --- a/railties/lib/rails/commands/console.rb +++ b/railties/lib/rails/commands/console.rb @@ -34,6 +34,10 @@ module Rails exit end end + + if defined?(ActiveRecord) + ActiveRecord::Base.logger = Logger.new(STDOUT) + end if options[:sandbox] puts "Loading #{Rails.env} environment in sandbox (Rails #{Rails.version})" From 63e4e218c57a1b5f19ba18acaa3c83ff0034a2dc Mon Sep 17 00:00:00 2001 From: burningTyger Date: Fri, 1 Apr 2011 00:12:58 +0200 Subject: [PATCH 45/54] fix typo --- railties/guides/source/testing.textile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/guides/source/testing.textile b/railties/guides/source/testing.textile index d937f30609..b42ea15f62 100644 --- a/railties/guides/source/testing.textile +++ b/railties/guides/source/testing.textile @@ -391,7 +391,7 @@ There are a bunch of different types of assertions you can use. Here's the compl |+assert_nil( obj, [msg] )+ |Ensures that +obj.nil?+ is true.| |+assert_not_nil( obj, [msg] )+ |Ensures that +obj.nil?+ is false.| |+assert_match( regexp, string, [msg] )+ |Ensures that a string matches the regular expression.| -|+assert_no_match( regexp, string, [msg] )+ |Ensures that a string doesn't matches the regular expression.| +|+assert_no_match( regexp, string, [msg] )+ |Ensures that a string doesn't match the regular expression.| |+assert_in_delta( expecting, actual, delta, [msg] )+ |Ensures that the numbers +expecting+ and +actual+ are within +delta+ of each other.| |+assert_throws( symbol, [msg] ) { block }+ |Ensures that the given block throws the symbol.| |+assert_raise( exception1, exception2, ... ) { block }+ |Ensures that the given block raises one of the given exceptions.| From 0eb6e5e270c0a1114fdafe4a8daa35ee88e176e3 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 31 Mar 2011 16:20:59 -0700 Subject: [PATCH 46/54] Moved Turn activation/dependency to railties --- activesupport/CHANGELOG | 2 -- activesupport/lib/active_support/test_case.rb | 1 - .../active_support/testing/turn_formatting.rb | 33 ------------------- rails.gemspec | 1 - railties/CHANGELOG | 2 ++ railties/lib/rails/commands/console.rb | 2 +- railties/lib/rails/test_help.rb | 8 +++++ railties/railties.gemspec | 1 + 8 files changed, 12 insertions(+), 38 deletions(-) delete mode 100644 activesupport/lib/active_support/testing/turn_formatting.rb diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index 1be5f39ac4..373236ce9a 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,7 +1,5 @@ *Rails 3.1.0 (unreleased)* -* Add using Turn with natural language test case names if the library is available (which it will be in Rails 3.1) [DHH] - * LocalCache strategy is now a real middleware class, not an anonymous class posing for pictures. diff --git a/activesupport/lib/active_support/test_case.rb b/activesupport/lib/active_support/test_case.rb index 7ac9ad46e9..8d6c27e381 100644 --- a/activesupport/lib/active_support/test_case.rb +++ b/activesupport/lib/active_support/test_case.rb @@ -5,7 +5,6 @@ require 'active_support/testing/deprecation' require 'active_support/testing/declarative' require 'active_support/testing/pending' require 'active_support/testing/isolation' -require 'active_support/testing/turn_formatting' require 'active_support/testing/mochaing' require 'active_support/core_ext/kernel/reporting' diff --git a/activesupport/lib/active_support/testing/turn_formatting.rb b/activesupport/lib/active_support/testing/turn_formatting.rb deleted file mode 100644 index ed9381e298..0000000000 --- a/activesupport/lib/active_support/testing/turn_formatting.rb +++ /dev/null @@ -1,33 +0,0 @@ -# Turn gives you prettier formatting for MiniTest and inline failure reporting. -# It also allows us to report test cases in natural language rather than with underscores. Example: -# -# CommentsControllerTest: -# PASS the truth (0.03s) -# -# APITest -# test_api_without_subdomain PASS -# test_create_milestone_using_typed_xml FAIL -# /test/integration/api_test.rb:50:in `test_create_milestone_using_typed_xml' -# <2006-05-01> expected but was -# . -# test_create_milestone_using_untyped_xml FAIL -# /test/integration/api_test.rb:38:in `test_create_milestone_using_untyped_xml' -# <2006-05-01> expected but was -# . - -# -# vs: -# -# .FF - -if defined?(MiniTest) - begin - silence_warnings { require 'turn' } - - if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) - MiniTest::Unit.use_natural_language_case_names = true - end - rescue LoadError - # If there's no turn, that's fine, it's just formatting - end -end \ No newline at end of file diff --git a/rails.gemspec b/rails.gemspec index 2711d95f34..98b5f46554 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -25,6 +25,5 @@ Gem::Specification.new do |s| s.add_dependency('activeresource', version) s.add_dependency('actionmailer', version) s.add_dependency('railties', version) - s.add_dependency('turn', '~> 0.8.2') s.add_dependency('bundler', '~> 1.0') end diff --git a/railties/CHANGELOG b/railties/CHANGELOG index 07e7e461de..f159247308 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* Add using Turn with natural language test case names for test_help.rb when running with minitest (Ruby 1.9.2+) [DHH] + * Direct logging of Active Record to STDOUT so it's shown inline with the results in the console [DHH] * Added `config.force_ssl` configuration which loads Rack::SSL middleware and force all requests to be under HTTPS protocol [DHH, Prem Sichanugrist, and Josh Peek] diff --git a/railties/lib/rails/commands/console.rb b/railties/lib/rails/commands/console.rb index 2a6cdca440..2b7faf9715 100644 --- a/railties/lib/rails/commands/console.rb +++ b/railties/lib/rails/commands/console.rb @@ -36,7 +36,7 @@ module Rails end if defined?(ActiveRecord) - ActiveRecord::Base.logger = Logger.new(STDOUT) + ActiveRecord::Base.logger = Logger.new(STDERR) end if options[:sandbox] diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index 00029e627e..b9f7bdc2eb 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -13,6 +13,14 @@ if defined?(Test::Unit::Util::BacktraceFilter) && ENV['BACKTRACE'].nil? Test::Unit::Util::BacktraceFilter.module_eval { include Rails::BacktraceFilterForTestUnit } end +if defined?(MiniTest) + require 'turn' + + if MiniTest::Unit.respond_to?(:use_natural_language_case_names=) + MiniTest::Unit.use_natural_language_case_names = true + end +end + if defined?(ActiveRecord) require 'active_record/test_case' diff --git a/railties/railties.gemspec b/railties/railties.gemspec index c51fe856be..1e233c885e 100644 --- a/railties/railties.gemspec +++ b/railties/railties.gemspec @@ -22,6 +22,7 @@ Gem::Specification.new do |s| s.add_dependency('rake', '>= 0.8.7') s.add_dependency('thor', '~> 0.14.4') s.add_dependency('rack-ssl', '~> 1.3.2') + s.add_dependency('turn', '~> 0.8.2') s.add_dependency('activesupport', version) s.add_dependency('actionpack', version) end From c17b8e4047443b416685e30c8825ae01909f8d27 Mon Sep 17 00:00:00 2001 From: Jon Cooper Date: Thu, 31 Mar 2011 16:19:18 -0700 Subject: [PATCH 47/54] Trivial fix to HTTP Digest auth MD5 example --- actionpack/lib/action_controller/metal/http_authentication.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 39c804d707..c305abf5eb 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -77,7 +77,7 @@ module ActionController # class PostsController < ApplicationController # REALM = "SuperSecret" # USERS = {"dhh" => "secret", #plain text password - # "dap" => Digest:MD5::hexdigest(["dap",REALM,"secret"].join(":")) #ha1 digest password + # "dap" => Digest::MD5.hexdigest(["dap",REALM,"secret"].join(":")) #ha1 digest password # # before_filter :authenticate, :except => [:index] # From 9de8dea2e886dad608cc0de6e62b8365b0ed9760 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 1 Apr 2011 23:58:37 -0300 Subject: [PATCH 48/54] default_executable is deprecated since rubygems 1.7.0 --- rails.gemspec | 1 - 1 file changed, 1 deletion(-) diff --git a/rails.gemspec b/rails.gemspec index 98b5f46554..4ee814c507 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -17,7 +17,6 @@ Gem::Specification.new do |s| s.bindir = 'bin' s.executables = ['rails'] - s.default_executable = 'rails' s.add_dependency('activesupport', version) s.add_dependency('actionpack', version) From 99da42c29944beed0cb9e892ae90bfeb2590c57e Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 2 Apr 2011 00:35:33 -0300 Subject: [PATCH 49/54] Gem::Specification#has_rdoc= is deprecated since rubygems 1.7.0 --- actionmailer/actionmailer.gemspec | 2 -- actionpack/actionpack.gemspec | 2 -- activemodel/activemodel.gemspec | 2 -- activerecord/activerecord.gemspec | 1 - activeresource/activeresource.gemspec | 1 - activesupport/activesupport.gemspec | 2 -- railties/railties.gemspec | 1 - 7 files changed, 11 deletions(-) diff --git a/actionmailer/actionmailer.gemspec b/actionmailer/actionmailer.gemspec index 2ae85f8b57..ee02cf6945 100644 --- a/actionmailer/actionmailer.gemspec +++ b/actionmailer/actionmailer.gemspec @@ -17,8 +17,6 @@ Gem::Specification.new do |s| s.require_path = 'lib' s.requirements << 'none' - s.has_rdoc = true - s.add_dependency('actionpack', version) s.add_dependency('mail', '~> 2.2.15') end diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 651f3b164a..6c55842eea 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -17,8 +17,6 @@ Gem::Specification.new do |s| s.require_path = 'lib' s.requirements << 'none' - s.has_rdoc = true - s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) s.add_dependency('rack-cache', '~> 1.0.0') diff --git a/activemodel/activemodel.gemspec b/activemodel/activemodel.gemspec index fec9c7ff8b..9f80673bb8 100644 --- a/activemodel/activemodel.gemspec +++ b/activemodel/activemodel.gemspec @@ -17,8 +17,6 @@ Gem::Specification.new do |s| s.files = Dir['CHANGELOG', 'MIT-LICENSE', 'README.rdoc', 'lib/**/*'] s.require_path = 'lib' - s.has_rdoc = true - s.add_dependency('activesupport', version) s.add_dependency('builder', '~> 3.0.0') s.add_dependency('i18n', '~> 0.5.0') diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index b1df24844a..c3cd76a714 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -17,7 +17,6 @@ Gem::Specification.new do |s| s.files = Dir['CHANGELOG', 'README.rdoc', 'examples/**/*', 'lib/**/*'] s.require_path = 'lib' - s.has_rdoc = true s.extra_rdoc_files = %w( README.rdoc ) s.rdoc_options.concat ['--main', 'README.rdoc'] diff --git a/activeresource/activeresource.gemspec b/activeresource/activeresource.gemspec index a71168722b..c2fd707e9b 100644 --- a/activeresource/activeresource.gemspec +++ b/activeresource/activeresource.gemspec @@ -17,7 +17,6 @@ Gem::Specification.new do |s| s.files = Dir['CHANGELOG', 'README.rdoc', 'examples/**/*', 'lib/**/*'] s.require_path = 'lib' - s.has_rdoc = true s.extra_rdoc_files = %w( README.rdoc ) s.rdoc_options.concat ['--main', 'README.rdoc'] diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index df7f68fecf..eaecb30090 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -16,6 +16,4 @@ Gem::Specification.new do |s| s.files = Dir['CHANGELOG', 'README.rdoc', 'lib/**/*'] s.require_path = 'lib' - - s.has_rdoc = true end diff --git a/railties/railties.gemspec b/railties/railties.gemspec index 1e233c885e..b1eda71c7f 100644 --- a/railties/railties.gemspec +++ b/railties/railties.gemspec @@ -17,7 +17,6 @@ Gem::Specification.new do |s| s.require_path = 'lib' s.rdoc_options << '--exclude' << '.' - s.has_rdoc = false s.add_dependency('rake', '>= 0.8.7') s.add_dependency('thor', '~> 0.14.4') From cc58fe79ac6f4d5fd54a39ff6e7f087c6a04fee8 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 2 Apr 2011 23:45:07 -0300 Subject: [PATCH 50/54] Implicit actions named not_implemented can be rendered --- actionpack/CHANGELOG | 2 ++ actionpack/lib/abstract_controller/base.rb | 2 ++ .../new_base/render_implicit_action_test.rb | 13 +++++++++++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 3eba2281c4..25e2d27a01 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* Implicit actions named not_implemented can be rendered [Santiago Pastorino] + * Wildcard route will always matching the optional format segment by default. For example if you have this route: map '*pages' => 'pages#show' diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 07ff5ad9f3..0951267fea 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -1,3 +1,4 @@ +require 'erubis' require 'active_support/configurable' require 'active_support/descendants_tracker' require 'active_support/core_ext/module/anonymous' @@ -18,6 +19,7 @@ module AbstractController include ActiveSupport::Configurable extend ActiveSupport::DescendantsTracker + undef_method :not_implemented class << self attr_reader :abstract alias_method :abstract?, :abstract diff --git a/actionpack/test/controller/new_base/render_implicit_action_test.rb b/actionpack/test/controller/new_base/render_implicit_action_test.rb index 667a9021be..3bb3016fdb 100644 --- a/actionpack/test/controller/new_base/render_implicit_action_test.rb +++ b/actionpack/test/controller/new_base/render_implicit_action_test.rb @@ -3,8 +3,9 @@ require 'abstract_unit' module RenderImplicitAction class SimpleController < ::ApplicationController self.view_paths = [ActionView::FixtureResolver.new( - "render_implicit_action/simple/hello_world.html.erb" => "Hello world!", - "render_implicit_action/simple/hyphen-ated.html.erb" => "Hello hyphen-ated!" + "render_implicit_action/simple/hello_world.html.erb" => "Hello world!", + "render_implicit_action/simple/hyphen-ated.html.erb" => "Hello hyphen-ated!", + "render_implicit_action/simple/not_implemented.html.erb" => "Not Implemented" )] def hello_world() end @@ -25,9 +26,17 @@ module RenderImplicitAction assert_status 200 end + test "render an action called not_implemented" do + get "/render_implicit_action/simple/not_implemented" + + assert_body "Not Implemented" + assert_status 200 + end + test "action_method? returns true for implicit actions" do assert SimpleController.new.action_method?(:hello_world) assert SimpleController.new.action_method?(:"hyphen-ated") + assert SimpleController.new.action_method?(:not_implemented) end end end From a000fc58b8bc288ca9c60566afc1e1943f5d6083 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 2 Apr 2011 23:56:52 -0300 Subject: [PATCH 51/54] Bump up erubis to 2.7.0 --- actionpack/actionpack.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 6c55842eea..d3c66800d9 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -26,5 +26,5 @@ Gem::Specification.new do |s| s.add_dependency('rack-test', '~> 0.5.7') s.add_dependency('rack-mount', '~> 0.7.1') s.add_dependency('tzinfo', '~> 0.3.23') - s.add_dependency('erubis', '~> 2.6.6') + s.add_dependency('erubis', '~> 2.7.0') end From bd3cdeea354ebff97b0d5102a0857ce85eedcfa4 Mon Sep 17 00:00:00 2001 From: Akira Matsuda Date: Sun, 3 Apr 2011 12:47:51 +0900 Subject: [PATCH 52/54] s/ERb/ERB/g The author of ERB sais, his eRuby implementation was originally named "ERb/ERbLight" and then renamed to "ERB" when started bundled as a Ruby standard lib. http://www2a.biglobe.ne.jp/~seki/ruby/erb.html --- actionpack/lib/action_controller/base.rb | 4 ++-- actionpack/lib/action_view/base.rb | 10 +++++----- actionpack/lib/action_view/helpers/atom_feed_helper.rb | 2 +- actionpack/lib/action_view/helpers/capture_helper.rb | 2 +- actionpack/lib/action_view/helpers/prototype_helper.rb | 2 +- actionpack/lib/action_view/helpers/url_helper.rb | 2 +- actionpack/lib/action_view/template/handlers/erb.rb | 2 +- activerecord/lib/active_record/fixtures.rb | 6 +++--- .../active_support/core_ext/string/output_safety.rb | 2 +- railties/lib/rails/source_annotation_extractor.rb | 4 ++-- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index e6523e56d2..5f9e082cd3 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -105,7 +105,7 @@ module ActionController # == Renders # # Action Controller sends content to the user by using one of five rendering methods. The most versatile and common is the rendering - # of a template. Included in the Action Pack is the Action View, which enables rendering of ERb templates. It's automatically configured. + # of a template. Included in the Action Pack is the Action View, which enables rendering of ERB templates. It's automatically configured. # The controller passes objects to the view by assigning instance variables: # # def show @@ -128,7 +128,7 @@ module ActionController # end # end # - # Read more about writing ERb and Builder templates in ActionView::Base. + # Read more about writing ERB and Builder templates in ActionView::Base. # # == Redirects # diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index ab8c6259c5..5519103627 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -8,13 +8,13 @@ require 'action_view/log_subscriber' module ActionView #:nodoc: # = Action View Base # - # Action View templates can be written in three ways. If the template file has a .erb (or .rhtml) extension then it uses a mixture of ERb + # Action View templates can be written in three ways. If the template file has a .erb (or .rhtml) extension then it uses a mixture of ERB # (included in Ruby) and HTML. If the template file has a .builder (or .rxml) extension then Jim Weirich's Builder::XmlMarkup library is used. # If the template file has a .rjs extension then it will use ActionView::Helpers::PrototypeHelper::JavaScriptGenerator. # - # == ERb + # == ERB # - # You trigger ERb by using embeddings such as <% %>, <% -%>, and <%= %>. The <%= %> tag set is used when you want output. Consider the + # You trigger ERB by using embeddings such as <% %>, <% -%>, and <%= %>. The <%= %> tag set is used when you want output. Consider the # following loop for names: # # Names of all the people @@ -23,7 +23,7 @@ module ActionView #:nodoc: # <% end %> # # The loop is setup in regular embedding tags <% %> and the name is written using the output embedding tag <%= %>. Note that this - # is not just a usage suggestion. Regular output functions like print or puts won't work with ERb templates. So this would be wrong: + # is not just a usage suggestion. Regular output functions like print or puts won't work with ERB templates. So this would be wrong: # # <%# WRONG %> # Hi, Mr. <% puts "Frodo" %> @@ -81,7 +81,7 @@ module ActionView #:nodoc: # # == Builder # - # Builder templates are a more programmatic alternative to ERb. They are especially useful for generating XML content. An XmlMarkup object + # Builder templates are a more programmatic alternative to ERB. They are especially useful for generating XML content. An XmlMarkup object # named +xml+ is automatically made available to templates with a .builder extension. # # Here are some basic examples: diff --git a/actionpack/lib/action_view/helpers/atom_feed_helper.rb b/actionpack/lib/action_view/helpers/atom_feed_helper.rb index db9d7a08ff..96e5722252 100644 --- a/actionpack/lib/action_view/helpers/atom_feed_helper.rb +++ b/actionpack/lib/action_view/helpers/atom_feed_helper.rb @@ -4,7 +4,7 @@ module ActionView # = Action View Atom Feed Helpers module Helpers #:nodoc: module AtomFeedHelper - # Adds easy defaults to writing Atom feeds with the Builder template engine (this does not work on ERb or any other + # Adds easy defaults to writing Atom feeds with the Builder template engine (this does not work on ERB or any other # template languages). # # Full usage example: diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb index c88bd1efd5..9ac7dff1ec 100644 --- a/actionpack/lib/action_view/helpers/capture_helper.rb +++ b/actionpack/lib/action_view/helpers/capture_helper.rb @@ -14,7 +14,7 @@ module ActionView # variable. You can then use this variable anywhere in your templates or layout. # # ==== Examples - # The capture method can be used in ERb templates... + # The capture method can be used in ERB templates... # # <% @greeting = capture do %> # Welcome to my shiny new web page! The date and time is diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index 18e303778c..506db24dc2 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -584,7 +584,7 @@ module ActionView # Works like update_page but wraps the generated JavaScript in a # \