Skip to content

Commit 31d28f2

Browse files
committed
Break ties in timestamp migrator version handling using lexicographic sort of rest of migration filename
While I consider it a bad practice to have multiple migrations with the same version that are interdependent, having non-deterministic behavior in this case isn't great either. This makes Sequel::Migrator::MIGRATION_FILE_PATTERN add another capture, which could affect users using it directly, but that seems unlikely.
1 parent 267daa5 commit 31d28f2

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed

CHANGELOG

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
=== master
22

3+
* Break ties in timestamp migrator version handling using lexicographic sort of rest of migration filename (jeremyevans)
4+
35
* Fix strict_unused_block warnings when running specs on Ruby 3.4 (jeremyevans)
46

57
=== 5.87.0 (2024-12-01)

lib/sequel/extensions/migration.rb

+18-2
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ def self.migration(&block)
371371
#
372372
# Part of the +migration+ extension.
373373
class Migrator
374-
MIGRATION_FILE_PATTERN = /\A(\d+)_.+\.rb\z/i.freeze
374+
MIGRATION_FILE_PATTERN = /\A(\d+)_(.+)\.rb\z/i.freeze
375375

376376
# Mutex used around migration file loading
377377
MUTEX = Mutex.new
@@ -791,7 +791,23 @@ def get_migration_files
791791
next unless MIGRATION_FILE_PATTERN.match(file)
792792
files << File.join(directory, file)
793793
end
794-
files.sort_by{|f| MIGRATION_FILE_PATTERN.match(File.basename(f))[1].to_i}
794+
files.sort! do |a, b|
795+
a_ver, a_name = split_migration_filename(a)
796+
b_ver, b_name = split_migration_filename(b)
797+
x = a_ver <=> b_ver
798+
if x.zero?
799+
x = a_name <=> b_name
800+
end
801+
x
802+
end
803+
files
804+
end
805+
806+
# Return an integer and name (without extension) for the given path.
807+
def split_migration_filename(path)
808+
version, name = MIGRATION_FILE_PATTERN.match(File.basename(path)).captures
809+
version = version.to_i
810+
[version, name]
795811
end
796812

797813
# Returns tuples of migration, filename, and direction

spec/extensions/migration_spec.rb

+16
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,22 @@ def create_table(name, *args, &block)
828828
@db[:schema_migrations].select_order_map(:filename).must_equal []
829829
end
830830

831+
it "should sort lexicographically by name for same timestamp" do
832+
@dir = 'spec/files/duplicate_timestamped_migrations'
833+
files = nil
834+
migrator_class = Class.new(Sequel::TimestampMigrator) do
835+
define_method(:get_migration_files) do
836+
files = super()
837+
end
838+
end
839+
@m = migrator_class.new(@db, @dir)
840+
@m.run
841+
files.map{|x| File.basename(x)}.must_equal %w'1273253849_create_sessions.rb 1273253853_create_nodes.rb 1273253853_create_users.rb'
842+
843+
[:schema_migrations, :sm1111, :sm2222, :sm3333].each{|n| @db.table_exists?(n).must_equal true}
844+
@db[:schema_migrations].select_order_map(:filename).must_equal %w'1273253849_create_sessions.rb 1273253853_create_nodes.rb 1273253853_create_users.rb'
845+
end
846+
831847
it "should convert schema_info table to schema_migrations table" do
832848
@dir = 'spec/files/integer_migrations'
833849
@m.apply(@db, @dir)

0 commit comments

Comments
 (0)