diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-03-08 12:12:26 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-03-08 12:12:26 +0300 |
commit | 3c050fb24b757425987a7df4cb3497e1d792be8e (patch) | |
tree | e4031413c679ecf3e3126da1d1a8cbc3084d8377 /spec | |
parent | 14ac28d7c7fcf1aba321f521f3a980b03b7b090e (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/factories/gitlab/database/async_foreign_keys/postgres_async_constraint_validation.rb (renamed from spec/factories/gitlab/database/async_foreign_keys/postgres_async_foreign_key_validation.rb) | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/database/async_constraints/validators/check_constraint_spec.rb | 20 | ||||
-rw-r--r-- | spec/lib/gitlab/database/async_constraints/validators/foreign_key_spec.rb | 35 | ||||
-rw-r--r-- | spec/lib/gitlab/database/async_constraints/validators_spec.rb | 21 | ||||
-rw-r--r-- | spec/support/shared_examples/lib/gitlab/database/async_constraints_validation_shared_examples.rb | 131 |
5 files changed, 215 insertions, 0 deletions
diff --git a/spec/factories/gitlab/database/async_foreign_keys/postgres_async_foreign_key_validation.rb b/spec/factories/gitlab/database/async_foreign_keys/postgres_async_constraint_validation.rb index 23f4049f1cb..81f67e958c0 100644 --- a/spec/factories/gitlab/database/async_foreign_keys/postgres_async_foreign_key_validation.rb +++ b/spec/factories/gitlab/database/async_foreign_keys/postgres_async_constraint_validation.rb @@ -5,5 +5,13 @@ FactoryBot.define do class: 'Gitlab::Database::AsyncConstraints::PostgresAsyncConstraintValidation' do sequence(:name) { |n| "fk_users_id_#{n}" } table_name { "users" } + + trait :foreign_key do + constraint_type { :foreign_key } + end + + trait :check_constraint do + constraint_type { :check_constraint } + end end end diff --git a/spec/lib/gitlab/database/async_constraints/validators/check_constraint_spec.rb b/spec/lib/gitlab/database/async_constraints/validators/check_constraint_spec.rb new file mode 100644 index 00000000000..7622b39feb1 --- /dev/null +++ b/spec/lib/gitlab/database/async_constraints/validators/check_constraint_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::AsyncConstraints::Validators::CheckConstraint, feature_category: :database do + it_behaves_like 'async constraints validation' do + let(:constraint_type) { :check_constraint } + + before do + connection.create_table(table_name) do |t| + t.integer :parent_id + end + + connection.execute(<<~SQL.squish) + ALTER TABLE #{table_name} ADD CONSTRAINT #{constraint_name} + CHECK ( parent_id = 101 ) NOT VALID; + SQL + end + end +end diff --git a/spec/lib/gitlab/database/async_constraints/validators/foreign_key_spec.rb b/spec/lib/gitlab/database/async_constraints/validators/foreign_key_spec.rb new file mode 100644 index 00000000000..0e345e0e9ae --- /dev/null +++ b/spec/lib/gitlab/database/async_constraints/validators/foreign_key_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::AsyncConstraints::Validators::ForeignKey, feature_category: :database do + it_behaves_like 'async constraints validation' do + let(:constraint_type) { :foreign_key } + + before do + connection.create_table(table_name) do |t| + t.references :parent, foreign_key: { to_table: table_name, validate: false, name: constraint_name } + end + end + + context 'with fully qualified table names' do + let(:validation) do + create(:postgres_async_constraint_validation, + table_name: "public.#{table_name}", + name: constraint_name, + constraint_type: constraint_type + ) + end + + it 'validates the constraint' do + allow(connection).to receive(:execute).and_call_original + + expect(connection).to receive(:execute) + .with(/ALTER TABLE "public"."#{table_name}" VALIDATE CONSTRAINT "#{constraint_name}";/) + .ordered.and_call_original + + subject.perform + end + end + end +end diff --git a/spec/lib/gitlab/database/async_constraints/validators_spec.rb b/spec/lib/gitlab/database/async_constraints/validators_spec.rb new file mode 100644 index 00000000000..e903b79dd1b --- /dev/null +++ b/spec/lib/gitlab/database/async_constraints/validators_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::AsyncConstraints::Validators, feature_category: :database do + describe '.for' do + subject { described_class.for(record) } + + context 'with foreign keys validations' do + let(:record) { build(:postgres_async_constraint_validation, :foreign_key) } + + it { is_expected.to be_a(described_class::ForeignKey) } + end + + context 'with check constraint validations' do + let(:record) { build(:postgres_async_constraint_validation, :check_constraint) } + + it { is_expected.to be_a(described_class::CheckConstraint) } + end + end +end diff --git a/spec/support/shared_examples/lib/gitlab/database/async_constraints_validation_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/database/async_constraints_validation_shared_examples.rb new file mode 100644 index 00000000000..b9d71183851 --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/database/async_constraints_validation_shared_examples.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'async constraints validation' do + include ExclusiveLeaseHelpers + + let!(:lease) { stub_exclusive_lease(lease_key, :uuid, timeout: lease_timeout) } + let(:lease_key) { "gitlab/database/asyncddl/actions/#{Gitlab::Database::PRIMARY_DATABASE_NAME}" } + let(:lease_timeout) { described_class::TIMEOUT_PER_ACTION } + + let(:constraints_model) { Gitlab::Database::AsyncConstraints::PostgresAsyncConstraintValidation } + let(:table_name) { '_test_async_constraints' } + let(:constraint_name) { 'constraint_parent_id' } + + let(:validation) do + create(:postgres_async_constraint_validation, + table_name: table_name, + name: constraint_name, + constraint_type: constraint_type) + end + + let(:connection) { validation.connection } + + subject { described_class.new(validation) } + + it 'validates the constraint while controlling statement timeout' do + allow(connection).to receive(:execute).and_call_original + expect(connection).to receive(:execute) + .with("SET statement_timeout TO '43200s'").ordered.and_call_original + expect(connection).to receive(:execute) + .with(/ALTER TABLE "#{table_name}" VALIDATE CONSTRAINT "#{constraint_name}";/).ordered.and_call_original + expect(connection).to receive(:execute) + .with("RESET statement_timeout").ordered.and_call_original + + subject.perform + end + + it 'removes the constraint validation record from table' do + expect(validation).to receive(:destroy!).and_call_original + + expect { subject.perform }.to change { constraints_model.count }.by(-1) + end + + it 'skips logic if not able to acquire exclusive lease' do + expect(lease).to receive(:try_obtain).ordered.and_return(false) + expect(connection).not_to receive(:execute).with(/ALTER TABLE/) + expect(validation).not_to receive(:destroy!) + + expect { subject.perform }.not_to change { constraints_model.count } + end + + it 'logs messages around execution' do + allow(Gitlab::AppLogger).to receive(:info).and_call_original + + subject.perform + + expect(Gitlab::AppLogger) + .to have_received(:info) + .with(a_hash_including(message: 'Starting to validate constraint')) + + expect(Gitlab::AppLogger) + .to have_received(:info) + .with(a_hash_including(message: 'Finished validating constraint')) + end + + context 'when the constraint does not exist' do + before do + connection.create_table(table_name, force: true) + end + + it 'skips validation and removes the record' do + expect(connection).not_to receive(:execute).with(/ALTER TABLE/) + + expect { subject.perform }.to change { constraints_model.count }.by(-1) + end + + it 'logs an appropriate message' do + expected_message = /Skipping #{constraint_name} validation since it does not exist/ + + allow(Gitlab::AppLogger).to receive(:info).and_call_original + + subject.perform + + expect(Gitlab::AppLogger) + .to have_received(:info) + .with(a_hash_including(message: expected_message)) + end + end + + context 'with error handling' do + before do + allow(connection).to receive(:execute).and_call_original + + allow(connection).to receive(:execute) + .with(/ALTER TABLE "#{table_name}" VALIDATE CONSTRAINT "#{constraint_name}";/) + .and_raise(ActiveRecord::StatementInvalid) + end + + context 'on production' do + before do + allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(false) + end + + it 'increases execution attempts' do + expect { subject.perform }.to change { validation.attempts }.by(1) + + expect(validation.last_error).to be_present + expect(validation).not_to be_destroyed + end + + it 'logs an error message including the constraint_name' do + expect(Gitlab::AppLogger) + .to receive(:error) + .with(a_hash_including(:message, :constraint_name)) + .and_call_original + + subject.perform + end + end + + context 'on development' do + it 'also raises errors' do + expect { subject.perform } + .to raise_error(ActiveRecord::StatementInvalid) + .and change { validation.attempts }.by(1) + + expect(validation.last_error).to be_present + expect(validation).not_to be_destroyed + end + end + end +end |