From ea7202cfe3576ba5d4e1beb4f954dd2a210f8036 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Oct 2021 21:13:01 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- GITALY_SERVER_VERSION | 2 +- .../_visibility_and_access.html.haml | 8 +- .../_default_branch_protection.html.haml | 7 +- .../shared/_project_creation_levels.html.haml | 7 ++ .../dependency_proxy_workhorse.yml | 2 +- .../documentation/styleguide/word_list.md | 4 + doc/development/workspaces/index.md | 95 ++++++++++++++++++ lib/gitlab/access.rb | 30 ++++-- .../detached_partition_dropper.rb | 96 ++++++++++++++++--- lib/gitlab/database/postgres_foreign_key.rb | 6 ++ locale/gitlab.pot | 27 ++++++ .../detached_partition_dropper_spec.rb | 95 ++++++++++++++---- .../database/postgres_foreign_key_spec.rb | 12 +++ 13 files changed, 343 insertions(+), 48 deletions(-) create mode 100644 app/views/shared/_project_creation_levels.html.haml create mode 100644 doc/development/workspaces/index.md diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9a383554ed5..9e0e86f9b5f 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -79a0dfb018671957bbc1a02e21684c8cc2858160 +bccabad35ea7b90a2bfae196cf583611a0484c51 diff --git a/app/views/admin/application_settings/_visibility_and_access.html.haml b/app/views/admin/application_settings/_visibility_and_access.html.haml index b6266c3ea34..e56c898b236 100644 --- a/app/views/admin/application_settings/_visibility_and_access.html.haml +++ b/app/views/admin/application_settings/_visibility_and_access.html.haml @@ -1,13 +1,11 @@ -= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-visibility-settings'), html: { class: 'fieldset-form', id: 'visibility-settings' } do |f| += gitlab_ui_form_for @application_setting, url: general_admin_application_settings_path(anchor: 'js-visibility-settings'), html: { class: 'fieldset-form', id: 'visibility-settings' } do |f| = form_errors(@application_setting) %fieldset - = render 'shared/default_branch_protection', f: f, selected_level: @application_setting.default_branch_protection + = render 'shared/default_branch_protection', f: f = render_if_exists 'admin/application_settings/group_owners_can_manage_default_branch_protection_setting', form: f - .form-group - = f.label s_('ProjectCreationLevel|Default project creation protection'), class: 'label-bold' - = f.select :default_project_creation, options_for_select(Gitlab::Access.project_creation_options, @application_setting.default_project_creation), {}, class: 'form-control' + = render 'shared/project_creation_levels', f: f, method: :default_project_creation, legend: s_('ProjectCreationLevel|Default project creation protection') = render_if_exists 'admin/application_settings/default_project_deletion_protection_setting', form: f = render_if_exists 'admin/application_settings/default_delayed_project_deletion_setting', form: f = render_if_exists 'admin/application_settings/default_project_deletion_adjourned_period_setting', form: f diff --git a/app/views/shared/_default_branch_protection.html.haml b/app/views/shared/_default_branch_protection.html.haml index d7ae21debd8..7a6152f6d96 100644 --- a/app/views/shared/_default_branch_protection.html.haml +++ b/app/views/shared/_default_branch_protection.html.haml @@ -1,3 +1,4 @@ -.form-group - = f.label :default_branch_protection, class: 'label-bold' - = f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, selected_level), {}, class: 'form-control' +%fieldset.form-group + %legend.h5.gl-border-none.gl-mt-0.gl-mb-3= _('Default branch protection') + - Gitlab::Access.protection_options.each do |option| + = f.gitlab_ui_radio_component :default_branch_protection, option[:value], option[:label], help_text: option[:help_text] diff --git a/app/views/shared/_project_creation_levels.html.haml b/app/views/shared/_project_creation_levels.html.haml new file mode 100644 index 00000000000..00f495a26dc --- /dev/null +++ b/app/views/shared/_project_creation_levels.html.haml @@ -0,0 +1,7 @@ +- method = local_assigns.fetch(:method, nil) +- legend = local_assigns.fetch(:legend, nil) + +%fieldset.form-group + %legend.h5.gl-border-none.gl-mt-0.gl-mb-3= legend + - Gitlab::Access.project_creation_options.each do |label, value| + = f.gitlab_ui_radio_component method, value, label diff --git a/config/feature_flags/development/dependency_proxy_workhorse.yml b/config/feature_flags/development/dependency_proxy_workhorse.yml index a3545d32cd5..a14f38fa001 100644 --- a/config/feature_flags/development/dependency_proxy_workhorse.yml +++ b/config/feature_flags/development/dependency_proxy_workhorse.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339639 milestone: '14.3' type: development group: group::source code -default_enabled: false +default_enabled: true diff --git a/doc/development/documentation/styleguide/word_list.md b/doc/development/documentation/styleguide/word_list.md index 2e7ebf4366f..595dab09bf5 100644 --- a/doc/development/documentation/styleguide/word_list.md +++ b/doc/development/documentation/styleguide/word_list.md @@ -572,6 +572,10 @@ or collapsing a section, don't include the word **section**. Use **select** with buttons, links, menu items, and lists. **Select** applies to more devices, while **click** is more specific to a mouse. +## Service Desk + +Use title case for **Service Desk**. + ## setup, set up Use **setup** as a noun, and **set up** as a verb. For example: diff --git a/doc/development/workspaces/index.md b/doc/development/workspaces/index.md new file mode 100644 index 00000000000..9fb5f404061 --- /dev/null +++ b/doc/development/workspaces/index.md @@ -0,0 +1,95 @@ +--- +comments: false +type: index, dev +stage: none +group: Development +info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +description: "Development Guidelines: learn about workspaces when developing GitLab." +--- + +# Workspaces + +[Read more](../../user/workspace/index.md) about the workspaces initiative. + +## Consolidate Groups and Projects + +- [Architecture blueprint](../../architecture/blueprints/consolidating_groups_and_projects/index.md) + +Consolidate Groups and Projects is one facet of the workspaces initiative, addressing the feature disparity between +groups and projects. + +There is feature disparity between groups and projects. Some features only available at group level (for example epics). +Some features only available at project level (for example issues). And some features available at both levels +(for example labels, milestones). + +We get more and more requests to get one feature or another added to either group or project level. This takes +engineering time, to just move features around to different levels. This also adds a UX overhead of keeping a mental +model of which features are available at which level. + +This also creates lots of redundant code. Features get copied from project, to group to instance level with small +nuances between them. This also causes extra maintenance, when something needs to be fixed. The fix is copied to +several locations. This also creates different user experience for the same feature but in the different locations. + +To solve this we are moving towards consolidating groups and projects into a single entity, namespace. The work on this +solution is split into several phases and is tracked in [epic 6473](https://gitlab.com/groups/gitlab-org/-/epics/6473). + +### Phase 1 + +Epic tracking [Phase 1](https://gitlab.com/groups/gitlab-org/-/epics/6697) + +Goal of Phase 1 is to ensure that every project gets a corresponding record in `namespaces` table with `type='Project'`. +For user namespaces, type changes from `NULL` to `User`. + +Places where we should make sure project and project namespace go hand in hand: + +- Create project. + - Use Rails callbacks to ensure a new project namespace is created for each project. + - In this case project namespace records should have `created_at`/`updated_at` attributes equal to project's `created_at`/`updated_at` attributes. +- Update Project. + - Use Rails `after_save` callback to ensure some attributes are kept in sync between project and project namespaces, + see [project#after_save](https://gitlab.com/gitlab-org/gitlab/blob/6d26634e864d7b748dda0e283eb2477362263bc3/app/models/project.rb#L101-L101). +- Delete project. + - Use FKs cascade delete or Rails callbacks to ensure when either a `Project` or its `ProjectNamespace` is removed its + corresponding `ProjectNamespace` or `Project` respectively is removed as well. +- Transfer project to a different group. + - Make sure that when a project is transferred, its corresponding project namespace is transferred to the same group. +- Transfer group. + - Make sure when transferring a group that all of its sub projects, either direct or through descendant groups, have their + corresponding project namespaces transferred correctly as well. +- Export/import project. + - Export project would continue to only export the project and not its project namespace in this phase. Project + namespace does not contain any specific information that has to be exported at this point. Eventually we want the + project namespace to be exported as well. + - Import creates a new project, so project namespace is created through Rails `after_save` callback on the project model. +- Export/import group. + - As of this writing, when importing or exporting a `Group`, `Project`s are not included in the operation. If that functionality is changed to include `Project` when its Group is imported/exported, the logic must be sure to include their corresponding project namespaces in the import/export. + +After ensuring the above points, we plan to run a DB migration to create a `ProjectNamespace` record for every `Project`. +Project namespace records created during migration should have `created_at`/`updated_at` attributes set to migration +runtime. That means that project namespaces `created_at`/`updated_at` attributes don't match their corresponding +project's `created_at`/`updated_at` attributes. We want the different dates to help audit any of the created project +namespaces, in case we need it. We plan to run the back-filling migration in 14.5 milestone. + +### Phase 2 + +Epic tracking [Phase 2](https://gitlab.com/groups/gitlab-org/-/epics/6768) + +The focus of this phase is to make `ProjectNamespace` the front entity to interact with instead of `Project` . +Problems to solve as part of this phase: + +- Routes handling through project namespace rather than project. +- Memberships handling. +- Policies handling. +- Import/export. +- Other interactions between project namespace and project models. + +Communicate the changes company wide at the engineers level. We want engineers to be aware of the upcoming changes even +though active collaboration of teams is expected only in phase 3. Raise awareness to avoid regressions, conflicting or duplicate work +that can be taken care of even before phase 3. + +### Phase 3 + +Epic tracking [Phase 3](https://gitlab.com/groups/gitlab-org/-/epics/6585) + +The focus of this phase is to get feature parity between the namespace types. Phase 3 is when active migration +of features from `Project` to `ProjectNamespace` or directly to `Namespace` happens. diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index d3c96a0f934..3e09d488bc3 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -74,16 +74,32 @@ module Gitlab end def protection_options - { - "Not protected: Both developers and maintainers can push new commits and force push." => PROTECTION_NONE, - "Protected against pushes: Developers cannot push new commits, but are allowed to accept merge requests to the branch. Maintainers can push to the branch." => PROTECTION_DEV_CAN_MERGE, - "Partially protected: Both developers and maintainers can push new commits, but cannot force push." => PROTECTION_DEV_CAN_PUSH, - "Fully protected: Developers cannot push new commits, but maintainers can. No one can force push." => PROTECTION_FULL - } + [ + { + label: s_('DefaultBranchProtection|Not protected'), + help_text: s_('DefaultBranchProtection|Both developers and maintainers can push new commits, force push, or delete the branch.'), + value: PROTECTION_NONE + }, + { + label: s_('DefaultBranchProtection|Protected against pushes'), + help_text: s_('DefaultBranchProtection|Developers cannot push new commits, but are allowed to accept merge requests to the branch. Maintainers can push to the branch.'), + value: PROTECTION_DEV_CAN_MERGE + }, + { + label: s_('DefaultBranchProtection|Partially protected'), + help_text: s_('DefaultBranchProtection|Both developers and maintainers can push new commits, but cannot force push.'), + value: PROTECTION_DEV_CAN_PUSH + }, + { + label: s_('DefaultBranchProtection|Fully protected'), + help_text: s_('DefaultBranchProtection|Developers cannot push new commits, but maintainers can. No one can force push.'), + value: PROTECTION_FULL + } + ] end def protection_values - protection_options.values + protection_options.map { |option| option[:value] } end def human_access(access) diff --git a/lib/gitlab/database/partitioning/detached_partition_dropper.rb b/lib/gitlab/database/partitioning/detached_partition_dropper.rb index 3e7ddece20b..593824384b5 100644 --- a/lib/gitlab/database/partitioning/detached_partition_dropper.rb +++ b/lib/gitlab/database/partitioning/detached_partition_dropper.rb @@ -9,13 +9,10 @@ module Gitlab Gitlab::AppLogger.info(message: "Checking for previously detached partitions to drop") Postgresql::DetachedPartition.ready_to_drop.find_each do |detached_partition| - connection.transaction do - # Another process may have already dropped the table and deleted this entry - next unless (detached_partition = Postgresql::DetachedPartition.lock.find_by(id: detached_partition.id)) - - drop_detached_partition(detached_partition.table_name) - - detached_partition.destroy! + if partition_attached?(qualify_partition_name(detached_partition.table_name)) + unmark_partition(detached_partition) + else + drop_partition(detached_partition) end rescue StandardError => e Gitlab::AppLogger.error(message: "Failed to drop previously detached partition", @@ -27,31 +24,100 @@ module Gitlab private + def unmark_partition(detached_partition) + connection.transaction do + # Another process may have already encountered this case and deleted this entry + next unless try_lock_detached_partition(detached_partition.id) + + # The current partition was scheduled for deletion incorrectly + # Dropping it now could delete in-use data and take locks that interrupt other database activity + Gitlab::AppLogger.error(message: "Prevented an attempt to drop an attached database partition", partition_name: detached_partition.table_name) + detached_partition.destroy! + end + end + + def drop_partition(detached_partition) + remove_foreign_keys(detached_partition) + + connection.transaction do + # Another process may have already dropped the table and deleted this entry + next unless try_lock_detached_partition(detached_partition.id) + + drop_detached_partition(detached_partition.table_name) + + detached_partition.destroy! + end + end + + def remove_foreign_keys(detached_partition) + partition_identifier = qualify_partition_name(detached_partition.table_name) + + # We want to load all of these into memory at once to get a consistent view to loop over, + # since we'll be deleting from this list as we go + fks_to_drop = PostgresForeignKey.by_constrained_table_identifier(partition_identifier).to_a + fks_to_drop.each do |foreign_key| + drop_foreign_key_if_present(detached_partition, foreign_key) + end + end + + # Drops the given foreign key for the given detached partition, but only if another process has not already + # detached the partition first. This method must be safe to call even if the associated partition table has already + # been detached, as it could be called by multiple processes at once. + def drop_foreign_key_if_present(detached_partition, foreign_key) + # It is important to only drop one foreign key per transaction. + # Dropping a foreign key takes an ACCESS EXCLUSIVE lock on both tables participating in the foreign key. + + partition_identifier = qualify_partition_name(detached_partition.table_name) + with_lock_retries do + connection.transaction(requires_new: false) do + next unless try_lock_detached_partition(detached_partition.id) + + # Another process may have already dropped this foreign key + next unless PostgresForeignKey.by_constrained_table_identifier(partition_identifier).where(name: foreign_key.name).exists? + + connection.execute("ALTER TABLE #{connection.quote_table_name(partition_identifier)} DROP CONSTRAINT #{connection.quote_table_name(foreign_key.name)}") + + Gitlab::AppLogger.info(message: "Dropped foreign key for previously detached partition", + partition_name: detached_partition.table_name, + referenced_table_name: foreign_key.referenced_table_identifier, + foreign_key_name: foreign_key.name) + end + end + end + def drop_detached_partition(partition_name) partition_identifier = qualify_partition_name(partition_name) - if partition_detached?(partition_identifier) - connection.drop_table(partition_identifier, if_exists: true) + connection.drop_table(partition_identifier, if_exists: true) - Gitlab::AppLogger.info(message: "Dropped previously detached partition", partition_name: partition_name) - else - Gitlab::AppLogger.error(message: "Attempt to drop attached database partition", partition_name: partition_name) - end + Gitlab::AppLogger.info(message: "Dropped previously detached partition", partition_name: partition_name) end def qualify_partition_name(table_name) "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.#{table_name}" end - def partition_detached?(partition_identifier) + def partition_attached?(partition_identifier) # PostgresPartition checks the pg_inherits view, so our partition will only show here if it's still attached # and thus should not be dropped - !Gitlab::Database::PostgresPartition.for_identifier(partition_identifier).exists? + Gitlab::Database::PostgresPartition.for_identifier(partition_identifier).exists? + end + + def try_lock_detached_partition(id) + Postgresql::DetachedPartition.lock.find_by(id: id).present? end def connection Postgresql::DetachedPartition.connection end + + def with_lock_retries(&block) + Gitlab::Database::WithLockRetries.new( + klass: self.class, + logger: Gitlab::AppLogger, + connection: connection + ).run(raise_on_exhaustion: true, &block) + end end end end diff --git a/lib/gitlab/database/postgres_foreign_key.rb b/lib/gitlab/database/postgres_foreign_key.rb index 72640f8785d..241b6f009f7 100644 --- a/lib/gitlab/database/postgres_foreign_key.rb +++ b/lib/gitlab/database/postgres_foreign_key.rb @@ -10,6 +10,12 @@ module Gitlab where(referenced_table_identifier: identifier) end + + scope :by_constrained_table_identifier, ->(identifier) do + raise ArgumentError, "Constrained table name is not fully qualified with a schema: #{identifier}" unless identifier =~ /^\w+\.\w+$/ + + where(constrained_table_identifier: identifier) + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3841c7bcbec..1cfd1094f18 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10897,6 +10897,9 @@ msgstr "" msgid "Default branch and protected branches" msgstr "" +msgid "Default branch protection" +msgstr "" + msgid "Default classification label" msgstr "" @@ -10936,6 +10939,30 @@ msgstr "" msgid "DefaultBranchLabel|default" msgstr "" +msgid "DefaultBranchProtection|Both developers and maintainers can push new commits, but cannot force push." +msgstr "" + +msgid "DefaultBranchProtection|Both developers and maintainers can push new commits, force push, or delete the branch." +msgstr "" + +msgid "DefaultBranchProtection|Developers cannot push new commits, but are allowed to accept merge requests to the branch. Maintainers can push to the branch." +msgstr "" + +msgid "DefaultBranchProtection|Developers cannot push new commits, but maintainers can. No one can force push." +msgstr "" + +msgid "DefaultBranchProtection|Fully protected" +msgstr "" + +msgid "DefaultBranchProtection|Not protected" +msgstr "" + +msgid "DefaultBranchProtection|Partially protected" +msgstr "" + +msgid "DefaultBranchProtection|Protected against pushes" +msgstr "" + msgid "Define a custom deploy freeze pattern with %{cronSyntaxStart}cron syntax%{cronSyntaxEnd}" msgstr "" diff --git a/spec/lib/gitlab/database/partitioning/detached_partition_dropper_spec.rb b/spec/lib/gitlab/database/partitioning/detached_partition_dropper_spec.rb index 8c406c90e36..b2c4e4b54a4 100644 --- a/spec/lib/gitlab/database/partitioning/detached_partition_dropper_spec.rb +++ b/spec/lib/gitlab/database/partitioning/detached_partition_dropper_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do include Database::TableSchemaHelpers + subject(:dropper) { described_class.new } + let(:connection) { ActiveRecord::Base.connection } def expect_partition_present(name) @@ -23,10 +25,18 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do before do connection.execute(<<~SQL) + CREATE TABLE referenced_table ( + id bigserial primary key not null + ) + SQL + connection.execute(<<~SQL) + CREATE TABLE parent_table ( id bigserial not null, + referenced_id bigint not null, created_at timestamptz not null, - primary key (id, created_at) + primary key (id, created_at), + constraint fk_referenced foreign key (referenced_id) references referenced_table(id) ) PARTITION BY RANGE(created_at) SQL end @@ -59,7 +69,7 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do attached: false, drop_after: 1.day.from_now) - subject.perform + dropper.perform expect_partition_present('test_partition') end @@ -75,7 +85,7 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do end it 'drops the partition' do - subject.perform + dropper.perform expect(table_oid('test_partition')).to be_nil end @@ -86,16 +96,62 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do end it 'does not drop the partition' do - subject.perform + dropper.perform expect(table_oid('test_partition')).not_to be_nil end end + context 'removing foreign keys' do + it 'removes foreign keys from the table before dropping it' do + expect(dropper).to receive(:drop_detached_partition).and_wrap_original do |drop_method, partition_name| + expect(partition_name).to eq('test_partition') + expect(foreign_key_exists_by_name(partition_name, 'fk_referenced', schema: Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA)).to be_falsey + + drop_method.call(partition_name) + end + + expect(foreign_key_exists_by_name('test_partition', 'fk_referenced', schema: Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA)).to be_truthy + + dropper.perform + end + + it 'does not remove foreign keys from the parent table' do + expect { dropper.perform }.not_to change { foreign_key_exists_by_name('parent_table', 'fk_referenced') }.from(true) + end + + context 'when another process drops the foreign key' do + it 'skips dropping that foreign key' do + expect(dropper).to receive(:drop_foreign_key_if_present).and_wrap_original do |drop_meth, *args| + connection.execute('alter table gitlab_partitions_dynamic.test_partition drop constraint fk_referenced;') + drop_meth.call(*args) + end + + dropper.perform + + expect_partition_removed('test_partition') + end + end + + context 'when another process drops the partition' do + it 'skips dropping the foreign key' do + expect(dropper).to receive(:drop_foreign_key_if_present).and_wrap_original do |drop_meth, *args| + connection.execute('drop table gitlab_partitions_dynamic.test_partition') + Postgresql::DetachedPartition.where(table_name: 'test_partition').delete_all + end + + expect(Gitlab::AppLogger).not_to receive(:error) + dropper.perform + end + end + end + context 'when another process drops the table while the first waits for a lock' do it 'skips the table' do + # First call to .lock is for removing foreign keys + expect(Postgresql::DetachedPartition).to receive(:lock).once.ordered.and_call_original # Rspec's receive_method_chain does not support .and_wrap_original, so we need to nest here. - expect(Postgresql::DetachedPartition).to receive(:lock).and_wrap_original do |lock_meth| + expect(Postgresql::DetachedPartition).to receive(:lock).once.ordered.and_wrap_original do |lock_meth| locked = lock_meth.call expect(locked).to receive(:find_by).and_wrap_original do |find_meth, *find_args| # Another process drops the table then deletes this entry @@ -106,9 +162,9 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do locked end - expect(subject).not_to receive(:drop_one) + expect(dropper).not_to receive(:drop_one) - subject.perform + dropper.perform end end end @@ -123,19 +179,26 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do end it 'does not drop the partition, but does remove the DetachedPartition entry' do - subject.perform + dropper.perform aggregate_failures do expect(table_oid('test_partition')).not_to be_nil expect(Postgresql::DetachedPartition.find_by(table_name: 'test_partition')).to be_nil end end - it 'removes the detached_partition entry' do - detached_partition = Postgresql::DetachedPartition.find_by!(table_name: 'test_partition') + context 'when another process removes the entry before this process' do + it 'does nothing' do + expect(Postgresql::DetachedPartition).to receive(:lock).and_wrap_original do |lock_meth| + Postgresql::DetachedPartition.delete_all + lock_meth.call + end - subject.perform + expect(Gitlab::AppLogger).not_to receive(:error) - expect(Postgresql::DetachedPartition.exists?(id: detached_partition.id)).to be_falsey + dropper.perform + + expect(table_oid('test_partition')).not_to be_nil + end end end @@ -155,7 +218,7 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do end it 'drops both partitions' do - subject.perform + dropper.perform expect_partition_removed('partition_1') expect_partition_removed('partition_2') @@ -163,10 +226,10 @@ RSpec.describe Gitlab::Database::Partitioning::DetachedPartitionDropper do context 'when the first drop returns an error' do it 'still drops the second partition' do - expect(subject).to receive(:drop_detached_partition).ordered.and_raise('injected error') - expect(subject).to receive(:drop_detached_partition).ordered.and_call_original + expect(dropper).to receive(:drop_detached_partition).ordered.and_raise('injected error') + expect(dropper).to receive(:drop_detached_partition).ordered.and_call_original - subject.perform + dropper.perform # We don't know which partition we tried to drop first, so the tests here have to work with either one expect(Postgresql::DetachedPartition.count).to eq(1) diff --git a/spec/lib/gitlab/database/postgres_foreign_key_spec.rb b/spec/lib/gitlab/database/postgres_foreign_key_spec.rb index ec39e5bfee7..b0e08ca1e67 100644 --- a/spec/lib/gitlab/database/postgres_foreign_key_spec.rb +++ b/spec/lib/gitlab/database/postgres_foreign_key_spec.rb @@ -38,4 +38,16 @@ RSpec.describe Gitlab::Database::PostgresForeignKey, type: :model do expect(described_class.by_referenced_table_identifier('public.referenced_table')).to contain_exactly(expected) end end + + describe '#by_constrained_table_identifier' do + it 'throws an error when the identifier name is not fully qualified' do + expect { described_class.by_constrained_table_identifier('constrained_table') }.to raise_error(ArgumentError, /not fully qualified/) + end + + it 'finds the foreign keys for the constrained table' do + expected = described_class.where(name: %w[fk_constrained_to_referenced fk_constrained_to_other_referenced]).to_a + + expect(described_class.by_constrained_table_identifier('public.constrained_table')).to match_array(expected) + end + end end