From a8c4a5b875179bfea1c8253bb0096be4e73ab4a2 Mon Sep 17 00:00:00 2001 From: Rodrigo Tomonari Date: Tue, 23 May 2023 22:05:50 -0300 Subject: [PATCH] Remove legacy BitBucket Server importer BitBucket Server importer was replaced by the parallel version which imports projects using multiple workers Changelog: removed --- .../bitbucket_server_parallel_importer.yml | 8 - .../bitbucket_server_import/importer.rb | 468 ------------- lib/gitlab/import_sources.rb | 6 +- .../bitbucket_server_import/importer_spec.rb | 653 ------------------ spec/lib/gitlab/import_sources_spec.rb | 38 - 5 files changed, 2 insertions(+), 1171 deletions(-) delete mode 100644 config/feature_flags/development/bitbucket_server_parallel_importer.yml delete mode 100644 lib/gitlab/bitbucket_server_import/importer.rb delete mode 100644 spec/lib/gitlab/bitbucket_server_import/importer_spec.rb diff --git a/config/feature_flags/development/bitbucket_server_parallel_importer.yml b/config/feature_flags/development/bitbucket_server_parallel_importer.yml deleted file mode 100644 index be553a5e6dacf4..00000000000000 --- a/config/feature_flags/development/bitbucket_server_parallel_importer.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: bitbucket_server_parallel_importer -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120931 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/411796 -milestone: '16.1' -type: development -group: group::import and integrate -default_enabled: true diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb deleted file mode 100644 index 1871dd3a89d5a9..00000000000000 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ /dev/null @@ -1,468 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BitbucketServerImport - class Importer - include Loggable - - attr_reader :recover_missing_commits - attr_reader :project, :project_key, :repository_slug, :client, :errors, :users, :already_imported_cache_key - - BATCH_SIZE = 100 - # The base cache key to use for tracking already imported objects. - ALREADY_IMPORTED_CACHE_KEY = - 'bitbucket_server-importer/already-imported/%{project}/%{collection}' - - TempBranch = Struct.new(:name, :sha) - - def self.imports_repository? - true - end - - def self.refmap - # We omit :heads and :tags since these are fetched in the import_repository - ['+refs/pull-requests/*/to:refs/merge-requests/*/head'] - end - - # Unlike GitHub, you can't grab the commit SHAs for pull requests that - # have been closed but not merged even though Bitbucket has these - # commits internally. We can recover these pull requests by creating a - # branch with the Bitbucket REST API, but by default we turn this - # behavior off. - def initialize(project, recover_missing_commits: false) - @project = project - @recover_missing_commits = recover_missing_commits - @project_key = project.import_data.data['project_key'] - @repository_slug = project.import_data.data['repo_slug'] - @client = BitbucketServer::Client.new(project.import_data.credentials) - @formatter = Gitlab::ImportFormatter.new - @errors = [] - @users = {} - @temp_branches = [] - @already_imported_cache_key = ALREADY_IMPORTED_CACHE_KEY % - { project: project.id, collection: collection_method } - end - - def collection_method - :pull_requests - end - - def execute - import_repository - import_pull_requests - download_lfs_objects - delete_temp_branches - handle_errors - metrics.track_finished_import - - log_info(import_stage: "complete") - - Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT) - true - end - - private - - def handle_errors - return unless errors.any? - - project.import_state.update_column(:last_error, { - message: 'The remote data could not be fully imported.', - errors: errors - }.to_json) - end - - def find_user_id(by:, value:) - return unless value - - return users[value] if users.key?(value) - - user = if by == :email - User.find_by_any_email(value, confirmed: true) - else - User.find_by_username(value) - end - - users[value] = user&.id - - user&.id - end - - def repo - @repo ||= client.repo(project_key, repository_slug) - end - - def sha_exists?(sha) - project.repository.commit(sha) - end - - def temp_branch_name(pull_request, suffix) - "gitlab/import/pull-request/#{pull_request.iid}/#{suffix}" - end - - # This method restores required SHAs that GitLab needs to create diffs - # into branch names as the following: - # - # gitlab/import/pull-request/N/{to,from} - def restore_branches(pull_requests) - shas_to_restore = [] - - pull_requests.each do |pull_request| - shas_to_restore << TempBranch.new(temp_branch_name(pull_request, :from), - pull_request.source_branch_sha) - shas_to_restore << TempBranch.new(temp_branch_name(pull_request, :to), - pull_request.target_branch_sha) - end - - # Create the branches on the Bitbucket Server first - created_branches = restore_branch_shas(shas_to_restore) - - @temp_branches += created_branches - # Now sync the repository so we get the new branches - import_repository unless created_branches.empty? - end - - def restore_branch_shas(shas_to_restore) - shas_to_restore.each_with_object([]) do |temp_branch, branches_created| - branch_name = temp_branch.name - sha = temp_branch.sha - - next if sha_exists?(sha) - - begin - client.create_branch(project_key, repository_slug, branch_name, sha) - branches_created << temp_branch - rescue BitbucketServer::Connection::ConnectionError => e - log_warn(message: "Unable to recreate branch", sha: sha, error: e.message) - end - end - end - - def import_repository - log_info(import_stage: 'import_repository', message: 'starting import') - - project.repository.import_repository(project.import_url) - project.repository.fetch_as_mirror(project.import_url, refmap: self.class.refmap) - - log_info(import_stage: 'import_repository', message: 'finished import') - rescue ::Gitlab::Git::CommandError => e - Gitlab::ErrorTracking.log_exception( - e, - import_stage: 'import_repository', message: 'failed import', error: e.message - ) - - # Expire cache to prevent scenarios such as: - # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true - # 2. Retried import, repo is broken or not imported but +exists?+ still returns true - project.repository.expire_content_cache if project.repository_exists? - - raise - end - - def download_lfs_objects - result = Projects::LfsPointers::LfsImportService.new(project).execute - - if result[:status] == :error - errors << { type: :lfs_objects, errors: "The Lfs import process failed. #{result[:message]}" } - end - end - - # Bitbucket Server keeps tracks of references for open pull requests in - # refs/heads/pull-requests, but closed and merged requests get moved - # into hidden internal refs under stash-refs/pull-requests. Unless the - # SHAs involved are at the tip of a branch or tag, there is no way to - # retrieve the server for those commits. - # - # To avoid losing history, we use the Bitbucket API to re-create the branch - # on the remote server. Then we have to issue a `git fetch` to download these - # branches. - def import_pull_requests - page = 0 - - log_info(import_stage: 'import_pull_requests', message: "starting") - - loop do - log_debug(import_stage: 'import_pull_requests', message: "importing page #{page} and batch-size #{BATCH_SIZE} from #{page * BATCH_SIZE} to #{(page + 1) * BATCH_SIZE}") - - pull_requests = client.pull_requests(project_key, repository_slug, page_offset: page, limit: BATCH_SIZE).to_a - - break if pull_requests.empty? - - # Creating branches on the server and fetching the newly-created branches - # may take a number of network round-trips. This used to be done in batches to - # avoid doing a git fetch for every new branch, as the whole process is now - # batched, we do not need to separately do this in batches. - restore_branches(pull_requests) if recover_missing_commits - - pull_requests.each do |pull_request| - if already_imported?(pull_request) - log_info(import_stage: 'import_pull_requests', message: 'already imported', iid: pull_request.iid) - else - import_bitbucket_pull_request(pull_request) - end - rescue StandardError => e - Gitlab::ErrorTracking.log_exception( - e, - import_stage: 'import_pull_requests', iid: pull_request.iid, error: e.message - ) - - backtrace = Gitlab::BacktraceCleaner.clean_backtrace(e.backtrace) - errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, backtrace: backtrace.join("\n"), raw_response: pull_request.raw } - end - - log_debug(import_stage: 'import_pull_requests', message: "finished page #{page} and batch-size #{BATCH_SIZE}") - page += 1 - end - end - - # Returns true if the given object has already been imported, false - # otherwise. - # - # object - The object to check. - def already_imported?(pull_request) - Gitlab::Cache::Import::Caching.set_includes?(already_imported_cache_key, pull_request.iid) - end - - # Marks the given object as "already imported". - def mark_as_imported(pull_request) - Gitlab::Cache::Import::Caching.set_add(already_imported_cache_key, pull_request.iid) - end - - def delete_temp_branches - @temp_branches.each do |branch| - client.delete_branch(project_key, repository_slug, branch.name, branch.sha) - project.repository.delete_branch(branch.name) - rescue BitbucketServer::Connection::ConnectionError => e - Gitlab::ErrorTracking.log_exception( - e, - import_stage: 'delete_temp_branches', branch: branch.name, error: e.message - ) - - @errors << { type: :delete_temp_branches, branch_name: branch.name, errors: e.message } - end - end - - def import_bitbucket_pull_request(pull_request) - log_info(import_stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid) - - description = '' - description += author_line(pull_request) - description += pull_request.description if pull_request.description - - attributes = { - iid: pull_request.iid, - title: pull_request.title, - description: description, - reviewer_ids: reviewers(pull_request.reviewers), - source_project_id: project.id, - source_branch: Gitlab::Git.ref_name(pull_request.source_branch_name), - source_branch_sha: pull_request.source_branch_sha, - target_project_id: project.id, - target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name), - target_branch_sha: pull_request.target_branch_sha, - state_id: MergeRequest.available_states[pull_request.state], - author_id: author_id(pull_request), - created_at: pull_request.created_at, - updated_at: pull_request.updated_at - } - - creator = Gitlab::Import::MergeRequestCreator.new(project) - merge_request = creator.execute(attributes) - - if merge_request.persisted? - import_pull_request_comments(pull_request, merge_request) - - metrics.merge_requests_counter.increment - end - - log_info(import_stage: 'import_bitbucket_pull_requests', message: 'finished', iid: pull_request.iid) - mark_as_imported(pull_request) - end - - def import_pull_request_comments(pull_request, merge_request) - log_info(import_stage: 'import_pull_request_comments', message: 'starting', iid: merge_request.iid) - - comments, other_activities = client.activities(project_key, repository_slug, pull_request.iid).partition(&:comment?) - - merge_event = other_activities.find(&:merge_event?) - import_merge_event(merge_request, merge_event) if merge_event - - inline_comments, pr_comments = comments.partition(&:inline_comment?) - - import_inline_comments(inline_comments.map(&:comment), merge_request) - import_standalone_pr_comments(pr_comments.map(&:comment), merge_request) - - log_info(import_stage: 'import_pull_request_comments', message: 'finished', iid: merge_request.iid, - merge_event_found: merge_event.present?, - inline_comments_count: inline_comments.count, - standalone_pr_comments: pr_comments.count) - end - - # rubocop: disable CodeReuse/ActiveRecord - def import_merge_event(merge_request, merge_event) - log_info(import_stage: 'import_merge_event', message: 'starting', iid: merge_request.iid) - - committer = merge_event.committer_email - - user_id = find_user_id(by: :email, value: committer) || project.creator_id - timestamp = merge_event.merge_timestamp - merge_request.update({ merge_commit_sha: merge_event.merge_commit }) - metric = MergeRequest::Metrics.find_or_initialize_by(merge_request: merge_request) - metric.update(merged_by_id: user_id, merged_at: timestamp) - - log_info(import_stage: 'import_merge_event', message: 'finished', iid: merge_request.iid) - end - # rubocop: enable CodeReuse/ActiveRecord - - def import_inline_comments(inline_comments, merge_request) - log_info(import_stage: 'import_inline_comments', message: 'starting', iid: merge_request.iid) - - inline_comments.each do |comment| - position = build_position(merge_request, comment) - parent = create_diff_note(merge_request, comment, position) - - next unless parent&.persisted? - - discussion_id = parent.discussion_id - - comment.comments.each do |reply| - create_diff_note(merge_request, reply, position, discussion_id) - end - end - - log_info(import_stage: 'import_inline_comments', message: 'finished', iid: merge_request.iid) - end - - def create_diff_note(merge_request, comment, position, discussion_id = nil) - attributes = pull_request_comment_attributes(comment) - attributes.merge!(position: position, type: 'DiffNote') - attributes[:discussion_id] = discussion_id if discussion_id - - note = merge_request.notes.build(attributes) - - if note.valid? - note.save - return note - end - - log_info(import_stage: 'create_diff_note', message: 'creating fallback DiffNote', iid: merge_request.iid) - - # Bitbucket Server supports the ability to comment on any line, not just the - # line in the diff. If we can't add the note as a DiffNote, fallback to creating - # a regular note. - create_fallback_diff_note(merge_request, comment, position) - rescue StandardError => e - Gitlab::ErrorTracking.log_exception( - e, - import_stage: 'create_diff_note', comment_id: comment.id, error: e.message - ) - - errors << { type: :pull_request, id: comment.id, errors: e.message } - nil - end - - def create_fallback_diff_note(merge_request, comment, position) - attributes = pull_request_comment_attributes(comment) - note = "*Comment on" - - note += " #{position.old_path}:#{position.old_line} -->" if position.old_line - note += " #{position.new_path}:#{position.new_line}" if position.new_line - note += "*\n\n#{comment.note}" - - attributes[:note] = note - merge_request.notes.create!(attributes) - end - - def build_position(merge_request, pr_comment) - params = { - diff_refs: merge_request.diff_refs, - old_path: pr_comment.file_path, - new_path: pr_comment.file_path, - old_line: pr_comment.old_pos, - new_line: pr_comment.new_pos - } - - Gitlab::Diff::Position.new(params) - end - - def import_standalone_pr_comments(pr_comments, merge_request) - pr_comments.each do |comment| - merge_request.notes.create!(pull_request_comment_attributes(comment)) - - comment.comments.each do |replies| - merge_request.notes.create!(pull_request_comment_attributes(replies)) - end - rescue StandardError => e - Gitlab::ErrorTracking.log_exception( - e, - import_stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message - ) - - errors << { type: :pull_request, comment_id: comment.id, errors: e.message } - end - end - - def pull_request_comment_attributes(comment) - author = uid(comment) - note = '' - - unless author - author = project.creator_id - note = "*By #{comment.author_username} (#{comment.author_email})*\n\n" - end - - note += - # Provide some context for replying - if comment.parent_comment - "> #{comment.parent_comment.note.truncate(80)}\n\n#{comment.note}" - else - comment.note - end - - { - project: project, - note: note, - author_id: author, - created_at: comment.created_at, - updated_at: comment.updated_at - } - end - - def metrics - @metrics ||= Gitlab::Import::Metrics.new(:bitbucket_server_importer, @project) - end - - def author_line(rep_object) - return '' if uid(rep_object) - - @formatter.author_line(rep_object.author) - end - - def author_id(rep_object) - uid(rep_object) || project.creator_id - end - - def uid(rep_object) - # We want this to only match either username or email depending on the flag state. - # There should be no fall-through. - if Feature.enabled?(:bitbucket_server_user_mapping_by_username, type: :ops) - find_user_id(by: :username, value: rep_object.author_username) - else - find_user_id(by: :email, value: rep_object.author_email) - end - end - - def reviewers(reviewers) - return [] unless reviewers.present? - - reviewers.filter_map do |reviewer| - if Feature.enabled?(:bitbucket_server_user_mapping_by_username, type: :ops) - find_user_id(by: :username, value: reviewer.dig('user', 'slug')) - else - find_user_id(by: :email, value: reviewer.dig('user', 'emailAddress')) - end - end - end - end - end -end diff --git a/lib/gitlab/import_sources.rb b/lib/gitlab/import_sources.rb index 5823fafa2aaafb..fec8b3a7708fca 100644 --- a/lib/gitlab/import_sources.rb +++ b/lib/gitlab/import_sources.rb @@ -12,7 +12,7 @@ module ImportSources IMPORT_TABLE = [ ImportSource.new('github', 'GitHub', Gitlab::GithubImport::ParallelImporter), ImportSource.new('bitbucket', 'Bitbucket Cloud', Gitlab::BitbucketImport::Importer), - ImportSource.new('bitbucket_server', 'Bitbucket Server', Gitlab::BitbucketServerImport::Importer), + ImportSource.new('bitbucket_server', 'Bitbucket Server', Gitlab::BitbucketServerImport::ParallelImporter), ImportSource.new('fogbugz', 'FogBugz', Gitlab::FogbugzImport::Importer), ImportSource.new('git', 'Repository by URL', nil), ImportSource.new('gitlab_project', 'GitLab export', Gitlab::ImportExport::Importer), @@ -45,14 +45,12 @@ def title(name) def import_table bitbucket_parallel_enabled = Feature.enabled?(:bitbucket_parallel_importer) - bitbucket_server_parallel_enabled = Feature.enabled?(:bitbucket_server_parallel_importer) - return IMPORT_TABLE unless bitbucket_parallel_enabled || bitbucket_server_parallel_enabled + return IMPORT_TABLE unless bitbucket_parallel_enabled import_table = IMPORT_TABLE.deep_dup import_table[1].importer = Gitlab::BitbucketImport::ParallelImporter if bitbucket_parallel_enabled - import_table[2].importer = Gitlab::BitbucketServerImport::ParallelImporter if bitbucket_server_parallel_enabled import_table end diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb deleted file mode 100644 index 4ff61bf329c9c5..00000000000000 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ /dev/null @@ -1,653 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BitbucketServerImport::Importer, feature_category: :importers do - include ImportSpecHelper - - let(:import_url) { 'http://my-bitbucket' } - let(:bitbucket_user) { 'bitbucket' } - let(:project_creator) { create(:user, username: 'project_creator', email: 'project_creator@example.org') } - let(:password) { 'test' } - let(:project) { create(:project, :repository, import_url: import_url, creator: project_creator) } - let(:now) { Time.now.utc.change(usec: 0) } - let(:project_key) { 'TEST' } - let(:repo_slug) { 'rouge-repo' } - let(:sample) { RepoHelpers.sample_compare } - - subject { described_class.new(project, recover_missing_commits: true) } - - before do - data = project.create_or_update_import_data( - data: { project_key: project_key, repo_slug: repo_slug }, - credentials: { base_uri: import_url, user: bitbucket_user, password: password } - ) - data.save! - project.save! - end - - describe '#import_repository' do - let(:repo_url) { 'http://bitbucket:test@my-bitbucket' } - - before do - expect(project.repository).to receive(:import_repository).with(repo_url) - end - - it 'adds a remote' do - expect(subject).to receive(:import_pull_requests) - expect(subject).to receive(:delete_temp_branches) - expect(project.repository).to receive(:fetch_as_mirror) - .with(repo_url, - refmap: ['+refs/pull-requests/*/to:refs/merge-requests/*/head']) - - subject.execute - end - - it 'raises a Gitlab::Git::CommandError in the fetch' do - expect(project.repository).to receive(:fetch_as_mirror).and_raise(::Gitlab::Git::CommandError) - - expect { subject.execute }.to raise_error(::Gitlab::Git::CommandError) - end - - it 'raises an unhandled exception in the fetch' do - expect(project.repository).to receive(:fetch_as_mirror).and_raise(RuntimeError) - - expect { subject.execute }.to raise_error(RuntimeError) - end - end - - describe '#import_pull_requests' do - let(:pull_request_author) { create(:user, username: 'pull_request_author', email: 'pull_request_author@example.org') } - let(:note_author) { create(:user, username: 'note_author', email: 'note_author@example.org') } - - let(:pull_request) do - instance_double( - BitbucketServer::Representation::PullRequest, - iid: 10, - source_branch_sha: sample.commits.last, - source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, - target_branch_sha: sample.commits.first, - target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, - title: 'This is a title', - description: 'This is a test pull request', - reviewers: [], - state: 'merged', - author: 'Test Author', - author_email: pull_request_author.email, - author_username: pull_request_author.username, - created_at: Time.now, - updated_at: Time.now, - raw: {}, - merged?: true) - end - - let(:merge_event) do - instance_double( - BitbucketServer::Representation::Activity, - comment?: false, - merge_event?: true, - committer_email: pull_request_author.email, - merge_timestamp: now, - merge_commit: '12345678' - ) - end - - let(:pr_note) do - instance_double( - BitbucketServer::Representation::Comment, - note: 'Hello world', - author_email: note_author.email, - author_username: note_author.username, - comments: [], - created_at: now, - updated_at: now, - parent_comment: nil) - end - - let(:pr_comment) do - instance_double( - BitbucketServer::Representation::Activity, - comment?: true, - inline_comment?: false, - merge_event?: false, - comment: pr_note) - end - - before do - allow(subject).to receive(:import_repository) - allow(subject).to receive(:delete_temp_branches) - allow(subject).to receive(:restore_branches) - - allow(subject.client).to receive(:pull_requests).and_return([pull_request], []) - end - - # As we are using Caching with redis, it is best to clean the cache after each test run, else we need to wait for - # the expiration by the importer - after do - Gitlab::Cache::Import::Caching.expire(subject.already_imported_cache_key, 0) - end - - it 'imports merge event' do - expect(subject.client).to receive(:activities).and_return([merge_event]) - - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.metrics.merged_by).to eq(pull_request_author) - expect(merge_request.metrics.merged_at).to eq(merge_event.merge_timestamp) - expect(merge_request.merge_commit_sha).to eq('12345678') - expect(merge_request.state_id).to eq(3) - end - - describe 'pull request author user mapping' do - before do - allow(subject.client).to receive(:activities).and_return([merge_event]) - end - - shared_examples 'imports pull requests' do - it 'maps user' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.author).to eq(expected_author) - end - end - - context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do - before do - stub_feature_flags(bitbucket_server_user_mapping_by_username: false) - end - - context 'when email is not present' do - before do - allow(pull_request).to receive(:author_email).and_return(nil) - end - - let(:expected_author) { project_creator } - - include_examples 'imports pull requests' - end - - context 'when email is present' do - before do - allow(pull_request).to receive(:author_email).and_return(pull_request_author.email) - end - - let(:expected_author) { pull_request_author } - - include_examples 'imports pull requests' - end - end - - context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do - before do - stub_feature_flags(bitbucket_server_user_mapping_by_username: true) - end - - context 'when username is not present' do - before do - allow(pull_request).to receive(:author_username).and_return(nil) - end - - let(:expected_author) { project_creator } - - include_examples 'imports pull requests' - end - - context 'when username is present' do - before do - allow(pull_request).to receive(:author_username).and_return(pull_request_author.username) - end - - let(:expected_author) { pull_request_author } - - include_examples 'imports pull requests' - end - end - - context 'when user is not found' do - before do - allow(pull_request).to receive(:author_username).and_return(nil) - allow(pull_request).to receive(:author_email).and_return(nil) - end - - it 'maps importer user' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.author).to eq(project_creator) - end - end - end - - describe 'comments' do - shared_examples 'imports comments' do - it 'imports comments' do - expect(subject.client).to receive(:activities).and_return([pr_comment]) - - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.notes.count).to eq(1) - note = merge_request.notes.first - expect(note.note).to end_with(pr_note.note) - expect(note.author).to eq(note_author) - expect(note.created_at).to eq(pr_note.created_at) - expect(note.updated_at).to eq(pr_note.created_at) - end - end - - context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do - before do - stub_feature_flags(bitbucket_server_user_mapping_by_username: false) - end - - include_examples 'imports comments' - end - - context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do - before do - stub_feature_flags(bitbucket_server_user_mapping_by_username: true) - end - - include_examples 'imports comments' - - context 'when username is not present' do - before do - allow(pr_note).to receive(:author_username).and_return(nil) - allow(subject.client).to receive(:activities).and_return([pr_comment]) - end - - it 'defaults to import user' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.notes.count).to eq(1) - note = merge_request.notes.first - expect(note.author).to eq(project_creator) - end - end - - context 'when username is present' do - before do - allow(pr_note).to receive(:author_username).and_return(note_author.username) - allow(subject.client).to receive(:activities).and_return([pr_comment]) - end - - it 'maps by username' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.notes.count).to eq(1) - note = merge_request.notes.first - expect(note.author).to eq(note_author) - end - end - end - end - - context 'metrics' do - let(:histogram) { double(:histogram).as_null_object } - let(:counter) { double('counter', increment: true) } - - before do - allow(Gitlab::Metrics).to receive(:counter) { counter } - allow(Gitlab::Metrics).to receive(:histogram) { histogram } - allow(subject.client).to receive(:activities).and_return([merge_event]) - end - - it 'counts and measures duration of imported projects' do - expect(Gitlab::Metrics).to receive(:counter).with( - :bitbucket_server_importer_imported_projects_total, - 'The number of imported projects' - ) - - expect(Gitlab::Metrics).to receive(:histogram).with( - :bitbucket_server_importer_total_duration_seconds, - 'Total time spent importing projects, in seconds', - {}, - Gitlab::Import::Metrics::IMPORT_DURATION_BUCKETS - ) - - expect(counter).to receive(:increment) - expect(histogram).to receive(:observe).with({ importer: :bitbucket_server_importer }, anything) - - subject.execute - end - - it 'counts imported pull requests' do - expect(Gitlab::Metrics).to receive(:counter).with( - :bitbucket_server_importer_imported_merge_requests_total, - 'The number of imported merge (pull) requests' - ) - - expect(counter).to receive(:increment) - - subject.execute - end - end - - describe 'threaded discussions' do - let(:reply_author) { create(:user, username: 'reply_author', email: 'reply_author@example.org') } - let(:inline_note_author) { create(:user, username: 'inline_note_author', email: 'inline_note_author@example.org') } - - let(:reply) do - instance_double( - BitbucketServer::Representation::PullRequestComment, - author_email: reply_author.email, - author_username: reply_author.username, - note: 'I agree', - created_at: now, - updated_at: now) - end - - # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad - let(:inline_note) do - instance_double( - BitbucketServer::Representation::PullRequestComment, - file_type: 'ADDED', - from_sha: sample.commits.first, - to_sha: sample.commits.last, - file_path: '.gitmodules', - old_pos: nil, - new_pos: 4, - note: 'Hello world', - author_email: inline_note_author.email, - author_username: inline_note_author.username, - comments: [reply], - created_at: now, - updated_at: now, - parent_comment: nil) - end - - let(:inline_comment) do - instance_double( - BitbucketServer::Representation::Activity, - comment?: true, - inline_comment?: true, - merge_event?: false, - comment: inline_note) - end - - before do - allow(reply).to receive(:parent_comment).and_return(inline_note) - allow(subject.client).to receive(:activities).and_return([inline_comment]) - end - - shared_examples 'imports threaded discussions' do - it 'imports threaded discussions' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.notes.count).to eq(2) - expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1) - - notes = merge_request.notes.order(:id).to_a - start_note = notes.first - expect(start_note.type).to eq('DiffNote') - expect(start_note.note).to end_with(inline_note.note) - expect(start_note.created_at).to eq(inline_note.created_at) - expect(start_note.updated_at).to eq(inline_note.updated_at) - expect(start_note.position.base_sha).to eq(inline_note.from_sha) - expect(start_note.position.start_sha).to eq(inline_note.from_sha) - expect(start_note.position.head_sha).to eq(inline_note.to_sha) - expect(start_note.position.old_line).to be_nil - expect(start_note.position.new_line).to eq(inline_note.new_pos) - expect(start_note.author).to eq(inline_note_author) - - reply_note = notes.last - # Make sure author and reply context is included - expect(reply_note.note).to start_with("> #{inline_note.note}\n\n#{reply.note}") - expect(reply_note.author).to eq(reply_author) - expect(reply_note.created_at).to eq(reply.created_at) - expect(reply_note.updated_at).to eq(reply.created_at) - expect(reply_note.position.base_sha).to eq(inline_note.from_sha) - expect(reply_note.position.start_sha).to eq(inline_note.from_sha) - expect(reply_note.position.head_sha).to eq(inline_note.to_sha) - expect(reply_note.position.old_line).to be_nil - expect(reply_note.position.new_line).to eq(inline_note.new_pos) - end - end - - context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do - before do - stub_feature_flags(bitbucket_server_user_mapping_by_username: false) - end - - include_examples 'imports threaded discussions' - end - - context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do - before do - stub_feature_flags(bitbucket_server_user_mapping_by_username: true) - end - - include_examples 'imports threaded discussions' do - context 'when username is not present' do - before do - allow(reply).to receive(:author_username).and_return(nil) - allow(inline_note).to receive(:author_username).and_return(nil) - end - - it 'defaults to import user' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - notes = MergeRequest.first.notes.order(:id).to_a - - expect(notes.first.author).to eq(project_creator) - expect(notes.last.author).to eq(project_creator) - end - end - end - end - - context 'when user is not found' do - before do - allow(reply).to receive(:author_username).and_return(nil) - allow(reply).to receive(:author_email).and_return(nil) - allow(inline_note).to receive(:author_username).and_return(nil) - allow(inline_note).to receive(:author_email).and_return(nil) - end - - it 'maps importer user' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - notes = MergeRequest.first.notes.order(:id).to_a - - expect(notes.first.author).to eq(project_creator) - expect(notes.last.author).to eq(project_creator) - end - end - end - - it 'falls back to comments if diff comments fail to validate' do - reply = instance_double( - BitbucketServer::Representation::Comment, - author_email: 'someuser@gitlab.com', - author_username: 'Aquaman', - note: 'I agree', - created_at: now, - updated_at: now) - - # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad - inline_note = instance_double( - BitbucketServer::Representation::PullRequestComment, - file_type: 'REMOVED', - from_sha: sample.commits.first, - to_sha: sample.commits.last, - file_path: '.gitmodules', - old_pos: 8, - new_pos: 9, - note: 'This is a note with an invalid line position.', - author_email: project.owner.email, - author_username: 'Owner', - comments: [reply], - created_at: now, - updated_at: now, - parent_comment: nil) - - inline_comment = instance_double( - BitbucketServer::Representation::Activity, - comment?: true, - inline_comment?: true, - merge_event?: false, - comment: inline_note) - - allow(reply).to receive(:parent_comment).and_return(inline_note) - - expect(subject.client).to receive(:activities).and_return([inline_comment]) - - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.notes.count).to eq(2) - notes = merge_request.notes - - expect(notes.first.note).to start_with('*Comment on .gitmodules') - expect(notes.second.note).to start_with('*Comment on .gitmodules') - end - - it 'reports an error if an exception is raised' do - allow(subject).to receive(:import_bitbucket_pull_request).and_raise(RuntimeError) - expect(Gitlab::ErrorTracking).to receive(:log_exception) - - subject.execute - end - - describe 'import pull requests with caching' do - let(:pull_request_already_imported) do - instance_double( - BitbucketServer::Representation::PullRequest, - iid: 11) - end - - let(:pull_request_to_be_imported) do - instance_double( - BitbucketServer::Representation::PullRequest, - iid: 12, - source_branch_sha: sample.commits.last, - source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, - target_branch_sha: sample.commits.first, - target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, - title: 'This is a title', - description: 'This is a test pull request', - reviewers: sample.reviewers, - state: 'merged', - author: 'Test Author', - author_email: pull_request_author.email, - author_username: pull_request_author.username, - created_at: Time.now, - updated_at: Time.now, - raw: {}, - merged?: true) - end - - before do - Gitlab::Cache::Import::Caching.set_add(subject.already_imported_cache_key, pull_request_already_imported.iid) - allow(subject.client).to receive(:pull_requests).and_return([pull_request_to_be_imported, pull_request_already_imported], []) - end - - it 'only imports one Merge Request, as the other on is in the cache' do - expect(subject.client).to receive(:activities).and_return([merge_event]) - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - expect(Gitlab::Cache::Import::Caching.set_includes?(subject.already_imported_cache_key, pull_request_already_imported.iid)).to eq(true) - expect(Gitlab::Cache::Import::Caching.set_includes?(subject.already_imported_cache_key, pull_request_to_be_imported.iid)).to eq(true) - end - end - end - - describe 'inaccessible branches' do - let(:id) { 10 } - let(:temp_branch_from) { "gitlab/import/pull-request/#{id}/from" } - let(:temp_branch_to) { "gitlab/import/pull-request/#{id}/to" } - - before do - pull_request = instance_double( - BitbucketServer::Representation::PullRequest, - iid: id, - source_branch_sha: '12345678', - source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, - target_branch_sha: '98765432', - target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, - title: 'This is a title', - description: 'This is a test pull request', - reviewers: [], - state: 'merged', - author: 'Test Author', - author_email: project.owner.email, - author_username: 'author', - created_at: Time.now, - updated_at: Time.now, - merged?: true) - - expect(subject.client).to receive(:pull_requests).and_return([pull_request], []) - expect(subject.client).to receive(:activities).and_return([]) - expect(subject).to receive(:import_repository).twice - end - - it '#restore_branches' do - expect(subject).to receive(:restore_branches).and_call_original - expect(subject).to receive(:delete_temp_branches) - expect(subject.client).to receive(:create_branch) - .with(project_key, repo_slug, - temp_branch_from, - '12345678') - expect(subject.client).to receive(:create_branch) - .with(project_key, repo_slug, - temp_branch_to, - '98765432') - - expect { subject.execute }.to change { MergeRequest.count }.by(1) - end - - it '#delete_temp_branches' do - expect(subject.client).to receive(:create_branch).twice - expect(subject).to receive(:delete_temp_branches).and_call_original - expect(subject.client).to receive(:delete_branch) - .with(project_key, repo_slug, - temp_branch_from, - '12345678') - expect(subject.client).to receive(:delete_branch) - .with(project_key, repo_slug, - temp_branch_to, - '98765432') - expect(project.repository).to receive(:delete_branch).with(temp_branch_from) - expect(project.repository).to receive(:delete_branch).with(temp_branch_to) - - expect { subject.execute }.to change { MergeRequest.count }.by(1) - end - end - - context "lfs files" do - before do - allow(project).to receive(:lfs_enabled?).and_return(true) - allow(subject).to receive(:import_repository) - allow(subject).to receive(:import_pull_requests) - end - - it "downloads lfs objects if lfs_enabled is enabled for project" do - expect_next_instance_of(Projects::LfsPointers::LfsImportService) do |lfs_import_service| - expect(lfs_import_service).to receive(:execute).and_return(status: :success) - end - - subject.execute - end - - it "adds the error message when the lfs download fails" do - allow_next_instance_of(Projects::LfsPointers::LfsImportService) do |lfs_import_service| - expect(lfs_import_service).to receive(:execute).and_return(status: :error, message: "LFS server not reachable") - end - - subject.execute - - expect(project.import_state.reload.last_error).to eq(Gitlab::Json.dump({ - message: "The remote data could not be fully imported.", - errors: [{ - type: "lfs_objects", - errors: "The Lfs import process failed. LFS server not reachable" - }] - })) - end - end -end diff --git a/spec/lib/gitlab/import_sources_spec.rb b/spec/lib/gitlab/import_sources_spec.rb index 13547609f68ccd..db23e3b1fd4b8c 100644 --- a/spec/lib/gitlab/import_sources_spec.rb +++ b/spec/lib/gitlab/import_sources_spec.rb @@ -72,49 +72,11 @@ expect(described_class.importer(name)).to eq(klass) end end - - context 'when flag is disabled' do - before do - stub_feature_flags(bitbucket_server_parallel_importer: false) - end - - it 'returns Gitlab::BitbucketServerImport::Importer when given bitbucket_server' do - expect(described_class.importer('bitbucket_server')).to eq(Gitlab::BitbucketServerImport::Importer) - end - end end describe '.import_table' do subject { described_class.import_table } - describe 'Bitbucket server' do - it 'returns the ParallelImporter' do - is_expected.to include( - described_class::ImportSource.new( - 'bitbucket_server', - 'Bitbucket Server', - Gitlab::BitbucketServerImport::ParallelImporter - ) - ) - end - - context 'when flag is disabled' do - before do - stub_feature_flags(bitbucket_server_parallel_importer: false) - end - - it 'returns the legacy Importer' do - is_expected.to include( - described_class::ImportSource.new( - 'bitbucket_server', - 'Bitbucket Server', - Gitlab::BitbucketServerImport::Importer - ) - ) - end - end - end - describe 'Bitbucket cloud' do it 'returns the ParallelImporter' do is_expected.to include( -- GitLab