Add CreateMigration action

This Thor-action isolates the logic whether to (over-)write migration and
what is shown to the user. It's modelled after Thor's CreateFile-action.

This solves the issue that removing a non-existing migration, tried to
remove the template-path (#13588).

Related issues: #12674

railties/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Added Thor-action for creation of migrations.
+
+    Fixes #13588 and #12674.
+
+    *Gert Goet*
+
 *   Ensure that `bin/rails` is a file before trying to execute it.
 
     Fixes #13825.

railties/lib/rails/generators/actions/create_migration.rb

@@ -0,0 +1,68 @@
+require 'thor/actions/create_file'
+
+module Rails
+  module Generators
+    module Actions
+      class CreateMigration < Thor::Actions::CreateFile
+
+        def migration_dir
+          File.dirname(@destination)
+        end
+
+        def migration_file_name
+          @base.migration_file_name
+        end
+
+        def identical?
+          exists? && File.binread(existing_migration) == render
+        end
+
+        def revoke!
+          say_destination = exists? ? relative_existing_migration : relative_destination
+          say_status :remove, :red, say_destination
+          return unless exists?
+          ::FileUtils.rm_rf(existing_migration) unless pretend?
+          existing_migration
+        end
+
+        def relative_existing_migration
+          base.relative_to_original_destination_root(existing_migration)
+        end
+
+        def existing_migration
+          @existing_migration ||= begin
+            @base.class.migration_exists?(migration_dir, migration_file_name) ||
+            File.exist?(@destination) && @destination
+          end
+        end
+        alias :exists? :existing_migration
+
+        protected
+
+        def on_conflict_behavior(&block)
+          options = base.options.merge(config)
+          if identical?
+            say_status :identical, :blue, relative_existing_migration
+          elsif options[:force]
+            say_status :remove, :green, relative_existing_migration
+            say_status :create, :green
+            unless pretend?
+              ::FileUtils.rm_rf(existing_migration)
+              block.call
+            end
+          elsif options[:skip]
+            say_status :skip, :yellow
+          else
+            say_status :conflict, :red
+            raise Error, "Another migration is already named #{migration_file_name}: " +
+                         "#{existing_migration}. Use --force to replace this migration file."
+          end
+        end
+
+        def say_status(status, color, message = relative_destination)
+          base.shell.say_status(status, message, color) if config[:verbose]
+        end
+      end
+    end
+  end
+end

railties/lib/rails/generators/migration.rb

@@ -1,4 +1,5 @@
 require 'active_support/concern'
+require 'rails/generators/actions/create_migration'
 
 module Rails
   module Generators
@@ -29,6 +30,19 @@ module Rails
         end
       end
 
+      def create_migration(destination, data, config = {}, &block)
+        action Rails::Generators::Actions::CreateMigration.new(self, destination, block || data.to_s, config)
+      end
+
+      def set_migration_assigns!(destination)
+        destination = File.expand_path(destination, self.destination_root)
+
+        migration_dir = File.dirname(destination)
+        @migration_number     = self.class.next_migration_number(migration_dir)
+        @migration_file_name  = File.basename(destination, '.rb')
+        @migration_class_name = @migration_file_name.camelize
+      end
+
       # Creates a migration template at the given destination. The difference
       # to the default template method is that the migration version is appended
       # to the destination file name.
@@ -37,26 +51,18 @@ module Rails
       # available as instance variables in the template to be rendered.
       #
       #   migration_template "migration.rb", "db/migrate/add_foo_to_bar.rb"
-      def migration_template(source, destination=nil, config={})
-        destination = File.expand_path(destination || source, self.destination_root)
+      def migration_template(source, destination, config = {})
+        source  = File.expand_path(find_in_source_paths(source.to_s))
 
-        migration_dir = File.dirname(destination)
-        @migration_number     = self.class.next_migration_number(migration_dir)
-        @migration_file_name  = File.basename(destination).sub(/\.rb$/, '')
-        @migration_class_name = @migration_file_name.camelize
+        set_migration_assigns!(destination)
+        context = instance_eval('binding')
 
-        destination = self.class.migration_exists?(migration_dir, @migration_file_name)
+        dir, base = File.split(destination)
+        numbered_destination = File.join(dir, ["%migration_number%", base].join('_'))
 
-        if !(destination && options[:skip]) && behavior == :invoke
-          if destination && options.force?
-            remove_file(destination)
-          elsif destination
-            raise Error, "Another migration is already named #{@migration_file_name}: #{destination}. Use --force to remove the old migration file and replace it."
-          end
-          destination = File.join(migration_dir, "#{@migration_number}_#{@migration_file_name}.rb")
+        create_migration numbered_destination, nil, config do
+          ERB.new(::File.binread(source), nil, '-', '@output_buffer').result(context)
         end
-
-        template(source, destination, config)
       end
     end
   end

railties/test/generators/create_migration_test.rb

@@ -0,0 +1,134 @@
+require 'generators/generators_test_helper'
+require 'rails/generators/rails/migration/migration_generator'
+
+class CreateMigrationTest < Rails::Generators::TestCase
+  include GeneratorsTestHelper
+
+  class Migrator < Rails::Generators::MigrationGenerator
+    include Rails::Generators::Migration
+
+    def self.next_migration_number(dirname)
+      current_migration_number(dirname) + 1
+    end
+  end
+
+  tests Migrator
+
+  def default_destination_path
+    "db/migrate/create_articles.rb"
+  end
+
+  def create_migration(destination_path = default_destination_path, config = {}, generator_options = {}, &block)
+    migration_name = File.basename(destination_path, '.rb')
+    generator([migration_name], generator_options)
+    generator.set_migration_assigns!(destination_path)
+
+    dir, base = File.split(destination_path)
+    timestamped_destination_path = File.join(dir, ["%migration_number%", base].join('_'))
+
+    @migration = Rails::Generators::Actions::CreateMigration.new(generator, timestamped_destination_path, block || "contents", config)
+  end
+
+  def migration_exists!(*args)
+    @existing_migration = create_migration(*args)
+    invoke!
+    @generator = nil
+  end
+
+  def invoke!
+    capture(:stdout) { @migration.invoke! }
+  end
+
+  def revoke!
+    capture(:stdout) { @migration.revoke! }
+  end
+
+  def test_invoke
+    create_migration
+
+    assert_match(/create  db\/migrate\/1_create_articles.rb\n/, invoke!)
+    assert_file @migration.destination
+  end
+
+  def test_invoke_pretended
+    create_migration(default_destination_path, {}, { pretend: true })
+
+    assert_no_file @migration.destination
+  end
+
+  def test_invoke_when_exists
+    migration_exists!
+    create_migration
+
+    assert_equal @existing_migration.destination, @migration.existing_migration
+  end
+
+  def test_invoke_when_exists_identical
+    migration_exists!
+    create_migration
+
+    assert_match(/identical  db\/migrate\/1_create_articles.rb\n/, invoke!)
+    assert @migration.identical?
+  end
+
+  def test_invoke_when_exists_not_identical
+    migration_exists!
+    create_migration { "different content" }
+
+    assert_raise(Rails::Generators::Error) { invoke! }
+  end
+
+  def test_invoke_forced_when_exists_not_identical
+    dest = "db/migrate/migration.rb"
+    migration_exists!(dest)
+    create_migration(dest, force: true) { "different content" }
+
+    stdout = invoke!
+    assert_match(/remove  db\/migrate\/1_migration.rb\n/, stdout)
+    assert_match(/create  db\/migrate\/2_migration.rb\n/, stdout)
+    assert_file @migration.destination
+    assert_no_file @existing_migration.destination
+  end
+
+  def test_invoke_forced_pretended_when_exists_not_identical
+    migration_exists!
+    create_migration(default_destination_path, { force: true }, { pretend: true }) do
+      "different content"
+    end
+
+    stdout = invoke!
+    assert_match(/remove  db\/migrate\/1_create_articles.rb\n/, stdout)
+    assert_match(/create  db\/migrate\/2_create_articles.rb\n/, stdout)
+    assert_no_file @migration.destination
+  end
+
+  def test_invoke_skipped_when_exists_not_identical
+    migration_exists!
+    create_migration(default_destination_path, {}, { skip: true }) { "different content" }
+
+    assert_match(/skip  db\/migrate\/2_create_articles.rb\n/, invoke!)
+    assert_no_file @migration.destination
+  end
+
+  def test_revoke
+    migration_exists!
+    create_migration
+
+    assert_match(/remove  db\/migrate\/1_create_articles.rb\n/, revoke!)
+    assert_no_file @existing_migration.destination
+  end
+
+  def test_revoke_pretended
+    migration_exists!
+    create_migration(default_destination_path, {}, { pretend: true })
+
+    assert_match(/remove  db\/migrate\/1_create_articles.rb\n/, revoke!)
+    assert_file @existing_migration.destination
+  end
+
+  def test_revoke_when_no_exists
+    create_migration
+
+    assert_match(/remove  db\/migrate\/1_create_articles.rb\n/, revoke!)
+  end
+end