From d5994ee48af14d67f0eec7d23863d4b19211b078 Mon Sep 17 00:00:00 2001 From: John Mileham Date: Thu, 3 Mar 2011 23:26:45 -0500 Subject: Change behavior of count(:limit => x, :offset => y) to limit/offset before counting. --- .../lib/active_record/relation/calculations.rb | 40 ++++++++++++++-------- activerecord/test/cases/calculations_test.rb | 37 ++++++++++++++------ activerecord/test/cases/relations_test.rb | 28 +++++++++++++++ 3 files changed, 80 insertions(+), 25 deletions(-) (limited to 'activerecord') 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,27 +109,42 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [2, 6], c.keys.compact end - def test_limit_with_offset_is_kept + 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).offset(1).count } + queries = assert_sql { Account.limit(1).count } assert_equal 1, queries.length assert_match(/LIMIT/, queries.first) - assert_match(/OFFSET/, queries.first) end - def test_offset_without_limit_removes_offset + def test_offset_is_kept + return if current_adapter?(:OracleAdapter) + queries = assert_sql { Account.offset(1).count } assert_equal 1, queries.length - assert_no_match(/LIMIT/, queries.first) - assert_no_match(/OFFSET/, queries.first) + assert_match(/OFFSET/, queries.first) end - def test_limit_without_offset_removes_limit - queries = assert_sql { Account.limit(1).count } + def test_limit_with_offset_is_kept + return if current_adapter?(:OracleAdapter) + + queries = assert_sql { Account.limit(1).offset(1).count } assert_equal 1, queries.length - assert_no_match(/LIMIT/, queries.first) - assert_no_match(/OFFSET/, queries.first) + assert_match(/LIMIT/, queries.first) + assert_match(/OFFSET/, queries.first) end def test_no_limit_no_offset 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") -- cgit v1.2.3 From 5214e73850916de3c9127d35a4ecee0424d364a3 Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Wed, 23 Mar 2011 21:52:53 -0700 Subject: add #first! and #last! to models & relations --- .../lib/active_record/relation/finder_methods.rb | 10 +++++++++ activerecord/test/cases/finder_test.rb | 24 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 426000fde1..25e23a9d55 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -123,6 +123,11 @@ module ActiveRecord end end + # Same as #first! but raises RecordNotFound if no record is returned + def first!(*args) + self.first(*args) or raise RecordNotFound + end + # A convenience wrapper for find(:last, *args). You can pass in all the # same arguments to this method as you can to find(:last). def last(*args) @@ -137,6 +142,11 @@ module ActiveRecord end end + # Same as #last! but raises RecordNotFound if no record is returned + def last!(*args) + self.last(*args) or raise RecordNotFound + end + # A convenience wrapper for find(:all, *args). You can pass in all the # same arguments to this method as you can to find(:all). def all(*args) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 2e620d8b03..543981b4a0 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -191,6 +191,30 @@ class FinderTest < ActiveRecord::TestCase assert_nil Topic.where("title = 'The Second Topic of the day!'").first end + def test_first_bang_present + assert_nothing_raised do + assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").first! + end + end + + def test_first_bang_missing + assert_raises ActiveRecord::RecordNotFound do + Topic.where("title = 'This title does not exist'").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! + end + end + + def test_last_bang_missing + assert_raises ActiveRecord::RecordNotFound do + Topic.where("title = 'This title does not exist'").last! + end + end + def test_unexisting_record_exception_handling assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1).parent -- cgit v1.2.3 From 28c73f012328c8386acfc608f0dfb1a459dbf170 Mon Sep 17 00:00:00 2001 From: John Mileham Date: Thu, 24 Mar 2011 15:09:24 -0400 Subject: Use Arel to build subquery. Adapt tests to changed fixtures. --- .../lib/active_record/relation/calculations.rb | 25 +++++++++++----------- activerecord/test/cases/relations_test.rb | 6 +++--- 2 files changed, 15 insertions(+), 16 deletions(-) (limited to 'activerecord') 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) - - 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) + column_alias = Arel.sql('count_column') + subquery_alias = Arel.sql('subquery_for_count') + + 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 -- cgit v1.2.3 From 7333f50abce3ad6c83d13c66b85e371e7951c84d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 25 Mar 2011 12:10:11 -0700 Subject: fixing whitespace errors. :bomb: --- activerecord/test/cases/attribute_methods_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index d8638ee776..d03c3ee1a1 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -120,11 +120,11 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_read_attributes_before_type_cast_on_datetime in_time_zone "Pacific Time (US & Canada)" do record = @target.new - + record.written_on = "345643456" assert_equal "345643456", record.written_on_before_type_cast assert_equal nil, record.written_on - + record.written_on = "2009-10-11 12:13:14" assert_equal "2009-10-11 12:13:14", record.written_on_before_type_cast assert_equal Time.zone.parse("2009-10-11 12:13:14"), record.written_on -- cgit v1.2.3 From 65dce01091ec88f3485ccc26f8799c609b91e7cc Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Fri, 25 Mar 2011 09:41:06 -0700 Subject: comment typo fix --- activerecord/lib/active_record/relation/finder_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 25e23a9d55..9d4c6d60f5 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -123,7 +123,7 @@ module ActiveRecord end end - # Same as #first! but raises RecordNotFound if no record is returned + # Same as #first but raises RecordNotFound if no record is returned def first!(*args) self.first(*args) or raise RecordNotFound end @@ -142,7 +142,7 @@ module ActiveRecord end end - # Same as #last! but raises RecordNotFound if no record is returned + # Same as #last but raises RecordNotFound if no record is returned def last!(*args) self.last(*args) or raise RecordNotFound end -- cgit v1.2.3 From 25be204e3c2c46b4eb7738daa0ac622738aa211b Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 25 Mar 2011 22:30:36 +0000 Subject: No arguments for first! and last! --- activerecord/lib/active_record/relation/finder_methods.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 9d4c6d60f5..d41a360821 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -124,8 +124,8 @@ module ActiveRecord end # Same as #first but raises RecordNotFound if no record is returned - def first!(*args) - self.first(*args) or raise RecordNotFound + def first! + self.first or raise RecordNotFound end # A convenience wrapper for find(:last, *args). You can pass in all the @@ -143,8 +143,8 @@ module ActiveRecord end # Same as #last but raises RecordNotFound if no record is returned - def last!(*args) - self.last(*args) or raise RecordNotFound + def last! + self.last or raise RecordNotFound end # A convenience wrapper for find(:all, *args). You can pass in all the -- cgit v1.2.3 From 9d9b873b9501f5fc90c0b6a4a550c5e29fc742a5 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Sat, 26 Mar 2011 00:02:40 +0100 Subject: removes unnecessary selfs, and mentions that first! and last! take no arguments in their API docs --- activerecord/lib/active_record/relation/finder_methods.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index d41a360821..563843f3cc 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -123,9 +123,10 @@ module ActiveRecord end end - # Same as #first but raises RecordNotFound if no record is returned + # Same as +first+ but raises ActiveRecord::RecordNotFound if no record + # is found. Note that first! accepts no arguments. def first! - self.first or raise RecordNotFound + first or raise RecordNotFound end # A convenience wrapper for find(:last, *args). You can pass in all the @@ -142,9 +143,10 @@ module ActiveRecord end end - # Same as #last but raises RecordNotFound if no record is returned + # Same as +last+ but raises ActiveRecord::RecordNotFound if no record + # is found. Note that last! accepts no arguments. def last! - self.last or raise RecordNotFound + last or raise RecordNotFound end # A convenience wrapper for find(:all, *args). You can pass in all the -- cgit v1.2.3 From 45c233ef819dc7b67e259dd73f24721fec28b8c8 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Sat, 26 Mar 2011 11:42:26 -0300 Subject: Removed #update_attribute method. New #update_column method. Signed-off-by: Santiago Pastorino --- .../associations/has_one_association.rb | 3 +- activerecord/lib/active_record/persistence.rb | 18 ++--- .../has_and_belongs_to_many_associations_test.rb | 2 +- .../associations/has_many_associations_test.rb | 6 +- .../has_many_through_associations_test.rb | 4 +- .../has_one_through_associations_test.rb | 4 +- .../test/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 | 88 +++++++++++----------- activerecord/test/cases/timestamp_test.rb | 6 +- 13 files changed, 78 insertions(+), 80 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 1d2e8667e4..dfcb116392 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -34,7 +34,8 @@ module ActiveRecord when :destroy target.destroy when :nullify - target.update_attribute(reflection.foreign_key, nil) + target.send("#{reflection.foreign_key}=", nil) + target.save(:validations => false) end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 17a64b6e86..3377a5934b 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -104,19 +104,17 @@ module ActiveRecord became end - # Updates a single attribute and saves the record. - # This is especially useful for boolean flags on existing records. Also note that + # Updates a single attribute of an object, without calling save. # # * Validation is skipped. - # * 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. + # * Callbacks are skipped. + # * updated_at/updated_on column is not updated in any case. # - def update_attribute(name, value) + def update_column(name, value) name = name.to_s raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name) send("#{name}=", value) - save(:validate => false) + self.class.update_all({ name => value }, self.class.primary_key => id) end # Updates the attributes of the model from the passed-in hash and saves the @@ -156,7 +154,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_attribute(attribute, self[attribute]) + increment(attribute, by).update_column(attribute, self[attribute]) end # Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1). @@ -173,7 +171,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_attribute(attribute, self[attribute]) + decrement(attribute, by).update_column(attribute, self[attribute]) end # Assigns to +attribute+ the boolean opposite of attribute?. So @@ -190,7 +188,7 @@ module ActiveRecord # Saving is not subjected to validation checks. Returns +true+ if the # record could be saved. def toggle!(attribute) - toggle(attribute).update_attribute(attribute, self[attribute]) + toggle(attribute).update_column(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 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..b75482a956 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 @@ -487,7 +487,8 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.previous_changes.key?('created_on') pirate = Pirate.find_by_catchphrase("Ahoy!") - pirate.update_attribute(:catchphrase, "Ninjas suck!") + pirate.catchphrase = "Ninjas suck!" + pirate.save(:validations => false) 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 8ca9d626d1..68d861c9c2 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -327,66 +327,62 @@ class PersistencesTest < ActiveRecord::TestCase assert_raise(ActiveSupport::FrozenObjectError) { client.name = "something else" } end - def test_update_attribute - assert !Topic.find(1).approved? - Topic.find(1).update_attribute("approved", true) - assert Topic.find(1).approved? + def test_update_column + topic = Topic.find(1) + topic.update_column("approved", true) + assert topic.approved? + topic.reload + assert topic.approved? - Topic.find(1).update_attribute(:approved, false) - assert !Topic.find(1).approved? + topic.update_column(:approved, false) + assert !topic.approved? + topic.reload + assert !topic.approved? end - def test_update_attribute_for_readonly_attribute + def test_update_column_with_model_having_primary_key_other_than_id minivan = Minivan.find('m1') - assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } - end - - # 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' + new_name = 'sebavan' - t.reload - assert_equal 'super_title', t.title + minivan.update_column(:name, new_name) + assert_equal new_name, minivan.name end - def test_update_attribute_for_updated_at_on + 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_attribute(:updated_at, prev_month) + developer.update_column(:updated_at, prev_month) assert_equal prev_month, developer.updated_at - developer.update_attribute(:salary, 80001) - assert_not_equal prev_month, developer.updated_at + developer.update_column(:salary, 80001) + assert_equal prev_month, developer.updated_at developer.reload - assert_not_equal prev_month, developer.updated_at + 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 end def test_update_attributes diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 1c21f0f3b6..08bbd14d38 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -113,7 +113,8 @@ class TimestampTest < ActiveRecord::TestCase pet = Pet.first owner = pet.owner - owner.update_attribute(:happy_at, 3.days.ago) + owner.happy_at = 3.days.ago + owner.save previously_owner_updated_at = owner.updated_at pet.name = "I'm a parrot" @@ -131,8 +132,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 -- cgit v1.2.3 From dea3d2dd203fe2008ccfae6b6575da0b0001e466 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sat, 26 Mar 2011 11:47:20 -0700 Subject: adding a test for attributes after type cast. thanks nragaz. :heart: --- activerecord/test/cases/attribute_methods_test.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index d03c3ee1a1..eacb18980c 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -132,6 +132,23 @@ class AttributeMethodsTest < ActiveRecord::TestCase end end + def test_read_attributes_after_type_cast_on_datetime + in_time_zone "Pacific Time (US & Canada)" do + record = @target.new + + record.written_on = "2011-03-24" + assert_equal "2011-03-24", record.written_on_before_type_cast + assert_equal Time.zone.parse("2011-03-24"), record.written_on + assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], + record.written_on.time_zone + + record.save + record.reload + + assert_equal Time.zone.parse("2011-03-24"), record.written_on + end + end + def test_hash_content topic = Topic.new topic.content = { "one" => 1, "two" => 2 } -- cgit v1.2.3 From 5013fe3a1dcc0472ff11b6f94260d5dc224b5305 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sat, 26 Mar 2011 17:04:54 -0700 Subject: refactoring tz to a variable rather than repeating it --- activerecord/test/cases/attribute_methods_test.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index eacb18980c..34271be8b0 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -133,13 +133,15 @@ class AttributeMethodsTest < ActiveRecord::TestCase end def test_read_attributes_after_type_cast_on_datetime - in_time_zone "Pacific Time (US & Canada)" do + tz = "Pacific Time (US & Canada)" + + in_time_zone tz do record = @target.new record.written_on = "2011-03-24" assert_equal "2011-03-24", record.written_on_before_type_cast assert_equal Time.zone.parse("2011-03-24"), record.written_on - assert_equal ActiveSupport::TimeZone["Pacific Time (US & Canada)"], + assert_equal ActiveSupport::TimeZone[tz], record.written_on.time_zone record.save -- cgit v1.2.3 From a9d27c04abf24dc85be061ff9772d71897af02b1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sat, 26 Mar 2011 17:16:24 -0700 Subject: cleaning up typecast test a little --- activerecord/test/cases/attribute_methods_test.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 34271be8b0..84f75cc803 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -138,16 +138,18 @@ class AttributeMethodsTest < ActiveRecord::TestCase in_time_zone tz do record = @target.new - record.written_on = "2011-03-24" - assert_equal "2011-03-24", record.written_on_before_type_cast - assert_equal Time.zone.parse("2011-03-24"), record.written_on - assert_equal ActiveSupport::TimeZone[tz], - record.written_on.time_zone + date_string = "2011-03-24" + time = Time.zone.parse date_string + + record.written_on = date_string + assert_equal date_string, record.written_on_before_type_cast + assert_equal time, record.written_on + assert_equal ActiveSupport::TimeZone[tz], record.written_on.time_zone record.save record.reload - assert_equal Time.zone.parse("2011-03-24"), record.written_on + assert_equal time, record.written_on end end -- cgit v1.2.3 From 0e1fed537a7699a7ded02f77b71d222d7207c5ad Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Sun, 27 Mar 2011 18:52:21 -0300 Subject: 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 +++-- .../has_and_belongs_to_many_associations_test.rb | 2 +- .../associations/has_many_associations_test.rb | 6 +- .../has_many_through_associations_test.rb | 4 +- .../has_one_through_associations_test.rb | 4 +- .../test/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 | 88 +++++++++++----------- activerecord/test/cases/timestamp_test.rb | 6 +- 13 files changed, 80 insertions(+), 78 deletions(-) (limited to 'activerecord') 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 - end + assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') } + end + + # 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' - 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 + 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 -- cgit v1.2.3 From 245542ea2994961731be105db6c076256a22a7a9 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Sun, 27 Mar 2011 19:12:28 -0300 Subject: Added new #update_column method. Signed-off-by: Santiago Pastorino --- activerecord/CHANGELOG | 9 +++ .../lib/active_record/attribute_methods/write.rb | 1 + activerecord/lib/active_record/persistence.rb | 14 ++++ .../has_and_belongs_to_many_associations_test.rb | 2 +- .../associations/has_many_associations_test.rb | 6 +- .../has_many_through_associations_test.rb | 4 +- .../has_one_through_associations_test.rb | 4 +- .../test/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(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From fb215110401c70cfc7013c6e2ad5753fa4e374e9 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Mon, 28 Mar 2011 10:19:41 -0300 Subject: 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(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From e1a5007207b8bd9e91711428acc3104f4c8e8fb8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 15:27:37 -0700 Subject: sql logger ignores schema statements --- .../connection_adapters/sqlite_adapter.rb | 4 ++-- activerecord/lib/active_record/log_subscriber.rb | 3 +++ activerecord/test/cases/log_subscriber_test.rb | 27 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From 7b4866e0aef1ed8ec08fd9eac104df42bce3c1c9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 16:08:42 -0700 Subject: namespacing connection management tests. :heart: --- .../test/cases/connection_management_test.rb | 40 ++++++++++++---------- 1 file changed, 22 insertions(+), 18 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From ec0cacc2938cb7159ae6085fc653b2be906a748f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 16:22:37 -0700 Subject: testing app delegation from the ConnectionManagement middleware --- .../test/cases/connection_management_test.rb | 23 +++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From 4211866b7a1d0abf0c9150fd61ea67a8043b831d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 16:43:34 -0700 Subject: adding active_connection? to the connection pool --- .../active_record/connection_adapters/abstract/connection_pool.rb | 6 ++++++ activerecord/test/cases/connection_pool_test.rb | 8 ++++++++ 2 files changed, 14 insertions(+) (limited to 'activerecord') 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'] -- cgit v1.2.3 From 25f94971abf71fe51089f53b72cc08b636c230b3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 17:29:37 -0700 Subject: adding active_connections? to the connection pool for finding open connections --- .../abstract/connection_pool.rb | 6 ++++ .../connection_adapters/connection_handler_test.rb | 33 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 activerecord/test/cases/connection_adapters/connection_handler_test.rb (limited to 'activerecord') 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 -- cgit v1.2.3 From aea1477362b640ebe52cf991b915ad32e7bf2571 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Mar 2011 17:47:46 -0700 Subject: make sure we have an active database connection before running each connection management test --- .../connection_adapters/abstract/connection_specification.rb | 6 +++++- activerecord/test/cases/connection_management_test.rb | 9 +++++---- 2 files changed, 10 insertions(+), 5 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From b155fdadf334cff32a7e648c86c3c97f2f51257f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 29 Mar 2011 12:51:58 +0100 Subject: Change exists? so that it doesn't instantiate records [#6127 state:resolved] --- activerecord/lib/active_record/relation/finder_methods.rb | 14 ++++++++++---- activerecord/test/cases/finder_test.rb | 5 +++++ 2 files changed, 15 insertions(+), 4 deletions(-) (limited to 'activerecord') 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) -- cgit v1.2.3 From a9dafbb28de3e34c31ebf184fbc4e2042c7ff207 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Tue, 29 Mar 2011 15:08:40 +0100 Subject: 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 ++++++++++++++++- 3 files changed, 34 insertions(+), 2 deletions(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From 555d0163897601010ab1305f41ed393ec517b61e Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sun, 27 Mar 2011 09:54:38 +0100 Subject: Quote find_in_batches ORDER BY clause [#6620 state:resolved] --- .../lib/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(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From e5246092d1ce30961af4f7d9b5ad86071298cf1c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 15:37:07 -0700 Subject: 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(-) (limited to 'activerecord') 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) -- cgit v1.2.3 From 3b2a032677a2261499aa5d2de019f31f1173a858 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 15:42:32 -0700 Subject: clearing active connections in the ConnectionManagement middleware if an exception happens --- .../active_record/connection_adapters/abstract/connection_pool.rb | 3 +++ activerecord/test/cases/connection_management_test.rb | 7 +++++++ 2 files changed, 10 insertions(+) (limited to 'activerecord') 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) -- cgit v1.2.3 From c7b7c6ad1c773102753f1a11b857d0e37ceb6a21 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 15:47:16 -0700 Subject: make sure that active connections are not cleared during test when an exception happens --- .../active_record/connection_adapters/abstract/connection_pool.rb | 6 ++++-- activerecord/test/cases/connection_management_test.rb | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'activerecord') 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) -- cgit v1.2.3 From 6067d29a1fd78a60c44f18d4c0815002ff2f8c84 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 17:09:22 -0700 Subject: 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(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From 58becf116580c37c63b89f4a660ebe293f6e7c4e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 17:27:32 -0700 Subject: 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(-) (limited to 'activerecord') 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! -- cgit v1.2.3 From 2be383b94619595a2b4886e35f599757eab6c878 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 29 Mar 2011 17:50:39 -0700 Subject: 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(-) (limited to 'activerecord') 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 -- cgit v1.2.3 From cfb6f77ac0574d3cd613c99fcc32135016b4284c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 30 Mar 2011 09:54:46 -0700 Subject: 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(-) (limited to 'activerecord') 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) -- cgit v1.2.3