Skip to content

Commit

Permalink
Change the method for copying migrations, do not add scope.
Browse files Browse the repository at this point in the history
The purpose of this change is to allow copying fail on the same names.
Migrations change database and they should be treated with caution,
if 2 migrations are named the same it's much better to skip migration
and allow user decide if it should be copied or not.
  • Loading branch information
drogus committed Oct 9, 2010
1 parent 5d5eb2b commit 4377f8e
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 30 deletions.
18 changes: 9 additions & 9 deletions activerecord/lib/active_record/migration.rb
Expand Up @@ -387,18 +387,18 @@ def method_missing(method, *arguments, &block)
def copy(destination, sources) def copy(destination, sources)
copied = [] copied = []


sources.each do |scope, path| sources.each do |name, path|
destination_migrations = ActiveRecord::Migrator.migrations(destination) destination_migrations = ActiveRecord::Migrator.migrations(destination)
source_migrations = ActiveRecord::Migrator.migrations(path) source_migrations = ActiveRecord::Migrator.migrations(path)
last = destination_migrations.last last = destination_migrations.last


source_migrations.each do |migration| source_migrations.each do |migration|
next if destination_migrations.any? { |m| m.name == migration.name && m.scope == scope.to_s } next if destination_migrations.any? { |m| m.name == migration.name }


migration.version = next_migration_number(last ? last.version + 1 : 0).to_i migration.version = next_migration_number(last ? last.version + 1 : 0).to_i
last = migration last = migration


new_path = File.join(destination, "#{migration.version}_#{migration.name.underscore}.#{scope}.rb") new_path = File.join(destination, "#{migration.version}_#{migration.name.underscore}.rb")
FileUtils.cp(migration.filename, new_path) FileUtils.cp(migration.filename, new_path)
copied << new_path copied << new_path
end end
Expand All @@ -419,9 +419,9 @@ def next_migration_number(number)


# MigrationProxy is used to defer loading of the actual migration classes # MigrationProxy is used to defer loading of the actual migration classes
# until they are needed # until they are needed
class MigrationProxy < Struct.new(:name, :version, :filename, :scope) class MigrationProxy < Struct.new(:name, :version, :filename)


def initialize(name, version, filename, scope) def initialize(name, version, filename)
super super
@migration = nil @migration = nil
end end
Expand Down Expand Up @@ -510,18 +510,18 @@ def migrations(path)
seen = Hash.new false seen = Hash.new false


migrations = files.map do |file| migrations = files.map do |file|
version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first version, name = file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first


raise IllegalMigrationNameError.new(file) unless version raise IllegalMigrationNameError.new(file) unless version
version = version.to_i version = version.to_i
name = name.camelize name = name.camelize


raise DuplicateMigrationVersionError.new(version) if seen[version] raise DuplicateMigrationVersionError.new(version) if seen[version]
raise DuplicateMigrationNameError.new(name) if seen[[name, scope]] raise DuplicateMigrationNameError.new(name) if seen[name]


seen[version] = seen[[name, scope]] = true seen[version] = seen[name] = true


MigrationProxy.new(name, version, file, scope) MigrationProxy.new(name, version, file)
end end


migrations.sort_by(&:version) migrations.sort_by(&:version)
Expand Down
44 changes: 23 additions & 21 deletions activerecord/test/cases/migration_test.rb
Expand Up @@ -1910,9 +1910,9 @@ def test_copying_migrations_without_timestamps
@existing_migrations = Dir[@migrations_path + "/*.rb"] @existing_migrations = Dir[@migrations_path + "/*.rb"]


copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"}) copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"})
assert File.exists?(@migrations_path + "/4_people_have_hobbies.bukkits.rb") assert File.exists?(@migrations_path + "/4_people_have_hobbies.rb")
assert File.exists?(@migrations_path + "/5_people_have_descriptions.bukkits.rb") assert File.exists?(@migrations_path + "/5_people_have_descriptions.rb")
assert_equal [@migrations_path + "/4_people_have_hobbies.bukkits.rb", @migrations_path + "/5_people_have_descriptions.bukkits.rb"], copied assert_equal [@migrations_path + "/4_people_have_hobbies.rb", @migrations_path + "/5_people_have_descriptions.rb"], copied


files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"}) copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"})
Expand All @@ -1928,12 +1928,13 @@ def test_copying_migrations_without_timestamps_from_2_sources
@existing_migrations = Dir[@migrations_path + "/*.rb"] @existing_migrations = Dir[@migrations_path + "/*.rb"]


sources = ActiveSupport::OrderedHash.new sources = ActiveSupport::OrderedHash.new
sources[:bukkits] = sources[:omg] = MIGRATIONS_ROOT + "/to_copy" sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy"
sources[:omg] = MIGRATIONS_ROOT + "/to_copy2"
ActiveRecord::Migration.copy(@migrations_path, sources) ActiveRecord::Migration.copy(@migrations_path, sources)
assert File.exists?(@migrations_path + "/4_people_have_hobbies.omg.rb") assert File.exists?(@migrations_path + "/4_people_have_hobbies.rb")
assert File.exists?(@migrations_path + "/5_people_have_descriptions.omg.rb") assert File.exists?(@migrations_path + "/5_people_have_descriptions.rb")
assert File.exists?(@migrations_path + "/6_people_have_hobbies.bukkits.rb") assert File.exists?(@migrations_path + "/6_create_articles.rb")
assert File.exists?(@migrations_path + "/7_people_have_descriptions.bukkits.rb") assert File.exists?(@migrations_path + "/7_create_comments.rb")


files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
ActiveRecord::Migration.copy(@migrations_path, sources) ActiveRecord::Migration.copy(@migrations_path, sources)
Expand All @@ -1948,10 +1949,10 @@ def test_copying_migrations_with_timestamps


Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do
copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb") assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.rb")
assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb") assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.rb")
expected = [@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb", expected = [@migrations_path + "/20100726101010_people_have_hobbies.rb",
@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb"] @migrations_path + "/20100726101011_people_have_descriptions.rb"]
assert_equal expected, copied assert_equal expected, copied


files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
Expand All @@ -1968,14 +1969,15 @@ def test_copying_migrations_with_timestamps_from_2_sources
@existing_migrations = Dir[@migrations_path + "/*.rb"] @existing_migrations = Dir[@migrations_path + "/*.rb"]


sources = ActiveSupport::OrderedHash.new sources = ActiveSupport::OrderedHash.new
sources[:bukkits] = sources[:omg] = MIGRATIONS_ROOT + "/to_copy_with_timestamps" sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy_with_timestamps"
sources[:omg] = MIGRATIONS_ROOT + "/to_copy_with_timestamps2"


Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do
copied = ActiveRecord::Migration.copy(@migrations_path, sources) copied = ActiveRecord::Migration.copy(@migrations_path, sources)
assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.omg.rb") assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.rb")
assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.omg.rb") assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.rb")
assert File.exists?(@migrations_path + "/20100726101012_people_have_hobbies.bukkits.rb") assert File.exists?(@migrations_path + "/20100726101012_create_articles.rb")
assert File.exists?(@migrations_path + "/20100726101013_people_have_descriptions.bukkits.rb") assert File.exists?(@migrations_path + "/20100726101013_create_comments.rb")
assert_equal 4, copied.length assert_equal 4, copied.length


files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
Expand All @@ -1992,8 +1994,8 @@ def test_copying_migrations_with_timestamps_to_destination_with_timestamps_in_fu


Time.travel_to(created_at = Time.utc(2010, 2, 20, 10, 10, 10)) do Time.travel_to(created_at = Time.utc(2010, 2, 20, 10, 10, 10)) do
ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
assert File.exists?(@migrations_path + "/20100301010102_people_have_hobbies.bukkits.rb") assert File.exists?(@migrations_path + "/20100301010102_people_have_hobbies.rb")
assert File.exists?(@migrations_path + "/20100301010103_people_have_descriptions.bukkits.rb") assert File.exists?(@migrations_path + "/20100301010103_people_have_descriptions.rb")


files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
Expand All @@ -2010,8 +2012,8 @@ def test_copying_migrations_to_empty_directory


Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do Time.travel_to(created_at = Time.utc(2010, 7, 26, 10, 10, 10)) do
copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"}) copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy_with_timestamps"})
assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.bukkits.rb") assert File.exists?(@migrations_path + "/20100726101010_people_have_hobbies.rb")
assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.bukkits.rb") assert File.exists?(@migrations_path + "/20100726101011_people_have_descriptions.rb")
assert_equal 2, copied.length assert_equal 2, copied.length
end end
ensure ensure
Expand Down
7 changes: 7 additions & 0 deletions activerecord/test/migrations/to_copy2/1_create_articles.rb
@@ -0,0 +1,7 @@
class CreateArticles < ActiveRecord::Migration
def self.up
end

def self.down
end
end
7 changes: 7 additions & 0 deletions activerecord/test/migrations/to_copy2/2_create_comments.rb
@@ -0,0 +1,7 @@
class CreateArticles < ActiveRecord::Migration
def self.up
end

def self.down
end
end
@@ -0,0 +1,7 @@
class CreateArticles < ActiveRecord::Migration
def self.up
end

def self.down
end
end
@@ -0,0 +1,7 @@
class CreateComments < ActiveRecord::Migration
def self.up
end

def self.down
end
end

0 comments on commit 4377f8e

Please sign in to comment.