Fix enqueue_background_data_migration to be idempotent

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## master (unreleased)
 
+- Fix `enqueue_background_data_migration` to be idempotent
+
 ## 0.19.1 (2024-05-24)
 
 - Fix `add_index_in_background` to be idempotent

lib/online_migrations/background_migrations/migration_helpers.rb

@@ -440,26 +440,22 @@ def create_background_data_migration(migration_name, *arguments, **options)
 
         migration_name = migration_name.name if migration_name.is_a?(Class)
 
-        migration = Migration.new(
-          migration_name: migration_name,
-          arguments: arguments,
-          **options
-        )
-
-        shards = Utils.shard_names(migration.migration_model)
-        if shards.size > 1
-          migration.children = shards.map do |shard|
-            child = migration.dup
-            child.shard = shard
-            child
+        # Can't use `find_or_create_by` or hash syntax here, because it does not correctly work with json `arguments`.
+        existing_migration = Migration.find_by("migration_name = ? AND arguments = ? AND shard IS NULL", migration_name, arguments.to_json)
+        return existing_migration if existing_migration
+
+        Migration.create!(**options, migration_name: migration_name, arguments: arguments, shard: nil) do |migration|
+          shards = Utils.shard_names(migration.migration_model)
+          if shards.size > 1
+            migration.children = shards.map do |shard|
+              child = migration.dup
+              child.shard = shard
+              child
+            end
+
+            migration.composite = true
           end
-
-          migration.composite = true
         end
-
-        # This will save all the records using a transaction.
-        migration.save!
-        migration
       end
     end
   end

lib/online_migrations/background_migrations/migration_runner.rb

@@ -13,7 +13,7 @@ def initialize(migration)
       # Runs one background migration job.
       def run_migration_job
         raise "Should not be called on a composite (with sharding) migration" if migration.composite?
-        return if migration.cancelled?
+        return if migration.cancelled? || migration.succeeded?
 
         mark_as_running if migration.enqueued?
         migration_payload = notifications_payload(migration)

test/schema_statements/misc_test.rb

@@ -172,6 +172,12 @@ def test_enqueue_background_data_migration
       assert_equal OnlineMigrations.config.background_migrations.batch_size, m.batch_size
     end
 
+    def test_enqueue_background_data_migration_is_idempotent
+      assert_equal 0, OnlineMigrations::BackgroundMigrations::Migration.count
+      2.times { @connection.enqueue_background_data_migration("MakeAllNonAdmins") }
+      assert_equal 1, OnlineMigrations::BackgroundMigrations::Migration.count
+    end
+
     def test_run_background_migrations_inline_true_in_local
       user = User.create!
       assert_nil user.admin