From e5b39862cb4f8f3097cc8ab2dc606ddb1bd1febb Mon Sep 17 00:00:00 2001 From: schneems Date: Sun, 3 Jun 2012 15:07:11 -0500 Subject: [PATCH 1/3] add convenience methods for checking migrations if a rails project needs to be migrated ActiveRecord::Migrator.needs_migration? will be true or false if the current version matches the last version. --- activerecord/lib/active_record/migration.rb | 8 ++++++++ activerecord/test/cases/migration_test.rb | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index ac4f53c774..2975b045e7 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -605,6 +605,14 @@ module ActiveRecord end end + def needs_migration? + current_version != last_version + end + + def last_version + migrations(migrations_paths).last.try(:version)||0 + end + def proper_table_name(name) # Use the Active Record objects own table_name, or pre/suffix from ActiveRecord::Base if name is a symbol/string name.table_name rescue "#{ActiveRecord::Base.table_name_prefix}#{name}#{ActiveRecord::Base.table_name_suffix}" diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 5d1bad0d54..45c847bae8 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -56,6 +56,21 @@ class MigrationTest < ActiveRecord::TestCase Person.reset_column_information end + def test_migrator_versions + migrations_path = MIGRATIONS_ROOT + "/valid" + ActiveRecord::Migrator.migrations_paths = migrations_path + + ActiveRecord::Migrator.up(migrations_path) + assert_equal 3, ActiveRecord::Migrator.current_version + assert_equal 3, ActiveRecord::Migrator.last_version + assert_equal false, ActiveRecord::Migrator.needs_migration? + + ActiveRecord::Migrator.down(MIGRATIONS_ROOT + "/valid") + assert_equal 0, ActiveRecord::Migrator.current_version + assert_equal 3, ActiveRecord::Migrator.last_version + assert_equal true, ActiveRecord::Migrator.needs_migration? + end + def test_create_table_with_force_true_does_not_drop_nonexisting_table if Person.connection.table_exists?(:testings2) Person.connection.drop_table :testings2 From 96f19f6cf62fb4705a75cf4a81e4e2f145a4cee2 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 5 Jun 2012 19:15:16 -0500 Subject: [PATCH 2/3] raise error for pending migration can be configured by setting config.active_record.migration. Setting to :page_load will raise an error on each page refresh if there are migrations that are pending. Setting to :page_load is defaulted in development for new applications. --- activerecord/lib/active_record/migration.rb | 28 +++++++++++++++++-- activerecord/lib/active_record/railtie.rb | 7 +++++ activerecord/test/cases/migration_test.rb | 4 +-- .../config/environments/development.rb.tt | 3 ++ .../rails/app/templates/test/test_helper.rb | 2 ++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 2975b045e7..a0169203b0 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -32,6 +32,12 @@ module ActiveRecord end end + class PendingMigrationError < ActiveRecordError#:nodoc: + def initialize + super("Migrations are pending run 'bundle exec rake db:migrate RAILS_ENV=#{ENV['RAILS_ENV']}' to resolve the issue") + end + end + # = Active Record Migrations # # Migrations can manage the evolution of a schema used by several physical @@ -326,10 +332,28 @@ module ActiveRecord class Migration autoload :CommandRecorder, 'active_record/migration/command_recorder' + + # This class is used to verify that all migrations have been run before + # loading a web page if config.active_record.migration_error is set to :page_load + class CheckPending + def initialize(app) + @app = app + end + + def call(env) + ActiveRecord::Migration.check_pending! + status, headers, body = @app.call(env) + end + end + class << self attr_accessor :delegate # :nodoc: end + def self.check_pending! + raise ActiveRecord::PendingMigrationError if ActiveRecord::Migrator::needs_migrations? + end + def self.method_missing(name, *args, &block) # :nodoc: (delegate || superclass.delegate).send(name, *args, &block) end @@ -605,8 +629,8 @@ module ActiveRecord end end - def needs_migration? - current_version != last_version + def needs_migrations? + current_version < last_version end def last_version diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 1e497b2a79..319516413b 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -59,6 +59,13 @@ module ActiveRecord ActiveSupport.on_load(:active_record) { self.logger ||= ::Rails.logger } end + initializer "active_record.migration_error" do |app| + if config.active_record.delete(:migration_error) == :page_load + config.app_middleware.insert_after "::ActionDispatch::Callbacks", + "ActiveRecord::Migration::CheckPending" + end + end + initializer "active_record.set_configs" do |app| ActiveSupport.on_load(:active_record) do if app.config.active_record.delete(:whitelist_attributes) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 45c847bae8..33d146dac3 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -63,12 +63,12 @@ class MigrationTest < ActiveRecord::TestCase ActiveRecord::Migrator.up(migrations_path) assert_equal 3, ActiveRecord::Migrator.current_version assert_equal 3, ActiveRecord::Migrator.last_version - assert_equal false, ActiveRecord::Migrator.needs_migration? + assert_equal false, ActiveRecord::Migrator.needs_migrations? ActiveRecord::Migrator.down(MIGRATIONS_ROOT + "/valid") assert_equal 0, ActiveRecord::Migrator.current_version assert_equal 3, ActiveRecord::Migrator.last_version - assert_equal true, ActiveRecord::Migrator.needs_migration? + assert_equal true, ActiveRecord::Migrator.needs_migrations? end def test_create_table_with_force_true_does_not_drop_nonexisting_table diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt index 24bcec854c..01f9396403 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt @@ -26,6 +26,9 @@ # Log the query plan for queries taking more than this (works # with SQLite, MySQL, and PostgreSQL). config.active_record.auto_explain_threshold_in_seconds = 0.5 + + # Raise an error on page load if there are pending migrations + config.active_record.migration_error = :page_load <%- end -%> <%- unless options.skip_sprockets? -%> diff --git a/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb b/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb index a8f7aeac7d..0090293200 100644 --- a/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb +++ b/railties/lib/rails/generators/rails/app/templates/test/test_helper.rb @@ -4,6 +4,8 @@ require 'rails/test_help' class ActiveSupport::TestCase <% unless options[:skip_active_record] -%> + ActiveRecord::Migration.check_pending! + # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in alphabetical order. # # Note: You'll currently still have to declare fixtures explicitly in integration tests From d741a4c6f863778c5ebf04b21f6c3292091c13a7 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 6 Jun 2012 15:47:03 -0500 Subject: [PATCH 3/3] test errors for pending migrations App should raise error on page_load --- railties/test/application/configuration_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index d7689863e6..8579a942dd 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -41,6 +41,21 @@ module ApplicationTests FileUtils.rm_rf(new_app) if File.directory?(new_app) end + test "a renders exception on pending migration" do + add_to_config <<-RUBY + config.active_record.migration_error = :page_load + config.consider_all_requests_local = true + config.action_dispatch.show_exceptions = true + RUBY + + require "#{app_path}/config/environment" + ActiveRecord::Migrator.stubs(:needs_migrations?).returns(true) + + get "/foo" + assert_equal 500, last_response.status + assert_match "ActiveRecord::PendingMigrationError", last_response.body + end + test "multiple queue construction is possible" do require 'rails' require "#{app_path}/config/environment"