From 429563313f830b1f64c0849565823183500ffc4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu>
Date: Thu, 28 Apr 2022 11:40:24 +0200
Subject: [PATCH] Make `Database/MultipleDatabases` to discover
`::ActiveRecord::Base`
---
.rubocop_todo/database/multiple_databases.yml | 28 +++++++++++++++++++
.../attr_encrypted_no_db_connection.rb | 2 +-
.../database/batched_background_migrations.md | 2 +-
rubocop/cop/database/multiple_databases.rb | 2 +-
.../cop/database/multiple_databases_spec.rb | 7 +++++
5 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/.rubocop_todo/database/multiple_databases.yml b/.rubocop_todo/database/multiple_databases.yml
index 8e372b89bbd9a..019fb41094d6c 100644
--- a/.rubocop_todo/database/multiple_databases.yml
+++ b/.rubocop_todo/database/multiple_databases.yml
@@ -14,6 +14,34 @@ Database/MultipleDatabases:
- 'ee/spec/services/ee/merge_requests/update_service_spec.rb'
- 'lib/backup/database.rb'
- 'lib/backup/manager.rb'
+ - lib/gitlab/background_migration/backfill_integrations_type_new.rb
+ - lib/gitlab/background_migration/backfill_issue_search_data.rb
+ - lib/gitlab/background_migration/backfill_member_namespace_for_group_members.rb
+ - lib/gitlab/background_migration/backfill_namespace_id_for_namespace_route.rb
+ - lib/gitlab/background_migration/backfill_namespace_id_for_project_route.rb
+ - lib/gitlab/background_migration/backfill_namespace_traversal_ids_children.rb
+ - lib/gitlab/background_migration/backfill_projects_with_coverage.rb
+ - lib/gitlab/background_migration/backfill_upvotes_count_on_issues.rb
+ - lib/gitlab/background_migration/backfill_user_namespace.rb
+ - lib/gitlab/background_migration/copy_ci_builds_columns_to_security_scans.rb
+ - lib/gitlab/background_migration/delete_orphaned_deployments.rb
+ - lib/gitlab/background_migration/disable_expiration_policies_linked_to_no_container_images.rb
+ - lib/gitlab/background_migration/fix_duplicate_project_name_and_path.rb
+ - lib/gitlab/background_migration/fix_projects_without_project_feature.rb
+ - lib/gitlab/background_migration/fix_projects_without_prometheus_service.rb
+ - lib/gitlab/background_migration/migrate_shimo_confluence_integration_category.rb
+ - lib/gitlab/background_migration/migrate_stage_status.rb
+ - lib/gitlab/background_migration/move_container_registry_enabled_to_project_feature.rb
+ - lib/gitlab/background_migration/nullify_orphan_runner_id_on_ci_builds.rb
+ - lib/gitlab/background_migration/populate_container_repository_migration_plan.rb
+ - lib/gitlab/background_migration/populate_topics_non_private_projects_count.rb
+ - lib/gitlab/background_migration/populate_topics_total_projects_count_cache.rb
+ - lib/gitlab/background_migration/populate_vulnerability_reads.rb
+ - lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb
+ - lib/gitlab/background_migration/remove_vulnerability_finding_links.rb
+ - lib/gitlab/background_migration/update_timelogs_null_spent_at.rb
+ - lib/gitlab/background_migration/update_timelogs_project_id.rb
+ - lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb
- 'lib/gitlab/database.rb'
- 'lib/gitlab/database/load_balancing/load_balancer.rb'
- 'lib/gitlab/database/migrations/observers/query_log.rb'
diff --git a/config/initializers/attr_encrypted_no_db_connection.rb b/config/initializers/attr_encrypted_no_db_connection.rb
index 91eef7d0a5c2d..e46ee9f803623 100644
--- a/config/initializers/attr_encrypted_no_db_connection.rb
+++ b/config/initializers/attr_encrypted_no_db_connection.rb
@@ -17,7 +17,7 @@ def attribute_instance_methods_as_symbols_available?
# ensuring the connection is released
def attribute_instance_methods_as_symbols
# Use with_connection so the connection doesn't stay pinned to the thread.
- connected = ::ActiveRecord::Base.connection_pool.with_connection(&:active?) rescue false
+ connected = ::ActiveRecord::Base.connection_pool.with_connection(&:active?) rescue false # rubocop:disable Database/MultipleDatabases
if connected
# Call version from AttrEncrypted::Adapters::ActiveRecord
diff --git a/doc/development/database/batched_background_migrations.md b/doc/development/database/batched_background_migrations.md
index 029df4dc11edb..0064be1e4a535 100644
--- a/doc/development/database/batched_background_migrations.md
+++ b/doc/development/database/batched_background_migrations.md
@@ -51,7 +51,7 @@ for new versions to deploy while the migrations are still running.
Background Migration contrary to regular migrations does have access to multiple databases
and can be used to efficiently access and update data across them. To properly indicate
a database to be used it is desired to create ActiveRecord model inline the migration code.
-Such model should use a correct [`ApplicationRecord`](database/multiple_databases.md#gitlab-schema)
+Such model should use a correct [`ApplicationRecord`](multiple_databases.md#gitlab-schema)
depending on which database the table is located. As such usage of `ActiveRecord::Base`
is disallowed as it does not describe a explicitly database to be used to access given table.
diff --git a/rubocop/cop/database/multiple_databases.rb b/rubocop/cop/database/multiple_databases.rb
index f20348d9d1fea..470c282f60f4e 100644
--- a/rubocop/cop/database/multiple_databases.rb
+++ b/rubocop/cop/database/multiple_databases.rb
@@ -22,7 +22,7 @@ class MultipleDatabases < RuboCop::Cop::Cop
].freeze
def_node_matcher :active_record_base_method_is_used?, <<~PATTERN
- (send (const (const nil? :ActiveRecord) :Base) $_)
+ (send (const (const _ :ActiveRecord) :Base) $_)
PATTERN
def on_send(node)
diff --git a/spec/rubocop/cop/database/multiple_databases_spec.rb b/spec/rubocop/cop/database/multiple_databases_spec.rb
index 8bcd4710305ad..6ee1e7b13ca81 100644
--- a/spec/rubocop/cop/database/multiple_databases_spec.rb
+++ b/spec/rubocop/cop/database/multiple_databases_spec.rb
@@ -13,6 +13,13 @@
SOURCE
end
+ it 'flags the use of ::ActiveRecord::Base.connection' do
+ expect_offense(<<~SOURCE)
+ ::ActiveRecord::Base.connection.inspect
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use methods from ActiveRecord::Base, [...]
+ SOURCE
+ end
+
described_class::ALLOWED_METHODS.each do |method_name|
it "does not flag use of ActiveRecord::Base.#{method_name}" do
expect_no_offenses(<<~SOURCE)
--
GitLab