diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-15 18:10:57 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-15 18:10:57 +0300 |
commit | e5c7d631a84940c66e46f1824ba7ce0c7f1d0ea4 (patch) | |
tree | ac1cce129d8c8cdcee6a668e27d8034d2da172e4 /spec/lib/gitlab | |
parent | 0687020509cafe1e24eb0bfa0e0f5c9f6c4799d2 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/lib/gitlab')
23 files changed, 841 insertions, 441 deletions
diff --git a/spec/lib/gitlab/background_migration/job_coordinator_spec.rb b/spec/lib/gitlab/background_migration/job_coordinator_spec.rb index 8b5fb03820f..5e029f304c9 100644 --- a/spec/lib/gitlab/background_migration/job_coordinator_spec.rb +++ b/spec/lib/gitlab/background_migration/job_coordinator_spec.rb @@ -42,7 +42,7 @@ RSpec.describe Gitlab::BackgroundMigration::JobCoordinator do describe '#with_shared_connection' do it 'yields to the block after properly configuring SharedModel' do expect(Gitlab::Database::SharedModel).to receive(:using_connection) - .with(Gitlab::Database.main.scope.connection).and_yield + .with(ActiveRecord::Base.connection).and_yield expect { |b| coordinator.with_shared_connection(&b) }.to yield_with_no_args end diff --git a/spec/lib/gitlab/ci/config/entry/processable_spec.rb b/spec/lib/gitlab/ci/config/entry/processable_spec.rb index b872f6644a2..c9c28e2eb8b 100644 --- a/spec/lib/gitlab/ci/config/entry/processable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/processable_spec.rb @@ -33,6 +33,14 @@ RSpec.describe Gitlab::Ci::Config::Entry::Processable do end end + context 'when job name is more than 255' do + let(:entry) { node_class.new(config, name: ('a' * 256).to_sym) } + + it 'shows a validation error' do + expect(entry.errors).to include "job name is too long (maximum is 255 characters)" + end + end + context 'when job name is empty' do let(:entry) { node_class.new(config, name: ''.to_sym) } diff --git a/spec/lib/gitlab/config_checker/external_database_checker_spec.rb b/spec/lib/gitlab/config_checker/external_database_checker_spec.rb index 5a4e9001ac9..933b6d6be9e 100644 --- a/spec/lib/gitlab/config_checker/external_database_checker_spec.rb +++ b/spec/lib/gitlab/config_checker/external_database_checker_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Gitlab::ConfigChecker::ExternalDatabaseChecker do context 'when database meets minimum supported version' do before do - allow(Gitlab::Database.main).to receive(:postgresql_minimum_supported_version?).and_return(true) + allow(ApplicationRecord.database).to receive(:postgresql_minimum_supported_version?).and_return(true) end it { is_expected.to be_empty } @@ -16,7 +16,7 @@ RSpec.describe Gitlab::ConfigChecker::ExternalDatabaseChecker do context 'when database does not meet minimum supported version' do before do - allow(Gitlab::Database.main).to receive(:postgresql_minimum_supported_version?).and_return(false) + allow(ApplicationRecord.database).to receive(:postgresql_minimum_supported_version?).and_return(false) end let(:notice_deprecated_database) do @@ -26,7 +26,7 @@ RSpec.describe Gitlab::ConfigChecker::ExternalDatabaseChecker do '%{pg_version_minimum} is required for this version of GitLab. ' \ 'Please upgrade your environment to a supported PostgreSQL version, ' \ 'see %{pg_requirements_url} for details.') % { - pg_version_current: Gitlab::Database.main.version, + pg_version_current: ApplicationRecord.database.version, pg_version_minimum: Gitlab::Database::MINIMUM_POSTGRES_VERSION, pg_requirements_url: '<a href="https://docs.gitlab.com/ee/install/requirements.html#database">database requirements</a>' } diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index da13bc425d1..9831510f014 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Gitlab::Database::BatchCount do end before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(in_transaction) + allow(model.connection).to receive(:transaction_open?).and_return(in_transaction) end def calculate_batch_size(batch_size) diff --git a/spec/lib/gitlab/database/connection_spec.rb b/spec/lib/gitlab/database/connection_spec.rb deleted file mode 100644 index 32a262964db..00000000000 --- a/spec/lib/gitlab/database/connection_spec.rb +++ /dev/null @@ -1,384 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::Connection do - let(:connection) { described_class.new } - - describe '#config' do - it 'returns a HashWithIndifferentAccess' do - expect(connection.config).to be_an_instance_of(HashWithIndifferentAccess) - end - - it 'returns a default pool size' do - expect(connection.config) - .to include(pool: Gitlab::Database.default_pool_size) - end - - it 'does not cache its results' do - a = connection.config - b = connection.config - - expect(a).not_to equal(b) - end - end - - describe '#pool_size' do - context 'when no explicit size is configured' do - it 'returns the default pool size' do - expect(connection).to receive(:config).and_return({ pool: nil }) - - expect(connection.pool_size).to eq(Gitlab::Database.default_pool_size) - end - end - - context 'when an explicit pool size is set' do - it 'returns the pool size' do - expect(connection).to receive(:config).and_return({ pool: 4 }) - - expect(connection.pool_size).to eq(4) - end - end - end - - describe '#username' do - context 'when a username is set' do - it 'returns the username' do - allow(connection).to receive(:config).and_return(username: 'bob') - - expect(connection.username).to eq('bob') - end - end - - context 'when a username is not set' do - it 'returns the value of the USER environment variable' do - allow(connection).to receive(:config).and_return(username: nil) - allow(ENV).to receive(:[]).with('USER').and_return('bob') - - expect(connection.username).to eq('bob') - end - end - end - - describe '#database_name' do - it 'returns the name of the database' do - allow(connection).to receive(:config).and_return(database: 'test') - - expect(connection.database_name).to eq('test') - end - end - - describe '#adapter_name' do - it 'returns the database adapter name' do - allow(connection).to receive(:config).and_return(adapter: 'test') - - expect(connection.adapter_name).to eq('test') - end - end - - describe '#human_adapter_name' do - context 'when the adapter is PostgreSQL' do - it 'returns PostgreSQL' do - allow(connection).to receive(:config).and_return(adapter: 'postgresql') - - expect(connection.human_adapter_name).to eq('PostgreSQL') - end - end - - context 'when the adapter is not PostgreSQL' do - it 'returns Unknown' do - allow(connection).to receive(:config).and_return(adapter: 'kittens') - - expect(connection.human_adapter_name).to eq('Unknown') - end - end - end - - describe '#postgresql?' do - context 'when using PostgreSQL' do - it 'returns true' do - allow(connection).to receive(:adapter_name).and_return('PostgreSQL') - - expect(connection.postgresql?).to eq(true) - end - end - - context 'when not using PostgreSQL' do - it 'returns false' do - allow(connection).to receive(:adapter_name).and_return('MySQL') - - expect(connection.postgresql?).to eq(false) - end - end - end - - describe '#db_read_only?' do - it 'detects a read-only database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => "t" }]) - - expect(connection.db_read_only?).to be_truthy - end - - it 'detects a read-only database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => true }]) - - expect(connection.db_read_only?).to be_truthy - end - - it 'detects a read-write database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => "f" }]) - - expect(connection.db_read_only?).to be_falsey - end - - it 'detects a read-write database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => false }]) - - expect(connection.db_read_only?).to be_falsey - end - end - - describe '#db_read_write?' do - it 'detects a read-only database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => "t" }]) - - expect(connection.db_read_write?).to eq(false) - end - - it 'detects a read-only database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => true }]) - - expect(connection.db_read_write?).to eq(false) - end - - it 'detects a read-write database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => "f" }]) - - expect(connection.db_read_write?).to eq(true) - end - - it 'detects a read-write database' do - allow(connection.scope.connection) - .to receive(:execute) - .with('SELECT pg_is_in_recovery()') - .and_return([{ "pg_is_in_recovery" => false }]) - - expect(connection.db_read_write?).to eq(true) - end - end - - describe '#version' do - around do |example| - connection.instance_variable_set(:@version, nil) - example.run - connection.instance_variable_set(:@version, nil) - end - - context "on postgresql" do - it "extracts the version number" do - allow(connection) - .to receive(:database_version) - .and_return("PostgreSQL 9.4.4 on x86_64-apple-darwin14.3.0") - - expect(connection.version).to eq '9.4.4' - end - end - - it 'memoizes the result' do - count = ActiveRecord::QueryRecorder - .new { 2.times { connection.version } } - .count - - expect(count).to eq(1) - end - end - - describe '#postgresql_minimum_supported_version?' do - it 'returns false when using PostgreSQL 10' do - allow(connection).to receive(:version).and_return('10') - - expect(connection.postgresql_minimum_supported_version?).to eq(false) - end - - it 'returns false when using PostgreSQL 11' do - allow(connection).to receive(:version).and_return('11') - - expect(connection.postgresql_minimum_supported_version?).to eq(false) - end - - it 'returns true when using PostgreSQL 12' do - allow(connection).to receive(:version).and_return('12') - - expect(connection.postgresql_minimum_supported_version?).to eq(true) - end - end - - describe '#bulk_insert' do - before do - allow(connection).to receive(:connection).and_return(dummy_connection) - allow(dummy_connection).to receive(:quote_column_name, &:itself) - allow(dummy_connection).to receive(:quote, &:itself) - allow(dummy_connection).to receive(:execute) - end - - let(:dummy_connection) { double(:connection) } - - let(:rows) do - [ - { a: 1, b: 2, c: 3 }, - { c: 6, a: 4, b: 5 } - ] - end - - it 'does nothing with empty rows' do - expect(dummy_connection).not_to receive(:execute) - - connection.bulk_insert('test', []) - end - - it 'uses the ordering from the first row' do - expect(dummy_connection).to receive(:execute) do |sql| - expect(sql).to include('(1, 2, 3)') - expect(sql).to include('(4, 5, 6)') - end - - connection.bulk_insert('test', rows) - end - - it 'quotes column names' do - expect(dummy_connection).to receive(:quote_column_name).with(:a) - expect(dummy_connection).to receive(:quote_column_name).with(:b) - expect(dummy_connection).to receive(:quote_column_name).with(:c) - - connection.bulk_insert('test', rows) - end - - it 'quotes values' do - 1.upto(6) do |i| - expect(dummy_connection).to receive(:quote).with(i) - end - - connection.bulk_insert('test', rows) - end - - it 'does not quote values of a column in the disable_quote option' do - [1, 2, 4, 5].each do |i| - expect(dummy_connection).to receive(:quote).with(i) - end - - connection.bulk_insert('test', rows, disable_quote: :c) - end - - it 'does not quote values of columns in the disable_quote option' do - [2, 5].each do |i| - expect(dummy_connection).to receive(:quote).with(i) - end - - connection.bulk_insert('test', rows, disable_quote: [:a, :c]) - end - - it 'handles non-UTF-8 data' do - expect { connection.bulk_insert('test', [{ a: "\255" }]) }.not_to raise_error - end - - context 'when using PostgreSQL' do - it 'allows the returning of the IDs of the inserted rows' do - result = double(:result, values: [['10']]) - - expect(dummy_connection) - .to receive(:execute) - .with(/RETURNING id/) - .and_return(result) - - ids = connection - .bulk_insert('test', [{ number: 10 }], return_ids: true) - - expect(ids).to eq([10]) - end - - it 'allows setting the upsert to do nothing' do - expect(dummy_connection) - .to receive(:execute) - .with(/ON CONFLICT DO NOTHING/) - - connection - .bulk_insert('test', [{ number: 10 }], on_conflict: :do_nothing) - end - end - end - - describe '#cached_column_exists?' do - it 'only retrieves the data from the schema cache' do - queries = ActiveRecord::QueryRecorder.new do - 2.times do - expect(connection.cached_column_exists?(:projects, :id)).to be_truthy - expect(connection.cached_column_exists?(:projects, :bogus_column)).to be_falsey - end - end - - expect(queries.count).to eq(0) - end - end - - describe '#cached_table_exists?' do - it 'only retrieves the data from the schema cache' do - queries = ActiveRecord::QueryRecorder.new do - 2.times do - expect(connection.cached_table_exists?(:projects)).to be_truthy - expect(connection.cached_table_exists?(:bogus_table_name)).to be_falsey - end - end - - expect(queries.count).to eq(0) - end - - it 'returns false when database does not exist' do - expect(connection.scope).to receive(:connection) do - raise ActiveRecord::NoDatabaseError, 'broken' - end - - expect(connection.cached_table_exists?(:projects)).to be(false) - end - end - - describe '#exists?' do - it 'returns true if the database exists' do - expect(connection.exists?).to be(true) - end - - it "returns false if the database doesn't exist" do - expect(connection.scope.connection.schema_cache) - .to receive(:database_version) - .and_raise(ActiveRecord::NoDatabaseError) - - expect(connection.exists?).to be(false) - end - end - - describe '#system_id' do - it 'returns the PostgreSQL system identifier' do - expect(connection.system_id).to be_an_instance_of(Integer) - end - end -end diff --git a/spec/lib/gitlab/database/each_database_spec.rb b/spec/lib/gitlab/database/each_database_spec.rb index 8523249d804..9327fc4ff78 100644 --- a/spec/lib/gitlab/database/each_database_spec.rb +++ b/spec/lib/gitlab/database/each_database_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' RSpec.describe Gitlab::Database::EachDatabase do - describe '.each_database' do + describe '.each_database_connection' do let(:expected_connections) do - Gitlab::Database.databases.map { |name, wrapper| [wrapper.scope.connection, name] } + Gitlab::Database.database_base_models.map { |name, model| [model.connection, name] } end it 'yields each connection after connecting SharedModel' do diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb index 957a2da7f9c..37b83729125 100644 --- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb @@ -542,4 +542,17 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do end end end + + describe '#wal_diff' do + it 'returns the diff between two write locations' do + loc1 = lb.send(:get_write_location, lb.pool.connection) + + create(:user) # This ensures we get a new WAL location + + loc2 = lb.send(:get_write_location, lb.pool.connection) + diff = lb.wal_diff(loc2, loc1) + + expect(diff).to be_positive + end + end end diff --git a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb index 7c4cfcfb3a9..1c6f5c5c694 100644 --- a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb +++ b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb @@ -195,7 +195,7 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager do end # Postgres 11 does not support foreign keys to partitioned tables - if Gitlab::Database.main.version.to_f >= 12 + if ApplicationRecord.database.version.to_f >= 12 context 'when the model is the target of a foreign key' do before do connection.execute(<<~SQL) diff --git a/spec/lib/gitlab/database/postgres_hll/batch_distinct_counter_spec.rb b/spec/lib/gitlab/database/postgres_hll/batch_distinct_counter_spec.rb index 2c550f14a08..c9bbc32e059 100644 --- a/spec/lib/gitlab/database/postgres_hll/batch_distinct_counter_spec.rb +++ b/spec/lib/gitlab/database/postgres_hll/batch_distinct_counter_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Gitlab::Database::PostgresHll::BatchDistinctCounter do end before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(in_transaction) + allow(model.connection).to receive(:transaction_open?).and_return(in_transaction) end context 'unit test for different counting parameters' do diff --git a/spec/lib/gitlab/database/reflection_spec.rb b/spec/lib/gitlab/database/reflection_spec.rb new file mode 100644 index 00000000000..7c3d797817d --- /dev/null +++ b/spec/lib/gitlab/database/reflection_spec.rb @@ -0,0 +1,280 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Reflection do + let(:database) { described_class.new(ApplicationRecord) } + + describe '#username' do + context 'when a username is set' do + it 'returns the username' do + allow(database).to receive(:config).and_return(username: 'bob') + + expect(database.username).to eq('bob') + end + end + + context 'when a username is not set' do + it 'returns the value of the USER environment variable' do + allow(database).to receive(:config).and_return(username: nil) + allow(ENV).to receive(:[]).with('USER').and_return('bob') + + expect(database.username).to eq('bob') + end + end + end + + describe '#database_name' do + it 'returns the name of the database' do + allow(database).to receive(:config).and_return(database: 'test') + + expect(database.database_name).to eq('test') + end + end + + describe '#adapter_name' do + it 'returns the database adapter name' do + allow(database).to receive(:config).and_return(adapter: 'test') + + expect(database.adapter_name).to eq('test') + end + end + + describe '#human_adapter_name' do + context 'when the adapter is PostgreSQL' do + it 'returns PostgreSQL' do + allow(database).to receive(:config).and_return(adapter: 'postgresql') + + expect(database.human_adapter_name).to eq('PostgreSQL') + end + end + + context 'when the adapter is not PostgreSQL' do + it 'returns Unknown' do + allow(database).to receive(:config).and_return(adapter: 'kittens') + + expect(database.human_adapter_name).to eq('Unknown') + end + end + end + + describe '#postgresql?' do + context 'when using PostgreSQL' do + it 'returns true' do + allow(database).to receive(:adapter_name).and_return('PostgreSQL') + + expect(database.postgresql?).to eq(true) + end + end + + context 'when not using PostgreSQL' do + it 'returns false' do + allow(database).to receive(:adapter_name).and_return('MySQL') + + expect(database.postgresql?).to eq(false) + end + end + end + + describe '#db_read_only?' do + it 'detects a read-only database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => "t" }]) + + expect(database.db_read_only?).to be_truthy + end + + it 'detects a read-only database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => true }]) + + expect(database.db_read_only?).to be_truthy + end + + it 'detects a read-write database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => "f" }]) + + expect(database.db_read_only?).to be_falsey + end + + it 'detects a read-write database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => false }]) + + expect(database.db_read_only?).to be_falsey + end + end + + describe '#db_read_write?' do + it 'detects a read-only database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => "t" }]) + + expect(database.db_read_write?).to eq(false) + end + + it 'detects a read-only database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => true }]) + + expect(database.db_read_write?).to eq(false) + end + + it 'detects a read-write database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => "f" }]) + + expect(database.db_read_write?).to eq(true) + end + + it 'detects a read-write database' do + allow(database.model.connection) + .to receive(:execute) + .with('SELECT pg_is_in_recovery()') + .and_return([{ "pg_is_in_recovery" => false }]) + + expect(database.db_read_write?).to eq(true) + end + end + + describe '#version' do + around do |example| + database.instance_variable_set(:@version, nil) + example.run + database.instance_variable_set(:@version, nil) + end + + context "on postgresql" do + it "extracts the version number" do + allow(database) + .to receive(:database_version) + .and_return("PostgreSQL 9.4.4 on x86_64-apple-darwin14.3.0") + + expect(database.version).to eq '9.4.4' + end + end + + it 'memoizes the result' do + count = ActiveRecord::QueryRecorder + .new { 2.times { database.version } } + .count + + expect(count).to eq(1) + end + end + + describe '#postgresql_minimum_supported_version?' do + it 'returns false when using PostgreSQL 10' do + allow(database).to receive(:version).and_return('10') + + expect(database.postgresql_minimum_supported_version?).to eq(false) + end + + it 'returns false when using PostgreSQL 11' do + allow(database).to receive(:version).and_return('11') + + expect(database.postgresql_minimum_supported_version?).to eq(false) + end + + it 'returns true when using PostgreSQL 12' do + allow(database).to receive(:version).and_return('12') + + expect(database.postgresql_minimum_supported_version?).to eq(true) + end + end + + describe '#cached_column_exists?' do + it 'only retrieves the data from the schema cache' do + database = described_class.new(Project) + queries = ActiveRecord::QueryRecorder.new do + 2.times do + expect(database.cached_column_exists?(:id)).to be_truthy + expect(database.cached_column_exists?(:bogus_column)).to be_falsey + end + end + + expect(queries.count).to eq(0) + end + end + + describe '#cached_table_exists?' do + it 'only retrieves the data from the schema cache' do + dummy = Class.new(ActiveRecord::Base) do + self.table_name = 'bogus_table_name' + end + + queries = ActiveRecord::QueryRecorder.new do + 2.times do + expect(described_class.new(Project).cached_table_exists?).to be_truthy + expect(described_class.new(dummy).cached_table_exists?).to be_falsey + end + end + + expect(queries.count).to eq(0) + end + + it 'returns false when database does not exist' do + database = described_class.new(Project) + + expect(database.model).to receive(:connection) do + raise ActiveRecord::NoDatabaseError, 'broken' + end + + expect(database.cached_table_exists?).to be(false) + end + end + + describe '#exists?' do + it 'returns true if the database exists' do + expect(database.exists?).to be(true) + end + + it "returns false if the database doesn't exist" do + expect(database.model.connection.schema_cache) + .to receive(:database_version) + .and_raise(ActiveRecord::NoDatabaseError) + + expect(database.exists?).to be(false) + end + end + + describe '#system_id' do + it 'returns the PostgreSQL system identifier' do + expect(database.system_id).to be_an_instance_of(Integer) + end + end + + describe '#config' do + it 'returns a HashWithIndifferentAccess' do + expect(database.config) + .to be_an_instance_of(HashWithIndifferentAccess) + end + + it 'returns a default pool size' do + expect(database.config) + .to include(pool: Gitlab::Database.default_pool_size) + end + + it 'does not cache its results' do + a = database.config + b = database.config + + expect(a).not_to equal(b) + end + end +end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 1ab08314a39..5ec7c338a2a 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -15,13 +15,6 @@ RSpec.describe Gitlab::Database do end end - describe '.databases' do - it 'stores connections as a HashWithIndifferentAccess' do - expect(described_class.databases.has_key?('main')).to be true - expect(described_class.databases.has_key?(:main)).to be true - end - end - describe '.default_pool_size' do before do allow(Gitlab::Runtime).to receive(:max_threads).and_return(7) @@ -112,18 +105,30 @@ RSpec.describe Gitlab::Database do end describe '.check_postgres_version_and_print_warning' do + let(:reflect) { instance_spy(Gitlab::Database::Reflection) } + subject { described_class.check_postgres_version_and_print_warning } + before do + allow(Gitlab::Database::Reflection) + .to receive(:new) + .and_return(reflect) + end + it 'prints a warning if not compliant with minimum postgres version' do - allow(described_class.main).to receive(:postgresql_minimum_supported_version?).and_return(false) + allow(reflect).to receive(:postgresql_minimum_supported_version?).and_return(false) - expect(Kernel).to receive(:warn).with(/You are using PostgreSQL/) + expect(Kernel) + .to receive(:warn) + .with(/You are using PostgreSQL/) + .exactly(Gitlab::Database.database_base_models.length) + .times subject end it 'doesnt print a warning if compliant with minimum postgres version' do - allow(described_class.main).to receive(:postgresql_minimum_supported_version?).and_return(true) + allow(reflect).to receive(:postgresql_minimum_supported_version?).and_return(true) expect(Kernel).not_to receive(:warn).with(/You are using PostgreSQL/) @@ -131,7 +136,7 @@ RSpec.describe Gitlab::Database do end it 'doesnt print a warning in Rails runner environment' do - allow(described_class.main).to receive(:postgresql_minimum_supported_version?).and_return(false) + allow(reflect).to receive(:postgresql_minimum_supported_version?).and_return(false) allow(Gitlab::Runtime).to receive(:rails_runner?).and_return(true) expect(Kernel).not_to receive(:warn).with(/You are using PostgreSQL/) @@ -140,13 +145,13 @@ RSpec.describe Gitlab::Database do end it 'ignores ActiveRecord errors' do - allow(described_class.main).to receive(:postgresql_minimum_supported_version?).and_raise(ActiveRecord::ActiveRecordError) + allow(reflect).to receive(:postgresql_minimum_supported_version?).and_raise(ActiveRecord::ActiveRecordError) expect { subject }.not_to raise_error end it 'ignores Postgres errors' do - allow(described_class.main).to receive(:postgresql_minimum_supported_version?).and_raise(PG::Error) + allow(reflect).to receive(:postgresql_minimum_supported_version?).and_raise(PG::Error) expect { subject }.not_to raise_error end diff --git a/spec/lib/gitlab/github_import/bulk_importing_spec.rb b/spec/lib/gitlab/github_import/bulk_importing_spec.rb index 6c94973b5a8..e170496ff7b 100644 --- a/spec/lib/gitlab/github_import/bulk_importing_spec.rb +++ b/spec/lib/gitlab/github_import/bulk_importing_spec.rb @@ -116,13 +116,13 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do value: 5 ) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .ordered .with('kittens', rows.first(5)) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .ordered .with('kittens', rows.last(5)) diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb index b671b126851..0448ada6bca 100644 --- a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb @@ -82,8 +82,8 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail it 'does not import the note when a foreign key error is raised' do stub_user_finder(project.creator_id, false) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') expect { subject.execute } @@ -94,6 +94,8 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail describe '#execute' do context 'when the merge request no longer exists' do it 'does not import anything' do + expect(ApplicationRecord).not_to receive(:legacy_bulk_insert) + expect { subject.execute } .to not_change(DiffNote, :count) .and not_change(LegacyDiffNote, :count) diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb index 0926000428c..4287c32b947 100644 --- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb @@ -190,8 +190,8 @@ RSpec.describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redi .with(issue.assignees[1]) .and_return(5) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with( IssueAssignee.table_name, [{ issue_id: 1, user_id: 4 }, { issue_id: 1, user_id: 5 }] diff --git a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb index 241a0fef600..e68849755b2 100644 --- a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb @@ -39,8 +39,8 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do .and_return(1) freeze_time do - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with( LabelLink.table_name, [ @@ -64,8 +64,8 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do .with('bug') .and_return(nil) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with(LabelLink.table_name, []) importer.create_labels diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb index 820f46c7286..96d8acbd3de 100644 --- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb @@ -41,8 +41,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .with(github_note) .and_return([user.id, true]) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with( Note.table_name, [ @@ -71,8 +71,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .with(github_note) .and_return([project.creator_id, false]) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with( Note.table_name, [ @@ -115,7 +115,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do context 'when the noteable does not exist' do it 'does not import the note' do - expect(Gitlab::Database.main).not_to receive(:bulk_insert) + expect(ApplicationRecord).not_to receive(:legacy_bulk_insert) importer.execute end @@ -134,8 +134,8 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .with(github_note) .and_return([user.id, true]) - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') expect { importer.execute }.not_to raise_error diff --git a/spec/lib/gitlab/import/database_helpers_spec.rb b/spec/lib/gitlab/import/database_helpers_spec.rb index 079faed2518..05d1c0ae078 100644 --- a/spec/lib/gitlab/import/database_helpers_spec.rb +++ b/spec/lib/gitlab/import/database_helpers_spec.rb @@ -16,8 +16,8 @@ RSpec.describe Gitlab::Import::DatabaseHelpers do let(:project) { create(:project) } it 'returns the ID returned by the query' do - expect(Gitlab::Database.main) - .to receive(:bulk_insert) + expect(ApplicationRecord) + .to receive(:legacy_bulk_insert) .with(Issue.table_name, [attributes], return_ids: true) .and_return([10]) diff --git a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb index 7dda10ab41d..e97a4fdddcb 100644 --- a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb @@ -18,8 +18,8 @@ RSpec.describe Gitlab::Metrics::Samplers::DatabaseSampler do let(:labels) do { class: 'ActiveRecord::Base', - host: Gitlab::Database.main.config['host'], - port: Gitlab::Database.main.config['port'] + host: ApplicationRecord.database.config['host'], + port: ApplicationRecord.database.config['port'] } end diff --git a/spec/lib/gitlab/redis/multi_store_spec.rb b/spec/lib/gitlab/redis/multi_store_spec.rb new file mode 100644 index 00000000000..bf1bf65bb9b --- /dev/null +++ b/spec/lib/gitlab/redis/multi_store_spec.rb @@ -0,0 +1,474 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Redis::MultiStore do + using RSpec::Parameterized::TableSyntax + + let_it_be(:redis_store_class) do + Class.new(Gitlab::Redis::Wrapper) do + def config_file_name + config_file_name = "spec/fixtures/config/redis_new_format_host.yml" + Rails.root.join(config_file_name).to_s + end + + def self.name + 'Sessions' + end + end + end + + let_it_be(:primary_db) { 1 } + let_it_be(:secondary_db) { 2 } + let_it_be(:primary_store) { create_redis_store(redis_store_class.params, db: primary_db, serializer: nil) } + let_it_be(:secondary_store) { create_redis_store(redis_store_class.params, db: secondary_db, serializer: nil) } + let_it_be(:instance_name) { 'TestStore' } + let_it_be(:multi_store) { described_class.new(primary_store, secondary_store, instance_name)} + + subject { multi_store.send(name, *args) } + + after(:all) do + primary_store.flushdb + secondary_store.flushdb + end + + context 'when primary_store is nil' do + let(:multi_store) { described_class.new(nil, secondary_store, instance_name)} + + it 'fails with exception' do + expect { multi_store }.to raise_error(ArgumentError, /primary_store is required/) + end + end + + context 'when secondary_store is nil' do + let(:multi_store) { described_class.new(primary_store, nil, instance_name)} + + it 'fails with exception' do + expect { multi_store }.to raise_error(ArgumentError, /secondary_store is required/) + end + end + + context 'when primary_store is not a ::Redis instance' do + before do + allow(primary_store).to receive(:is_a?).with(::Redis).and_return(false) + end + + it 'fails with exception' do + expect { described_class.new(primary_store, secondary_store, instance_name) }.to raise_error(ArgumentError, /invalid primary_store/) + end + end + + context 'when secondary_store is not a ::Redis instance' do + before do + allow(secondary_store).to receive(:is_a?).with(::Redis).and_return(false) + end + + it 'fails with exception' do + expect { described_class.new(primary_store, secondary_store, instance_name) }.to raise_error(ArgumentError, /invalid secondary_store/) + end + end + + context 'with READ redis commands' do + let_it_be(:key1) { "redis:{1}:key_a" } + let_it_be(:key2) { "redis:{1}:key_b" } + let_it_be(:value1) { "redis_value1"} + let_it_be(:value2) { "redis_value2"} + let_it_be(:skey) { "redis:set:key" } + let_it_be(:keys) { [key1, key2] } + let_it_be(:values) { [value1, value2] } + let_it_be(:svalues) { [value2, value1] } + + where(:case_name, :name, :args, :value, :block) do + 'execute :get command' | :get | ref(:key1) | ref(:value1) | nil + 'execute :mget command' | :mget | ref(:keys) | ref(:values) | nil + 'execute :mget with block' | :mget | ref(:keys) | ref(:values) | ->(value) { value } + 'execute :smembers command' | :smembers | ref(:skey) | ref(:svalues) | nil + 'execute :scard command' | :scard | ref(:skey) | 2 | nil + end + + before(:all) do + primary_store.multi do |multi| + multi.set(key1, value1) + multi.set(key2, value2) + multi.sadd(skey, value1) + multi.sadd(skey, value2) + end + + secondary_store.multi do |multi| + multi.set(key1, value1) + multi.set(key2, value2) + multi.sadd(skey, value1) + multi.sadd(skey, value2) + end + end + + RSpec.shared_examples_for 'reads correct value' do + it 'returns the correct value' do + if value.is_a?(Array) + # :smembers does not guarantee the order it will return the values (unsorted set) + is_expected.to match_array(value) + else + is_expected.to eq(value) + end + end + end + + RSpec.shared_examples_for 'fallback read from the secondary store' do + it 'fallback and execute on secondary instance' do + expect(secondary_store).to receive(name).with(*args).and_call_original + + subject + end + + it 'logs the ReadFromPrimaryError' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(Gitlab::Redis::MultiStore::ReadFromPrimaryError), + hash_including(command_name: name, extra: hash_including(instance_name: instance_name))) + + subject + end + + it 'increment read fallback count metrics' do + expect(multi_store).to receive(:increment_read_fallback_count).with(name) + + subject + end + + include_examples 'reads correct value' + + context 'when fallback read from the secondary instance raises an exception' do + before do + allow(secondary_store).to receive(name).with(*args).and_raise(StandardError) + end + + it 'fails with exception' do + expect { subject }.to raise_error(StandardError) + end + end + end + + RSpec.shared_examples_for 'secondary store' do + it 'execute on the secondary instance' do + expect(secondary_store).to receive(name).with(*args).and_call_original + + subject + end + + include_examples 'reads correct value' + + it 'does not execute on the primary store' do + expect(primary_store).not_to receive(name) + + subject + end + end + + with_them do + describe "#{name}" do + before do + allow(primary_store).to receive(name).and_call_original + allow(secondary_store).to receive(name).and_call_original + end + + context 'with feature flag :use_multi_store enabled' do + before do + stub_feature_flags(use_multi_store: true) + end + + context 'when reading from the primary is successful' do + it 'returns the correct value' do + expect(primary_store).to receive(name).with(*args).and_call_original + + subject + end + + it 'does not execute on the secondary store' do + expect(secondary_store).not_to receive(name) + + subject + end + + include_examples 'reads correct value' + end + + context 'when reading from primary instance is raising an exception' do + before do + allow(primary_store).to receive(name).with(*args).and_raise(StandardError) + allow(Gitlab::ErrorTracking).to receive(:log_exception) + end + + it 'logs the exception' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), + hash_including(extra: hash_including(:multi_store_error_message, instance_name: instance_name), + command_name: name)) + + subject + end + + include_examples 'fallback read from the secondary store' + end + + context 'when reading from primary instance return no value' do + before do + allow(primary_store).to receive(name).and_return(nil) + end + + include_examples 'fallback read from the secondary store' + end + + context 'when the command is executed within pipelined block' do + subject do + multi_store.pipelined do + multi_store.send(name, *args) + end + end + + it 'is executed only 1 time on primary instance' do + expect(primary_store).to receive(name).with(*args).once + + subject + end + end + + if params[:block] + subject do + multi_store.send(name, *args, &block) + end + + context 'when block is provided' do + it 'yields to the block' do + expect(primary_store).to receive(name).and_yield(value) + + subject + end + + include_examples 'reads correct value' + end + end + end + + context 'with feature flag :use_multi_store is disabled' do + before do + stub_feature_flags(use_multi_store: false) + end + + it_behaves_like 'secondary store' + end + + context 'with both primary and secondary store using same redis instance' do + let(:primary_store) { create_redis_store(redis_store_class.params, db: primary_db, serializer: nil) } + let(:secondary_store) { create_redis_store(redis_store_class.params, db: primary_db, serializer: nil) } + let(:multi_store) { described_class.new(primary_store, secondary_store, instance_name)} + + it_behaves_like 'secondary store' + end + end + end + end + + context 'with WRITE redis commands' do + let_it_be(:key1) { "redis:{1}:key_a" } + let_it_be(:key2) { "redis:{1}:key_b" } + let_it_be(:value1) { "redis_value1"} + let_it_be(:value2) { "redis_value2"} + let_it_be(:key1_value1) { [key1, value1] } + let_it_be(:key1_value2) { [key1, value2] } + let_it_be(:ttl) { 10 } + let_it_be(:key1_ttl_value1) { [key1, ttl, value1] } + let_it_be(:skey) { "redis:set:key" } + let_it_be(:svalues1) { [value2, value1] } + let_it_be(:svalues2) { [value1] } + let_it_be(:skey_value1) { [skey, value1] } + let_it_be(:skey_value2) { [skey, value2] } + + where(:case_name, :name, :args, :expected_value, :verification_name, :verification_args) do + 'execute :set command' | :set | ref(:key1_value1) | ref(:value1) | :get | ref(:key1) + 'execute :setnx command' | :setnx | ref(:key1_value2) | ref(:value1) | :get | ref(:key2) + 'execute :setex command' | :setex | ref(:key1_ttl_value1) | ref(:ttl) | :ttl | ref(:key1) + 'execute :sadd command' | :sadd | ref(:skey_value2) | ref(:svalues1) | :smembers | ref(:skey) + 'execute :srem command' | :srem | ref(:skey_value1) | [] | :smembers | ref(:skey) + 'execute :del command' | :del | ref(:key2) | nil | :get | ref(:key2) + 'execute :flushdb command' | :flushdb | nil | 0 | :dbsize | nil + end + + before do + primary_store.flushdb + secondary_store.flushdb + + primary_store.multi do |multi| + multi.set(key2, value1) + multi.sadd(skey, value1) + end + + secondary_store.multi do |multi| + multi.set(key2, value1) + multi.sadd(skey, value1) + end + end + + RSpec.shared_examples_for 'verify that store contains values' do |store| + it "#{store} redis store contains correct values", :aggregate_errors do + subject + + redis_store = multi_store.send(store) + + if expected_value.is_a?(Array) + # :smembers does not guarantee the order it will return the values + expect(redis_store.send(verification_name, *verification_args)).to match_array(expected_value) + else + expect(redis_store.send(verification_name, *verification_args)).to eq(expected_value) + end + end + end + + with_them do + describe "#{name}" do + let(:expected_args) {args || no_args } + + before do + allow(primary_store).to receive(name).and_call_original + allow(secondary_store).to receive(name).and_call_original + end + + context 'with feature flag :use_multi_store enabled' do + before do + stub_feature_flags(use_multi_store: true) + end + + context 'when executing on primary instance is successful' do + it 'executes on both primary and secondary redis store', :aggregate_errors do + expect(primary_store).to receive(name).with(*expected_args).and_call_original + expect(secondary_store).to receive(name).with(*expected_args).and_call_original + + subject + end + + include_examples 'verify that store contains values', :primary_store + include_examples 'verify that store contains values', :secondary_store + end + + context 'when executing on the primary instance is raising an exception' do + before do + allow(primary_store).to receive(name).with(*expected_args).and_raise(StandardError) + allow(Gitlab::ErrorTracking).to receive(:log_exception) + end + + it 'logs the exception and execute on secondary instance', :aggregate_errors do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), + hash_including(extra: hash_including(:multi_store_error_message), command_name: name)) + expect(secondary_store).to receive(name).with(*expected_args).and_call_original + + subject + end + + include_examples 'verify that store contains values', :secondary_store + end + + context 'when the command is executed within pipelined block' do + subject do + multi_store.pipelined do + multi_store.send(name, *args) + end + end + + it 'is executed only 1 time on each instance', :aggregate_errors do + expect(primary_store).to receive(name).with(*expected_args).once + expect(secondary_store).to receive(name).with(*expected_args).once + + subject + end + + include_examples 'verify that store contains values', :primary_store + include_examples 'verify that store contains values', :secondary_store + end + end + + context 'with feature flag :use_multi_store is disabled' do + before do + stub_feature_flags(use_multi_store: false) + end + + it 'executes only on the secondary redis store', :aggregate_errors do + expect(secondary_store).to receive(name).with(*expected_args) + expect(primary_store).not_to receive(name).with(*expected_args) + + subject + end + + include_examples 'verify that store contains values', :secondary_store + end + end + end + end + + context 'with unsupported command' do + before do + primary_store.flushdb + secondary_store.flushdb + end + + let_it_be(:key) { "redis:counter" } + + subject do + multi_store.incr(key) + end + + it 'executes method missing' do + expect(multi_store).to receive(:method_missing) + + subject + end + + it 'logs MethodMissingError' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(Gitlab::Redis::MultiStore::MethodMissingError), + hash_including(command_name: :incr, extra: hash_including(instance_name: instance_name))) + + subject + end + + it 'increments method missing counter' do + expect(multi_store).to receive(:increment_method_missing_count).with(:incr) + + subject + end + + it 'fallback and executes only on the secondary store', :aggregate_errors do + expect(secondary_store).to receive(:incr).with(key).and_call_original + expect(primary_store).not_to receive(:incr) + + subject + end + + it 'correct value is stored on the secondary store', :aggregate_errors do + subject + + expect(primary_store.get(key)).to be_nil + expect(secondary_store.get(key)).to eq('1') + end + + context 'when the command is executed within pipelined block' do + subject do + multi_store.pipelined do + multi_store.incr(key) + end + end + + it 'is executed only 1 time on each instance', :aggregate_errors do + expect(primary_store).to receive(:incr).with(key).once + expect(secondary_store).to receive(:incr).with(key).once + + subject + end + + it "both redis stores are containing correct values", :aggregate_errors do + subject + + expect(primary_store.get(key)).to eq('1') + expect(secondary_store.get(key)).to eq('1') + end + end + end + + def create_redis_store(options, extras = {}) + ::Redis::Store.new(options.merge(extras)) + end +end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/generic_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/generic_metric_spec.rb index 158be34d39c..c8cb1bb4373 100644 --- a/spec/lib/gitlab/usage/metrics/instrumentations/generic_metric_spec.rb +++ b/spec/lib/gitlab/usage/metrics/instrumentations/generic_metric_spec.rb @@ -7,18 +7,18 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::GenericMetric do subject do Class.new(described_class) do fallback(custom_fallback) - value { Gitlab::Database.main.version } + value { ApplicationRecord.database.version } end.new(time_frame: 'none') end describe '#value' do it 'gives the correct value' do - expect(subject.value).to eq(Gitlab::Database.main.version) + expect(subject.value).to eq(ApplicationRecord.database.version) end context 'when raising an exception' do it 'return the custom fallback' do - expect(Gitlab::Database.main).to receive(:version).and_raise('Error') + expect(ApplicationRecord.database).to receive(:version).and_raise('Error') expect(subject.value).to eq(custom_fallback) end end @@ -28,18 +28,18 @@ RSpec.describe Gitlab::Usage::Metrics::Instrumentations::GenericMetric do context 'with default fallback' do subject do Class.new(described_class) do - value { Gitlab::Database.main.version } + value { ApplicationRecord.database.version } end.new(time_frame: 'none') end describe '#value' do it 'gives the correct value' do - expect(subject.value).to eq(Gitlab::Database.main.version ) + expect(subject.value).to eq(ApplicationRecord.database.version ) end context 'when raising an exception' do it 'return the default fallback' do - expect(Gitlab::Database.main).to receive(:version).and_raise('Error') + expect(ApplicationRecord.database).to receive(:version).and_raise('Error') expect(subject.value).to eq(described_class::FALLBACK) end end diff --git a/spec/lib/gitlab/usage_data_metrics_spec.rb b/spec/lib/gitlab/usage_data_metrics_spec.rb index c3bec5fff58..563eed75c38 100644 --- a/spec/lib/gitlab/usage_data_metrics_spec.rb +++ b/spec/lib/gitlab/usage_data_metrics_spec.rb @@ -13,7 +13,9 @@ RSpec.describe Gitlab::UsageDataMetrics do end before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) + allow_next_instance_of(Gitlab::Database::BatchCounter) do |batch_counter| + allow(batch_counter).to receive(:transaction_open?).and_return(false) + end end context 'with instrumentation_class' do diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 7cbf439affd..cf544c07195 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -978,9 +978,9 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do expect(subject[:gitlab_pages][:enabled]).to eq(Gitlab.config.pages.enabled) expect(subject[:gitlab_pages][:version]).to eq(Gitlab::Pages::VERSION) expect(subject[:git][:version]).to eq(Gitlab::Git.version) - expect(subject[:database][:adapter]).to eq(Gitlab::Database.main.adapter_name) - expect(subject[:database][:version]).to eq(Gitlab::Database.main.version) - expect(subject[:database][:pg_system_id]).to eq(Gitlab::Database.main.system_id) + expect(subject[:database][:adapter]).to eq(ApplicationRecord.database.adapter_name) + expect(subject[:database][:version]).to eq(ApplicationRecord.database.version) + expect(subject[:database][:pg_system_id]).to eq(ApplicationRecord.database.system_id) expect(subject[:mail][:smtp_server]).to eq(ActionMailer::Base.smtp_settings[:address]) expect(subject[:gitaly][:version]).to be_present expect(subject[:gitaly][:servers]).to be >= 1 diff --git a/spec/lib/gitlab/utils/usage_data_spec.rb b/spec/lib/gitlab/utils/usage_data_spec.rb index 4411b10cdfb..e721b28ac29 100644 --- a/spec/lib/gitlab/utils/usage_data_spec.rb +++ b/spec/lib/gitlab/utils/usage_data_spec.rb @@ -70,7 +70,7 @@ RSpec.describe Gitlab::Utils::UsageData do let(:relation) { double(:relation, connection: double(:connection)) } before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) # rubocop: disable Database/MultipleDatabases + allow(relation.connection).to receive(:transaction_open?).and_return(false) end it 'delegates counting to counter class instance' do @@ -122,7 +122,7 @@ RSpec.describe Gitlab::Utils::UsageData do let(:ci_builds_estimated_cardinality) { 2.0809220082170614 } before do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?).and_return(false) # rubocop: disable Database/MultipleDatabases + allow(model.connection).to receive(:transaction_open?).and_return(false) end context 'different counting parameters' do |