Merge branch '342795-use-yaml-file-to-define-gitlab_schema' into 'master'

Use a YAML file to define gitlab_schema for tables

See merge request gitlab-org/gitlab!72353

app/models/application_record.rb

 # frozen_string_literal: true
 
 class ApplicationRecord < ActiveRecord::Base
-  self.gitlab_schema = :gitlab_main
   self.abstract_class = true
 
   alias_method :reset, :reload

app/models/ci/application_record.rb

@@ -2,7 +2,6 @@
 
 module Ci
   class ApplicationRecord < ::ApplicationRecord
-    self.gitlab_schema = :gitlab_ci
     self.abstract_class = true
 
     def self.table_name_prefix

app/models/concerns/loose_foreign_key.rb

@@ -69,7 +69,7 @@ module LooseForeignKey
       end
 
       on_delete_options = %i[async_delete async_nullify]
-      gitlab_schema_options = [ApplicationRecord.gitlab_schema, Ci::ApplicationRecord.gitlab_schema]
+      gitlab_schema_options = %i(gitlab_main gitlab_ci)
 
       unless on_delete_options.include?(symbolized_options[:on_delete]&.to_sym)
         raise "Invalid on_delete option given: #{symbolized_options[:on_delete]}. Valid options: #{on_delete_options.join(', ')}"

config/initializers/00_active_record_gitlab_schema.rb

-# frozen_string_literal: true
-
-# This parameter describes a virtual context to indicate
-# table affinity to other tables.
-#
-# Table affinity limits cross-joins, cross-modifications,
-# foreign keys and validates relationship between tables
-#
-# By default it is undefined
-ActiveRecord::Base.class_attribute :gitlab_schema, default: nil

config/initializers/0_acts_as_taggable.rb

@@ -13,7 +13,3 @@ raise "Counter cache is not disabled" if
 ActsAsTaggableOn::Tagging.include IgnorableColumns
 ActsAsTaggableOn::Tagging.ignore_column :id_convert_to_bigint, remove_with: '14.5', remove_after: '2021-10-22'
 ActsAsTaggableOn::Tagging.ignore_column :taggable_id_convert_to_bigint, remove_with: '14.5', remove_after: '2021-10-22'
-
-# The tags and taggings are supposed to be part of `gitlab_ci`
-ActsAsTaggableOn::Tag.gitlab_schema = :gitlab_ci
-ActsAsTaggableOn::Tagging.gitlab_schema = :gitlab_ci

db/migrate/20210629031900_associate_existing_dast_builds_with_variables.rb

@@ -20,7 +20,6 @@ class AssociateExistingDastBuildsWithVariables < ActiveRecord::Migration[6.1]
   class Build < ApplicationRecord
     self.table_name = 'ci_builds'
     self.inheritance_column = :_type_disabled
-    self.gitlab_schema = :gitlab_ci
 
     default_scope { where(name: :dast, stage: :dast) } # rubocop:disable Cop/DefaultScope
   end

spec/support/database/cross-join-allowlist.yml

@@ -30,6 +30,7 @@
 - "./spec/lib/gitlab/background_migration/migrate_pages_metadata_spec.rb"
 - "./spec/migrations/20210907211557_finalize_ci_builds_bigint_conversion_spec.rb"
 - "./spec/migrations/associate_existing_dast_builds_with_variables_spec.rb"
+- "./spec/migrations/disable_job_token_scope_when_unused_spec.rb"
 - "./spec/migrations/schedule_copy_ci_builds_columns_to_security_scans2_spec.rb"
 - "./spec/migrations/schedule_pages_metadata_migration_spec.rb"
 - "./spec/models/ci/pipeline_spec.rb"

spec/support/database/gitlab_schema.rb

@@ -9,17 +9,12 @@ module Database
     end
 
     def self.table_schema(name)
-      tables_to_schema[name] || :undefined
+      # When undefined it's best to return a unique name so that we don't incorrectly assume that 2 undefined schemas belong on the same database
+      tables_to_schema[name] || :"undefined_#{name}"
     end
 
     def self.tables_to_schema
-      @tables_to_schema ||= all_classes_with_schema.to_h do |klass|
-        [klass.table_name, klass.gitlab_schema]
-      end
-    end
-
-    def self.all_classes_with_schema
-      ActiveRecord::Base.descendants.reject(&:abstract_class?).select(&:gitlab_schema?) # rubocop:disable Database/MultipleDatabases
+      @tables_to_schema ||= YAML.load_file(Rails.root.join('spec/support/database/gitlab_schemas.yml'))
     end
   end
 end

spec/support/database/prevent_cross_database_modification.rb

@@ -91,6 +91,11 @@ module Database
 
       return if tables.empty?
 
+      # All migrations will write to schema_migrations in the same transaction.
+      # It's safe to ignore this since schema_migrations exists in all
+      # databases
+      return if tables == ['schema_migrations']
+
       cross_database_context[:modified_tables_by_db][database].merge(tables)
 
       all_tables = cross_database_context[:modified_tables_by_db].values.map(&:to_a).flatten

spec/support_specs/database/gitlab_schema_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Database::GitlabSchema do
+  it 'matches all the tables in the database', :aggregate_failures do
+    # These tables do not need a gitlab_schema
+    excluded_tables = %w(ar_internal_metadata schema_migrations)
+
+    all_tables_in_database = ApplicationRecord.connection.tables
+
+    all_tables_with_gitlab_schema = described_class.tables_to_schema.keys
+
+    missing = []
+    all_tables_in_database.each do |table_in_database|
+      next if table_in_database.in?(excluded_tables)
+
+      missing << table_in_database unless all_tables_with_gitlab_schema.include?(table_in_database)
+    end
+
+    extras = []
+    all_tables_with_gitlab_schema.each do |table_with_gitlab_schema|
+      extras << table_with_gitlab_schema unless all_tables_in_database.include?(table_with_gitlab_schema)
+    end
+
+    expect(missing).to be_empty, "Missing table(s) #{missing} not found in #{described_class}.tables_to_schema. Any new tables must be added to spec/support/database/gitlab_schemas.yml ."
+
+    expect(extras).to be_empty, "Extra table(s) #{extras} found in #{described_class}.tables_to_schema. Any removed or renamed tables must be removed from spec/support/database/gitlab_schemas.yml ."
+  end
+end