From 31d28f2650df8d07af6d701dfab9797293770e28 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@jeremyevans.net>
Date: Wed, 18 Dec 2024 10:20:55 -0800
Subject: [PATCH] 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.
---
 CHANGELOG                          |  2 ++
 lib/sequel/extensions/migration.rb | 20 ++++++++++++++++++--
 spec/extensions/migration_spec.rb  | 16 ++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 89b0804b1..27e567cf1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,7 @@
 === master
 
+* Break ties in timestamp migrator version handling using lexicographic sort of rest of migration filename (jeremyevans)
+
 * Fix strict_unused_block warnings when running specs on Ruby 3.4 (jeremyevans)
 
 === 5.87.0 (2024-12-01)
diff --git a/lib/sequel/extensions/migration.rb b/lib/sequel/extensions/migration.rb
index 9a481dfbc..ec27006ff 100644
--- a/lib/sequel/extensions/migration.rb
+++ b/lib/sequel/extensions/migration.rb
@@ -371,7 +371,7 @@ def self.migration(&block)
   #
   # Part of the +migration+ extension.
   class Migrator
-    MIGRATION_FILE_PATTERN = /\A(\d+)_.+\.rb\z/i.freeze
+    MIGRATION_FILE_PATTERN = /\A(\d+)_(.+)\.rb\z/i.freeze
 
     # Mutex used around migration file loading
     MUTEX = Mutex.new
@@ -791,7 +791,23 @@ def get_migration_files
         next unless MIGRATION_FILE_PATTERN.match(file)
         files << File.join(directory, file)
       end
-      files.sort_by{|f| MIGRATION_FILE_PATTERN.match(File.basename(f))[1].to_i}
+      files.sort! do |a, b|
+        a_ver, a_name = split_migration_filename(a)
+        b_ver, b_name = split_migration_filename(b)
+        x = a_ver <=> b_ver
+        if x.zero?
+          x = a_name <=> b_name
+        end
+        x
+      end
+      files
+    end
+
+    # Return an integer and name (without extension) for the given path.
+    def split_migration_filename(path)
+      version, name = MIGRATION_FILE_PATTERN.match(File.basename(path)).captures
+      version = version.to_i
+      [version, name]
     end
     
     # Returns tuples of migration, filename, and direction
diff --git a/spec/extensions/migration_spec.rb b/spec/extensions/migration_spec.rb
index 5661597dc..fae55cb88 100644
--- a/spec/extensions/migration_spec.rb
+++ b/spec/extensions/migration_spec.rb
@@ -828,6 +828,22 @@ def create_table(name, *args, &block)
     @db[:schema_migrations].select_order_map(:filename).must_equal []
   end
 
+  it "should sort lexicographically by name for same timestamp" do
+    @dir = 'spec/files/duplicate_timestamped_migrations'
+    files = nil
+    migrator_class = Class.new(Sequel::TimestampMigrator) do
+      define_method(:get_migration_files) do
+        files = super()
+      end
+    end
+    @m = migrator_class.new(@db, @dir)
+    @m.run
+    files.map{|x| File.basename(x)}.must_equal %w'1273253849_create_sessions.rb 1273253853_create_nodes.rb 1273253853_create_users.rb'
+
+    [:schema_migrations, :sm1111, :sm2222, :sm3333].each{|n| @db.table_exists?(n).must_equal true}
+    @db[:schema_migrations].select_order_map(:filename).must_equal %w'1273253849_create_sessions.rb 1273253853_create_nodes.rb 1273253853_create_users.rb'
+  end
+
   it "should convert schema_info table to schema_migrations table" do
     @dir = 'spec/files/integer_migrations'
     @m.apply(@db, @dir)