diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 11 | ||||
| -rw-r--r-- | activerecord/lib/active_record/railties/databases.rake | 3 | ||||
| -rw-r--r-- | activerecord/lib/active_record/transactions.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/cases/transactions_test.rb | 42 | ||||
| -rw-r--r-- | activesupport/lib/active_support/core_ext/hash/deep_merge.rb | 33 | ||||
| -rw-r--r-- | activesupport/test/core_ext/hash_ext_test.rb | 18 | ||||
| -rw-r--r-- | railties/test/application/rake/migrations_test.rb | 2 |
7 files changed, 95 insertions, 18 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4164b928bd..b684703923 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,14 @@ +* Return a non zero status when running `rake db:migrate:status` and migration table does + not exist. + + *Paul B.* + +* Keep track of dirty attributes after transaction is rollback. + + Related #13166. + + *Bogdan Gusiev* *arthurnn* + * Add support for module-level `table_name_suffix` in models. This makes `table_name_suffix` work the same way as `table_name_prefix` when diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 9538ead5f1..9e8e5fe94b 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -83,8 +83,7 @@ db_namespace = namespace :db do desc 'Display status of migrations' task :status => [:environment, :load_config] do unless ActiveRecord::Base.connection.table_exists?(ActiveRecord::Migrator.schema_migrations_table_name) - puts 'Schema migrations table does not exist yet.' - next # means "return" for rake task + abort 'Schema migrations table does not exist yet.' end db_list = ActiveRecord::Base.connection.select_values("SELECT version FROM #{ActiveRecord::Migrator.schema_migrations_table_name}") db_list.map! { |version| "%.3d" % version } diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 17f76b63b3..cf76d53bf7 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -350,6 +350,7 @@ module ActiveRecord end @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 @_start_transaction_state[:frozen?] = @attributes.frozen? + @_start_transaction_state[:changed_attributes] ||= changed_attributes end # Clear the new record state and id of a record. @@ -368,6 +369,9 @@ module ActiveRecord @attributes = @attributes.dup if @attributes.frozen? @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] + changed_attributes.replace(restore_state[:changed_attributes]).delete_if do |attribute, old_value| + old_value == @attributes[attribute] + end if restore_state.has_key?(:id) write_attribute(self.class.primary_key, restore_state[:id]) else diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index e6ed85394b..7f2e830083 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -274,6 +274,48 @@ class TransactionTest < ActiveRecord::TestCase end end + def test_rollback_when_changing_inside_transaction + assert !@first.approved? + Topic.transaction do + @first.approved = true + @first.save! + raise ActiveRecord::Rollback + end + assert @first.approved + assert @first.changes["approved"] + @first.save! + assert @first.reload.approved + end + + def test_rollback_when_changing_outside_transaction + assert !@first.approved? + @first.approved = true + Topic.transaction do + @first.save! + raise ActiveRecord::Rollback + end + assert @first.changes["approved"] + assert @first.approved + @first.save! + assert @first.reload.approved + end + + def test_rollback_when_changing_back_to_prev_stage + assert !@first.approved? + Topic.transaction do + @first.approved = true + @first.save! + @first.approved = false + @first.save! + raise ActiveRecord::Rollback + end + assert !@first.approved + assert !@first.changes["approved"] + @first.save! + assert !@first.reload.approved + end + + def test_force_savepoint_in_nested_transaction Topic.transaction do @first.approved = true diff --git a/activesupport/lib/active_support/core_ext/hash/deep_merge.rb b/activesupport/lib/active_support/core_ext/hash/deep_merge.rb index dc86c92003..763d563231 100644 --- a/activesupport/lib/active_support/core_ext/hash/deep_merge.rb +++ b/activesupport/lib/active_support/core_ext/hash/deep_merge.rb @@ -1,27 +1,38 @@ class Hash # Returns a new hash with +self+ and +other_hash+ merged recursively. # - # h1 = { x: { y: [4, 5, 6] }, z: [7, 8, 9] } - # h2 = { x: { y: [7, 8, 9] }, z: 'xyz' } + # h1 = { a: true, b: { c: [1, 2, 3] } } + # h2 = { a: false, b: { x: [3, 4, 5] } } # - # h1.deep_merge(h2) # => {x: {y: [7, 8, 9]}, z: "xyz"} - # h2.deep_merge(h1) # => {x: {y: [4, 5, 6]}, z: [7, 8, 9]} - # h1.deep_merge(h2) { |key, old, new| Array.wrap(old) + Array.wrap(new) } - # # => {:x=>{:y=>[4, 5, 6, 7, 8, 9]}, :z=>[7, 8, 9, "xyz"]} + # h1.deep_merge(h2) #=> { a: false, b: { c: [1, 2, 3], x: [3, 4, 5] } } + # + # Like with Hash#merge in the standard library, a block can be provided + # to merge values: + # + # h1 = { a: 100, b: 200, c: { c1: 100 } } + # h2 = { b: 250, c: { c1: 200 } } + # h1.deep_merge(h2) { |key, this_val, other_val| this_val + other_val } + # # => { a: 100, b: 450, c: { c1: 300 } } def deep_merge(other_hash, &block) dup.deep_merge!(other_hash, &block) end # Same as +deep_merge+, but modifies +self+. def deep_merge!(other_hash, &block) - other_hash.each_pair do |k,v| - tv = self[k] - if tv.is_a?(Hash) && v.is_a?(Hash) - self[k] = tv.deep_merge(v, &block) + other_hash.each_pair do |current_key, other_value| + this_value = self[current_key] + + self[current_key] = if this_value.is_a?(Hash) && other_value.is_a?(Hash) + this_value.deep_merge(other_value, &block) else - self[k] = block && tv ? block.call(k, tv, v) : v + if block_given? && key?(current_key) + block.call(current_key, this_value, other_value) + else + other_value + end end end + self end end diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index d824a16e98..ad354a4c30 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -697,6 +697,16 @@ class HashExtTest < ActiveSupport::TestCase assert_equal expected, hash_1 end + def test_deep_merge_with_falsey_values + hash_1 = { e: false } + hash_2 = { e: 'e' } + expected = { e: [:e, false, 'e'] } + assert_equal(expected, hash_1.deep_merge(hash_2) { |k, o, n| [k, o, n] }) + + hash_1.deep_merge!(hash_2) { |k, o, n| [k, o, n] } + assert_equal expected, hash_1 + end + def test_deep_merge_on_indifferent_access hash_1 = HashWithIndifferentAccess.new({ :a => "a", :b => "b", :c => { :c1 => "c1", :c2 => "c2", :c3 => { :d1 => "d1" } } }) hash_2 = HashWithIndifferentAccess.new({ :a => 1, :c => { :c1 => 2, :c3 => { :d2 => "d2" } } }) @@ -905,11 +915,11 @@ class HashExtTest < ActiveSupport::TestCase def test_compact hash_contain_nil_value = @symbols.merge(z: nil) hash_with_only_nil_values = { a: nil, b: nil } - + h = hash_contain_nil_value.dup assert_equal(@symbols, h.compact) assert_equal(hash_contain_nil_value, h) - + h = hash_with_only_nil_values.dup assert_equal({}, h.compact) assert_equal(hash_with_only_nil_values, h) @@ -918,11 +928,11 @@ class HashExtTest < ActiveSupport::TestCase def test_compact! hash_contain_nil_value = @symbols.merge(z: nil) hash_with_only_nil_values = { a: nil, b: nil } - + h = hash_contain_nil_value.dup assert_equal(@symbols, h.compact!) assert_equal(@symbols, h) - + h = hash_with_only_nil_values.dup assert_equal({}, h.compact!) assert_equal({}, h) diff --git a/railties/test/application/rake/migrations_test.rb b/railties/test/application/rake/migrations_test.rb index b7fd5d02c5..a6900a57c4 100644 --- a/railties/test/application/rake/migrations_test.rb +++ b/railties/test/application/rake/migrations_test.rb @@ -58,7 +58,7 @@ module ApplicationTests end test 'migration status when schema migrations table is not present' do - output = Dir.chdir(app_path){ `rake db:migrate:status` } + output = Dir.chdir(app_path){ `rake db:migrate:status 2>&1` } assert_equal "Schema migrations table does not exist yet.\n", output end |