Avoid name collision between migrations

Adds a `pre-push` hook to check name clashing between `db` migrations and `Elasticsearch` migrations.

lefthook.yml

@@ -83,6 +83,11 @@ pre-push:
       files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD
       glob: 'db/structure.sql'
       run: scripts/validate_schema_changes
+    db-migration-name-collisions:
+      tags: database
+      files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD
+      glob: '{db/migrate/*.rb,db/post_migrate/*.rb,ee/elastic/migrate/*.rb}'
+      run: scripts/validate_name_collisions_between_migrations
     static-verification:
       skip: true # This is disabled by default. You can enable this check by adding skip: false in lefthook-local.yml https://github.com/evilmartians/lefthook/blob/master/docs/configuration.md#skip
       tags: backend

scripts/database/migration_collision_checker.rb

+# frozen_string_literal: true
+
+require 'pathname'
+require 'open3'
+
+# Checks for class name collisions between Database migrations and Elasticsearch migrations
+class MigrationCollisionChecker
+  MIGRATION_FOLDERS = %w[db/migrate/*.rb db/post_migrate/*.rb ee/elastic/migrate/*.rb].freeze
+
+  CLASS_MATCHER = /^\s*class\s+:*([A-Z][A-Za-z0-9_]+\S+)/
+
+  ERROR_CODE = 1
+
+  # To be removed in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129012
+  SKIP_MIGRATIONS = %w[AddInternalToNotes BackfillInternalOnNotes].freeze
+
+  Result = Struct.new(:error_code, :error_message)
+
+  def initialize
+    @collisions = Hash.new { |h, k| h[k] = [] }
+  end
+
+  def check
+    check_for_collisions
+
+    return if collisions.empty?
+
+    Result.new(ERROR_CODE, "\e[31mError: Naming collisions were found between migrations\n\n#{message}\e[0m")
+  end
+
+  private
+
+  attr_reader :collisions
+
+  def check_for_collisions
+    MIGRATION_FOLDERS.each do |migration_folder|
+      Dir.glob(base_path.join(migration_folder)).each do |migration_path|
+        klass_name = CLASS_MATCHER.match(File.read(migration_path))[1]
+
+        next if SKIP_MIGRATIONS.include?(klass_name)
+
+        collisions[klass_name] << migration_path
+      end
+    end
+
+    collisions.select! { |_, v| v.size > 1 }
+  end
+
+  def message
+    collisions.map { |klass_name, paths| "#{klass_name}: #{paths.join(', ')}\n" }.join('')
+  end
+
+  def base_path
+    Pathname.new(File.expand_path('../../', __dir__))
+  end
+end

scripts/validate_name_collisions_between_migrations

+#!/usr/bin/env ruby
+
+# frozen_string_literal: true
+
+require_relative './database/migration_collision_checker'
+
+result = MigrationCollisionChecker.new.check
+
+if result
+  puts result.error_message
+  exit result.error_code
+end

spec/fixtures/migrations/db/migrate/database_migration.txt

+# frozen_string_literal: true
+
+class DatabaseMigration < Gitlab::Database::Migration[2.0]
+  def up
+    add_column :dast_site_profiles, :scan_file_path, :text
+  end
+
+  def down
+    remove_column :dast_site_profiles, :scan_file_path, :text
+  end
+end

spec/fixtures/migrations/db/migrate/database_migration_two.txt

+# frozen_string_literal: true
+
+class Gitlab::Database::DatabaseMigration < Gitlab::Database::Migration[2.0]
+  def up
+    add_column :dast_site_profiles, :scan_file_path, :text
+  end
+
+  def down
+    remove_column :dast_site_profiles, :scan_file_path, :text
+  end
+end

spec/fixtures/migrations/db/post_migrate/database_clash_migration.txt

+# frozen_string_literal: true
+
+class ::ClashMigration < Gitlab::Database::Migration[2.0]
+  def up
+    add_column :dast_site_profiles, :scan_file_path, :text
+  end
+
+  def down
+    remove_column :dast_site_profiles, :scan_file_path, :text
+  end
+end

spec/fixtures/migrations/db/post_migrate/database_clash_migration_two.txt

+# frozen_string_literal: true
+
+class Gitlab::ClashMigrationTwo < Gitlab::Database::Migration[2.0]
+  def up
+    add_column :dast_site_profiles, :scan_file_path, :text
+  end
+
+  def down
+    remove_column :dast_site_profiles, :scan_file_path, :text
+  end
+end

spec/fixtures/migrations/elasticsearch/elasticsearch_clash_migration.txt

+# frozen_string_literal: true
+
+class ClashMigration < Elastic::Migration
+  include Elastic::MigrationCreateIndex
+
+  retry_on_failure
+
+  def document_type
+    :epic
+  end
+
+  def target_class
+    Epic
+  end
+end

spec/fixtures/migrations/elasticsearch/elasticsearch_clash_migration_two.txt

+# frozen_string_literal: true
+
+class Gitlab::ClashMigrationTwo < Gitlab::Database::Migration[2.0]
+  def up
+    add_column :dast_site_profiles, :scan_file_path, :text
+  end
+
+  def down
+    remove_column :dast_site_profiles, :scan_file_path, :text
+  end
+end

spec/fixtures/migrations/elasticsearch/elasticsearch_migration.txt

+# frozen_string_literal: true
+
+class ElasticsearchMigration < Elastic::Migration
+    include Elastic::MigrationUpdateMappingsHelper
+
+    private
+
+    def index_name
+      Project.__elasticsearch__.index_name
+    end
+
+    def new_mappings
+      {
+        readme_content: {
+          type: 'text'
+        },
+        ci_catalog: {
+          type: 'boolean'
+        }
+      }
+    end
+end

spec/scripts/database/migration_collision_checker_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+require_relative '../../../scripts/database/migration_collision_checker'
+
+RSpec.describe MigrationCollisionChecker, feature_category: :database do
+  subject(:checker) { described_class.new }
+
+  before do
+    stub_const('MigrationCollisionChecker::MIGRATION_FOLDERS', [db_migration_path, elasticsearch_migration_path])
+  end
+
+  describe "#check" do
+    context "when there's no collision between migrations" do
+      let(:db_migration_path) { 'spec/fixtures/migrations/db/migrate/*.txt' }
+      let(:elasticsearch_migration_path) { 'spec/fixtures/migrations/elasticsearch/*.txt' }
+
+      it { expect(checker.check).to be_nil }
+    end
+
+    context 'when migration class name clashes' do
+      let(:db_migration_path) { 'spec/fixtures/migrations/db/*/*.txt' }
+      let(:elasticsearch_migration_path) { 'spec/fixtures/migrations/elasticsearch/*.txt' }
+
+      it 'returns the error code' do
+        expect(checker.check.error_code).to eq(1)
+      end
+
+      it 'returns the error message' do
+        expect(checker.check.error_message).to include(
+          'Naming collisions were found between migrations', 'ClashMigration', 'Gitlab::ClashMigrationTwo'
+        )
+      end
+    end
+
+    context 'when migration class name clashes but they are marked to be skipped' do
+      let(:db_migration_path) { 'spec/fixtures/migrations/db/*/*.txt' }
+      let(:elasticsearch_migration_path) { 'spec/fixtures/migrations/elasticsearch/*.txt' }
+
+      before do
+        stub_const('MigrationCollisionChecker::SKIP_MIGRATIONS', %w[ClashMigration Gitlab::ClashMigrationTwo])
+      end
+
+      it { expect(checker.check).to be_nil }
+    end
+  end
+end