aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_controller/metal/strong_parameters.rb8
-rw-r--r--actionpack/test/controller/parameters/accessors_test.rb18
-rw-r--r--activerecord/CHANGELOG.md7
-rw-r--r--activerecord/lib/active_record/associations.rb2
-rw-r--r--activerecord/lib/active_record/migration.rb10
-rw-r--r--activerecord/lib/active_record/migration/command_recorder.rb19
-rw-r--r--activerecord/lib/active_record/migration/compatibility.rb15
-rw-r--r--activerecord/lib/active_record/relation.rb16
-rw-r--r--activerecord/lib/arel/visitors/mysql.rb4
-rw-r--r--activerecord/lib/rails/generators/active_record/model/model_generator.rb1
-rw-r--r--activerecord/test/cases/invertible_migration_test.rb16
-rw-r--r--activerecord/test/cases/migration/command_recorder_test.rb10
-rw-r--r--activerecord/test/cases/migration/compatibility_test.rb14
-rw-r--r--railties/CHANGELOG.md16
-rw-r--r--railties/test/generators/model_generator_test.rb9
-rw-r--r--railties/test/generators/scaffold_generator_test.rb6
16 files changed, 163 insertions, 8 deletions
diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index a37f08d944..c1272ce667 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -348,6 +348,14 @@ module ActionController
end
alias_method :each, :each_pair
+ # Convert all hashes in values into parameters, then yield each value in
+ # the same way as <tt>Hash#each_value</tt>.
+ def each_value(&block)
+ @parameters.each_pair do |key, value|
+ yield [convert_hashes_to_parameters(key, value)]
+ end
+ end
+
# Attribute that keeps track of converted arrays, if any, to avoid double
# looping in the common use case permit + mass-assignment. Defined in a
# method to instantiate it only if needed.
diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb
index 68c7f2d9ea..9f1fb3d042 100644
--- a/actionpack/test/controller/parameters/accessors_test.rb
+++ b/actionpack/test/controller/parameters/accessors_test.rb
@@ -75,6 +75,24 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
end
end
+ test "each_value carries permitted status" do
+ @params.permit!
+ @params["person"].each_value { |value| assert(value.permitted?) if value == 32 }
+ end
+
+ test "each_value carries unpermitted status" do
+ @params["person"].each_value { |value| assert_not(value.permitted?) if value == 32 }
+ end
+
+ test "each_key converts to hash for permitted" do
+ @params.permit!
+ @params.each_key { |key| assert_kind_of(String, key) if key == "person" }
+ end
+
+ test "each_key converts to hash for unpermitted" do
+ @params.each_key { |key| assert_kind_of(String, key) if key == "person" }
+ end
+
test "empty? returns true when params contains no key/value pairs" do
params = ActionController::Parameters.new
assert_empty params
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9acc21510a..7bcf755258 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+* Fix `transaction` reverting for migrations.
+
+ Before: Commands inside a `transaction` in a reverted migration ran uninverted.
+ Now: This change fixes that by reverting commands inside `transaction` block.
+
+ *fatkodima*, *David Verhasselt*
+
* Raise an error instead of scanning the filesystem root when `fixture_path` is blank.
*Gannon McGibbon*, *Max Albrecht*
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b1778732dd..ab1e7ad269 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1582,7 +1582,7 @@ module ActiveRecord
# association will use "taggable_type" as the default <tt>:foreign_type</tt>.
# [:primary_key]
# Specify the method that returns the primary key of associated object used for the association.
- # By default this is id.
+ # By default this is +id+.
# [:dependent]
# If set to <tt>:destroy</tt>, the associated object is destroyed when this object is. If set to
# <tt>:delete</tt>, the associated object is deleted *without* calling its destroy method.
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 9651e69edd..d712d4b3cf 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -678,15 +678,13 @@ module ActiveRecord
if connection.respond_to? :revert
connection.revert { yield }
else
- recorder = CommandRecorder.new(connection)
+ recorder = command_recorder
@connection = recorder
suppress_messages do
connection.revert { yield }
end
@connection = recorder.delegate
- recorder.commands.each do |cmd, args, block|
- send(cmd, *args, &block)
- end
+ recorder.replay(self)
end
end
end
@@ -962,6 +960,10 @@ module ActiveRecord
yield
end
end
+
+ def command_recorder
+ CommandRecorder.new(connection)
+ end
end
# MigrationProxy is used to defer loading of the actual migration classes
diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb
index dea6d4ec08..82f5121d94 100644
--- a/activerecord/lib/active_record/migration/command_recorder.rb
+++ b/activerecord/lib/active_record/migration/command_recorder.rb
@@ -108,11 +108,17 @@ module ActiveRecord
yield delegate.update_table_definition(table_name, self)
end
+ def replay(migration)
+ commands.each do |cmd, args, block|
+ migration.send(cmd, *args, &block)
+ end
+ end
+
private
module StraightReversions # :nodoc:
private
- { transaction: :transaction,
+ {
execute_block: :execute_block,
create_table: :drop_table,
create_join_table: :drop_join_table,
@@ -133,6 +139,17 @@ module ActiveRecord
include StraightReversions
+ def invert_transaction(args)
+ sub_recorder = CommandRecorder.new(delegate)
+ sub_recorder.revert { yield }
+
+ invertions_proc = proc {
+ sub_recorder.replay(self)
+ }
+
+ [:transaction, args, invertions_proc]
+ end
+
def invert_drop_table(args, &block)
if args.size == 1 && block == nil
raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty)."
diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index 0edaaa0cf9..8f6fcfcaea 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -16,6 +16,21 @@ module ActiveRecord
V6_0 = Current
class V5_2 < V6_0
+ module CommandRecorder
+ def invert_transaction(args, &block)
+ [:transaction, args, block]
+ end
+ end
+
+ private
+
+ def command_recorder
+ recorder = super
+ class << recorder
+ prepend CommandRecorder
+ end
+ recorder
+ end
end
class V5_1 < V5_2
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 806f8a1cbb..771ca952e1 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -348,10 +348,14 @@ module ActiveRecord
end
stmt = Arel::UpdateManager.new
-
- stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates, table.name))
stmt.table(table)
+ if updates.is_a?(Hash)
+ stmt.set _substitute_values(updates)
+ else
+ stmt.set Arel.sql(klass.sanitize_sql_for_assignment(updates, table.name))
+ end
+
if has_join_values? || offset_value
@klass.connection.join_to_update(stmt, arel, arel_attribute(primary_key))
else
@@ -625,6 +629,14 @@ module ActiveRecord
end
private
+ def _substitute_values(values)
+ values.map do |name, value|
+ attr = arel_attribute(name)
+ type = klass.type_for_attribute(attr.name)
+ bind = predicate_builder.build_bind_attribute(attr.name, type.cast(value))
+ [attr, bind]
+ end
+ end
def has_join_values?
joins_values.any? || left_outer_joins_values.any?
diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb
index 37bfb661f0..ee75b6bb25 100644
--- a/activerecord/lib/arel/visitors/mysql.rb
+++ b/activerecord/lib/arel/visitors/mysql.rb
@@ -37,6 +37,10 @@ module Arel # :nodoc: all
visit o.expr, collector
end
+ def visit_Arel_Nodes_UnqualifiedColumn(o, collector)
+ visit o.expr, collector
+ end
+
###
# :'(
# http://dev.mysql.com/doc/refman/5.0/en/select.html#id3482214
diff --git a/activerecord/lib/rails/generators/active_record/model/model_generator.rb b/activerecord/lib/rails/generators/active_record/model/model_generator.rb
index 25e54f3ac8..747c8493b1 100644
--- a/activerecord/lib/rails/generators/active_record/model/model_generator.rb
+++ b/activerecord/lib/rails/generators/active_record/model/model_generator.rb
@@ -14,6 +14,7 @@ module ActiveRecord
class_option :parent, type: :string, desc: "The parent class for the generated model"
class_option :indexes, type: :boolean, default: true, desc: "Add indexes for references and belongs_to columns"
class_option :primary_key_type, type: :string, desc: "The type for primary key"
+ class_option :migrations_paths, type: :string, desc: "The migration path for your generated migrations. If this is not set it will default to db/migrate"
# creates the migration file for the model.
def create_migration_file
diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb
index 363beb4780..6cf17ac15d 100644
--- a/activerecord/test/cases/invertible_migration_test.rb
+++ b/activerecord/test/cases/invertible_migration_test.rb
@@ -22,6 +22,14 @@ module ActiveRecord
end
end
+ class InvertibleTransactionMigration < InvertibleMigration
+ def change
+ transaction do
+ super
+ end
+ end
+ end
+
class InvertibleRevertMigration < SilentMigration
def change
revert do
@@ -271,6 +279,14 @@ module ActiveRecord
assert_not revert.connection.table_exists?("horses")
end
+ def test_migrate_revert_transaction
+ migration = InvertibleTransactionMigration.new
+ migration.migrate :up
+ assert migration.connection.table_exists?("horses")
+ migration.migrate :down
+ assert_not migration.connection.table_exists?("horses")
+ end
+
def test_migrate_revert_change_column_default
migration1 = ChangeColumnDefault1.new
migration1.migrate(:up)
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index 199818fc90..01f8628fc5 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -360,6 +360,16 @@ module ActiveRecord
@recorder.inverse_of :remove_foreign_key, [:dogs]
end
end
+
+ def test_invert_transaction_with_irreversible_inside_is_irreversible
+ assert_raises(ActiveRecord::IrreversibleMigration) do
+ @recorder.revert do
+ @recorder.transaction do
+ @recorder.execute "some sql"
+ end
+ end
+ end
+ end
end
end
end
diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index 69a50674af..017ee7951e 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -127,6 +127,20 @@ module ActiveRecord
assert_match(/LegacyMigration < ActiveRecord::Migration\[4\.2\]/, e.message)
end
+ def test_legacy_migrations_not_raise_exception_on_reverting_transaction
+ migration = Class.new(ActiveRecord::Migration[5.2]) {
+ def change
+ transaction do
+ execute "select 1"
+ end
+ end
+ }.new
+
+ assert_nothing_raised do
+ migration.migrate(:down)
+ end
+ end
+
if current_adapter?(:PostgreSQLAdapter)
class Testing < ActiveRecord::Base
end
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md
index 4342cf6968..a839f80eb3 100644
--- a/railties/CHANGELOG.md
+++ b/railties/CHANGELOG.md
@@ -1,3 +1,19 @@
+* Adds an option to the model generator to allow setting the
+ migrations paths for that migration. This is useful for
+ applications that use multiple databases and put migrations
+ per database in their own directories.
+
+ ```
+ bin/rails g model Room capacity:integer --migrations-paths=db/kingston_migrate
+ invoke active_record
+ create db/kingston_migrate/20180830151055_create_rooms.rb
+ ```
+
+ Because rails scaffolding uses the model generator, you can
+ also specify migrations paths with the scaffold generator.
+
+ *Gannon McGibbon*
+
* Raise an error when "recyclable cache keys" are being used by a cache store
that does not explicitly support it. Custom cache keys that do support this feature
can bypass this error by implementing the `supports_cache_versioning?` method on their
diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb
index 7febdfae96..5a0c2f74c7 100644
--- a/railties/test/generators/model_generator_test.rb
+++ b/railties/test/generators/model_generator_test.rb
@@ -392,6 +392,15 @@ class ModelGeneratorTest < Rails::Generators::TestCase
end
end
+ def test_migrations_paths_puts_migrations_in_that_folder
+ run_generator ["account", "--migrations_paths=db/test_migrate"]
+ assert_migration "db/test_migrate/create_accounts.rb" do |content|
+ assert_method :change, content do |change|
+ assert_match(/create_table :accounts/, change)
+ end
+ end
+ end
+
def test_required_belongs_to_adds_required_association
run_generator ["account", "supplier:references{required}"]
diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb
index e90834bc2b..dbcf49290e 100644
--- a/railties/test/generators/scaffold_generator_test.rb
+++ b/railties/test/generators/scaffold_generator_test.rb
@@ -476,6 +476,12 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase
end
end
+ def test_scaffold_generator_migrations_paths
+ run_generator ["posts", "--migrations-paths=db/kingston_migrate"]
+
+ assert_migration "db/kingston_migrate/create_posts.rb"
+ end
+
def test_scaffold_generator_password_digest
run_generator ["user", "name", "password:digest"]