From 6a3c8fe69c16ad7360046f145db6689c18e91005 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 20 Mar 2018 18:20:50 +1100 Subject: [PATCH] FEATURE: protect against accidental column or table drops Often we need to amend our schema, it is tempting to use drop_table, rename_column and drop_column to amned schema trouble though is that existing code that is running in production can depend on the existance of previous schema leading to application breaking until new code base is deployed. The commit enforces new rules to ensure we can never drop tables or columns in migrations and instead use Migration::ColumnDropper and Migration::TableDropper to defer drop the db objects --- db/fixtures/001_categories.rb | 4 +- db/fixtures/002_groups.rb | 2 +- db/fixtures/009_users.rb | 8 +- db/fixtures/600_themes.rb | 2 +- db/fixtures/999_delayed.rb | 4 +- db/fixtures/999_topics.rb | 6 +- ...85227_create_topic_status_updates_again.rb | 4 +- .../20170717084947_create_user_emails.rb | 4 +- lib/column_dropper.rb | 75 ----------- lib/freedom_patches/safe_migrations.rb | 3 + lib/migration/base_dropper.rb | 72 +++++++++++ lib/migration/column_dropper.rb | 64 ++++++++++ lib/migration/safe_migrate.rb | 118 ++++++++++++++++++ lib/migration/table_dropper.rb | 69 ++++++++++ lib/table_migration_helper.rb | 66 ---------- .../{ => migration}/column_dropper_spec.rb | 12 +- .../components/migration/safe_migrate_spec.rb | 84 +++++++++++++ .../table_dropper_spec.rb} | 4 +- .../drop_table/20990309014014_drop_table.rb | 9 ++ .../20990309014014_remove_column.rb | 9 ++ .../20990309014014_rename_column.rb | 9 ++ 21 files changed, 462 insertions(+), 166 deletions(-) delete mode 100644 lib/column_dropper.rb create mode 100644 lib/freedom_patches/safe_migrations.rb create mode 100644 lib/migration/base_dropper.rb create mode 100644 lib/migration/column_dropper.rb create mode 100644 lib/migration/safe_migrate.rb create mode 100644 lib/migration/table_dropper.rb delete mode 100644 lib/table_migration_helper.rb rename spec/components/{ => migration}/column_dropper_spec.rb (93%) create mode 100644 spec/components/migration/safe_migrate_spec.rb rename spec/components/{table_migration_helper_spec.rb => migration/table_dropper_spec.rb} (95%) create mode 100644 spec/fixtures/migrate/drop_table/20990309014014_drop_table.rb create mode 100644 spec/fixtures/migrate/remove_column/20990309014014_remove_column.rb create mode 100644 spec/fixtures/migrate/rename_column/20990309014014_rename_column.rb diff --git a/db/fixtures/001_categories.rb b/db/fixtures/001_categories.rb index 98b0be1a32d..f21181180e7 100644 --- a/db/fixtures/001_categories.rb +++ b/db/fixtures/001_categories.rb @@ -1,4 +1,4 @@ -require 'column_dropper' +require 'migration/column_dropper' # fix any bust caches post initial migration ActiveRecord::Base.send(:subclasses).each { |m| m.reset_column_information } @@ -26,7 +26,7 @@ if uncat_id == -1 || !Category.exists?(uncat_id) VALUES ('uncategorized_category_id', 3, #{category_id}, now(), now())" end -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'categories', after_migration: 'AddSuppressFromLatestToCategories', columns: ['logo_url', 'background_url', 'suppress_from_homepage'], diff --git a/db/fixtures/002_groups.rb b/db/fixtures/002_groups.rb index 900f4b3ca80..cbf32466c3d 100644 --- a/db/fixtures/002_groups.rb +++ b/db/fixtures/002_groups.rb @@ -5,7 +5,7 @@ end Group.where(name: 'everyone').update_all(visibility_level: Group.visibility_levels[:owners]) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'groups', after_migration: 'SplitAliasLevels', columns: %w[visible public alias_level], diff --git a/db/fixtures/009_users.rb b/db/fixtures/009_users.rb index 6a97df4bb20..b0f83d0f9c9 100644 --- a/db/fixtures/009_users.rb +++ b/db/fixtures/009_users.rb @@ -33,7 +33,7 @@ UserOption.where(user_id: -1).update_all( Group.user_trust_level_change!(-1, TrustLevel[4]) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'users', after_migration: 'DropEmailFromUsers', columns: %w[ @@ -61,7 +61,7 @@ ColumnDropper.drop( } ) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'users', after_migration: 'RenameBlockedSilence', columns: %w[ @@ -72,7 +72,7 @@ ColumnDropper.drop( } ) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'users', after_migration: 'AddSilencedTillToUsers', columns: %w[ @@ -83,7 +83,7 @@ ColumnDropper.drop( } ) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'users', after_migration: 'AddTrustLevelLocksToUsers', columns: %w[ diff --git a/db/fixtures/600_themes.rb b/db/fixtures/600_themes.rb index 5b141c57ad7..84694a5ec0c 100644 --- a/db/fixtures/600_themes.rb +++ b/db/fixtures/600_themes.rb @@ -18,7 +18,7 @@ if !Theme.exists? default_theme.set_default! end -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'theme_fields', after_migration: 'AddUploadIdToThemeFields', columns: ['target'], diff --git a/db/fixtures/999_delayed.rb b/db/fixtures/999_delayed.rb index 4d69a58267d..1794f79ae69 100644 --- a/db/fixtures/999_delayed.rb +++ b/db/fixtures/999_delayed.rb @@ -1,8 +1,8 @@ # Delayed migration steps -require 'table_migration_helper' +require 'migration/table_dropper' -TableMigrationHelper.delayed_drop( +Migration::TableDropper.delayed_drop( old_name: 'topic_status_updates', new_name: 'topic_timers', after_migration: 'RenameTopicStatusUpdatesToTopicTimers', diff --git a/db/fixtures/999_topics.rb b/db/fixtures/999_topics.rb index 25958a8c694..78ee61a92d3 100644 --- a/db/fixtures/999_topics.rb +++ b/db/fixtures/999_topics.rb @@ -64,7 +64,7 @@ end # run this later, cause we need to make sure new application controller resilience is in place first -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'user_stats', after_migration: 'DropUnreadTrackingColumns', columns: %w{ @@ -76,7 +76,7 @@ ColumnDropper.drop( } ) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'topics', after_migration: 'DropUnreadTrackingColumns', columns: %w{ @@ -92,7 +92,7 @@ ColumnDropper.drop( } ) -ColumnDropper.drop( +Migration::ColumnDropper.drop( table: 'topics', after_migration: 'RemoveAutoCloseColumnsFromTopics', columns: %w{ diff --git a/db/migrate/20170512185227_create_topic_status_updates_again.rb b/db/migrate/20170512185227_create_topic_status_updates_again.rb index 9883594e90c..785939f2b3a 100644 --- a/db/migrate/20170512185227_create_topic_status_updates_again.rb +++ b/db/migrate/20170512185227_create_topic_status_updates_again.rb @@ -1,4 +1,4 @@ -require 'table_migration_helper' +require 'migration/table_dropper' class CreateTopicStatusUpdatesAgain < ActiveRecord::Migration[4.2] def up @@ -14,7 +14,7 @@ class CreateTopicStatusUpdatesAgain < ActiveRecord::Migration[4.2] t.integer :category_id end - TableMigrationHelper.read_only_table('topic_status_updates') + Migration::TableDropper.read_only_table('topic_status_updates') end def down diff --git a/db/migrate/20170717084947_create_user_emails.rb b/db/migrate/20170717084947_create_user_emails.rb index 978c75bfdf8..449bf1a1777 100644 --- a/db/migrate/20170717084947_create_user_emails.rb +++ b/db/migrate/20170717084947_create_user_emails.rb @@ -1,4 +1,4 @@ -require_dependency 'column_dropper' +require 'migration/column_dropper' class CreateUserEmails < ActiveRecord::Migration[4.2] def up @@ -33,7 +33,7 @@ class CreateUserEmails < ActiveRecord::Migration[4.2] SQL change_column_null :users, :email, true - ColumnDropper.mark_readonly(:users, :email) + Migration::ColumnDropper.mark_readonly(:users, :email) end def down diff --git a/lib/column_dropper.rb b/lib/column_dropper.rb deleted file mode 100644 index 31479077c7b..00000000000 --- a/lib/column_dropper.rb +++ /dev/null @@ -1,75 +0,0 @@ -class ColumnDropper - def self.drop(table:, after_migration:, columns:, delay: nil, on_drop: nil) - raise ArgumentError.new("Invalid table name passed to drop #{table}") if table =~ /[^a-z0-9_]/i - - columns.each do |column| - raise ArgumentError.new("Invalid column name passed to drop #{column}") if column =~ /[^a-z0-9_]/i - end - - # in production we need some extra delay to allow for slow migrations - delay ||= Rails.env.production? ? 3600 : 0 - - sql = <<~SQL - SELECT 1 - FROM INFORMATION_SCHEMA.COLUMNS - WHERE table_schema = 'public' AND - table_name = :table AND - column_name IN (:columns) AND - EXISTS ( - SELECT 1 - FROM schema_migration_details - WHERE name = :after_migration AND - created_at <= (current_timestamp at time zone 'UTC' - interval :delay) - ) - LIMIT 1 - SQL - - if ActiveRecord::Base.exec_sql(sql, table: table, - columns: columns, - delay: "#{delay.to_i || 0} seconds", - after_migration: after_migration).to_a.length > 0 - on_drop&.call - - columns.each do |column| - ActiveRecord::Base.exec_sql <<~SQL - DROP TRIGGER IF EXISTS #{readonly_trigger_name(table, column)} ON #{table}; - DROP FUNCTION IF EXISTS #{readonly_function_name(table, column)} CASCADE; - SQL - - # safe cause it is protected on method entry, can not be passed in params - ActiveRecord::Base.exec_sql("ALTER TABLE #{table} DROP COLUMN IF EXISTS #{column}") - end - - Discourse.reset_active_record_cache - end - end - - def self.mark_readonly(table_name, column_name) - ActiveRecord::Base.exec_sql <<-SQL - CREATE OR REPLACE FUNCTION #{readonly_function_name(table_name, column_name)} RETURNS trigger AS $rcr$ - BEGIN - RAISE EXCEPTION 'Discourse: #{column_name} in #{table_name} is readonly'; - END - $rcr$ LANGUAGE plpgsql; - SQL - - ActiveRecord::Base.exec_sql <<-SQL - CREATE TRIGGER #{readonly_trigger_name(table_name, column_name)} - BEFORE INSERT OR UPDATE OF #{column_name} - ON #{table_name} - FOR EACH ROW - WHEN (NEW.#{column_name} IS NOT NULL) - EXECUTE PROCEDURE #{readonly_function_name(table_name, column_name)}; - SQL - end - - private - - def self.readonly_function_name(table_name, column_name) - "raise_#{table_name}_#{column_name}_readonly()" - end - - def self.readonly_trigger_name(table_name, column_name) - "#{table_name}_#{column_name}_readonly" - end -end diff --git a/lib/freedom_patches/safe_migrations.rb b/lib/freedom_patches/safe_migrations.rb new file mode 100644 index 00000000000..59eb1f7ce42 --- /dev/null +++ b/lib/freedom_patches/safe_migrations.rb @@ -0,0 +1,3 @@ +require_dependency 'migration/safe_migrate' + +Migration::SafeMigrate.patch_active_record! diff --git a/lib/migration/base_dropper.rb b/lib/migration/base_dropper.rb new file mode 100644 index 00000000000..c00035d4ab5 --- /dev/null +++ b/lib/migration/base_dropper.rb @@ -0,0 +1,72 @@ +module Migration + class BaseDropper + def initialize(after_migration, delay, on_drop) + @after_migration = after_migration + @on_drop = on_drop + + # in production we need some extra delay to allow for slow migrations + @delay = delay || (Rails.env.production? ? 3600 : 0) + end + + def delayed_drop + if droppable? + @on_drop&.call + execute_drop! + + Discourse.reset_active_record_cache + end + end + + private + + def droppable? + raise NotImplementedError + end + + def execute_drop! + raise NotImplementedError + end + + def previous_migration_done + <<~SQL + EXISTS( + SELECT 1 + FROM schema_migration_details + WHERE name = :after_migration AND + created_at <= (current_timestamp AT TIME ZONE 'UTC' - INTERVAL :delay) + ) + SQL + end + + def self.create_readonly_function(table_name, column_name = nil) + message = column_name ? + "Discourse: #{column_name} in #{table_name} is readonly" : + "Discourse: #{table_name} is read only" + + ActiveRecord::Base.exec_sql <<~SQL + CREATE OR REPLACE FUNCTION #{readonly_function_name(table_name, column_name)} RETURNS trigger AS $rcr$ + BEGIN + RAISE EXCEPTION '#{message}'; + END + $rcr$ LANGUAGE plpgsql; + SQL + end + private_class_method :create_readonly_function + + def self.validate_table_name(table_name) + raise ArgumentError.new("Invalid table name passed: #{table_name}") if table_name =~ /[^a-z0-9_]/i + end + + def self.validate_column_name(column_name) + raise ArgumentError.new("Invalid column name passed to drop #{column_name}") if column_name =~ /[^a-z0-9_]/i + end + + def self.readonly_function_name(table_name, column_name = nil) + ["raise", table_name, column_name, "readonly()"].compact.join("_") + end + + def self.readonly_trigger_name(table_name, column_name = nil) + [table_name, column_name, "readonly"].compact.join("_") + end + end +end diff --git a/lib/migration/column_dropper.rb b/lib/migration/column_dropper.rb new file mode 100644 index 00000000000..d7176505d15 --- /dev/null +++ b/lib/migration/column_dropper.rb @@ -0,0 +1,64 @@ +require_dependency 'migration/base_dropper' + +module Migration + class ColumnDropper < BaseDropper + def self.drop(table:, after_migration:, columns:, delay: nil, on_drop: nil) + validate_table_name(table) + columns.each { |column| validate_column_name(column) } + + ColumnDropper.new(table, columns, after_migration, delay, on_drop).delayed_drop + end + + def self.mark_readonly(table_name, column_name) + create_readonly_function(table_name, column_name) + + ActiveRecord::Base.exec_sql <<~SQL + CREATE TRIGGER #{readonly_trigger_name(table_name, column_name)} + BEFORE INSERT OR UPDATE OF #{column_name} + ON #{table_name} + FOR EACH ROW + WHEN (NEW.#{column_name} IS NOT NULL) + EXECUTE PROCEDURE #{readonly_function_name(table_name, column_name)}; + SQL + end + + private + + def initialize(table, columns, after_migration, delay, on_drop) + super(after_migration, delay, on_drop) + + @table = table + @columns = columns + end + + def droppable? + builder = SqlBuilder.new(<<~SQL) + SELECT 1 + FROM INFORMATION_SCHEMA.COLUMNS + /*where*/ + LIMIT 1 + SQL + + builder.where("table_schema = 'public'") + .where("table_name = :table") + .where("column_name IN (:columns)") + .where(previous_migration_done) + .exec(table: @table, + columns: @columns, + delay: "#{@delay} seconds", + after_migration: @after_migration).to_a.length > 0 + end + + def execute_drop! + @columns.each do |column| + ActiveRecord::Base.exec_sql <<~SQL + DROP TRIGGER IF EXISTS #{BaseDropper.readonly_trigger_name(@table, column)} ON #{@table}; + DROP FUNCTION IF EXISTS #{BaseDropper.readonly_function_name(@table, column)} CASCADE; + SQL + + # safe cause it is protected on method entry, can not be passed in params + ActiveRecord::Base.exec_sql("ALTER TABLE #{@table} DROP COLUMN IF EXISTS #{column}") + end + end + end +end diff --git a/lib/migration/safe_migrate.rb b/lib/migration/safe_migrate.rb new file mode 100644 index 00000000000..92486fe67b0 --- /dev/null +++ b/lib/migration/safe_migrate.rb @@ -0,0 +1,118 @@ +module Migration; end + +class Discourse::InvalidMigration < StandardError; end + +class Migration::SafeMigrate + module SafeMigration + UNSAFE_VERSION = 20180321015220 + @@enable_safe = true + + def self.enable_safe! + @@enable_safe = true + end + + def self.disable_safe! + @@enable_safe = false + end + + def migrate(direction) + if direction == :up && version && version > UNSAFE_VERSION && @@enable_safe != false + Migration::SafeMigrate.enable! + end + super + ensure + Migration::SafeMigrate.disable! + end + end + + module NiceErrors + def migrate + super + rescue => e + if e.cause.is_a?(Discourse::InvalidMigration) + def e.cause; nil; end + def e.backtrace + super.reject do |frame| + frame =~ /safe_migrate\.rb/ || frame =~ /schema_migration_details\.rb/ + end + end + raise e + else + raise e + end + end + end + + def self.enable! + return if PG::Connection.method_defined?(:exec_migrator_unpatched) + + PG::Connection.class_eval do + alias_method :exec_migrator_unpatched, :exec + alias_method :async_exec_migrator_unpatched, :async_exec + + def exec(*args, &blk) + Migration::SafeMigrate.protect!(args[0]) + exec_migrator_unpatched(*args, &blk) + end + + def async_exec(*args, &blk) + Migration::SafeMigrate.protect!(args[0]) + async_exec_migrator_unpatched(*args, &blk) + end + end + end + + def self.disable! + return if !PG::Connection.method_defined?(:exec_migrator_unpatched) + PG::Connection.class_eval do + alias_method :exec, :exec_migrator_unpatched + alias_method :async_exec, :async_exec_migrator_unpatched + + remove_method :exec_migrator_unpatched + remove_method :async_exec_migrator_unpatched + end + end + + def self.patch_active_record! + ActiveSupport.on_load(:active_record) do + ActiveRecord::Migration.prepend(SafeMigration) + end + + if defined?(ActiveRecord::Tasks::DatabaseTasks) + ActiveRecord::Tasks::DatabaseTasks.singleton_class.prepend(NiceErrors) + end + end + + def self.protect!(sql) + if sql =~ /^\s*drop\s+table/i + $stdout.puts("", <<~STR) + WARNING + ------------------------------------------------------------------------------------- + An attempt was made to drop a table in a migration + SQL used was: '#{sql}' + Please use the deferred pattrn using Migration::TableDropper in db/seeds to drop + the table. + + This protection is in place to protect us against dropping tables that are currently + in use by live applications. + STR + raise Discourse::InvalidMigration, "Attempt was made to drop a table" + elsif sql =~ /^\s*alter\s+table.*(rename|drop)/i + $stdout.puts("", <<~STR) + WARNING + ------------------------------------------------------------------------------------- + An attempt was made to drop or rename a column in a migration + SQL used was: '#{sql}' + Please use the deferred pattrn using Migration::ColumnDropper in db/seeds to drop + or rename columns. + + Note, to minimize disruption use self.ignored_columns = ["column name"] on your + ActiveRecord model, this can be removed 6 months or so later. + + This protection is in place to protect us against dropping columns that are currently + in use by live applications. + STR + raise Discourse::InvalidMigration, "Attempt was made to rename or delete column" + end + end +end diff --git a/lib/migration/table_dropper.rb b/lib/migration/table_dropper.rb new file mode 100644 index 00000000000..cc2dd1ea5ef --- /dev/null +++ b/lib/migration/table_dropper.rb @@ -0,0 +1,69 @@ +require_dependency 'migration/base_dropper' + +module Migration + class Migration::TableDropper < BaseDropper + def self.delayed_drop(old_name:, new_name:, after_migration:, delay: nil, on_drop: nil) + validate_table_name(old_name) + validate_table_name(new_name) + + TableDropper.new(old_name, new_name, after_migration, delay, on_drop).delayed_drop + end + + def self.read_only_table(table_name) + create_readonly_function(table_name) + + ActiveRecord::Base.exec_sql <<~SQL + CREATE TRIGGER #{readonly_trigger_name(table_name)} + BEFORE INSERT OR UPDATE OR DELETE OR TRUNCATE + ON #{table_name} + FOR EACH STATEMENT + EXECUTE PROCEDURE #{readonly_function_name(table_name)}; + SQL + end + + private + + def initialize(old_name, new_name, after_migration, delay, on_drop) + super(after_migration, delay, on_drop) + + @old_name = old_name + @new_name = new_name + end + + def droppable? + builder = SqlBuilder.new(<<~SQL) + SELECT 1 + FROM INFORMATION_SCHEMA.TABLES + /*where*/ + LIMIT 1 + SQL + + builder.where("table_schema = 'public'") + .where(previous_migration_done) + .where(new_table_exists) + .exec(old_name: @old_name, + new_name: @new_name, + delay: "#{@delay} seconds", + after_migration: @after_migration).to_a.length > 0 + end + + def new_table_exists + <<~SQL + EXISTS( + SELECT 1 + FROM INFORMATION_SCHEMA.TABLES + WHERE table_schema = 'public' AND + table_name = :new_name + ) + SQL + end + + def execute_drop! + ActiveRecord::Base.exec_sql("DROP TABLE IF EXISTS #{@old_name}") + + ActiveRecord::Base.exec_sql <<~SQL + DROP FUNCTION IF EXISTS #{BaseDropper.readonly_function_name(@old_name)} CASCADE; + SQL + end + end +end diff --git a/lib/table_migration_helper.rb b/lib/table_migration_helper.rb deleted file mode 100644 index 477cd7fa0e6..00000000000 --- a/lib/table_migration_helper.rb +++ /dev/null @@ -1,66 +0,0 @@ -class TableMigrationHelper - def self.read_only_table(table_name) - ActiveRecord::Base.exec_sql <<-SQL - CREATE OR REPLACE FUNCTION #{readonly_function_name(table_name)} RETURNS trigger AS $rro$ - BEGIN - RAISE EXCEPTION 'Discourse: Table is read only'; - RETURN null; - END - $rro$ LANGUAGE plpgsql; - SQL - - ActiveRecord::Base.exec_sql <<-SQL - CREATE TRIGGER #{readonly_trigger_name(table_name)} - BEFORE INSERT OR UPDATE OR DELETE OR TRUNCATE - ON #{table_name} - FOR EACH STATEMENT - EXECUTE PROCEDURE #{readonly_function_name(table_name)}; - SQL - end - - def self.delayed_drop(old_name:, new_name:, after_migration:, delay: nil, on_drop: nil) - delay ||= Rails.env.production? ? 3600 : 0 - - sql = <<~SQL - SELECT 1 - FROM INFORMATION_SCHEMA.TABLES - WHERE table_schema = 'public' AND - EXISTS ( - SELECT 1 - FROM schema_migration_details - WHERE name = :after_migration AND - created_at <= (current_timestamp at time zone 'UTC' - interval :delay) - ) - AND EXISTS ( - SELECT 1 - FROM INFORMATION_SCHEMA.TABLES - WHERE table_schema = 'public' AND - table_name = :new_name - ) - LIMIT 1 - SQL - - if ActiveRecord::Base.exec_sql(sql, old_name: old_name, - new_name: new_name, - delay: "#{delay.to_i || 0} seconds", - after_migration: after_migration).to_a.length > 0 - - on_drop&.call - ActiveRecord::Base.exec_sql("DROP TABLE IF EXISTS #{old_name}") - - ActiveRecord::Base.exec_sql <<~SQL - DROP FUNCTION IF EXISTS #{readonly_function_name(old_name)} CASCADE; - SQL - end - end - - private - - def self.readonly_function_name(table_name) - "public.raise_#{table_name}_read_only()" - end - - def self.readonly_trigger_name(table_name) - "#{table_name}_read_only" - end -end diff --git a/spec/components/column_dropper_spec.rb b/spec/components/migration/column_dropper_spec.rb similarity index 93% rename from spec/components/column_dropper_spec.rb rename to spec/components/migration/column_dropper_spec.rb index 3fd75815905..8d3c4ad6f39 100644 --- a/spec/components/column_dropper_spec.rb +++ b/spec/components/migration/column_dropper_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' -require 'column_dropper' +require_dependency 'migration/column_dropper' -RSpec.describe ColumnDropper do +RSpec.describe Migration::ColumnDropper do def has_column?(table, column) Topic.exec_sql("SELECT 1 FROM INFORMATION_SCHEMA.COLUMNS @@ -25,7 +25,7 @@ RSpec.describe ColumnDropper do dropped_proc_called = false - ColumnDropper.drop( + Migration::ColumnDropper.drop( table: 'topics', after_migration: name, columns: ['junk'], @@ -36,7 +36,7 @@ RSpec.describe ColumnDropper do expect(has_column?('topics', 'junk')).to eq(true) expect(dropped_proc_called).to eq(false) - ColumnDropper.drop( + Migration::ColumnDropper.drop( table: 'topics', after_migration: name, columns: ['junk'], @@ -60,7 +60,7 @@ RSpec.describe ColumnDropper do VALUES (1, 'something@email.com'); SQL - ColumnDropper.mark_readonly(table_name, 'email') + Migration::ColumnDropper.mark_readonly(table_name, 'email') end after do @@ -78,7 +78,7 @@ RSpec.describe ColumnDropper do .getvalue(0, 0) dropped_proc_called = false - ColumnDropper.drop( + Migration::ColumnDropper.drop( table: table_name, after_migration: name, columns: ['email'], diff --git a/spec/components/migration/safe_migrate_spec.rb b/spec/components/migration/safe_migrate_spec.rb new file mode 100644 index 00000000000..846a4494cf8 --- /dev/null +++ b/spec/components/migration/safe_migrate_spec.rb @@ -0,0 +1,84 @@ +require 'rails_helper' +require_dependency 'migration/safe_migrate' + +describe Migration::SafeMigrate do + before do + Migration::SafeMigrate::SafeMigration.disable_safe! + end + + after do + Migration::SafeMigrate.disable! + Migration::SafeMigrate::SafeMigration.enable_safe! + end + + def capture_stdout + old_stdout = $stdout + io = StringIO.new + $stdout = io + yield + io.string + ensure + $stdout = old_stdout + end + + it "bans all table removal" do + Migration::SafeMigrate.enable! + + path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/drop_table" + + output = capture_stdout do + expect(lambda do + ActiveRecord::Migrator.up([path]) + end).to raise_error(StandardError) + end + + expect(output).to include("TableDropper") + + expect(User.first).not_to eq(nil) + end + + it "bans all column removal" do + Migration::SafeMigrate.enable! + + path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/remove_column" + + output = capture_stdout do + expect(lambda do + ActiveRecord::Migrator.up([path]) + end).to raise_error(StandardError) + end + + expect(output).to include("ColumnDropper") + + expect(User.first).not_to eq(nil) + end + + it "bans all column renames" do + Migration::SafeMigrate.enable! + + path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/rename_column" + + output = capture_stdout do + expect(lambda do + ActiveRecord::Migrator.up([path]) + end).to raise_error(StandardError) + end + + expect(output).to include("ColumnDropper") + + expect(User.first).not_to eq(nil) + end + + it "supports being disabled" do + Migration::SafeMigrate.enable! + Migration::SafeMigrate.disable! + + path = File.expand_path "#{Rails.root}/spec/fixtures/migrate/drop_table" + + output = capture_stdout do + ActiveRecord::Migrator.up([path]) + end + + expect(output).to include("drop_table(:users)") + end +end diff --git a/spec/components/table_migration_helper_spec.rb b/spec/components/migration/table_dropper_spec.rb similarity index 95% rename from spec/components/table_migration_helper_spec.rb rename to spec/components/migration/table_dropper_spec.rb index 9f690140697..f6e07a08a0d 100644 --- a/spec/components/table_migration_helper_spec.rb +++ b/spec/components/migration/table_dropper_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' -require 'table_migration_helper' +require_dependency 'migration/table_dropper' -describe TableMigrationHelper do +describe Migration::TableDropper do def table_exists?(table_name) sql = <<-SQL diff --git a/spec/fixtures/migrate/drop_table/20990309014014_drop_table.rb b/spec/fixtures/migrate/drop_table/20990309014014_drop_table.rb new file mode 100644 index 00000000000..211c70d7643 --- /dev/null +++ b/spec/fixtures/migrate/drop_table/20990309014014_drop_table.rb @@ -0,0 +1,9 @@ +class DropTable < ActiveRecord::Migration[5.1] + def up + drop_table :users + end + + def down + raise "not tested" + end +end diff --git a/spec/fixtures/migrate/remove_column/20990309014014_remove_column.rb b/spec/fixtures/migrate/remove_column/20990309014014_remove_column.rb new file mode 100644 index 00000000000..25bc82b171a --- /dev/null +++ b/spec/fixtures/migrate/remove_column/20990309014014_remove_column.rb @@ -0,0 +1,9 @@ +class RemoveColumn < ActiveRecord::Migration[5.1] + def up + remove_column :users, :username + end + + def down + raise "not tested" + end +end diff --git a/spec/fixtures/migrate/rename_column/20990309014014_rename_column.rb b/spec/fixtures/migrate/rename_column/20990309014014_rename_column.rb new file mode 100644 index 00000000000..1e0b568ada7 --- /dev/null +++ b/spec/fixtures/migrate/rename_column/20990309014014_rename_column.rb @@ -0,0 +1,9 @@ +class RenameColumn < ActiveRecord::Migration[5.1] + def up + rename_column :users, :username, :username1 + end + + def down + raise "not tested" + end +end