Commit b452b892 authored by dfrazao-gitlab's avatar dfrazao-gitlab

Fix problematic Database/MultipleDatabases offenses

- Fix places where we are ignoring the Database/MultipleDatabase rule.
Instead of using ActiveRecord::Base connection/transaction,
we are using the proper class.

Changelog: changed
Related to https://gitlab.com/gitlab-org/gitlab/-/issues/335808
EE: true
parent 96c19ed3
...@@ -246,7 +246,7 @@ class NotifyPreview < ActionMailer::Preview ...@@ -246,7 +246,7 @@ class NotifyPreview < ActionMailer::Preview
def cleanup def cleanup
email = nil email = nil
ActiveRecord::Base.transaction do # rubocop: disable Database/MultipleDatabases ApplicationRecord.transaction do
email = yield email = yield
raise ActiveRecord::Rollback raise ActiveRecord::Rollback
end end
......
...@@ -57,7 +57,7 @@ class InternalId < ApplicationRecord ...@@ -57,7 +57,7 @@ class InternalId < ApplicationRecord
self.internal_id_transactions_total.increment( self.internal_id_transactions_total.increment(
operation: operation, operation: operation,
usage: usage.to_s, usage: usage.to_s,
in_transaction: ActiveRecord::Base.connection.transaction_open?.to_s # rubocop: disable Database/MultipleDatabases in_transaction: InternalId.connection.transaction_open?.to_s
) )
end end
......
...@@ -6,7 +6,7 @@ module AutoMerge ...@@ -6,7 +6,7 @@ module AutoMerge
include MergeRequests::AssignsMergeParams include MergeRequests::AssignsMergeParams
def execute(merge_request) def execute(merge_request)
ActiveRecord::Base.transaction do # rubocop: disable Database/MultipleDatabases ApplicationRecord.transaction do
register_auto_merge_parameters!(merge_request) register_auto_merge_parameters!(merge_request)
yield if block_given? yield if block_given?
end end
...@@ -29,7 +29,7 @@ module AutoMerge ...@@ -29,7 +29,7 @@ module AutoMerge
end end
def cancel(merge_request) def cancel(merge_request)
ActiveRecord::Base.transaction do # rubocop: disable Database/MultipleDatabases ApplicationRecord.transaction do
clear_auto_merge_parameters!(merge_request) clear_auto_merge_parameters!(merge_request)
yield if block_given? yield if block_given?
end end
...@@ -41,7 +41,7 @@ module AutoMerge ...@@ -41,7 +41,7 @@ module AutoMerge
end end
def abort(merge_request, reason) def abort(merge_request, reason)
ActiveRecord::Base.transaction do # rubocop: disable Database/MultipleDatabases ApplicationRecord.transaction do
clear_auto_merge_parameters!(merge_request) clear_auto_merge_parameters!(merge_request)
yield if block_given? yield if block_given?
end end
......
...@@ -26,7 +26,7 @@ module Deployments ...@@ -26,7 +26,7 @@ module Deployments
end end
def update_environment(deployment) def update_environment(deployment)
ActiveRecord::Base.transaction do # rubocop: disable Database/MultipleDatabases ApplicationRecord.transaction do
# Renew attributes at update # Renew attributes at update
renew_external_url renew_external_url
renew_auto_stop_in renew_auto_stop_in
......
...@@ -99,8 +99,8 @@ module EE ...@@ -99,8 +99,8 @@ module EE
yield yield
end end
ensure # rubocop: disable Layout/RescueEnsureAlignment ensure
ActiveRecord::Base.clear_active_connections! # rubocop: disable Database/MultipleDatabases ApplicationRecord.clear_active_connections!
end end
end end
end end
......
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
# We _must not_ use quote_table_name as this will produce double # We _must not_ use quote_table_name as this will produce double
# quotes on PostgreSQL and for "has_table_privilege" we need single # quotes on PostgreSQL and for "has_table_privilege" we need single
# quotes. # quotes.
connection = ActiveRecord::Base.connection # rubocop: disable Database/MultipleDatabases connection = ApplicationRecord.connection
quoted_table = connection.quote(table) quoted_table = connection.quote(table)
begin begin
......
...@@ -104,11 +104,9 @@ module Gitlab ...@@ -104,11 +104,9 @@ module Gitlab
end end
end end
# rubocop:disable Database/MultipleDatabases
def connection def connection
use_model_load_balancing? ? super : ActiveRecord::Base.connection use_model_load_balancing? ? super : ApplicationRecord.connection
end end
# rubocop:enable Database/MultipleDatabases
end end
end end
end end
......
...@@ -62,7 +62,7 @@ RSpec.describe Gitlab::Metrics::Samplers::DatabaseSampler do ...@@ -62,7 +62,7 @@ RSpec.describe Gitlab::Metrics::Samplers::DatabaseSampler do
end end
context 'when replica hosts are configured' do context 'when replica hosts are configured' do
let(:main_load_balancer) { ActiveRecord::Base.load_balancer } # rubocop:disable Database/MultipleDatabases let(:main_load_balancer) { ApplicationRecord.load_balancer }
let(:main_replica_host) { main_load_balancer.host } let(:main_replica_host) { main_load_balancer.host }
let(:ci_load_balancer) { double(:load_balancer, host_list: ci_host_list, configuration: configuration) } let(:ci_load_balancer) { double(:load_balancer, host_list: ci_host_list, configuration: configuration) }
...@@ -117,7 +117,7 @@ RSpec.describe Gitlab::Metrics::Samplers::DatabaseSampler do ...@@ -117,7 +117,7 @@ RSpec.describe Gitlab::Metrics::Samplers::DatabaseSampler do
end end
context 'when the base model has replica connections' do context 'when the base model has replica connections' do
let(:main_load_balancer) { ActiveRecord::Base.load_balancer } # rubocop:disable Database/MultipleDatabases let(:main_load_balancer) { ApplicationRecord.load_balancer }
let(:main_replica_host) { main_load_balancer.host } let(:main_replica_host) { main_load_balancer.host }
let(:ci_load_balancer) { double(:load_balancer, host_list: ci_host_list, configuration: configuration) } let(:ci_load_balancer) { double(:load_balancer, host_list: ci_host_list, configuration: configuration) }
......
...@@ -147,22 +147,20 @@ RSpec.describe ApplicationRecord do ...@@ -147,22 +147,20 @@ RSpec.describe ApplicationRecord do
end end
end end
# rubocop:disable Database/MultipleDatabases
it 'increments a counter when a transaction is created in ActiveRecord' do it 'increments a counter when a transaction is created in ActiveRecord' do
expect(described_class.connection.transaction_open?).to be false expect(described_class.connection.transaction_open?).to be false
expect(::Gitlab::Database::Metrics) expect(::Gitlab::Database::Metrics)
.to receive(:subtransactions_increment) .to receive(:subtransactions_increment)
.with('ActiveRecord::Base') .with('ApplicationRecord')
.once .once
ActiveRecord::Base.transaction do ApplicationRecord.transaction do
ActiveRecord::Base.transaction(requires_new: true) do ApplicationRecord.transaction(requires_new: true) do
expect(ActiveRecord::Base.connection.transaction_open?).to be true expect(ApplicationRecord.connection.transaction_open?).to be true
end end
end end
end end
# rubocop:enable Database/MultipleDatabases
end end
describe '.with_fast_read_statement_timeout' do describe '.with_fast_read_statement_timeout' do
......
...@@ -87,7 +87,7 @@ RSpec.describe InternalId do ...@@ -87,7 +87,7 @@ RSpec.describe InternalId do
context 'when executed outside of transaction' do context 'when executed outside of transaction' do
it 'increments counter with in_transaction: "false"' do it 'increments counter with in_transaction: "false"' do
allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases allow(ApplicationRecord.connection).to receive(:transaction_open?) { false }
expect(InternalId.internal_id_transactions_total).to receive(:increment) expect(InternalId.internal_id_transactions_total).to receive(:increment)
.with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original
...@@ -146,7 +146,7 @@ RSpec.describe InternalId do ...@@ -146,7 +146,7 @@ RSpec.describe InternalId do
let(:value) { 2 } let(:value) { 2 }
it 'increments counter with in_transaction: "false"' do it 'increments counter with in_transaction: "false"' do
allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases allow(ApplicationRecord.connection).to receive(:transaction_open?) { false }
expect(InternalId.internal_id_transactions_total).to receive(:increment) expect(InternalId.internal_id_transactions_total).to receive(:increment)
.with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original
...@@ -217,7 +217,7 @@ RSpec.describe InternalId do ...@@ -217,7 +217,7 @@ RSpec.describe InternalId do
context 'when executed outside of transaction' do context 'when executed outside of transaction' do
it 'increments counter with in_transaction: "false"' do it 'increments counter with in_transaction: "false"' do
allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases allow(ApplicationRecord.connection).to receive(:transaction_open?) { false }
expect(InternalId.internal_id_transactions_total).to receive(:increment) expect(InternalId.internal_id_transactions_total).to receive(:increment)
.with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original
......
...@@ -42,7 +42,7 @@ RSpec.describe 'Database::MultipleDatabases' do ...@@ -42,7 +42,7 @@ RSpec.describe 'Database::MultipleDatabases' do
context 'when reconnect is true' do context 'when reconnect is true' do
it 'does not raise exception' do it 'does not raise exception' do
with_reestablished_active_record_base(reconnect: true) do with_reestablished_active_record_base(reconnect: true) do
expect { ActiveRecord::Base.connection.execute("SELECT 1") }.not_to raise_error # rubocop:disable Database/MultipleDatabases expect { ApplicationRecord.connection.execute("SELECT 1") }.not_to raise_error
end end
end end
end end
...@@ -50,7 +50,7 @@ RSpec.describe 'Database::MultipleDatabases' do ...@@ -50,7 +50,7 @@ RSpec.describe 'Database::MultipleDatabases' do
context 'when reconnect is false' do context 'when reconnect is false' do
it 'does raise exception' do it 'does raise exception' do
with_reestablished_active_record_base(reconnect: false) do with_reestablished_active_record_base(reconnect: false) do
expect { ActiveRecord::Base.connection.execute("SELECT 1") }.to raise_error(ActiveRecord::ConnectionNotEstablished) # rubocop:disable Database/MultipleDatabases expect { ApplicationRecord.connection.execute("SELECT 1") }.to raise_error(ActiveRecord::ConnectionNotEstablished)
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment