From 18186e10c5bbd893c2b276eb9b2960a08cbec698 Mon Sep 17 00:00:00 2001 From: Carla Drago Date: Sat, 16 Jul 2022 11:45:12 +0200 Subject: [PATCH 1/5] Backfill project import level on namespace settings This migration updates a group's project import level to match the project creation level, wherever a group has a project creation level set. Changelog: other --- ...715152108_backfill_project_import_level.rb | 25 +++++++ db/schema_migrations/20220715152108 | 1 + .../backfill_project_import_level.rb | 33 +++++++++ .../backfill_project_import_level_spec.rb | 72 +++++++++++++++++++ .../backfill_project_import_level_spec.rb | 29 ++++++++ .../matchers/be_able_to_import_project.rb | 12 ++++ 6 files changed, 172 insertions(+) create mode 100644 db/post_migrate/20220715152108_backfill_project_import_level.rb create mode 100644 db/schema_migrations/20220715152108 create mode 100644 lib/gitlab/background_migration/backfill_project_import_level.rb create mode 100644 spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb create mode 100644 spec/migrations/backfill_project_import_level_spec.rb create mode 100644 spec/support/matchers/be_able_to_import_project.rb diff --git a/db/post_migrate/20220715152108_backfill_project_import_level.rb b/db/post_migrate/20220715152108_backfill_project_import_level.rb new file mode 100644 index 00000000000000..0af2e63370b093 --- /dev/null +++ b/db/post_migrate/20220715152108_backfill_project_import_level.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class BackfillProjectImportLevel < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MIGRATION = 'BackfillProjectImportLevel' + DELAY_INTERVAL = 120.seconds + + def up + queue_batched_background_migration( + MIGRATION, + :namespaces, + :id, + job_interval: DELAY_INTERVAL, + ) + end + + def down + delete_batched_background_migration(MIGRATION, :namespaces, :id, []) + end +end diff --git a/db/schema_migrations/20220715152108 b/db/schema_migrations/20220715152108 new file mode 100644 index 00000000000000..23d61b453341b8 --- /dev/null +++ b/db/schema_migrations/20220715152108 @@ -0,0 +1 @@ +76f4adebfb71dcd51f861097ba441ae5ee3f62eeb2060f147730d4e6c6006402 \ No newline at end of file diff --git a/lib/gitlab/background_migration/backfill_project_import_level.rb b/lib/gitlab/background_migration/backfill_project_import_level.rb new file mode 100644 index 00000000000000..6fd76c0c686bf8 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_project_import_level.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class BackfillProjectImportLevel < BatchedMigrationJob + + LEVEL = { + 0 => Gitlab::Access::OWNER, + 1 => Gitlab::Access::MAINTAINER, + 2 => Gitlab::Access::DEVELOPER + } + + def perform + each_sub_batch(operation_name: :update_import_level) do |sub_batch| + update_import_level(sub_batch) + end + end + + private + + def update_import_level(relation) + relation.where(type: 'Group').each do |group| + group.namespace_settings.project_import_level = LEVEL[group.project_creation_level] || Gitlab::Access::OWNER + end + end + + def logger + @logger ||= Gitlab::BackgroundMigration::Logger.build + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb new file mode 100644 index 00000000000000..3292d1cc0715cf --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe Gitlab::BackgroundMigration::BackfillProjectImportLevel, + :migration, + schema: 20220715152108 do + + let!(:owner) { table(:users).create!(email: 'owner@example.com', username: 'owner', projects_limit: 10) } + let!(:developer) { table(:users).create!(email: 'developer@example.com', username: 'developer', projects_limit: 10) } + let!(:guest) { table(:users).create!(email: 'guest@example.com', username: 'guest', projects_limit: 10) } + + let!(:group) { create(:group, projects: [project]) } + + let!(:project) { create(:project, name: 'project', path: 'project_path') } + + let(:migration) do + described_class.new( + start_id: 1, + end_id: 30, + batch_table: :namespaces, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: ApplicationRecord.connection + ) + end + + let(:perform_migration) { migration.perform } + + context 'when namespace is a Project' do + it 'does not update the project_import_level' do + expect { perform_migration } + .to not_change { group.namespace_settings } + end + end + + context 'when namespace is a Group with project_creation_level OWNER' do + + it 'does not update the project_import_level' do + expect { perform_migration } + .to not_change { group.namespace_settings } + + expect(owner).to be_able_to_import_project + expect(developer).to_not be_able_to_import_project + expect(guest).to_not be_able_to_import_project + end + end + + context 'when namespace is a Group with project_creation_level DEVELOPER' do + it 'updates the project_import_level' do + expect { perform_migration } + .to change { group.namespace_settings } + + expect(owner).to be_able_to_import_project + expect(developer).to be_able_to_import_project + expect(guest).to_not be_able_to_import_project + end + end + + context 'when namespace is a Group with project_creation_level GUEST' do + it 'updates the project_import_level' do + expect { perform_migration } + .to change { group.namespace_settings } + + expect(owner).to be_able_to_import_project + expect(developer).to be_able_to_import_project + expect(guest).to be_able_to_import_project + end + end +end diff --git a/spec/migrations/backfill_project_import_level_spec.rb b/spec/migrations/backfill_project_import_level_spec.rb new file mode 100644 index 00000000000000..a16bfa68d824b0 --- /dev/null +++ b/spec/migrations/backfill_project_import_level_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe BackfillProjectImportLevel do + let_it_be(:batched_migration) { described_class::MIGRATION } + + describe '#up' do + it 'schedules background jobs for each batch of namespaces' do + migrate! + + expect(migration).to have_scheduled_batched_migration( + table_name: :namespaces, + column_name: :id, + interval: described_class::INTERVAL + ) + end + end + + describe '#down' do + it 'deletes all batched migration records' do + migrate! + schema_migrate_down! + + expect(migration).not_to have_scheduled_batched_migration + end + end +end diff --git a/spec/support/matchers/be_able_to_import_project.rb b/spec/support/matchers/be_able_to_import_project.rb new file mode 100644 index 00000000000000..3e49a6b0bba3de --- /dev/null +++ b/spec/support/matchers/be_able_to_import_project.rb @@ -0,0 +1,12 @@ +RSpec::Matchers.define :be_able_to_import_project do |expected| + match do |member| + byebug + expect(member.can?(:import_project, :group)).to be(true) + end + + failure_message do |member| + + "expected #{member} to be able to import projects but their" \ + "access_level of #{member.access_level} did meet requirements." + end +end -- GitLab From 3b0607af4d6dc653cae64252aaac3fbdcf3a3f72 Mon Sep 17 00:00:00 2001 From: Carla Drago Date: Mon, 25 Jul 2022 16:53:39 +0200 Subject: [PATCH 2/5] Add backfill migration to align import_level to creation_level This batched background migration aligns the project_import_level in existing group namespaces to their corresponding values for project_creation_level. This is to avoid confusion when the ability to manage project_import_levels go live. Changelog: other Update mapping to include NO_ACCESS option Update start and end point of migration --- ...715152108_backfill_project_import_level.rb | 4 +- .../backfill_project_import_level.rb | 22 ++--- .../backfill_project_import_level_spec.rb | 98 +++++++++++-------- .../backfill_project_import_level_spec.rb | 4 +- .../matchers/be_able_to_import_project.rb | 12 --- 5 files changed, 72 insertions(+), 68 deletions(-) delete mode 100644 spec/support/matchers/be_able_to_import_project.rb diff --git a/db/post_migrate/20220715152108_backfill_project_import_level.rb b/db/post_migrate/20220715152108_backfill_project_import_level.rb index 0af2e63370b093..afe3e3369f22fb 100644 --- a/db/post_migrate/20220715152108_backfill_project_import_level.rb +++ b/db/post_migrate/20220715152108_backfill_project_import_level.rb @@ -8,14 +8,14 @@ class BackfillProjectImportLevel < Gitlab::Database::Migration[2.0] restrict_gitlab_migration gitlab_schema: :gitlab_main MIGRATION = 'BackfillProjectImportLevel' - DELAY_INTERVAL = 120.seconds + INTERVAL = 120.seconds def up queue_batched_background_migration( MIGRATION, :namespaces, :id, - job_interval: DELAY_INTERVAL, + job_interval: INTERVAL ) end diff --git a/lib/gitlab/background_migration/backfill_project_import_level.rb b/lib/gitlab/background_migration/backfill_project_import_level.rb index 6fd76c0c686bf8..3715db2231db23 100644 --- a/lib/gitlab/background_migration/backfill_project_import_level.rb +++ b/lib/gitlab/background_migration/backfill_project_import_level.rb @@ -4,12 +4,15 @@ module Gitlab module BackgroundMigration class BackfillProjectImportLevel < BatchedMigrationJob - LEVEL = { - 0 => Gitlab::Access::OWNER, - 1 => Gitlab::Access::MAINTAINER, - 2 => Gitlab::Access::DEVELOPER - } + nil => Gitlab::Access::OWNER, + 0 => Gitlab::Access::NO_ACCESS, + 1 => Gitlab::Access::MAINTAINER, + 2 => Gitlab::Access::DEVELOPER, + 3 => Gitlab::Access::DEVELOPER, + 4 => Gitlab::Access::DEVELOPER, + 5 => Gitlab::Access::DEVELOPER + }.freeze def perform each_sub_batch(operation_name: :update_import_level) do |sub_batch| @@ -20,14 +23,11 @@ def perform private def update_import_level(relation) - relation.where(type: 'Group').each do |group| - group.namespace_settings.project_import_level = LEVEL[group.project_creation_level] || Gitlab::Access::OWNER + relation.where(type: 'Group').each do |namespace| + NamespaceSetting.find_by(namespace_id: namespace.id) + .update!(project_import_level: LEVEL[namespace.project_creation_level]) end end - - def logger - @logger ||= Gitlab::BackgroundMigration::Logger.build - end end end end diff --git a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb index 3292d1cc0715cf..ec9fde1d564eeb 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb @@ -6,19 +6,10 @@ RSpec.describe Gitlab::BackgroundMigration::BackfillProjectImportLevel, :migration, schema: 20220715152108 do - - let!(:owner) { table(:users).create!(email: 'owner@example.com', username: 'owner', projects_limit: 10) } - let!(:developer) { table(:users).create!(email: 'developer@example.com', username: 'developer', projects_limit: 10) } - let!(:guest) { table(:users).create!(email: 'guest@example.com', username: 'guest', projects_limit: 10) } - - let!(:group) { create(:group, projects: [project]) } - - let!(:project) { create(:project, name: 'project', path: 'project_path') } - let(:migration) do described_class.new( - start_id: 1, - end_id: 30, + start_id: Namespace.minimum(:id), + end_id: Namespace.maximum(:id), batch_table: :namespaces, batch_column: :id, sub_batch_size: 2, @@ -27,46 +18,71 @@ ) end - let(:perform_migration) { migration.perform } + let(:namespaces_table) { table(:namespaces) } + let(:projects_table) { table(:projects) } + let(:namespace_settings_table) { table(:namespace_settings) } - context 'when namespace is a Project' do - it 'does not update the project_import_level' do - expect { perform_migration } - .to not_change { group.namespace_settings } - end + let!(:namespace1) do + namespaces_table.create!( + name: 'namespace1', + path: 'namespace1', + type: 'User', + project_creation_level: nil + ) end - context 'when namespace is a Group with project_creation_level OWNER' do + let!(:namespace2) do + namespaces_table.create!( + name: 'namespace2', + path: 'namespace2', + type: 'Group', + project_creation_level: 0 + ) + end - it 'does not update the project_import_level' do - expect { perform_migration } - .to not_change { group.namespace_settings } + let!(:namespace3) do + namespaces_table.create!( + name: 'namespace3', + path: 'namespace3', + type: 'Group', + project_creation_level: 2 + ) + end - expect(owner).to be_able_to_import_project - expect(developer).to_not be_able_to_import_project - expect(guest).to_not be_able_to_import_project - end + let!(:namespace4) do + namespaces_table.create!( + name: 'namespace4', + path: 'namespace4', + type: 'Group', + project_creation_level: 4 + ) end - context 'when namespace is a Group with project_creation_level DEVELOPER' do - it 'updates the project_import_level' do - expect { perform_migration } - .to change { group.namespace_settings } + let(:default_level) { ::Gitlab::Access::OWNER } + let(:lowest_level) { ::Gitlab::Access::DEVELOPER } + + subject(:perform_migration) { migration.perform } - expect(owner).to be_able_to_import_project - expect(developer).to be_able_to_import_project - expect(guest).to_not be_able_to_import_project - end + before do + namespace_settings_table.create!(namespace_id: namespace1.id) + namespace_settings_table.create!(namespace_id: namespace2.id) + namespace_settings_table.create!(namespace_id: namespace3.id) + namespace_settings_table.create!(namespace_id: namespace4.id) end - context 'when namespace is a Group with project_creation_level GUEST' do - it 'updates the project_import_level' do - expect { perform_migration } - .to change { group.namespace_settings } + it 'backfills the correct `project_import_level`', :aggregate_failures do + perform_migration + + expect(namespace_settings_table.find_by(namespace_id: namespace1.id).project_import_level) + .to eq(default_level) + + expect(namespace_settings_table.find_by(namespace_id: namespace2.id).project_import_level) + .to eq(::Gitlab::Access::NO_ACCESS) + + expect(namespace_settings_table.find_by(namespace_id: namespace3.id).project_import_level) + .to eq(::Gitlab::Access::DEVELOPER) - expect(owner).to be_able_to_import_project - expect(developer).to be_able_to_import_project - expect(guest).to be_able_to_import_project - end + expect(namespace_settings_table.find_by(namespace_id: namespace4.id).project_import_level) + .to eq(lowest_level) end end diff --git a/spec/migrations/backfill_project_import_level_spec.rb b/spec/migrations/backfill_project_import_level_spec.rb index a16bfa68d824b0..c24ddac0730fbc 100644 --- a/spec/migrations/backfill_project_import_level_spec.rb +++ b/spec/migrations/backfill_project_import_level_spec.rb @@ -10,7 +10,7 @@ it 'schedules background jobs for each batch of namespaces' do migrate! - expect(migration).to have_scheduled_batched_migration( + expect(batched_migration).to have_scheduled_batched_migration( table_name: :namespaces, column_name: :id, interval: described_class::INTERVAL @@ -23,7 +23,7 @@ migrate! schema_migrate_down! - expect(migration).not_to have_scheduled_batched_migration + expect(batched_migration).not_to have_scheduled_batched_migration end end end diff --git a/spec/support/matchers/be_able_to_import_project.rb b/spec/support/matchers/be_able_to_import_project.rb deleted file mode 100644 index 3e49a6b0bba3de..00000000000000 --- a/spec/support/matchers/be_able_to_import_project.rb +++ /dev/null @@ -1,12 +0,0 @@ -RSpec::Matchers.define :be_able_to_import_project do |expected| - match do |member| - byebug - expect(member.can?(:import_project, :group)).to be(true) - end - - failure_message do |member| - - "expected #{member} to be able to import projects but their" \ - "access_level of #{member.access_level} did meet requirements." - end -end -- GitLab From 934518a37f7ed2b43f23d4638596c573b5ff677c Mon Sep 17 00:00:00 2001 From: Carla Drago Date: Thu, 28 Jul 2022 17:18:07 +0200 Subject: [PATCH 3/5] Address comments in merge request --- ...715152108_backfill_project_import_level.rb | 3 -- .../backfill_project_import_level.rb | 21 ++++++------- .../backfill_project_import_level_spec.rb | 30 +++++++++++++++++-- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/db/post_migrate/20220715152108_backfill_project_import_level.rb b/db/post_migrate/20220715152108_backfill_project_import_level.rb index afe3e3369f22fb..65a0dc0a58afc6 100644 --- a/db/post_migrate/20220715152108_backfill_project_import_level.rb +++ b/db/post_migrate/20220715152108_backfill_project_import_level.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class BackfillProjectImportLevel < Gitlab::Database::Migration[2.0] disable_ddl_transaction! restrict_gitlab_migration gitlab_schema: :gitlab_main diff --git a/lib/gitlab/background_migration/backfill_project_import_level.rb b/lib/gitlab/background_migration/backfill_project_import_level.rb index 3715db2231db23..88fddae2e24858 100644 --- a/lib/gitlab/background_migration/backfill_project_import_level.rb +++ b/lib/gitlab/background_migration/backfill_project_import_level.rb @@ -5,13 +5,10 @@ module Gitlab module BackgroundMigration class BackfillProjectImportLevel < BatchedMigrationJob LEVEL = { - nil => Gitlab::Access::OWNER, - 0 => Gitlab::Access::NO_ACCESS, - 1 => Gitlab::Access::MAINTAINER, - 2 => Gitlab::Access::DEVELOPER, - 3 => Gitlab::Access::DEVELOPER, - 4 => Gitlab::Access::DEVELOPER, - 5 => Gitlab::Access::DEVELOPER + Gitlab::Access::NO_ACCESS => [0], + Gitlab::Access::DEVELOPER => [2, 3, 4, 5], + Gitlab::Access::MAINTAINER => [1], + Gitlab::Access::OWNER => [nil] }.freeze def perform @@ -23,9 +20,13 @@ def perform private def update_import_level(relation) - relation.where(type: 'Group').each do |namespace| - NamespaceSetting.find_by(namespace_id: namespace.id) - .update!(project_import_level: LEVEL[namespace.project_creation_level]) + LEVEL.each do |import_level, creation_level| + namespace_ids = relation + .where(type: 'Group', project_creation_level: creation_level) + + NamespaceSetting.where( + namespace_id: namespace_ids + ).update_all(project_import_level: import_level) end end end diff --git a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb index ec9fde1d564eeb..bbdc1fa10f9cb3 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb @@ -8,8 +8,8 @@ schema: 20220715152108 do let(:migration) do described_class.new( - start_id: Namespace.minimum(:id), - end_id: Namespace.maximum(:id), + start_id: table(:namespaces).minimum(:id), + end_id: table(:namespaces).maximum(:id), batch_table: :namespaces, batch_column: :id, sub_batch_size: 2, @@ -58,6 +58,24 @@ ) end + let!(:namespace5) do + namespaces_table.create!( + name: 'namespace5', + path: 'namespace5', + type: 'Group', + project_creation_level: 5 + ) + end + + let!(:namespace9999) do + namespaces_table.create!( + name: 'namespace9999', + path: 'namespace9999', + type: 'Group', + project_creation_level: 9999 + ) + end + let(:default_level) { ::Gitlab::Access::OWNER } let(:lowest_level) { ::Gitlab::Access::DEVELOPER } @@ -68,6 +86,8 @@ namespace_settings_table.create!(namespace_id: namespace2.id) namespace_settings_table.create!(namespace_id: namespace3.id) namespace_settings_table.create!(namespace_id: namespace4.id) + namespace_settings_table.create!(namespace_id: namespace5.id) + namespace_settings_table.create!(namespace_id: namespace9999.id) end it 'backfills the correct `project_import_level`', :aggregate_failures do @@ -84,5 +104,11 @@ expect(namespace_settings_table.find_by(namespace_id: namespace4.id).project_import_level) .to eq(lowest_level) + + expect(namespace_settings_table.find_by(namespace_id: namespace5.id).project_import_level) + .to eq(lowest_level) + + expect(namespace_settings_table.find_by(namespace_id: namespace9999.id).project_import_level) + .to eq(default_level) end end -- GitLab From 472f67e47fc14daefd51dfadd56a5b9c89d99723 Mon Sep 17 00:00:00 2001 From: Carla Drago Date: Fri, 29 Jul 2022 10:07:18 +0200 Subject: [PATCH 4/5] Further improvements to tests --- .../backfill_project_import_level_spec.rb | 97 +++++++++---------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb index bbdc1fa10f9cb3..756998037d9f4f 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb @@ -3,9 +3,7 @@ require 'spec_helper' require_migration! -RSpec.describe Gitlab::BackgroundMigration::BackfillProjectImportLevel, - :migration, - schema: 20220715152108 do +RSpec.describe Gitlab::BackgroundMigration::BackfillProjectImportLevel do let(:migration) do described_class.new( start_id: table(:namespaces).minimum(:id), @@ -19,96 +17,97 @@ end let(:namespaces_table) { table(:namespaces) } - let(:projects_table) { table(:projects) } let(:namespace_settings_table) { table(:namespace_settings) } - let!(:namespace1) do + let!(:user_namespace) do namespaces_table.create!( - name: 'namespace1', - path: 'namespace1', + name: 'user_namespace', + path: 'user_namespace', type: 'User', project_creation_level: nil ) end - let!(:namespace2) do + let!(:group_namespace1) do namespaces_table.create!( - name: 'namespace2', - path: 'namespace2', + name: 'group_namespace1', + path: 'group_namespace1', type: 'Group', project_creation_level: 0 ) end - let!(:namespace3) do + let!(:group_namespace2) do namespaces_table.create!( - name: 'namespace3', - path: 'namespace3', + name: 'group_namespace2', + path: 'group_namespace2', type: 'Group', project_creation_level: 2 ) end - let!(:namespace4) do + let!(:group_namespace3) do namespaces_table.create!( - name: 'namespace4', - path: 'namespace4', + name: 'group_namespace3', + path: 'group_namespace3', type: 'Group', project_creation_level: 4 ) end - let!(:namespace5) do + let!(:group_namespace4) do namespaces_table.create!( - name: 'namespace5', - path: 'namespace5', + name: 'group_namespace4', + path: 'group_namespace4', type: 'Group', project_creation_level: 5 ) end - let!(:namespace9999) do + let!(:group_namespace9999) do namespaces_table.create!( - name: 'namespace9999', - path: 'namespace9999', + name: 'group_namespace9999', + path: 'group_namespace9999', type: 'Group', project_creation_level: 9999 ) end - let(:default_level) { ::Gitlab::Access::OWNER } - let(:lowest_level) { ::Gitlab::Access::DEVELOPER } - subject(:perform_migration) { migration.perform } before do - namespace_settings_table.create!(namespace_id: namespace1.id) - namespace_settings_table.create!(namespace_id: namespace2.id) - namespace_settings_table.create!(namespace_id: namespace3.id) - namespace_settings_table.create!(namespace_id: namespace4.id) - namespace_settings_table.create!(namespace_id: namespace5.id) - namespace_settings_table.create!(namespace_id: namespace9999.id) + namespace_settings_table.create!(namespace_id: user_namespace.id) + namespace_settings_table.create!(namespace_id: group_namespace1.id) + namespace_settings_table.create!(namespace_id: group_namespace2.id) + namespace_settings_table.create!(namespace_id: group_namespace3.id) + namespace_settings_table.create!(namespace_id: group_namespace4.id) + namespace_settings_table.create!(namespace_id: group_namespace9999.id) end - it 'backfills the correct `project_import_level`', :aggregate_failures do - perform_migration - - expect(namespace_settings_table.find_by(namespace_id: namespace1.id).project_import_level) - .to eq(default_level) - - expect(namespace_settings_table.find_by(namespace_id: namespace2.id).project_import_level) - .to eq(::Gitlab::Access::NO_ACCESS) - - expect(namespace_settings_table.find_by(namespace_id: namespace3.id).project_import_level) - .to eq(::Gitlab::Access::DEVELOPER) - - expect(namespace_settings_table.find_by(namespace_id: namespace4.id).project_import_level) - .to eq(lowest_level) + describe 'Groups' do + using RSpec::Parameterized::TableSyntax + + where(:namespace_id, :prev_level, :new_level) do + lazy { group_namespace1.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::NO_ACCESS + lazy { group_namespace2.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER + lazy { group_namespace3.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER + lazy { group_namespace4.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER + end + + with_them do + it 'backfills the correct `project_import_level` of `Group` namespaces' do + expect { perform_migration } + .to change { namespace_settings_table.find_by(namespace_id: namespace_id).project_import_level } + .from(prev_level).to(new_level) + end + end + end - expect(namespace_settings_table.find_by(namespace_id: namespace5.id).project_import_level) - .to eq(lowest_level) + it 'does not update `User` namespaces' do + expect { perform_migration } + .not_to change { namespace_settings_table.find_by(namespace_id: user_namespace.id).project_import_level } - expect(namespace_settings_table.find_by(namespace_id: namespace9999.id).project_import_level) - .to eq(default_level) + expect { perform_migration } + .not_to change { namespace_settings_table.find_by(namespace_id: group_namespace9999.id).project_import_level } end end -- GitLab From d8220d0824351c01bcc5324243e612803b1fdbe6 Mon Sep 17 00:00:00 2001 From: Carla Drago Date: Thu, 4 Aug 2022 10:50:41 +0200 Subject: [PATCH 5/5] Remove reporter and guest, improve tests --- .../backfill_project_import_level.rb | 5 +- .../backfill_project_import_level_spec.rb | 96 ++++++++++--------- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/lib/gitlab/background_migration/backfill_project_import_level.rb b/lib/gitlab/background_migration/backfill_project_import_level.rb index 88fddae2e24858..06706b729ea968 100644 --- a/lib/gitlab/background_migration/backfill_project_import_level.rb +++ b/lib/gitlab/background_migration/backfill_project_import_level.rb @@ -1,12 +1,11 @@ # frozen_string_literal: true # rubocop:disable Style/Documentation - module Gitlab module BackgroundMigration class BackfillProjectImportLevel < BatchedMigrationJob LEVEL = { Gitlab::Access::NO_ACCESS => [0], - Gitlab::Access::DEVELOPER => [2, 3, 4, 5], + Gitlab::Access::DEVELOPER => [2], Gitlab::Access::MAINTAINER => [1], Gitlab::Access::OWNER => [nil] }.freeze @@ -32,3 +31,5 @@ def update_import_level(relation) end end end + +# rubocop:enable Style/Documentation diff --git a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb index 756998037d9f4f..ae296483166799 100644 --- a/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_project_import_level_spec.rb @@ -3,18 +3,20 @@ require 'spec_helper' require_migration! +# rubocop:disable Layout/HashAlignment RSpec.describe Gitlab::BackgroundMigration::BackfillProjectImportLevel do let(:migration) do described_class.new( - start_id: table(:namespaces).minimum(:id), - end_id: table(:namespaces).maximum(:id), - batch_table: :namespaces, - batch_column: :id, - sub_batch_size: 2, - pause_ms: 0, - connection: ApplicationRecord.connection + start_id: table(:namespaces).minimum(:id), + end_id: table(:namespaces).maximum(:id), + batch_table: :namespaces, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: ApplicationRecord.connection ) end + # rubocop:enable Layout/HashAlignment let(:namespaces_table) { table(:namespaces) } let(:namespace_settings_table) { table(:namespace_settings) } @@ -24,50 +26,50 @@ name: 'user_namespace', path: 'user_namespace', type: 'User', - project_creation_level: nil + project_creation_level: 100 ) end - let!(:group_namespace1) do + let!(:group_namespace_nil) do namespaces_table.create!( - name: 'group_namespace1', - path: 'group_namespace1', + name: 'group_namespace_nil', + path: 'group_namespace_nil', type: 'Group', - project_creation_level: 0 + project_creation_level: nil ) end - let!(:group_namespace2) do + let!(:group_namespace_0) do namespaces_table.create!( - name: 'group_namespace2', - path: 'group_namespace2', + name: 'group_namespace_0', + path: 'group_namespace_0', type: 'Group', - project_creation_level: 2 + project_creation_level: 0 ) end - let!(:group_namespace3) do + let!(:group_namespace_1) do namespaces_table.create!( - name: 'group_namespace3', - path: 'group_namespace3', + name: 'group_namespace_1', + path: 'group_namespace_1', type: 'Group', - project_creation_level: 4 + project_creation_level: 1 ) end - let!(:group_namespace4) do + let!(:group_namespace_2) do namespaces_table.create!( - name: 'group_namespace4', - path: 'group_namespace4', + name: 'group_namespace_2', + path: 'group_namespace_2', type: 'Group', - project_creation_level: 5 + project_creation_level: 2 ) end - let!(:group_namespace9999) do + let!(:group_namespace_9999) do namespaces_table.create!( - name: 'group_namespace9999', - path: 'group_namespace9999', + name: 'group_namespace_9999', + path: 'group_namespace_9999', type: 'Group', project_creation_level: 9999 ) @@ -77,37 +79,45 @@ before do namespace_settings_table.create!(namespace_id: user_namespace.id) - namespace_settings_table.create!(namespace_id: group_namespace1.id) - namespace_settings_table.create!(namespace_id: group_namespace2.id) - namespace_settings_table.create!(namespace_id: group_namespace3.id) - namespace_settings_table.create!(namespace_id: group_namespace4.id) - namespace_settings_table.create!(namespace_id: group_namespace9999.id) + namespace_settings_table.create!(namespace_id: group_namespace_nil.id) + namespace_settings_table.create!(namespace_id: group_namespace_0.id) + namespace_settings_table.create!(namespace_id: group_namespace_1.id) + namespace_settings_table.create!(namespace_id: group_namespace_2.id) + namespace_settings_table.create!(namespace_id: group_namespace_9999.id) end describe 'Groups' do using RSpec::Parameterized::TableSyntax where(:namespace_id, :prev_level, :new_level) do - lazy { group_namespace1.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::NO_ACCESS - lazy { group_namespace2.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER - lazy { group_namespace3.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER - lazy { group_namespace4.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER + lazy { group_namespace_0.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::NO_ACCESS + lazy { group_namespace_1.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::MAINTAINER + lazy { group_namespace_2.id } | ::Gitlab::Access::OWNER | ::Gitlab::Access::DEVELOPER end with_them do - it 'backfills the correct `project_import_level` of `Group` namespaces' do + it 'backfills the correct project_import_level of Group namespaces' do expect { perform_migration } .to change { namespace_settings_table.find_by(namespace_id: namespace_id).project_import_level } .from(prev_level).to(new_level) end end - end - it 'does not update `User` namespaces' do - expect { perform_migration } - .not_to change { namespace_settings_table.find_by(namespace_id: user_namespace.id).project_import_level } + it 'does not update `User` namespaces or values outside range' do + expect { perform_migration } + .not_to change { namespace_settings_table.find_by(namespace_id: user_namespace.id).project_import_level } + + expect { perform_migration } + .not_to change { namespace_settings_table.find_by(namespace_id: group_namespace_9999.id).project_import_level } + end + + it 'maintains default import_level if creation_level is nil' do + project_import_level = namespace_settings_table.find_by(namespace_id: group_namespace_nil.id).project_import_level + + expect { perform_migration } + .not_to change { project_import_level } - expect { perform_migration } - .not_to change { namespace_settings_table.find_by(namespace_id: group_namespace9999.id).project_import_level } + expect(project_import_level).to eq(::Gitlab::Access::OWNER) + end end end -- GitLab