Skip to content

Commit 3f17728

Browse files
committed
Fix migrating multiple DBs with pending migration action
This was supposed to work with multiple DBs, but the task code and the migration code have fallen out of sync and the pending migration action only works against primary. With this patch, we use `ActiveRecord::Tasks::DatabaseTasks.migrate_all` for both, and introduce `ActiveRecord::Tasks::DatabaseTasks.dump_all` to use in the task and action.
1 parent 2990ce3 commit 3f17728

File tree

5 files changed

+77
-9
lines changed

5 files changed

+77
-9
lines changed

activerecord/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Fix migrating multiple databases with `ActiveRecord::PendingMigration` action.
2+
3+
*Gannon McGibbon*
4+
15
* Enable automatically retrying idempotent association queries on connection
26
errors.
37

activerecord/lib/active_record/migration.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,10 @@ class PendingMigrationError < MigrationError # :nodoc:
148148
include ActiveSupport::ActionableError
149149

150150
action "Run pending migrations" do
151-
ActiveRecord::Tasks::DatabaseTasks.migrate
151+
ActiveRecord::Tasks::DatabaseTasks.migrate_all
152152

153153
if ActiveRecord.dump_schema_after_migration
154-
connection = ActiveRecord::Tasks::DatabaseTasks.migration_connection
155-
ActiveRecord::Tasks::DatabaseTasks.dump_schema(connection.pool.db_config)
154+
ActiveRecord::Tasks::DatabaseTasks.dump_all
156155
end
157156
end
158157

activerecord/lib/active_record/railties/databases.rake

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -447,11 +447,7 @@ db_namespace = namespace :db do
447447
namespace :schema do
448448
desc "Create a database schema file (either db/schema.rb or db/structure.sql, depending on `ENV['SCHEMA_FORMAT']` or `config.active_record.schema_format`)"
449449
task dump: :load_config do
450-
ActiveRecord::Tasks::DatabaseTasks.with_temporary_pool_for_each do |pool|
451-
db_config = pool.db_config
452-
schema_format = ENV.fetch("SCHEMA_FORMAT", ActiveRecord.schema_format).to_sym
453-
ActiveRecord::Tasks::DatabaseTasks.dump_schema(db_config, schema_format)
454-
end
450+
ActiveRecord::Tasks::DatabaseTasks.dump_all
455451

456452
db_namespace["schema:dump"].reenable
457453
end

activerecord/lib/active_record/tasks/database_tasks.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,14 @@ def reconstruct_from_schema(db_config, format = ActiveRecord.schema_format, file
428428
end
429429
end
430430

431+
def dump_all
432+
with_temporary_pool_for_each do |pool|
433+
db_config = pool.db_config
434+
schema_format = ENV.fetch("SCHEMA_FORMAT", ActiveRecord.schema_format).to_sym
435+
ActiveRecord::Tasks::DatabaseTasks.dump_schema(db_config, schema_format)
436+
end
437+
end
438+
431439
def dump_schema(db_config, format = ActiveRecord.schema_format) # :nodoc:
432440
return unless db_config.schema_dump
433441

railties/test/application/configuration_test.rb

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ class MyLogger < ::Logger
159159
assert_match(/You're using a cache/, error.message)
160160
end
161161

162-
test "a renders exception on pending migration" do
162+
test "renders an exception on pending migration" do
163163
add_to_config <<-RUBY
164164
config.active_record.migration_error = :page_load
165165
config.consider_all_requests_local = true
@@ -200,6 +200,67 @@ def change
200200
end
201201
end
202202

203+
test "renders an exception on pending migration for multiple DBs" do
204+
add_to_config <<-RUBY
205+
config.active_record.migration_error = :page_load
206+
config.consider_all_requests_local = true
207+
config.action_dispatch.show_exceptions = :all
208+
RUBY
209+
210+
app_file "config/database.yml", <<-YAML
211+
<%= Rails.env %>:
212+
primary:
213+
adapter: sqlite3
214+
database: 'dev_db'
215+
other:
216+
adapter: sqlite3
217+
database: 'other_dev_db'
218+
migrations_paths: db/other_migrate
219+
YAML
220+
221+
app_file "db/migrate/20140708012246_create_users.rb", <<-RUBY
222+
class CreateUsers < ActiveRecord::Migration::Current
223+
def change
224+
create_table :users
225+
end
226+
end
227+
RUBY
228+
229+
app_file "db/other_migrate/20140708012247_create_blogs.rb", <<-RUBY
230+
class CreateBlogs < ActiveRecord::Migration::Current
231+
def change
232+
create_table :blogs
233+
end
234+
end
235+
RUBY
236+
237+
app "development"
238+
239+
begin
240+
ActiveRecord::Migrator.migrations_paths = ["#{app_path}/db/migrate", "#{app_path}/db/other_migrate"]
241+
242+
get "/foo"
243+
assert_equal 500, last_response.status
244+
assert_match "ActiveRecord::PendingMigrationError", last_response.body
245+
246+
assert_changes -> { File.exist?(File.join(app_path, "db", "schema.rb")) }, from: false, to: true do
247+
output = capture(:stdout) do
248+
post "/rails/actions", { error: "ActiveRecord::PendingMigrationError", action: "Run pending migrations", location: "/foo" }
249+
end
250+
251+
assert_match(/\d{14}\s+CreateUsers/, output)
252+
assert_match(/\d{14}\s+CreateBlogs/, output)
253+
end
254+
255+
assert_equal 302, last_response.status
256+
257+
get "/foo"
258+
assert_equal 404, last_response.status
259+
ensure
260+
ActiveRecord::Migrator.migrations_paths = nil
261+
end
262+
end
263+
203264
test "Rails.groups returns available groups" do
204265
require "rails"
205266

0 commit comments

Comments
 (0)