diff --git a/.rubocop_todo/database/multiple_databases.yml b/.rubocop_todo/database/multiple_databases.yml index 40de71b120373f608fca046325558a44ea8521be..1c63431c4d7ba208ff213a22297e9154376acc7f 100644 --- a/.rubocop_todo/database/multiple_databases.yml +++ b/.rubocop_todo/database/multiple_databases.yml @@ -16,7 +16,6 @@ Database/MultipleDatabases: - 'lib/backup/manager.rb' - lib/gitlab/background_migration/backfill_projects_with_coverage.rb - lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans.rb - - lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds.rb - 'lib/gitlab/database.rb' - 'lib/gitlab/database/load_balancing/load_balancer.rb' - 'lib/gitlab/database/migrations/observers/query_log.rb' diff --git a/ee/spec/support/helpers/ee/migrations_helpers.rb b/ee/spec/support/helpers/ee/migrations_helpers.rb index 7f8a33c5eb10db6fa3c470b98869d43ce7f9eb2f..e34e32e7dd110cbb5edc4596bc2559659d5e32b7 100644 --- a/ee/spec/support/helpers/ee/migrations_helpers.rb +++ b/ee/spec/support/helpers/ee/migrations_helpers.rb @@ -11,7 +11,7 @@ def reset_column_information(klass) end override :active_record_base - def active_record_base + def active_record_base(...) if geo_migration? ::Geo::TrackingBase else diff --git a/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds.rb b/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds.rb index 78e897d9ae13e98eac6afe67c0e3125c77ceb12a..36d4e6492716751039911a689dd508e068b2758e 100644 --- a/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds.rb +++ b/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds.rb @@ -26,7 +26,7 @@ def batch_metrics private def connection - ActiveRecord::Base.connection + ::Ci::ApplicationRecord.connection end def relation_scoped_to_range(source_table, source_key_column, start_id, stop_id) diff --git a/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb b/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb index 90dd3e14606b5a2d580c6bef22d077044313cddd..e38edfc36432e8b1668cfce488c3e0b232c1be96 100644 --- a/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb +++ b/spec/lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds_spec.rb @@ -5,9 +5,9 @@ RSpec.describe Gitlab::BackgroundMigration::NullifyOrphanRunnerIdOnCiBuilds, :migration, schema: 20220223112304 do let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } - let(:ci_runners) { table(:ci_runners) } - let(:ci_pipelines) { table(:ci_pipelines) } - let(:ci_builds) { table(:ci_builds) } + let(:ci_runners) { table(:ci_runners, database: :ci) } + let(:ci_pipelines) { table(:ci_pipelines, database: :ci) } + let(:ci_builds) { table(:ci_builds, database: :ci) } subject { described_class.new } @@ -26,9 +26,9 @@ describe '#perform' do let(:namespace) { namespaces.create!(name: 'test', path: 'test', type: 'Group') } let(:project) { projects.create!(namespace_id: namespace.id, name: 'test') } - let(:pipeline) { ci_pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success') } it 'nullifies runner_id for orphan ci_builds in range' do + pipeline = ci_pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success') ci_runners.create!(id: 2, runner_type: 'project_type') ci_builds.create!(id: 5, type: 'Ci::Build', commit_id: pipeline.id, runner_id: 2) diff --git a/spec/support/helpers/migrations_helpers.rb b/spec/support/helpers/migrations_helpers.rb index afa7ee84bda69ccb114be28cfb3cc88e9d8b60eb..0c60871d2e746c1c3a50e13b385d6bb17c6bb64b 100644 --- a/spec/support/helpers/migrations_helpers.rb +++ b/spec/support/helpers/migrations_helpers.rb @@ -1,12 +1,18 @@ # frozen_string_literal: true module MigrationsHelpers - def active_record_base - Gitlab::Database.database_base_models.fetch(self.class.metadata[:database] || :main) + def active_record_base(database: nil) + database_name = database || self.class.metadata[:database] || :main + + unless Gitlab::Database::DATABASE_NAMES.include?(database_name.to_s) + raise ArgumentError, "#{database_name} is not a valid argument" + end + + Gitlab::Database.database_base_models[database_name] || Gitlab::Database.database_base_models[:main] end - def table(name) - Class.new(active_record_base) do + def table(name, database: nil) + Class.new(active_record_base(database: database)) do self.table_name = name self.inheritance_column = :_type_disabled diff --git a/spec/support_specs/helpers/migrations_helpers_spec.rb b/spec/support_specs/helpers/migrations_helpers_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b82eddad9bcf1145846e0294050119da0adcd714 --- /dev/null +++ b/spec/support_specs/helpers/migrations_helpers_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MigrationsHelpers do + let(:helper_class) do + Class.new.tap do |klass| + klass.include described_class + allow(klass).to receive(:metadata).and_return(metadata) + end + end + + let(:metadata) { {} } + let(:helper) { helper_class.new } + + describe '#active_record_base' do + it 'returns the main base model' do + expect(helper.active_record_base).to eq(ActiveRecord::Base) + end + + context 'ci database configured' do + before do + skip_if_multiple_databases_not_setup + end + + it 'returns the CI base model' do + expect(helper.active_record_base(database: :ci)).to eq(Ci::ApplicationRecord) + end + end + + context 'ci database not configured' do + before do + skip_if_multiple_databases_are_setup + end + + it 'returns the main base model' do + expect(helper.active_record_base(database: :ci)).to eq(ActiveRecord::Base) + end + end + + it 'raises ArgumentError for bad database argument' do + expect { helper.active_record_base(database: :non_existent) }.to raise_error(ArgumentError) + end + end + + describe '#table' do + it 'creates a class based on main base model' do + klass = helper.table(:projects) + expect(klass.connection_specification_name).to eq('ActiveRecord::Base') + end + + context 'ci database configured' do + before do + skip_if_multiple_databases_not_setup + end + + it 'create a class based on the CI base model' do + klass = helper.table(:ci_builds, database: :ci) + expect(klass.connection_specification_name).to eq('Ci::ApplicationRecord') + end + end + + context 'ci database not configured' do + before do + skip_if_multiple_databases_are_setup + end + + it 'creates a class based on main base model' do + klass = helper.table(:ci_builds, database: :ci) + expect(klass.connection_specification_name).to eq('ActiveRecord::Base') + end + end + end +end