diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 3d8c349910ee962b20db94020cc72a9c724967cd..bfbadc16960da3fc28947c9fb54c6c942597e9bb 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -2577,6 +2577,15 @@ :weight: 1 :idempotent: false :tags: [] +- :name: bitbucket_server_import_stage_import_users + :worker_name: Gitlab::BitbucketServerImport::Stage::ImportUsersWorker + :feature_category: :importers + :has_external_dependencies: true + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: false + :tags: [] - :name: bulk_import :worker_name: BulkImportWorker :feature_category: :importers diff --git a/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb b/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb index b378d07d59cd6040a7c68a0c69079522ed02dd2e..573c73cd7dfe66275ff118797ac211c68fbfda43 100644 --- a/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb +++ b/app/workers/gitlab/bitbucket_server_import/stage/import_repository_worker.rb @@ -14,7 +14,11 @@ def import(project) importer.execute - ImportPullRequestsWorker.perform_async(project.id) + if Feature.enabled?(:bitbucket_server_convert_mentions_to_users, project.creator) + ImportUsersWorker.perform_async(project.id) + else + ImportPullRequestsWorker.perform_async(project.id) + end end def importer_class diff --git a/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb b/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..dd18139fc9e3dc9fdd85dcff15045ef144784337 --- /dev/null +++ b/app/workers/gitlab/bitbucket_server_import/stage/import_users_worker.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module BitbucketServerImport + module Stage + class ImportUsersWorker # rubocop:disable Scalability/IdempotentWorker -- ImportPullRequestsWorker is not idempotent + include StageMethods + + private + + def import(project) + importer = importer_class.new(project) + + importer.execute + + ImportPullRequestsWorker.perform_async(project.id) + end + + def importer_class + Importers::UsersImporter + end + end + end + end +end diff --git a/config/feature_flags/development/bitbucket_server_convert_mentions_to_users.yml b/config/feature_flags/development/bitbucket_server_convert_mentions_to_users.yml new file mode 100644 index 0000000000000000000000000000000000000000..90ea8d56c0acca704cb4ef12d497d529b46d05b7 --- /dev/null +++ b/config/feature_flags/development/bitbucket_server_convert_mentions_to_users.yml @@ -0,0 +1,8 @@ +--- +name: bitbucket_server_convert_mentions_to_users +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139097 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/434453 +milestone: '16.7' +type: development +group: group::import and integrate +default_enabled: false diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 77503814158d3b2245343d598f02e3fe35bfe408..cb45c09560dd495b33af89a8d2046223c7e10de4 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -127,6 +127,8 @@ - 1 - - bitbucket_server_import_stage_import_repository - 1 +- - bitbucket_server_import_stage_import_users + - 1 - - bulk_import - 1 - - bulk_imports_entity diff --git a/lib/bitbucket_server/client.rb b/lib/bitbucket_server/client.rb index 8e84afe51d771a8f6d449fea0fdc5ca9740f56de..432928b0591c4f1d4860c378a9a8c9a795a6cbb4 100644 --- a/lib/bitbucket_server/client.rb +++ b/lib/bitbucket_server/client.rb @@ -29,6 +29,11 @@ def repos(page_offset: 0, limit: nil, filter: nil) get_collection(path, :repo, page_offset: page_offset, limit: limit) end + def users(project_key, page_offset: 0, limit: nil) + path = "/projects/#{project_key}/permissions/users" + get_collection(path, :user, page_offset: page_offset, limit: limit) + end + def create_branch(project_key, repo, branch_name, sha) payload = { name: branch_name, diff --git a/lib/bitbucket_server/representation/user.rb b/lib/bitbucket_server/representation/user.rb new file mode 100644 index 0000000000000000000000000000000000000000..433baec1c42f96ee84a71042fbd0de175ee63294 --- /dev/null +++ b/lib/bitbucket_server/representation/user.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module BitbucketServer + module Representation + class User < Representation::Base + def email + user['emailAddress'] + end + + def username + user['slug'] + end + + private + + def user + raw['user'] + end + end + end +end diff --git a/lib/gitlab/bitbucket_server_import/importers/users_importer.rb b/lib/gitlab/bitbucket_server_import/importers/users_importer.rb new file mode 100644 index 0000000000000000000000000000000000000000..f8d0521afb26118a6749f9136f68373c76f88321 --- /dev/null +++ b/lib/gitlab/bitbucket_server_import/importers/users_importer.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module Gitlab + module BitbucketServerImport + module Importers + class UsersImporter + include Loggable + include UserCaching + + BATCH_SIZE = 100 + + def initialize(project) + @project = project + @project_id = project.id + end + + attr_reader :project, :project_id + + def execute + log_info(import_stage: 'import_users', message: 'starting') + + page = 1 + + loop do + log_info( + import_stage: 'import_users', + message: "importing page #{page} using batch size #{BATCH_SIZE}" + ) + + users = client.users(project_key, page_offset: page, limit: BATCH_SIZE).to_a + + break if users.empty? + + cache_users(users) + + page += 1 + end + + log_info(import_stage: 'import_users', message: 'finished') + end + + private + + def cache_users(users) + users_hash = users.each_with_object({}) do |user, hash| + cache_key = source_user_cache_key(project_id, user.username) + hash[cache_key] = user.email + end + + ::Gitlab::Cache::Import::Caching.write_multiple(users_hash) + end + + def client + @client ||= BitbucketServer::Client.new(project.import_data.credentials) + end + + def project_key + project.import_data.data['project_key'] + end + end + end + end +end diff --git a/lib/gitlab/bitbucket_server_import/user_caching.rb b/lib/gitlab/bitbucket_server_import/user_caching.rb new file mode 100644 index 0000000000000000000000000000000000000000..0f0169122c513d7b302ac3b8dab59dc220af4560 --- /dev/null +++ b/lib/gitlab/bitbucket_server_import/user_caching.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Gitlab + module BitbucketServerImport + module UserCaching + SOURCE_USER_CACHE_KEY = 'bitbucket_server/project/%s/source/username/%s' + + def source_user_cache_key(project_id, username) + format(SOURCE_USER_CACHE_KEY, project_id, username) + end + end + end +end diff --git a/spec/lib/bitbucket_server/client_spec.rb b/spec/lib/bitbucket_server/client_spec.rb index 1d6e8492760129516b96e48cf15d9536dc104404..0d027234a0d393d31490ad405a89c97c4cb768c9 100644 --- a/spec/lib/bitbucket_server/client_spec.rb +++ b/spec/lib/bitbucket_server/client_spec.rb @@ -80,6 +80,25 @@ end end + describe '#users' do + let(:path) { "/projects/#{project}/permissions/users" } + + it 'requests a collection' do + expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :user, page_offset: 0, limit: nil) + + subject.users(project) + end + + it 'requests a collection with offset and limit' do + offset = 10 + limit = 100 + + expect(BitbucketServer::Paginator).to receive(:new).with(anything, path, :user, page_offset: offset, limit: limit) + + subject.users(project, page_offset: offset, limit: limit) + end + end + describe '#create_branch' do let(:branch) { 'test-branch' } let(:sha) { '12345678' } diff --git a/spec/lib/bitbucket_server/representation/user_spec.rb b/spec/lib/bitbucket_server/representation/user_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..32470e3a12ffc22f920dba35c33fc1325f6e1446 --- /dev/null +++ b/spec/lib/bitbucket_server/representation/user_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BitbucketServer::Representation::User, feature_category: :importers do + let(:email) { 'test@email.com' } + let(:username) { 'test_user' } + let(:sample_data) { { 'user' => { 'emailAddress' => email, 'slug' => username } } } + + subject(:user) { described_class.new(sample_data) } + + describe '#email' do + it { expect(user.email).to eq(email) } + end + + describe '#username' do + it { expect(user.username).to eq(username) } + end +end diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..33d6ab9451364050f4ac283869599d66c298a675 --- /dev/null +++ b/spec/lib/gitlab/bitbucket_server_import/importers/users_importer_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BitbucketServerImport::Importers::UsersImporter, feature_category: :importers do + let(:logger) { Gitlab::BitbucketServerImport::Logger } + + let_it_be(:project) do + create(:project, :with_import_url, :import_started, :empty_repo, + import_data_attributes: { + data: { 'project_key' => 'key', 'repo_slug' => 'slug' }, + credentials: { 'base_uri' => 'http://bitbucket.org/', 'user' => 'bitbucket', 'password' => 'password' } + } + ) + end + + let(:user_1) do + BitbucketServer::Representation::User.new( + { 'user' => { 'emailAddress' => 'email1', 'slug' => 'username1' } } + ) + end + + let(:user_2) do + BitbucketServer::Representation::User.new( + { 'user' => { 'emailAddress' => 'email2', 'slug' => 'username2' } } + ) + end + + let(:user_3) do + BitbucketServer::Representation::User.new( + { 'user' => { 'emailAddress' => 'email3', 'slug' => 'username3' } } + ) + end + + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + + allow_next_instance_of(BitbucketServer::Client) do |client| + allow(client).to receive(:users).with('key', limit: 2, page_offset: 1).and_return([user_1, user_2]) + allow(client).to receive(:users).with('key', limit: 2, page_offset: 2).and_return([user_3]) + allow(client).to receive(:users).with('key', limit: 2, page_offset: 3).and_return([]) + end + end + + subject(:importer) { described_class.new(project) } + + describe '#execute' do + it 'writes the username and email to cache for every user in batches' do + expect(logger).to receive(:info).with(hash_including(message: 'starting')) + expect(logger).to receive(:info).with(hash_including(message: 'importing page 1 using batch size 2')) + expect(logger).to receive(:info).with(hash_including(message: 'importing page 2 using batch size 2')) + expect(logger).to receive(:info).with(hash_including(message: 'importing page 3 using batch size 2')) + expect(logger).to receive(:info).with(hash_including(message: 'finished')) + + expect(Gitlab::Cache::Import::Caching).to receive(:write_multiple).and_call_original.twice + + importer.execute + + cache_key_prefix = "bitbucket_server/project/#{project.id}/source/username" + expect(Gitlab::Cache::Import::Caching.read("#{cache_key_prefix}/username1")).to eq('email1') + expect(Gitlab::Cache::Import::Caching.read("#{cache_key_prefix}/username2")).to eq('email2') + expect(Gitlab::Cache::Import::Caching.read("#{cache_key_prefix}/username3")).to eq('email3') + end + end +end diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 136fa6b4aa0f6cc9762479b81fe5079c9f6d89aa..ecc2774afc4df9b5ce19de0d56e02c68a21ed231 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -262,6 +262,7 @@ 'Gitlab::BitbucketServerImport::Stage::ImportNotesWorker' => 3, 'Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker' => 3, 'Gitlab::BitbucketServerImport::Stage::ImportRepositoryWorker' => 3, + 'Gitlab::BitbucketServerImport::Stage::ImportUsersWorker' => 3, 'Gitlab::GithubImport::AdvanceStageWorker' => 3, 'Gitlab::GithubImport::Attachments::ImportReleaseWorker' => 5, 'Gitlab::GithubImport::Attachments::ImportNoteWorker' => 5, diff --git a/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb b/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb index 7ea23041e791b3f442653457976c49c2a08e7e4a..1531b30089c9c2346cffc8232056c7fded81f6f4 100644 --- a/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb +++ b/spec/workers/gitlab/bitbucket_server_import/stage/import_repository_worker_spec.rb @@ -18,12 +18,25 @@ end it 'schedules the next stage' do - expect(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async) + expect(Gitlab::BitbucketServerImport::Stage::ImportUsersWorker).to receive(:perform_async) .with(project.id) worker.perform(project.id) end + context 'when the bitbucket_server_convert_mentions_to_users flag is disabled' do + before do + stub_feature_flags(bitbucket_server_convert_mentions_to_users: false) + end + + it 'skips the user import and schedules the next stage' do + expect(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async) + .with(project.id) + + worker.perform(project.id) + end + end + it 'logs stage start and finish' do expect(Gitlab::BitbucketServerImport::Logger) .to receive(:info).with(hash_including(message: 'starting stage', project_id: project.id)) diff --git a/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb b/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d4cd1b823495291cdeb598297c01716e46e1091a --- /dev/null +++ b/spec/workers/gitlab/bitbucket_server_import/stage/import_users_worker_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BitbucketServerImport::Stage::ImportUsersWorker, feature_category: :importers do + let_it_be(:project) { create(:project, :import_started) } + + let(:worker) { described_class.new } + + it_behaves_like Gitlab::BitbucketServerImport::StageMethods + + describe '#perform' do + context 'when the import succeeds' do + before do + allow_next_instance_of(Gitlab::BitbucketServerImport::Importers::UsersImporter) do |importer| + allow(importer).to receive(:execute) + end + end + + it 'schedules the next stage' do + expect(Gitlab::BitbucketServerImport::Stage::ImportPullRequestsWorker).to receive(:perform_async) + .with(project.id) + + worker.perform(project.id) + end + + it 'logs stage start and finish' do + expect(Gitlab::BitbucketServerImport::Logger) + .to receive(:info).with(hash_including(message: 'starting stage', project_id: project.id)) + expect(Gitlab::BitbucketServerImport::Logger) + .to receive(:info).with(hash_including(message: 'stage finished', project_id: project.id)) + + worker.perform(project.id) + end + end + + context 'when project does not exists' do + it 'does not call importer' do + expect(Gitlab::BitbucketServerImport::Importers::UsersImporter).not_to receive(:new) + + worker.perform(-1) + end + end + + context 'when project import state is not `started`' do + it 'does not call importer' do + project = create(:project, :import_canceled) + + expect(Gitlab::BitbucketServerImport::Importers::UsersImporter).not_to receive(:new) + + worker.perform(project.id) + end + end + + context 'when the importer fails' do + it 'does not schedule the next stage and raises error' do + exception = StandardError.new('Error') + + allow_next_instance_of(Gitlab::BitbucketServerImport::Importers::UsersImporter) do |importer| + allow(importer).to receive(:execute).and_raise(exception) + end + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track).with( + project_id: project.id, + exception: exception, + error_source: described_class.name, + fail_import: false + ).and_call_original + + expect { worker.perform(project.id) } + .to change { Gitlab::BitbucketServerImport::Stage::ImportUsersWorker.jobs.size }.by(0) + .and raise_error(exception) + end + end + end +end