diff options
Diffstat (limited to 'spec/lib/gitlab/database')
17 files changed, 721 insertions, 1008 deletions
diff --git a/spec/lib/gitlab/database/click_house_client_spec.rb b/spec/lib/gitlab/database/click_house_client_spec.rb index 6e63ae56557..271500ed3f6 100644 --- a/spec/lib/gitlab/database/click_house_client_spec.rb +++ b/spec/lib/gitlab/database/click_house_client_spec.rb @@ -112,6 +112,28 @@ RSpec.describe 'ClickHouse::Client', :click_house, feature_category: :database d results = ClickHouse::Client.select(select_query, :main) expect(results).to be_empty + + # Async, lazy deletion + # Set the `deleted` field to 1 and update the `updated_at` timestamp. + # Based on the highest version of the given row (updated_at), CH will eventually remove the row. + # See: https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/replacingmergetree#is_deleted + soft_delete_query = ClickHouse::Client::Query.new( + raw_query: %{ + INSERT INTO events (id, deleted, updated_at) + VALUES ({id:UInt64}, 1, {updated_at:DateTime64(6, 'UTC')}) + }, + placeholders: { id: event2.id, updated_at: (event2.updated_at + 2.hours).utc.to_f } + ) + + ClickHouse::Client.execute(soft_delete_query, :main) + + select_query = ClickHouse::Client::Query.new( + raw_query: 'SELECT * FROM events FINAL WHERE id = {id:UInt64}', + placeholders: { id: event2.id } + ) + + results = ClickHouse::Client.select(select_query, :main) + expect(results).to be_empty end end end diff --git a/spec/lib/gitlab/database/gitlab_schema_spec.rb b/spec/lib/gitlab/database/gitlab_schema_spec.rb index e402014df90..a6de695c345 100644 --- a/spec/lib/gitlab/database/gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/gitlab_schema_spec.rb @@ -226,57 +226,83 @@ RSpec.describe Gitlab::Database::GitlabSchema, feature_category: :database do allow_cross_joins: %i[gitlab_shared], allow_cross_transactions: %i[gitlab_internal gitlab_shared], allow_cross_foreign_keys: %i[] + ), + Gitlab::Database::GitlabSchemaInfo.new( + name: "gitlab_main_cell", + allow_cross_joins: [ + :gitlab_shared, + :gitlab_main, + { gitlab_main_clusterwide: { specific_tables: %w[plans] } } + ], + allow_cross_transactions: [ + :gitlab_internal, + :gitlab_shared, + :gitlab_main, + { gitlab_main_clusterwide: { specific_tables: %w[plans] } } + ], + allow_cross_foreign_keys: [ + { gitlab_main_clusterwide: { specific_tables: %w[plans] } } + ] ) ].index_by(&:name) ) end describe '.cross_joins_allowed?' do - where(:schemas, :result) do - %i[] | true - %i[gitlab_main_clusterwide gitlab_main] | true - %i[gitlab_main_clusterwide gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_internal] | false - %i[gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | true - %i[gitlab_main_clusterwide gitlab_shared] | true + where(:schemas, :tables, :result) do + %i[] | %i[] | true + %i[gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_internal] | %i[] | false + %i[gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[users namespaces] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[plans namespaces] | true end with_them do - it { expect(described_class.cross_joins_allowed?(schemas)).to eq(result) } + it { expect(described_class.cross_joins_allowed?(schemas, tables)).to eq(result) } end end describe '.cross_transactions_allowed?' do - where(:schemas, :result) do - %i[] | true - %i[gitlab_main_clusterwide gitlab_main] | true - %i[gitlab_main_clusterwide gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_internal] | true - %i[gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | true - %i[gitlab_main_clusterwide gitlab_shared] | true + where(:schemas, :tables, :result) do + %i[] | %i[] | true + %i[gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_internal] | %i[] | true + %i[gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_shared] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[users namespaces] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[plans namespaces] | true end with_them do - it { expect(described_class.cross_transactions_allowed?(schemas)).to eq(result) } + it { expect(described_class.cross_transactions_allowed?(schemas, tables)).to eq(result) } end end describe '.cross_foreign_key_allowed?' do - where(:schemas, :result) do - %i[] | false - %i[gitlab_main_clusterwide gitlab_main] | true - %i[gitlab_main_clusterwide gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_internal] | false - %i[gitlab_main gitlab_ci] | false - %i[gitlab_main_clusterwide gitlab_shared] | false + where(:schemas, :tables, :result) do + %i[] | %i[] | false + %i[gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_main] | %i[] | true + %i[gitlab_main_clusterwide gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_internal] | %i[] | false + %i[gitlab_main gitlab_ci] | %i[] | false + %i[gitlab_main_clusterwide gitlab_shared] | %i[] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[users namespaces] | false + %i[gitlab_main_clusterwide gitlab_main_cell] | %w[plans namespaces] | true end with_them do - it { expect(described_class.cross_foreign_key_allowed?(schemas)).to eq(result) } + it { expect(described_class.cross_foreign_key_allowed?(schemas, tables)).to eq(result) } end end end diff --git a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb index 7197b99fe33..442fa678d4e 100644 --- a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb @@ -194,7 +194,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery, feature_catego describe '#replace_hosts' do before do - stub_env('LOAD_BALANCER_PARALLEL_DISCONNECT', 'true') allow(service) .to receive(:load_balancer) .and_return(load_balancer) @@ -257,26 +256,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery, feature_catego service.replace_hosts([address_foo, address_bar]) end end - - context 'when LOAD_BALANCER_PARALLEL_DISCONNECT is false' do - before do - stub_env('LOAD_BALANCER_PARALLEL_DISCONNECT', 'false') - end - - it 'disconnects them sequentially' do - host = load_balancer.host_list.hosts.first - - allow(service) - .to receive(:disconnect_timeout) - .and_return(2) - - expect(host) - .to receive(:disconnect!) - .with(timeout: 2) - - service.replace_hosts([address_bar]) - end - end end describe '#addresses_from_dns' do diff --git a/spec/lib/gitlab/database/migration_helpers/swapping_spec.rb b/spec/lib/gitlab/database/migration_helpers/swapping_spec.rb new file mode 100644 index 00000000000..0940c6f4c30 --- /dev/null +++ b/spec/lib/gitlab/database/migration_helpers/swapping_spec.rb @@ -0,0 +1,172 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::MigrationHelpers::Swapping, feature_category: :database do + let(:connection) { ApplicationRecord.connection } + let(:migration_context) do + ActiveRecord::Migration + .new + .extend(described_class) + .extend(Gitlab::Database::MigrationHelpers) + end + + let(:service_instance) { instance_double('Gitlab::Database::Migrations::SwapColumns', execute: nil) } + + describe '#reset_trigger_function' do + let(:trigger_function_name) { 'existing_trigger_function' } + + before do + connection.execute(<<~SQL) + CREATE FUNCTION #{trigger_function_name}() RETURNS trigger + LANGUAGE plpgsql + AS $$ + BEGIN + NEW."bigint_column" := NEW."integer_column"; + RETURN NEW; + END; + $$; + SQL + end + + it 'resets' do + recorder = ActiveRecord::QueryRecorder.new do + migration_context.reset_trigger_function(trigger_function_name) + end + expect(recorder.log).to include(/ALTER FUNCTION "existing_trigger_function" RESET ALL/) + end + end + + describe '#swap_columns' do + let(:table) { :ci_pipeline_variables } + let(:column1) { :pipeline_id } + let(:column2) { :pipeline_id_convert_to_bigint } + + it 'calls service' do + expect(::Gitlab::Database::Migrations::SwapColumns).to receive(:new).with( + migration_context: migration_context, + table: table, + column1: column1, + column2: column2 + ).and_return(service_instance) + + migration_context.swap_columns(table, column1, column2) + end + end + + describe '#swap_columns_default' do + let(:table) { :_test_table } + let(:column1) { :pipeline_id } + let(:column2) { :pipeline_id_convert_to_bigint } + + it 'calls service' do + expect(::Gitlab::Database::Migrations::SwapColumnsDefault).to receive(:new).with( + migration_context: migration_context, + table: table, + column1: column1, + column2: column2 + ).and_return(service_instance) + + migration_context.swap_columns_default(table, column1, column2) + end + end + + describe '#swap_foreign_keys' do + let(:table) { :_test_swap_foreign_keys } + let(:referenced_table) { "#{table}_referenced" } + let(:foreign_key1) { :fkey_on_integer_column } + let(:foreign_key2) { :fkey_on_bigint_column } + + before do + connection.execute(<<~SQL) + CREATE TABLE #{table} ( + integer_column integer NOT NULL, + bigint_column bigint DEFAULT 0 NOT NULL + ); + CREATE TABLE #{referenced_table} ( + id bigint NOT NULL + ); + + ALTER TABLE ONLY #{referenced_table} + ADD CONSTRAINT pk PRIMARY KEY (id); + + ALTER TABLE ONLY #{table} + ADD CONSTRAINT #{foreign_key1} + FOREIGN KEY (integer_column) REFERENCES #{referenced_table}(id) ON DELETE SET NULL; + + ALTER TABLE ONLY #{table} + ADD CONSTRAINT #{foreign_key2} + FOREIGN KEY (bigint_column) REFERENCES #{referenced_table}(id) ON DELETE SET NULL; + SQL + end + + shared_examples_for 'swapping foreign keys correctly' do + specify do + expect { migration_context.swap_foreign_keys(table, foreign_key1, foreign_key2) } + .to change { + find_foreign_key_by(foreign_key1).options[:column] + }.from('integer_column').to('bigint_column') + .and change { + find_foreign_key_by(foreign_key2).options[:column] + }.from('bigint_column').to('integer_column') + end + end + + it_behaves_like 'swapping foreign keys correctly' + + context 'when foreign key names are 63 bytes' do + let(:foreign_key1) { :f1_012345678901234567890123456789012345678901234567890123456789 } + let(:foreign_key2) { :f2_012345678901234567890123456789012345678901234567890123456789 } + + it_behaves_like 'swapping foreign keys correctly' + end + + private + + def find_foreign_key_by(name) + connection.foreign_keys(table).find { |k| k.options[:name].to_s == name.to_s } + end + end + + describe '#swap_indexes' do + let(:table) { :_test_swap_indexes } + let(:index1) { :index_on_integer } + let(:index2) { :index_on_bigint } + + before do + connection.execute(<<~SQL) + CREATE TABLE #{table} ( + integer_column integer NOT NULL, + bigint_column bigint DEFAULT 0 NOT NULL + ); + + CREATE INDEX #{index1} ON #{table} USING btree (integer_column); + + CREATE INDEX #{index2} ON #{table} USING btree (bigint_column); + SQL + end + + shared_examples_for 'swapping indexes correctly' do + specify do + expect { migration_context.swap_indexes(table, index1, index2) } + .to change { find_index_by(index1).columns }.from(['integer_column']).to(['bigint_column']) + .and change { find_index_by(index2).columns }.from(['bigint_column']).to(['integer_column']) + end + end + + it_behaves_like 'swapping indexes correctly' + + context 'when index names are 63 bytes' do + let(:index1) { :i1_012345678901234567890123456789012345678901234567890123456789 } + let(:index2) { :i2_012345678901234567890123456789012345678901234567890123456789 } + + it_behaves_like 'swapping indexes correctly' + end + + private + + def find_index_by(name) + connection.indexes(table).find { |c| c.name == name.to_s } + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index f3c181db3aa..dd51cca688c 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1774,6 +1774,35 @@ RSpec.describe Gitlab::Database::MigrationHelpers, feature_category: :database d end describe '#copy_indexes' do + context 'when index name is too long' do + it 'does not fail' do + index = double(:index, + columns: %w(uuid), + name: 'index_vuln_findings_on_uuid_including_vuln_id_1', + using: nil, + where: nil, + opclasses: {}, + unique: true, + lengths: [], + orders: []) + + allow(model).to receive(:indexes_for).with(:vulnerability_occurrences, 'uuid') + .and_return([index]) + + expect(model).to receive(:add_concurrent_index) + .with(:vulnerability_occurrences, + %w(tmp_undo_cleanup_column_8cbf300838), + { + unique: true, + name: 'idx_copy_191a1af1a0', + length: [], + order: [] + }) + + model.copy_indexes(:vulnerability_occurrences, :uuid, :tmp_undo_cleanup_column_8cbf300838) + end + end + context 'using a regular index using a single column' do it 'copies the index' do index = double(:index, @@ -2326,6 +2355,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers, feature_category: :database d end describe '#revert_initialize_conversion_of_integer_to_bigint' do + let(:setup_table) { true } let(:table) { :_test_table } before do @@ -2334,7 +2364,18 @@ RSpec.describe Gitlab::Database::MigrationHelpers, feature_category: :database d t.integer :other_id end - model.initialize_conversion_of_integer_to_bigint(table, columns) + model.initialize_conversion_of_integer_to_bigint(table, columns) if setup_table + end + + context 'when column and trigger do not exist' do + let(:setup_table) { false } + let(:columns) { :id } + + it 'does not raise an error' do + expect do + model.revert_initialize_conversion_of_integer_to_bigint(table, columns) + end.not_to raise_error + end end context 'when single column is given' do @@ -2906,4 +2947,20 @@ RSpec.describe Gitlab::Database::MigrationHelpers, feature_category: :database d it { expect(recorder.log).to be_empty } end end + + describe '#lock_tables' do + let(:lock_statement) do + /LOCK TABLE ci_builds, ci_pipelines IN ACCESS EXCLUSIVE MODE/ + end + + subject(:recorder) do + ActiveRecord::QueryRecorder.new do + model.lock_tables(:ci_builds, :ci_pipelines) + end + end + + it 'locks the tables' do + expect(recorder.log).to include(lock_statement) + end + end end diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb index 158497b1fef..f1271f2434c 100644 --- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers do +RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers, feature_category: :database do let(:migration_class) do Class.new(ActiveRecord::Migration[6.1]) .include(described_class) @@ -70,39 +70,54 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d end end - it 'creates the database record for the migration' do - expect(Gitlab::Database::PgClass).to receive(:for_table).with(:projects).and_return(pgclass_info) + context "when the migration doesn't exist already" do + before do + allow(Gitlab::Database::PgClass).to receive(:for_table).with(:projects).and_return(pgclass_info) + end - expect do + subject(:enqueue_batched_background_migration) do migration.queue_batched_background_migration( job_class.name, :projects, :id, job_interval: 5.minutes, + queued_migration_version: format("%.14d", 123), batch_min_value: 5, batch_max_value: 1000, batch_class_name: 'MyBatchClass', batch_size: 100, max_batch_size: 10000, sub_batch_size: 10, - gitlab_schema: :gitlab_ci) - end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) - - expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to have_attributes( - job_class_name: 'MyJobClass', - table_name: 'projects', - column_name: 'id', - interval: 300, - min_value: 5, - max_value: 1000, - batch_class_name: 'MyBatchClass', - batch_size: 100, - max_batch_size: 10000, - sub_batch_size: 10, - job_arguments: %w[], - status_name: :active, - total_tuple_count: pgclass_info.cardinality_estimate, - gitlab_schema: 'gitlab_ci') + gitlab_schema: :gitlab_ci + ) + end + + it 'enqueues exactly one batched migration' do + expect { enqueue_batched_background_migration } + .to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + end + + it 'creates the database record for the migration' do + batched_background_migration = enqueue_batched_background_migration + + expect(batched_background_migration.reload).to have_attributes( + job_class_name: 'MyJobClass', + table_name: 'projects', + column_name: 'id', + interval: 300, + min_value: 5, + max_value: 1000, + batch_class_name: 'MyBatchClass', + batch_size: 100, + max_batch_size: 10000, + sub_batch_size: 10, + job_arguments: %w[], + status_name: :active, + total_tuple_count: pgclass_info.cardinality_estimate, + gitlab_schema: 'gitlab_ci', + queued_migration_version: format("%.14d", 123) + ) + end end context 'when the job interval is lower than the minimum' do diff --git a/spec/lib/gitlab/database/migrations/milestone_mixin_spec.rb b/spec/lib/gitlab/database/migrations/milestone_mixin_spec.rb new file mode 100644 index 00000000000..e375af494a2 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/milestone_mixin_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::MilestoneMixin, feature_category: :database do + let(:migration_no_mixin) do + Class.new(Gitlab::Database::Migration[2.1]) do + def change + # no-op here to make rubocop happy + end + end + end + + let(:migration_mixin) do + Class.new(Gitlab::Database::Migration[2.1]) do + include Gitlab::Database::Migrations::MilestoneMixin + end + end + + let(:migration_mixin_version) do + Class.new(Gitlab::Database::Migration[2.1]) do + include Gitlab::Database::Migrations::MilestoneMixin + milestone '16.4' + end + end + + context 'when the mixin is not included' do + it 'does not raise an error' do + expect { migration_no_mixin.new(4, 4) }.not_to raise_error + end + end + + context 'when the mixin is included' do + context 'when a milestone is not specified' do + it "raises MilestoneNotSetError" do + expect { migration_mixin.new(4, 4, :regular) }.to raise_error( + "#{described_class}::MilestoneNotSetError".constantize + ) + end + end + + context 'when a milestone is specified' do + it "does not raise an error" do + expect { migration_mixin_version.new(4, 4, :regular) }.not_to raise_error + end + end + end +end diff --git a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb index 66de25d65bb..330c9d18fb2 100644 --- a/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb +++ b/spec/lib/gitlab/database/migrations/observers/query_statistics_spec.rb @@ -41,7 +41,13 @@ RSpec.describe Gitlab::Database::Migrations::Observers::QueryStatistics do let(:result) { double } let(:pgss_query) do <<~SQL - SELECT query, calls, total_time, max_time, mean_time, rows + SELECT + query, + calls, + total_exec_time + total_plan_time AS total_time, + max_exec_time + max_plan_time AS max_time, + mean_exec_time + mean_plan_time AS mean_time, + "rows" FROM pg_stat_statements WHERE pg_get_userbyid(userid) = current_user ORDER BY total_time DESC diff --git a/spec/lib/gitlab/database/migrations/swap_columns_default_spec.rb b/spec/lib/gitlab/database/migrations/swap_columns_default_spec.rb new file mode 100644 index 00000000000..e53480d453e --- /dev/null +++ b/spec/lib/gitlab/database/migrations/swap_columns_default_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::SwapColumnsDefault, feature_category: :database do + describe '#execute' do + let(:connection) { ApplicationRecord.connection } + let(:migration_context) do + Gitlab::Database::Migration[2.1] + .new('name', 'version') + .extend(Gitlab::Database::MigrationHelpers::Swapping) + end + + let(:table) { :_test_swap_columns_and_defaults } + let(:column1) { :integer_column } + let(:column2) { :bigint_column } + + subject(:execute_service) do + described_class.new( + migration_context: migration_context, + table: table, + column1: column1, + column2: column2 + ).execute + end + + before do + connection.execute(sql) + end + + context 'when defaults are static values' do + let(:sql) do + <<~SQL + CREATE TABLE #{table} ( + id integer NOT NULL, + #{column1} integer DEFAULT 8 NOT NULL, + #{column2} bigint DEFAULT 100 NOT NULL + ); + SQL + end + + it 'swaps the default correctly' do + expect { execute_service } + .to change { find_column_by(column1).default }.to('100') + .and change { find_column_by(column2).default }.to('8') + .and not_change { find_column_by(column1).default_function }.from(nil) + .and not_change { find_column_by(column2).default_function }.from(nil) + end + end + + context 'when default is sequence' do + let(:sql) do + <<~SQL + CREATE TABLE #{table} ( + id integer NOT NULL, + #{column1} integer NOT NULL, + #{column2} bigint DEFAULT 100 NOT NULL + ); + + CREATE SEQUENCE #{table}_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + ALTER SEQUENCE #{table}_seq OWNED BY #{table}.#{column1}; + ALTER TABLE ONLY #{table} ALTER COLUMN #{column1} SET DEFAULT nextval('#{table}_seq'::regclass); + SQL + end + + it 'swaps the default correctly' do + recorder = nil + expect { recorder = ActiveRecord::QueryRecorder.new { execute_service } } + .to change { find_column_by(column1).default }.to('100') + .and change { find_column_by(column1).default_function }.to(nil) + .and change { find_column_by(column2).default }.to(nil) + .and change { + find_column_by(column2).default_function + }.to("nextval('_test_swap_columns_and_defaults_seq'::regclass)") + expect(recorder.log).to include( + /SEQUENCE "_test_swap_columns_and_defaults_seq" OWNED BY "_test_swap_columns_and_defaults"."bigint_column"/ + ) + expect(recorder.log).to include( + /COLUMN "bigint_column" SET DEFAULT nextval\('_test_swap_columns_and_defaults_seq'::regclass\)/ + ) + end + end + + context 'when defaults are the same' do + let(:sql) do + <<~SQL + CREATE TABLE #{table} ( + id integer NOT NULL, + #{column1} integer DEFAULT 100 NOT NULL, + #{column2} bigint DEFAULT 100 NOT NULL + ); + SQL + end + + it 'does nothing' do + recorder = nil + expect { recorder = ActiveRecord::QueryRecorder.new { execute_service } } + .to not_change { find_column_by(column1).default } + .and not_change { find_column_by(column1).default_function } + .and not_change { find_column_by(column2).default } + .and not_change { find_column_by(column2).default_function } + expect(recorder.log).not_to include(/ALTER TABLE/) + end + end + + private + + def find_column_by(name) + connection.columns(table).find { |c| c.name == name.to_s } + end + end +end diff --git a/spec/lib/gitlab/database/migrations/swap_columns_spec.rb b/spec/lib/gitlab/database/migrations/swap_columns_spec.rb new file mode 100644 index 00000000000..a119b23dda4 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/swap_columns_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::SwapColumns, feature_category: :database do + describe '#execute' do + let(:connection) { ApplicationRecord.connection } + let(:sql) do + <<~SQL + CREATE TABLE #{table} ( + id integer NOT NULL, + #{column1} integer DEFAULT 8 NOT NULL, + #{column2} bigint DEFAULT 100 NOT NULL + ); + SQL + end + + let(:migration_context) do + Gitlab::Database::Migration[2.1] + .new('name', 'version') + .extend(Gitlab::Database::MigrationHelpers::Swapping) + end + + let(:table) { :_test_swap_columns_and_defaults } + let(:column1) { :integer_column } + let(:column2) { :bigint_column } + + subject(:execute_service) do + described_class.new( + migration_context: migration_context, + table: table, + column1: column1, + column2: column2 + ).execute + end + + before do + connection.execute(sql) + end + + shared_examples_for 'swapping columns correctly' do + specify do + expect { execute_service } + .to change { find_column_by(column1).sql_type }.from('integer').to('bigint') + .and change { find_column_by(column2).sql_type }.from('bigint').to('integer') + end + end + + it_behaves_like 'swapping columns correctly' + + context 'when column names are 63 bytes' do + let(:column1) { :int012345678901234567890123456789012345678901234567890123456789 } + let(:column2) { :big012345678901234567890123456789012345678901234567890123456789 } + + it_behaves_like 'swapping columns correctly' + end + + private + + def find_column_by(name) + connection.columns(table).find { |c| c.name == name.to_s } + end + end +end diff --git a/spec/lib/gitlab/database/migrations/version_spec.rb b/spec/lib/gitlab/database/migrations/version_spec.rb new file mode 100644 index 00000000000..821a2156539 --- /dev/null +++ b/spec/lib/gitlab/database/migrations/version_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Migrations::Version, feature_category: :database do + let(:test_versions) do + [ + 4, + 5, + described_class.new(6, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + 7, + described_class.new(8, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + described_class.new(9, Gitlab::VersionInfo.parse_from_milestone('10.4'), :regular), + described_class.new(10, Gitlab::VersionInfo.parse_from_milestone('10.3'), :post), + described_class.new(11, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular) + ] + end + + describe "#<=>" do + it 'sorts by existence of milestone, then by milestone, then by type, then by timestamp when sorted by version' do + expect(test_versions.sort.map(&:to_i)).to eq [4, 5, 7, 6, 8, 11, 10, 9] + end + end + + describe 'initialize' do + context 'when the type is :post or :regular' do + it 'does not raise an error' do + expect { described_class.new(4, 4, :regular) }.not_to raise_error + expect { described_class.new(4, 4, :post) }.not_to raise_error + end + end + + context 'when the type is anything else' do + it 'does not raise an error' do + expect { described_class.new(4, 4, 'foo') }.to raise_error("#{described_class}::InvalidTypeError".constantize) + end + end + end + + describe 'eql?' do + where(:version1, :version2, :expected_equality) do + [ + [ + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + true + ], + [ + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.4'), :regular), + false + ], + [ + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :post), + false + ], + [ + described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + described_class.new(5, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular), + false + ] + ] + end + + with_them do + it 'correctly evaluates deep equality' do + expect(version1.eql?(version2)).to eq(expected_equality) + end + + it 'correctly evaluates deep equality using ==' do + expect(version1 == version2).to eq(expected_equality) + end + end + end + + describe 'type' do + subject { described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), migration_type) } + + context 'when the migration is regular' do + let(:migration_type) { :regular } + + it 'correctly identifies the migration type' do + expect(subject.type).to eq(:regular) + expect(subject.regular?).to eq(true) + expect(subject.post_deployment?).to eq(false) + end + end + + context 'when the migration is post_deployment' do + let(:migration_type) { :post } + + it 'correctly identifies the migration type' do + expect(subject.type).to eq(:post) + expect(subject.regular?).to eq(false) + expect(subject.post_deployment?).to eq(true) + end + end + end + + describe 'to_s' do + subject { described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular) } + + it 'returns the given timestamp value as a string' do + expect(subject.to_s).to eql('4') + end + end + + describe 'hash' do + subject { described_class.new(4, Gitlab::VersionInfo.parse_from_milestone('10.3'), :regular) } + + let(:expected_hash) { subject.hash } + + it 'deterministically returns a hash of the timestamp, milestone, and type value' do + 3.times do + expect(subject.hash).to eq(expected_hash) + end + end + end +end diff --git a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb index 2fa4c9e562f..c6cd5e55754 100644 --- a/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb +++ b/spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb @@ -23,8 +23,6 @@ RSpec.describe 'cross-database foreign keys' do 'merge_requests.merge_user_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/422080 'merge_requests.author_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/422080 'project_authorizations.user_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/422044 - 'projects.creator_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/421844 - 'projects.marked_for_deletion_by_user_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/421844 'user_group_callouts.user_id' # https://gitlab.com/gitlab-org/gitlab/-/issues/421287 ] end @@ -34,9 +32,11 @@ RSpec.describe 'cross-database foreign keys' do end def is_cross_db?(fk_record) - table_schemas = Gitlab::Database::GitlabSchema.table_schemas!([fk_record.from_table, fk_record.to_table]) + tables = [fk_record.from_table, fk_record.to_table] - !Gitlab::Database::GitlabSchema.cross_foreign_key_allowed?(table_schemas) + table_schemas = Gitlab::Database::GitlabSchema.table_schemas!(tables) + + !Gitlab::Database::GitlabSchema.cross_foreign_key_allowed?(table_schemas, tables) end it 'onlies have allowed list of cross-database foreign keys', :aggregate_failures do diff --git a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb index c41228777ca..80ffa708d8a 100644 --- a/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb +++ b/spec/lib/gitlab/database/partitioning/partition_manager_spec.rb @@ -322,74 +322,33 @@ RSpec.describe Gitlab::Database::Partitioning::PartitionManager, feature_categor allow(connection).to receive(:select_value).and_return(nil, Time.current, Time.current) end - context 'when feature flag database_analyze_on_partitioned_tables is enabled' do - before do - stub_feature_flags(database_analyze_on_partitioned_tables: true) - end - - it_behaves_like 'run only once analyze within interval' + it_behaves_like 'run only once analyze within interval' - context 'when analyze is false' do - let(:analyze) { false } + context 'when analyze is false' do + let(:analyze) { false } - it_behaves_like 'not to run the analyze at all' - end + it_behaves_like 'not to run the analyze at all' + end - context 'when model does not set analyze_interval' do - let(:my_model) do - Class.new(ApplicationRecord) do - include PartitionedTable + context 'when model does not set analyze_interval' do + let(:my_model) do + Class.new(ApplicationRecord) do + include PartitionedTable - partitioned_by :partition_id, - strategy: :ci_sliding_list, - next_partition_if: proc { false }, - detach_partition_if: proc { false } - end + partitioned_by :partition_id, + strategy: :ci_sliding_list, + next_partition_if: proc { false }, + detach_partition_if: proc { false } end - - it_behaves_like 'not to run the analyze at all' - end - - context 'when no partition is created' do - let(:create_partition) { false } - - it_behaves_like 'run only once analyze within interval' - end - end - - context 'when feature flag database_analyze_on_partitioned_tables is disabled' do - before do - stub_feature_flags(database_analyze_on_partitioned_tables: false) end it_behaves_like 'not to run the analyze at all' + end - context 'when analyze is false' do - let(:analyze) { false } - - it_behaves_like 'not to run the analyze at all' - end - - context 'when model does not set analyze_interval' do - let(:my_model) do - Class.new(ApplicationRecord) do - include PartitionedTable - - partitioned_by :partition_id, - strategy: :ci_sliding_list, - next_partition_if: proc { false }, - detach_partition_if: proc { false } - end - end - - it_behaves_like 'not to run the analyze at all' - end - - context 'when no partition is created' do - let(:create_partition) { false } + context 'when no partition is created' do + let(:create_partition) { false } - it_behaves_like 'not to run the analyze at all' - end + it_behaves_like 'run only once analyze within interval' end end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb deleted file mode 100644 index 370d03b495c..00000000000 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_base_spec.rb +++ /dev/null @@ -1,292 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameBase, :delete, feature_category: :groups_and_projects do - let(:migration) { FakeRenameReservedPathMigrationV1.new } - let(:subject) { described_class.new(['the-path'], migration) } - - before do - allow(migration).to receive(:say) - TestEnv.clean_test_path - end - - def migration_namespace(namespace) - Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses:: - Namespace.find(namespace.id) - end - - def migration_project(project) - Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses:: - Project.find(project.id) - end - - describe "#remove_last_occurrence" do - it "removes only the last occurrence of a string" do - input = "this/is/a-word-to-replace/namespace/with/a-word-to-replace" - - expect(subject.remove_last_occurrence(input, "a-word-to-replace")) - .to eq("this/is/a-word-to-replace/namespace/with/") - end - end - - describe '#remove_cached_html_for_projects' do - let(:project) { create(:project, description_html: 'Project description') } - - it 'removes description_html from projects' do - subject.remove_cached_html_for_projects([project.id]) - - expect(project.reload.description_html).to be_nil - end - - it 'removes issue descriptions' do - issue = create(:issue, project: project, description_html: 'Issue description') - - subject.remove_cached_html_for_projects([project.id]) - - expect(issue.reload.description_html).to be_nil - end - - it 'removes merge request descriptions' do - merge_request = create(:merge_request, - source_project: project, - target_project: project, - description_html: 'MergeRequest description') - - subject.remove_cached_html_for_projects([project.id]) - - expect(merge_request.reload.description_html).to be_nil - end - - it 'removes note html' do - note = create(:note, - project: project, - noteable: create(:issue, project: project), - note_html: 'note description') - - subject.remove_cached_html_for_projects([project.id]) - - expect(note.reload.note_html).to be_nil - end - - it 'removes milestone description' do - milestone = create(:milestone, - project: project, - description_html: 'milestone description') - - subject.remove_cached_html_for_projects([project.id]) - - expect(milestone.reload.description_html).to be_nil - end - end - - describe '#rename_path_for_routable' do - context 'for personal namespaces' do - let(:namespace) { create(:namespace, path: 'the-path') } - - it "renames namespaces called the-path" do - subject.rename_path_for_routable(migration_namespace(namespace)) - - expect(namespace.reload.path).to eq("the-path0") - end - - it "renames the route to the namespace" do - subject.rename_path_for_routable(migration_namespace(namespace)) - - expect(Namespace.find(namespace.id).full_path).to eq("the-path0") - end - - it "renames the route for projects of the namespace" do - project = create(:project, :repository, path: "project-path", namespace: namespace) - - subject.rename_path_for_routable(migration_namespace(namespace)) - - expect(project.route.reload.path).to eq("the-path0/project-path") - end - - it 'returns the old & the new path' do - old_path, new_path = subject.rename_path_for_routable(migration_namespace(namespace)) - - expect(old_path).to eq('the-path') - expect(new_path).to eq('the-path0') - end - - it "doesn't rename routes that start with a similar name" do - other_namespace = create(:namespace, path: 'the-path-but-not-really') - project = create(:project, path: 'the-project', namespace: other_namespace) - - subject.rename_path_for_routable(migration_namespace(namespace)) - - expect(project.route.reload.path).to eq('the-path-but-not-really/the-project') - end - end - - context 'for groups' do - context "the-path group -> subgroup -> the-path0 project" do - it "updates the route of the project correctly" do - group = create(:group, path: 'the-path') - subgroup = create(:group, path: "subgroup", parent: group) - project = create(:project, :repository, path: "the-path0", namespace: subgroup) - - subject.rename_path_for_routable(migration_namespace(group)) - - expect(project.route.reload.path).to eq("the-path0/subgroup/the-path0") - end - end - end - - context 'for projects' do - let(:parent) { create(:namespace, path: 'the-parent') } - let(:project) { create(:project, path: 'the-path', namespace: parent) } - - it 'renames the project called `the-path`' do - subject.rename_path_for_routable(migration_project(project)) - - expect(project.reload.path).to eq('the-path0') - end - - it 'renames the route for the project' do - subject.rename_path_for_routable(project) - - expect(project.reload.route.path).to eq('the-parent/the-path0') - end - - it 'returns the old & new path' do - old_path, new_path = subject.rename_path_for_routable(migration_project(project)) - - expect(old_path).to eq('the-parent/the-path') - expect(new_path).to eq('the-parent/the-path0') - end - end - end - - describe '#perform_rename' do - context 'for personal namespaces' do - it 'renames the path' do - namespace = create(:namespace, path: 'the-path') - - subject.perform_rename(migration_namespace(namespace), 'the-path', 'renamed') - - expect(namespace.reload.path).to eq('renamed') - expect(namespace.reload.route.path).to eq('renamed') - end - end - - context 'for groups' do - it 'renames all the routes for the group' do - group = create(:group, path: 'the-path') - child = create(:group, path: 'child', parent: group) - project = create(:project, :repository, namespace: child, path: 'the-project') - other_one = create(:group, path: 'the-path-is-similar') - - subject.perform_rename(migration_namespace(group), 'the-path', 'renamed') - - expect(group.reload.route.path).to eq('renamed') - expect(child.reload.route.path).to eq('renamed/child') - expect(project.reload.route.path).to eq('renamed/child/the-project') - expect(other_one.reload.route.path).to eq('the-path-is-similar') - end - end - end - - describe '#move_pages' do - it 'moves the pages directory' do - expect(subject).to receive(:move_folders) - .with(TestEnv.pages_path, 'old-path', 'new-path') - - subject.move_pages('old-path', 'new-path') - end - end - - describe "#move_uploads" do - let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'rename_reserved_paths') } - let(:uploads_dir) { File.join(test_dir, 'public', 'uploads') } - - it 'moves subdirectories in the uploads folder' do - expect(subject).to receive(:uploads_dir).and_return(uploads_dir) - expect(subject).to receive(:move_folders).with(uploads_dir, 'old_path', 'new_path') - - subject.move_uploads('old_path', 'new_path') - end - - it "doesn't move uploads when they are stored in object storage" do - expect(subject).to receive(:file_storage?).and_return(false) - expect(subject).not_to receive(:move_folders) - - subject.move_uploads('old_path', 'new_path') - end - end - - describe '#move_folders' do - let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'rename_reserved_paths') } - let(:uploads_dir) { File.join(test_dir, 'public', 'uploads') } - - before do - FileUtils.remove_dir(test_dir) if File.directory?(test_dir) - FileUtils.mkdir_p(uploads_dir) - allow(subject).to receive(:uploads_dir).and_return(uploads_dir) - end - - it 'moves a folder with files' do - source = File.join(uploads_dir, 'parent-group', 'sub-group') - FileUtils.mkdir_p(source) - destination = File.join(uploads_dir, 'parent-group', 'moved-group') - FileUtils.touch(File.join(source, 'test.txt')) - expected_file = File.join(destination, 'test.txt') - - subject.move_folders(uploads_dir, File.join('parent-group', 'sub-group'), File.join('parent-group', 'moved-group')) - - expect(File.exist?(expected_file)).to be(true) - end - end - - describe '#track_rename', :redis do - it 'tracks a rename in redis' do - key = 'rename:FakeRenameReservedPathMigrationV1:namespace' - - subject.track_rename('namespace', 'path/to/namespace', 'path/to/renamed') - - old_path = nil - new_path = nil - Gitlab::Redis::SharedState.with do |redis| - rename_info = redis.lpop(key) - old_path, new_path = Gitlab::Json.parse(rename_info) - end - - expect(old_path).to eq('path/to/namespace') - expect(new_path).to eq('path/to/renamed') - end - end - - describe '#reverts_for_type', :redis do - it 'yields for each tracked rename' do - subject.track_rename('project', 'old_path', 'new_path') - subject.track_rename('project', 'old_path2', 'new_path2') - subject.track_rename('namespace', 'namespace_path', 'new_namespace_path') - - expect { |b| subject.reverts_for_type('project', &b) } - .to yield_successive_args(%w(old_path2 new_path2), %w(old_path new_path)) - expect { |b| subject.reverts_for_type('namespace', &b) } - .to yield_with_args('namespace_path', 'new_namespace_path') - end - - it 'keeps the revert in redis if it failed' do - subject.track_rename('project', 'old_path', 'new_path') - - subject.reverts_for_type('project') do - raise 'whatever happens, keep going!' - end - - key = 'rename:FakeRenameReservedPathMigrationV1:project' - stored_renames = nil - rename_count = 0 - Gitlab::Redis::SharedState.with do |redis| - stored_renames = redis.lrange(key, 0, 1) - rename_count = redis.llen(key) - end - - expect(rename_count).to eq(1) - expect(Gitlab::Json.parse(stored_renames.first)).to eq(%w(old_path new_path)) - end - end -end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb deleted file mode 100644 index b00a1d4a9e1..00000000000 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces_spec.rb +++ /dev/null @@ -1,313 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameNamespaces, :delete, -feature_category: :groups_and_projects do - let(:migration) { FakeRenameReservedPathMigrationV1.new } - let(:subject) { described_class.new(['the-path'], migration) } - let(:namespace) { create(:group, name: 'the-path') } - - before do - allow(migration).to receive(:say) - TestEnv.clean_test_path - end - - def migration_namespace(namespace) - Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses:: - Namespace.find(namespace.id) - end - - describe '#namespaces_for_paths' do - context 'nested namespaces' do - let(:subject) { described_class.new(['parent/the-Path'], migration) } - - it 'includes the namespace' do - parent = create(:group, path: 'parent') - child = create(:group, path: 'the-path', parent: parent) - - found_ids = subject.namespaces_for_paths(type: :child) - .map(&:id) - - expect(found_ids).to contain_exactly(child.id) - end - end - - context 'for child namespaces' do - it 'only returns child namespaces with the correct path' do - _root_namespace = create(:group, path: 'THE-path') - _other_path = create(:group, - path: 'other', - parent: create(:group)) - namespace = create(:group, - path: 'the-path', - parent: create(:group)) - - found_ids = subject.namespaces_for_paths(type: :child) - .map(&:id) - - expect(found_ids).to contain_exactly(namespace.id) - end - - it 'has no namespaces that look the same' do - _root_namespace = create(:group, path: 'THE-path') - _similar_path = create(:group, - path: 'not-really-the-path', - parent: create(:group)) - namespace = create(:group, - path: 'the-path', - parent: create(:group)) - - found_ids = subject.namespaces_for_paths(type: :child) - .map(&:id) - - expect(found_ids).to contain_exactly(namespace.id) - end - end - - context 'for top levelnamespaces' do - it 'only returns child namespaces with the correct path' do - root_namespace = create(:group, path: 'the-path') - _other_path = create(:group, path: 'other') - _child_namespace = create(:group, - path: 'the-path', - parent: create(:group)) - - found_ids = subject.namespaces_for_paths(type: :top_level) - .map(&:id) - - expect(found_ids).to contain_exactly(root_namespace.id) - end - - it 'has no namespaces that just look the same' do - root_namespace = create(:group, path: 'the-path') - _similar_path = create(:group, path: 'not-really-the-path') - _child_namespace = create(:group, - path: 'the-path', - parent: create(:group)) - - found_ids = subject.namespaces_for_paths(type: :top_level) - .map(&:id) - - expect(found_ids).to contain_exactly(root_namespace.id) - end - end - end - - describe '#move_repositories' do - let(:namespace) { create(:group, name: 'hello-group') } - - it 'moves a project for a namespace' do - project = create(:project, :repository, :legacy_storage, namespace: namespace, path: 'hello-project') - expected_repository = Gitlab::Git::Repository.new( - project.repository_storage, - 'bye-group/hello-project.git', - nil, - nil - ) - - subject.move_repositories(namespace, 'hello-group', 'bye-group') - - expect(expected_repository).to exist - end - - it 'moves a namespace in a subdirectory correctly' do - child_namespace = create(:group, name: 'sub-group', parent: namespace) - project = create(:project, :repository, :legacy_storage, namespace: child_namespace, path: 'hello-project') - - expected_repository = Gitlab::Git::Repository.new( - project.repository_storage, - 'hello-group/renamed-sub-group/hello-project.git', - nil, - nil - ) - - subject.move_repositories(child_namespace, 'hello-group/sub-group', 'hello-group/renamed-sub-group') - - expect(expected_repository).to exist - end - - it 'moves a parent namespace with subdirectories' do - child_namespace = create(:group, name: 'sub-group', parent: namespace) - project = create(:project, :repository, :legacy_storage, namespace: child_namespace, path: 'hello-project') - expected_repository = Gitlab::Git::Repository.new( - project.repository_storage, - 'renamed-group/sub-group/hello-project.git', - nil, - nil - ) - - subject.move_repositories(child_namespace, 'hello-group', 'renamed-group') - - expect(expected_repository).to exist - end - end - - describe "#child_ids_for_parent" do - it "collects child ids for all levels" do - parent = create(:group) - first_child = create(:group, parent: parent) - second_child = create(:group, parent: parent) - third_child = create(:group, parent: second_child) - all_ids = [parent.id, first_child.id, second_child.id, third_child.id] - - collected_ids = subject.child_ids_for_parent(parent, ids: [parent.id]) - - expect(collected_ids).to contain_exactly(*all_ids) - end - end - - describe "#rename_namespace" do - it 'renames paths & routes for the namespace' do - expect(subject).to receive(:rename_path_for_routable) - .with(namespace) - .and_call_original - - subject.rename_namespace(namespace) - - expect(namespace.reload.path).to eq('the-path0') - end - - it 'tracks the rename' do - expect(subject).to receive(:track_rename) - .with('namespace', 'the-path', 'the-path0') - - subject.rename_namespace(namespace) - end - - it 'renames things related to the namespace' do - expect(subject).to receive(:rename_namespace_dependencies) - .with(namespace, 'the-path', 'the-path0') - - subject.rename_namespace(namespace) - end - end - - describe '#rename_namespace_dependencies' do - it "moves the repository for a project in the namespace" do - project = create(:project, :repository, :legacy_storage, namespace: namespace, path: "the-path-project") - expected_repository = Gitlab::Git::Repository.new( - project.repository_storage, - "the-path0/the-path-project.git", - nil, - nil - ) - - subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0') - - expect(expected_repository).to exist - end - - it "moves the uploads for the namespace" do - expect(subject).to receive(:move_uploads).with("the-path", "the-path0") - - subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0') - end - - it "moves the pages for the namespace" do - expect(subject).to receive(:move_pages).with("the-path", "the-path0") - - subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0') - end - - it 'invalidates the markdown cache of related projects' do - project = create(:project, :legacy_storage, namespace: namespace, path: "the-path-project") - - expect(subject).to receive(:remove_cached_html_for_projects).with([project.id]) - - subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0') - end - - it "doesn't rename users for other namespaces" do - expect(subject).not_to receive(:rename_user) - - subject.rename_namespace_dependencies(namespace, 'the-path', 'the-path0') - end - - it 'renames the username of a namespace for a user' do - user = create(:user, username: 'the-path') - - expect(subject).to receive(:rename_user).with('the-path', 'the-path0') - - subject.rename_namespace_dependencies(user.namespace, 'the-path', 'the-path0') - end - end - - describe '#rename_user' do - it 'renames a username' do - subject = described_class.new([], migration) - user = create(:user, username: 'broken') - - subject.rename_user('broken', 'broken0') - - expect(user.reload.username).to eq('broken0') - end - end - - describe '#rename_namespaces' do - let!(:top_level_namespace) { create(:group, path: 'the-path') } - let!(:child_namespace) do - create(:group, path: 'the-path', parent: create(:group)) - end - - it 'renames top level namespaces the namespace' do - expect(subject).to receive(:rename_namespace) - .with(migration_namespace(top_level_namespace)) - - subject.rename_namespaces(type: :top_level) - end - - it 'renames child namespaces' do - expect(subject).to receive(:rename_namespace) - .with(migration_namespace(child_namespace)) - - subject.rename_namespaces(type: :child) - end - end - - describe '#revert_renames', :redis do - it 'renames the routes back to the previous values' do - project = create(:project, :legacy_storage, :repository, path: 'a-project', namespace: namespace) - subject.rename_namespace(namespace) - - expect(subject).to receive(:perform_rename) - .with( - kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Namespace), - 'the-path0', - 'the-path' - ).and_call_original - - subject.revert_renames - - expect(namespace.reload.path).to eq('the-path') - expect(namespace.reload.route.path).to eq('the-path') - expect(project.reload.route.path).to eq('the-path/a-project') - end - - it 'moves the repositories back to their original place' do - project = create(:project, :repository, :legacy_storage, path: 'a-project', namespace: namespace) - project.create_repository - subject.rename_namespace(namespace) - - expected_repository = Gitlab::Git::Repository.new(project.repository_storage, 'the-path/a-project.git', nil, nil) - - expect(subject).to receive(:rename_namespace_dependencies) - .with( - kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Namespace), - 'the-path0', - 'the-path' - ).and_call_original - - subject.revert_renames - - expect(expected_repository).to exist - end - - it "doesn't break when the namespace was renamed" do - subject.rename_namespace(namespace) - namespace.update!(path: 'renamed-afterwards') - - expect { subject.revert_renames }.not_to raise_error - end - end -end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb deleted file mode 100644 index d2665664fb0..00000000000 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects_spec.rb +++ /dev/null @@ -1,190 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1::RenameProjects, :delete, -feature_category: :groups_and_projects do - let(:migration) { FakeRenameReservedPathMigrationV1.new } - let(:subject) { described_class.new(['the-path'], migration) } - let(:project) do - create(:project, - :legacy_storage, - path: 'the-path', - namespace: create(:namespace, path: 'known-parent' )) - end - - before do - allow(migration).to receive(:say) - TestEnv.clean_test_path - end - - describe '#projects_for_paths' do - it 'searches using nested paths' do - namespace = create(:namespace, path: 'hello') - project = create(:project, :legacy_storage, path: 'THE-path', namespace: namespace) - - result_ids = described_class.new(['Hello/the-path'], migration) - .projects_for_paths.map(&:id) - - expect(result_ids).to contain_exactly(project.id) - end - - it 'includes the correct projects' do - project = create(:project, :legacy_storage, path: 'THE-path') - _other_project = create(:project, :legacy_storage) - - result_ids = subject.projects_for_paths.map(&:id) - - expect(result_ids).to contain_exactly(project.id) - end - end - - describe '#rename_projects' do - let!(:projects) { create_list(:project, 2, :legacy_storage, path: 'the-path') } - - it 'renames each project' do - expect(subject).to receive(:rename_project).twice - - subject.rename_projects - end - - it 'invalidates the markdown cache of related projects' do - expect(subject).to receive(:remove_cached_html_for_projects) - .with(a_collection_containing_exactly(*projects.map(&:id))) - - subject.rename_projects - end - end - - describe '#rename_project' do - it 'renames path & route for the project' do - expect(subject).to receive(:rename_path_for_routable) - .with(project) - .and_call_original - - subject.rename_project(project) - - expect(project.reload.path).to eq('the-path0') - end - - it 'tracks the rename' do - expect(subject).to receive(:track_rename) - .with('project', 'known-parent/the-path', 'known-parent/the-path0') - - subject.rename_project(project) - end - - it 'renames the folders for the project' do - expect(subject).to receive(:move_project_folders).with(project, 'known-parent/the-path', 'known-parent/the-path0') - - subject.rename_project(project) - end - end - - describe '#move_project_folders' do - it 'moves the wiki & the repo' do - expect(subject).to receive(:move_repository) - .with(project, 'known-parent/the-path.wiki', 'known-parent/the-path0.wiki') - expect(subject).to receive(:move_repository) - .with(project, 'known-parent/the-path', 'known-parent/the-path0') - - subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0') - end - - it 'does not move the repositories when hashed storage is enabled' do - project.update!(storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) - - expect(subject).not_to receive(:move_repository) - - subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0') - end - - it 'moves uploads' do - expect(subject).to receive(:move_uploads) - .with('known-parent/the-path', 'known-parent/the-path0') - - subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0') - end - - it 'does not move uploads when hashed storage is enabled for attachments' do - project.update!(storage_version: Project::HASHED_STORAGE_FEATURES[:attachments]) - - expect(subject).not_to receive(:move_uploads) - - subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0') - end - - it 'moves pages' do - expect(subject).to receive(:move_pages) - .with('known-parent/the-path', 'known-parent/the-path0') - - subject.move_project_folders(project, 'known-parent/the-path', 'known-parent/the-path0') - end - end - - describe '#move_repository' do - let(:known_parent) { create(:namespace, path: 'known-parent') } - let(:project) { create(:project, :repository, :legacy_storage, path: 'the-path', namespace: known_parent) } - - it 'moves the repository for a project' do - expected_repository = Gitlab::Git::Repository.new( - project.repository_storage, - 'known-parent/new-repo.git', - nil, - nil - ) - - subject.move_repository(project, 'known-parent/the-path', 'known-parent/new-repo') - - expect(expected_repository).to exist - end - end - - describe '#revert_renames', :redis do - it 'renames the routes back to the previous values' do - subject.rename_project(project) - - expect(subject).to receive(:perform_rename) - .with( - kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Project), - 'known-parent/the-path0', - 'known-parent/the-path' - ).and_call_original - - subject.revert_renames - - expect(project.reload.path).to eq('the-path') - expect(project.route.path).to eq('known-parent/the-path') - end - - it 'moves the repositories back to their original place' do - project.create_repository - subject.rename_project(project) - - expected_repository = Gitlab::Git::Repository.new( - project.repository_storage, - 'known-parent/the-path.git', - nil, - nil - ) - - expect(subject).to receive(:move_project_folders) - .with( - kind_of(Gitlab::Database::RenameReservedPathsMigration::V1::MigrationClasses::Project), - 'known-parent/the-path0', - 'known-parent/the-path' - ).and_call_original - - subject.revert_renames - - expect(expected_repository).to exist - end - - it "doesn't break when the project was renamed" do - subject.rename_project(project) - project.update!(path: 'renamed-afterwards') - - expect { subject.revert_renames }.not_to raise_error - end - end -end diff --git a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb b/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb deleted file mode 100644 index 3b2d3ab1354..00000000000 --- a/spec/lib/gitlab/database/rename_reserved_paths_migration/v1_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.shared_examples 'renames child namespaces' do |type| - it 'renames namespaces' do - rename_namespaces = double - expect(described_class::RenameNamespaces) - .to receive(:new).with(%w[first-path second-path], subject) - .and_return(rename_namespaces) - expect(rename_namespaces).to receive(:rename_namespaces) - .with(type: :child) - - subject.rename_wildcard_paths(%w[first-path second-path]) - end -end - -RSpec.describe Gitlab::Database::RenameReservedPathsMigration::V1, :delete do - let(:subject) { FakeRenameReservedPathMigrationV1.new } - - before do - allow(subject).to receive(:say) - end - - describe '#rename_child_paths' do - it_behaves_like 'renames child namespaces' - end - - describe '#rename_wildcard_paths' do - it_behaves_like 'renames child namespaces' - - it 'renames projects' do - rename_projects = double - expect(described_class::RenameProjects) - .to receive(:new).with(['the-path'], subject) - .and_return(rename_projects) - - expect(rename_projects).to receive(:rename_projects) - - subject.rename_wildcard_paths(['the-path']) - end - end - - describe '#rename_root_paths' do - it 'renames namespaces' do - rename_namespaces = double - expect(described_class::RenameNamespaces) - .to receive(:new).with(['the-path'], subject) - .and_return(rename_namespaces) - expect(rename_namespaces).to receive(:rename_namespaces) - .with(type: :top_level) - - subject.rename_root_paths('the-path') - end - end - - describe '#revert_renames' do - it 'renames namespaces' do - rename_namespaces = double - expect(described_class::RenameNamespaces) - .to receive(:new).with([], subject) - .and_return(rename_namespaces) - expect(rename_namespaces).to receive(:revert_renames) - - subject.revert_renames - end - - it 'renames projects' do - rename_projects = double - expect(described_class::RenameProjects) - .to receive(:new).with([], subject) - .and_return(rename_projects) - expect(rename_projects).to receive(:revert_renames) - - subject.revert_renames - end - end -end |