diff options
| -rw-r--r-- | actionpack/lib/action_controller/metal/helpers.rb | 2 | ||||
| -rw-r--r-- | activemodel/lib/active_model/dirty.rb | 9 | ||||
| -rw-r--r-- | activemodel/lib/active_model/validations/numericality.rb | 11 | ||||
| -rw-r--r-- | activemodel/test/cases/validations/numericality_validation_test.rb | 2 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 18 | ||||
| -rwxr-xr-x | activerecord/test/cases/base_test.rb | 50 | ||||
| -rw-r--r-- | activerecord/test/cases/dirty_test.rb | 9 | ||||
| -rw-r--r-- | activerecord/test/cases/transaction_callbacks_test.rb | 34 |
8 files changed, 104 insertions, 31 deletions
diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index 1110d7c117..b6d4fb1284 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -104,7 +104,7 @@ module ActionController def all_application_helpers helpers = [] helpers_path.each do |path| - extract = /^#{Regexp.quote(path)}\/?(.*)_helper.rb$/ + extract = /^#{Regexp.quote(path.to_s)}\/?(.*)_helper.rb$/ helpers += Dir["#{path}/**/*_helper.rb"].map { |file| file.sub(extract, '\1') } end helpers.sort! diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index a7ee15a7f6..bbcc345e4b 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -124,11 +124,12 @@ module ActiveModel @previously_changed end + # Map of change <tt>attr => original value</tt>. + def changed_attributes + @changed_attributes ||= {} + end + private - # Map of change <tt>attr => original value</tt>. - def changed_attributes - @changed_attributes ||= {} - end # Handle <tt>*_changed?</tt> for +method_missing+. def attribute_changed?(attr) diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 716010e88b..ac8308b0d7 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -57,10 +57,15 @@ module ActiveModel protected def parse_raw_value_as_a_number(raw_value) - begin - Kernel.Float(raw_value) - rescue ArgumentError, TypeError + case raw_value + when /\A0x/ nil + else + begin + Kernel.Float(raw_value) + rescue ArgumentError, TypeError + nil + end end end diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index be620c53fa..963ad6450b 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -18,7 +18,7 @@ class NumericalityValidationTest < ActiveModel::TestCase FLOATS = [0.0, 10.0, 10.5, -10.5, -0.0001] + FLOAT_STRINGS INTEGERS = [0, 10, -10] + INTEGER_STRINGS BIGDECIMAL = BIGDECIMAL_STRINGS.collect! { |bd| BigDecimal.new(bd) } - JUNK = ["not a number", "42 not a number", "0xdeadbeef", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12", "123\nnot a number"] + JUNK = ["not a number", "42 not a number", "0xdeadbeef", "0xinvalidhex", "00-1", "--3", "+-3", "+3-1", "-+019.0", "12.12.13.12", "123\nnot a number"] INFINITY = [1.0/0.0] def test_default_validates_numericality_of diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 04c474c9a1..18f75af11b 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1362,7 +1362,8 @@ module ActiveRecord #:nodoc: def replace_bind_variables(statement, values) #:nodoc: raise_if_bind_arity_mismatch(statement, statement.count('?'), values.size) bound = values.dup - statement.gsub('?') { quote_bound_value(bound.shift) } + c = connection + statement.gsub('?') { quote_bound_value(bound.shift, c) } end def replace_named_bind_variables(statement, bind_vars) #:nodoc: @@ -1394,15 +1395,15 @@ module ActiveRecord #:nodoc: expanded end - def quote_bound_value(value) #:nodoc: + def quote_bound_value(value, c = connection) #:nodoc: if value.respond_to?(:map) && !value.acts_like?(:string) if value.respond_to?(:empty?) && value.empty? - connection.quote(nil) + c.quote(nil) else - value.map { |v| connection.quote(v) }.join(',') + value.map { |v| c.quote(v) }.join(',') end else - connection.quote(value) + c.quote(value) end end @@ -1462,7 +1463,14 @@ module ActiveRecord #:nodoc: callback(:after_initialize) if respond_to_without_attributes?(:after_initialize) cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast) cloned_attributes.delete(self.class.primary_key) + @attributes = cloned_attributes + + @changed_attributes = {} + attributes_from_column_definition.each do |attr, orig_value| + @changed_attributes[attr] = orig_value if field_changed?(attr, orig_value, @attributes[attr]) + end + clear_aggregation_cache @attributes_cache = {} @new_record = true diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b7ae619787..7c4d92f3f8 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1456,6 +1456,56 @@ class BasicsTest < ActiveRecord::TestCase assert_kind_of Client, clone end + def test_clone_of_new_object_with_defaults + developer = Developer.new + assert !developer.name_changed? + assert !developer.salary_changed? + + cloned_developer = developer.clone + assert !cloned_developer.name_changed? + assert !cloned_developer.salary_changed? + end + + def test_clone_of_new_object_marks_attributes_as_dirty + developer = Developer.new :name => 'Bjorn', :salary => 100000 + assert developer.name_changed? + assert developer.salary_changed? + + cloned_developer = developer.clone + assert cloned_developer.name_changed? + assert cloned_developer.salary_changed? + end + + def test_clone_of_new_object_marks_as_dirty_only_changed_attributes + developer = Developer.new :name => 'Bjorn' + assert developer.name_changed? # obviously + assert !developer.salary_changed? # attribute has non-nil default value, so treated as not changed + + cloned_developer = developer.clone + assert cloned_developer.name_changed? + assert !cloned_developer.salary_changed? # ... and cloned instance should behave same + end + + def test_clone_of_saved_object_marks_attributes_as_dirty + developer = Developer.create! :name => 'Bjorn', :salary => 100000 + assert !developer.name_changed? + assert !developer.salary_changed? + + cloned_developer = developer.clone + assert cloned_developer.name_changed? # both attributes differ from defaults + assert cloned_developer.salary_changed? + end + + def test_clone_of_saved_object_marks_as_dirty_only_changed_attributes + developer = Developer.create! :name => 'Bjorn' + assert !developer.name_changed? # both attributes of saved object should be threated as not changed + assert !developer.salary_changed? + + cloned_developer = developer.clone + assert cloned_developer.name_changed? # ... but on cloned object should be + assert !cloned_developer.salary_changed? # ... BUT salary has non-nil default which should be threated as not changed on cloned instance + end + def test_bignum company = Company.find(1) company.rating = 2147483647 diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 7a17ef1ee0..3ea2948f62 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -338,6 +338,15 @@ class DirtyTest < ActiveRecord::TestCase assert !pirate.changed? end + def test_cloned_objects_should_not_copy_dirty_flag_from_creator + pirate = Pirate.create!(:catchphrase => "shiver me timbers") + pirate_clone = pirate.clone + pirate_clone.reset_catchphrase! + pirate.catchphrase = "I love Rum" + assert pirate.catchphrase_changed? + assert !pirate_clone.catchphrase_changed? + end + def test_reverted_changes_are_not_dirty phrase = "shiver me timbers" pirate = Pirate.create!(:catchphrase => phrase) diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index a07da093f1..9c74dce965 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -54,7 +54,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first.after_rollback_block{|r| r.history << :after_rollback} @first.save! - assert @first.history, [:after_commit] + assert_equal [:after_commit], @first.history end def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record @@ -67,7 +67,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy} @first.save! - assert @first.history, [:commit_on_update] + assert_equal [:commit_on_update], @first.history end def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record @@ -80,7 +80,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy} @first.destroy - assert @first.history, [:commit_on_destroy] + assert_equal [:commit_on_destroy], @first.history end def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record @@ -93,7 +93,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @new_record.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy} @new_record.save! - assert @new_record.history, [:commit_on_create] + assert_equal [:commit_on_create], @new_record.history end def test_call_after_rollback_after_transaction_rollsback @@ -105,7 +105,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase raise ActiveRecord::Rollback end - assert @first.history, [:after_rollback] + assert_equal [:after_rollback], @first.history end def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record @@ -122,7 +122,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase raise ActiveRecord::Rollback end - assert @first.history, [:rollback_on_update] + assert_equal [:rollback_on_update], @first.history end def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record @@ -139,7 +139,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase raise ActiveRecord::Rollback end - assert @first.history, [:rollback_on_destroy] + assert_equal [:rollback_on_destroy], @first.history end def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record @@ -156,7 +156,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase raise ActiveRecord::Rollback end - assert @new_record.history, [:rollback_on_create] + assert_equal [:rollback_on_create], @new_record.history end def test_call_after_rollback_when_commit_fails @@ -170,7 +170,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first.after_rollback_block{|r| r.history << :after_rollback} assert !@first.save rescue nil - assert @first.history == [:after_rollback] + assert_equal [:after_rollback], @first.history ensure @first.connection.class.send(:remove_method, :commit_db_transaction) @first.connection.class.send(:alias_method, :commit_db_transaction, :real_method_commit_db_transaction) @@ -196,10 +196,10 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end end - assert 1, @first.commits - assert 0, @first.rollbacks - assert 1, @second.commits - assert 1, @second.rollbacks + assert_equal 1, @first.commits + assert_equal 0, @first.rollbacks + assert_equal 1, @second.commits + assert_equal 1, @second.rollbacks end def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails @@ -221,8 +221,8 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end end - assert 1, @first.commits - assert 2, @first.rollbacks + assert_equal 1, @first.commits + assert_equal 2, @first.rollbacks end def test_after_transaction_callbacks_should_not_raise_errors @@ -232,13 +232,13 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";} @first.save! - assert_equal @first.last_after_transaction_error, :commit + assert_equal :commit, @first.last_after_transaction_error Topic.transaction do @first.save! raise ActiveRecord::Rollback end - assert_equal @first.last_after_transaction_error, :rollback + assert_equal :rollback, @first.last_after_transaction_error end end |