Merge branch 'pb-background-migration-tracking-fix-class-names' into 'master'

Remove leading :: from BG migration class names

See merge request gitlab-org/gitlab!36722

lib/gitlab/database/background_migration_job.rb

@@ -6,7 +6,7 @@ module Gitlab
       self.table_name = :background_migration_jobs
 
       scope :for_migration_execution, -> (class_name, arguments) do
-        where('class_name = ? AND arguments = ?', class_name, arguments.to_json)
+        where('class_name = ? AND arguments = ?', normalize_class_name(class_name), arguments.to_json)
       end
 
       enum status: {
@@ -18,6 +18,16 @@ module Gitlab
         self.pending.for_migration_execution(class_name, arguments)
           .update_all("status = #{statuses[:succeeded]}, updated_at = NOW()")
       end
+
+      def self.normalize_class_name(class_name)
+        return class_name unless class_name.present? && class_name.start_with?('::')
+
+        class_name[2..]
+      end
+
+      def class_name=(value)
+        write_attribute(:class_name, self.class.normalize_class_name(value))
+      end
     end
   end
 end

spec/lib/gitlab/database/background_migration_job_spec.rb

@@ -16,6 +16,13 @@ RSpec.describe Gitlab::Database::BackgroundMigrationJob do
       expect(relation.count).to eq(1)
       expect(relation.first).to have_attributes(class_name: 'TestJob', arguments: ['hi', 2])
     end
+
+    it 'normalizes class names by removing leading ::' do
+      relation = described_class.for_migration_execution('::TestJob', ['hi', 2])
+
+      expect(relation.count).to eq(1)
+      expect(relation.first).to have_attributes(class_name: 'TestJob', arguments: ['hi', 2])
+    end
   end
 
   describe '.mark_all_as_succeeded' do
@@ -34,6 +41,16 @@ RSpec.describe Gitlab::Database::BackgroundMigrationJob do
       expect(job4.reload).to be_pending
     end
 
+    it 'normalizes class_names by removing leading ::' do
+      expect { described_class.mark_all_as_succeeded('::TestJob', [1, 100]) }
+        .to change { described_class.succeeded.count }.from(0).to(2)
+
+      expect(job1.reload).to be_succeeded
+      expect(job2.reload).to be_succeeded
+      expect(job3.reload).to be_pending
+      expect(job4.reload).to be_pending
+    end
+
     context 'when previous matching jobs have already succeeded' do
       let(:initial_time) { Time.now.round }
       let!(:job1) { create(:background_migration_job, :succeeded, created_at: initial_time, updated_at: initial_time) }
@@ -51,4 +68,38 @@ RSpec.describe Gitlab::Database::BackgroundMigrationJob do
       end
     end
   end
+
+  describe '#class_name=' do
+    context 'when the class_name is given without the leading ::' do
+      it 'sets the class_name to the given value' do
+        job = described_class.new(class_name: 'TestJob')
+
+        expect(job.class_name).to eq('TestJob')
+      end
+    end
+
+    context 'when the class_name is given with the leading ::' do
+      it 'removes the leading :: when setting the class_name' do
+        job = described_class.new(class_name: '::TestJob')
+
+        expect(job.class_name).to eq('TestJob')
+      end
+    end
+
+    context 'when the value is nil' do
+      it 'sets the class_name to nil' do
+        job = described_class.new(class_name: nil)
+
+        expect(job.class_name).to be_nil
+      end
+    end
+
+    context 'when the values is blank' do
+      it 'sets the class_name to the given value' do
+        job = described_class.new(class_name: '')
+
+        expect(job.class_name).to eq('')
+      end
+    end
+  end
 end

spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb

@@ -161,7 +161,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do
         it 'creates a record for each job in the database' do
           Sidekiq::Testing.fake! do
             expect do
-              model.queue_background_migration_jobs_by_range_at_intervals(User, 'FooJob', 10.minutes,
+              model.queue_background_migration_jobs_by_range_at_intervals(User, '::FooJob', 10.minutes,
                 other_job_arguments: [1, 2], track_jobs: true)
             end.to change { Gitlab::Database::BackgroundMigrationJob.count }.from(0).to(1)
 

spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb

@@ -97,7 +97,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::BackfillPartition
     end
 
     it 'marks each job record as succeeded after processing' do
-      create(:background_migration_job, class_name: described_class.name,
+      create(:background_migration_job, class_name: "::#{described_class.name}",
         arguments: [source1.id, source3.id, source_table, destination_table, unique_key])
 
       expect(::Gitlab::Database::BackgroundMigrationJob).to receive(:mark_all_as_succeeded).and_call_original