diff --git a/lib/gitlab/legacy_github_import/base_formatter.rb b/lib/gitlab/legacy_github_import/base_formatter.rb index 9e6e08c37a37db3af5994f7bedab9a30626cb2ed..6e24f023e1bfbad6cf9cf60c8147c0ad003a052c 100644 --- a/lib/gitlab/legacy_github_import/base_formatter.rb +++ b/lib/gitlab/legacy_github_import/base_formatter.rb @@ -41,6 +41,9 @@ def push_placeholder_references(record, contributing_users: nil) def push_placeholder_reference(record, user_reference_column, source_user) return unless project.import_data.user_mapping_enabled? + return if project.root_ancestor.user_namespace? && + project.import_data.user_mapping_to_personal_namespace_owner_enabled? + user_id = record[user_reference_column] return if user_id.nil? || source_user.nil? diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index 9968a5c6589392b86c5b755ef8390607e572c052..6fc61a334b862a341b978dfc93e235e74bbef646 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -337,7 +337,7 @@ def fetch_resources(resource_type, *opts) end def load_placeholder_references - return unless project.import_data.user_mapping_enabled? + return unless should_map_users? ::Import::LoadPlaceholderReferencesWorker.perform_async( project.import_type, @@ -346,12 +346,12 @@ def load_placeholder_references end def placeholder_references_loaded? - return true unless project.import_data.user_mapping_enabled? - project.placeholder_reference_store.empty? end def wait_for_placeholder_references + return unless should_map_users? + # Since this importer is synchronous, wait until all placeholder references have been saved # to the database before completing the import time_waited = 0 @@ -376,6 +376,15 @@ def wait_for_placeholder_references end end + def should_map_users? + return false unless project.import_data.user_mapping_enabled? + + return false if project.root_ancestor.user_namespace? && + project.import_data.user_mapping_to_personal_namespace_owner_enabled? + + true + end + def imported?(resource_type) Rails.cache.read("#{cache_key_prefix}:#{resource_type}:imported") end diff --git a/lib/gitlab/legacy_github_import/user_formatter.rb b/lib/gitlab/legacy_github_import/user_formatter.rb index c0d47539f7b3c690e4c972c7e7d842973d888840..a70db346e668a14033a9c7569b32f2f033ef3c00 100644 --- a/lib/gitlab/legacy_github_import/user_formatter.rb +++ b/lib/gitlab/legacy_github_import/user_formatter.rb @@ -26,15 +26,15 @@ def login def gitlab_id return find_by_email unless user_mapping_enabled? - return GithubImport.ghost_user_id if ghost_user? + return project.root_ancestor.owner_id if map_to_personal_namespace_owner? gitlab_user&.id end strong_memoize_attr :gitlab_id def source_user - return if !user_mapping_enabled? || ghost_user? + return if !user_mapping_enabled? || map_to_personal_namespace_owner? || ghost_user? source_user_mapper.find_or_create_source_user( source_name: gitea_user[:full_name].presence || gitea_user[:login], @@ -87,6 +87,12 @@ def user_mapping_enabled? project.import_data.reset.user_mapping_enabled? end strong_memoize_attr :user_mapping_enabled? + + def map_to_personal_namespace_owner? + project.root_ancestor.user_namespace? && + project.import_data.user_mapping_to_personal_namespace_owner_enabled? + end + strong_memoize_attr :map_to_personal_namespace_owner? end end end diff --git a/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb index ab61488026aa83eb705911c2a98b7e0c97aaa2f6..cecdb1def2e3ed4dde4c6ce621afc1efb1e65411 100644 --- a/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb @@ -3,17 +3,13 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::CommentFormatter, :clean_gitlab_redis_shared_state, feature_category: :importers do - include Import::GiteaHelper + include Import::UserMappingHelper - let_it_be(:project) do - create(:project, :with_import_url, :import_user_mapping_enabled, import_type: ::Import::SOURCE_GITEA) - end - - let_it_be(:source_user_mapper) do - Gitlab::Import::SourceUserMapper.new( - namespace: project.root_ancestor, - import_type: project.import_type, - source_hostname: 'https://gitea.com' + let_it_be_with_reload(:project) do + create( + :project, :in_group, :with_import_url, + :import_user_mapping_enabled, :user_mapping_to_personal_namespace_owner_enabled, + import_type: ::Import::SOURCE_GITEA ) end @@ -28,6 +24,14 @@ ) end + let(:source_user_mapper) do + Gitlab::Import::SourceUserMapper.new( + namespace: project.root_ancestor, + import_type: project.import_type, + source_hostname: 'https://gitea.com' + ) + end + let(:client) { instance_double(Gitlab::LegacyGithubImport::Client) } let(:ghost_user) { { id: -1, login: 'Ghost' } } let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } @@ -185,11 +189,38 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + let(:raw) { base } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'maps the author to the personal namespace owner' do + expect(comment.attributes.fetch(:author_id)).to eq(user_namespace.owner_id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'maps the author to the import user' do + expect(comment.attributes.fetch(:author_id)).to eq( + user_namespace.namespace_import_user.user_id + ) + end + end + end + context 'when user contribution mapping is disabled' do let(:raw) { base.merge(user: octocat) } before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end context 'when author is a GitLab user' do @@ -245,7 +276,7 @@ describe '#create!', :aggregate_failures do let(:issuable) { create(:issue, project: project) } let(:raw) { base } - let(:store) { project.placeholder_reference_store } + let(:cached_references) { placeholder_user_references(Import::SOURCE_GITEA, project.import_state.id) } before do comment.gitlab_issuable = issuable @@ -257,11 +288,10 @@ it 'pushes placeholder references for comments made by existing users in Gitea' do comment.create! - cached_references = store.get(100).map { |ref| Import::SourceUserPlaceholderReference.from_serialized(ref) } - expect(cached_references.map(&:model)).to eq(['Note']) - expect(cached_references.map(&:source_user_id)).to eq([import_source_user.id]) - expect(cached_references.map(&:user_reference_column)).to match_array(['author_id']) + expect(cached_references).to match_array([ + ['Note', an_instance_of(Integer), 'author_id', import_source_user.id] + ]) end context 'when the comment was made by a deleted user in Gitea' do @@ -269,18 +299,52 @@ it 'does not push any placeholder references' do comment.create! - expect(store).to be_empty + expect(cached_references).to be_empty + end + end + + context 'when importing into a personal namespace' do + before_all do + project.update!(namespace: create(:namespace)) + end + + it 'does not push any placeholder references' do + comment.create! + expect(cached_references).to be_empty + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'pushes placeholder references for the pull request and assignees' do + comment.create! + + import_source_user = Import::SourceUser.find_source_user( + source_user_identifier: octocat[:id], + namespace: project.root_ancestor, + source_hostname: 'https://gitea.com', + import_type: ::Import::SOURCE_GITEA + ) + + expect(cached_references).to match_array([ + ['Note', an_instance_of(Integer), 'author_id', import_source_user.id] + ]) + end end end context 'when user contribution mapping is disabled' do before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'does not push any placeholder references' do comment.create! - expect(store).to be_empty + expect(cached_references).to be_empty end end end diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index 6e24a86e56d480e87c2e592eb6c6d2eb38b4da75..e6911f141d5332e0abc18f16bae58bbe0fe49cb2 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -3,15 +3,14 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::Importer, :clean_gitlab_redis_shared_state, feature_category: :importers do - include Import::GiteaHelper - subject(:importer) { described_class.new(project) } let_it_be(:api_root) { 'https://try.gitea.io/api/v1' } let_it_be(:repo_root) { 'https://try.gitea.io' } - let_it_be(:project) do + let_it_be_with_reload(:project) do create( - :project, :repository, :wiki_disabled, :import_user_mapping_enabled, + :project, :repository, :wiki_disabled, :in_group, + :import_user_mapping_enabled, :user_mapping_to_personal_namespace_owner_enabled, import_url: "#{repo_root}/foo/group/project.git", import_type: ::Import::SOURCE_GITEA ) @@ -184,7 +183,6 @@ context 'with stages' do before do - allow(project).to receive(:import_data).and_return(double.as_null_object) allow(project).to receive_message_chain(:wiki, :repository_exists?).and_return(true) allow(importer).to receive(:fetch_resources).and_return(nil) end @@ -253,9 +251,64 @@ }) end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'does not enqueue the worker to load placeholder references' do + expect(Import::LoadPlaceholderReferencesWorker).not_to receive(:perform_async) + + importer.execute + end + + it 'does not sleep' do + allow(store).to receive(:empty?).and_return(false) + + expect(Kernel).not_to receive(:sleep) + + importer.execute + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'loads placeholder references after each relevant stage' do + stages_that_push_placeholder_references = [ + :import_pull_requests, :import_issues, :import_comments + ] + + expect(::Import::LoadPlaceholderReferencesWorker).to receive(:perform_async).exactly( + stages_that_push_placeholder_references.length + ).times.with( + project.import_type, + project.import_state.id + ) + + importer.execute + end + + it 'waits for the placeholder references to be loaded from the store without error' do + allow(store).to receive(:empty?).and_return(false, false, false, false, true) + + expect(Kernel).to receive(:sleep).with(0.01).exactly(4).times + + importer.execute + + expect(Gitlab::Json.parse(project.import_state.last_error)).to be_nil + end + end + end + context 'when user contribution mapping is disabled' do before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'does not enqueue the worker to load placeholder references' do @@ -276,8 +329,6 @@ context 'when an error occurs' do before do - allow(project).to receive(:import_data).and_return(double.as_null_object) - allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) allow_any_instance_of(Octokit::Client).to receive(:rate_limit!).and_raise(Octokit::NotFound) diff --git a/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb index 515c90a33c23f433844f78083c9c6043a9cae304..bdca93e5acdf6ec682a4db9673dbae8372f30462 100644 --- a/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb @@ -3,25 +3,18 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::IssueFormatter, :clean_gitlab_redis_shared_state, feature_category: :importers do - include Import::GiteaHelper + include Import::UserMappingHelper - let_it_be(:project) do + let_it_be_with_reload(:project) do create( :project, :with_import_url, :import_user_mapping_enabled, + :user_mapping_to_personal_namespace_owner_enabled, :in_group, import_type: ::Import::SOURCE_GITEA) end - let_it_be(:source_user_mapper) do - Gitlab::Import::SourceUserMapper.new( - namespace: project.root_ancestor, - import_type: project.import_type, - source_hostname: 'https://gitea.com' - ) - end - let_it_be(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } let_it_be(:import_source_user) do create( @@ -33,6 +26,14 @@ ) end + let(:source_user_mapper) do + Gitlab::Import::SourceUserMapper.new( + namespace: project.root_ancestor, + import_type: project.import_type, + source_hostname: 'https://gitea.com' + ) + end + let(:client) { instance_double(Gitlab::LegacyGithubImport::Client) } let(:ghost_user) { { id: -1, login: 'Ghost' } } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } @@ -145,11 +146,38 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + let(:raw_data) { base_data.merge(assignee: octocat) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'maps the assignee to the personal namespace owner' do + expect(issue.attributes.fetch(:assignee_ids)).to contain_exactly(user_namespace.owner_id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'maps the assignee to the import user' do + expect(issue.attributes.fetch(:assignee_ids)).to contain_exactly( + user_namespace.namespace_import_user.user_id + ) + end + end + end + context 'and user contribution mapping is disabled' do let(:raw_data) { base_data.merge(assignee: octocat) } before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns nil as assignee_id when is not a GitLab user' do @@ -213,11 +241,38 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + let(:raw_data) { base_data.merge(assignee: octocat) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'maps the author to the personal namespace owner' do + expect(issue.attributes.fetch(:author_id)).to eq(user_namespace.owner_id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'maps the author to the import user' do + expect(issue.attributes.fetch(:author_id)).to eq( + user_namespace.namespace_import_user.user_id + ) + end + end + end + context 'and user contribution mapping is disabled' do let(:raw_data) { base_data.merge(user: octocat) } before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns project creator_id as author_id when is not a GitLab user' do @@ -253,23 +308,16 @@ end context 'when importing a GitHub project' do - let_it_be(:project) do + let_it_be_with_reload(:project) do create( :project, :with_import_url, :in_group, :import_user_mapping_enabled, + :user_mapping_to_personal_namespace_owner_enabled, import_type: ::Import::SOURCE_GITHUB) end - let_it_be(:source_user_mapper) do - Gitlab::Import::SourceUserMapper.new( - namespace: project.root_ancestor, - import_type: project.import_type, - source_hostname: 'https://github.com' - ) - end - let_it_be(:import_source_user) do create( :import_source_user, @@ -280,6 +328,14 @@ ) end + let(:source_user_mapper) do + Gitlab::Import::SourceUserMapper.new( + namespace: project.root_ancestor, + import_type: project.import_type, + source_hostname: 'https://github.com' + ) + end + let(:imported_from) { ::Import::SOURCE_GITHUB } it_behaves_like 'Gitlab::LegacyGithubImport::IssueFormatter#attributes' @@ -362,7 +418,7 @@ describe '#create!', :aggregate_failures do let(:raw_data) { base_data.merge(assignee: octocat) } - let(:store) { project.placeholder_reference_store } + let(:cached_references) { placeholder_user_references(Import::SOURCE_GITEA, project.import_state.id) } it 'saves the issue and assignees' do issue.create! @@ -372,47 +428,77 @@ expect(created_issue&.issue_assignees).not_to be_empty end - it 'pushes placeholder references for user references on the issue' do + it 'pushes placeholder references for the issue and assignees' do issue.create! - cached_references = store.get(100).filter_map do |item| - reference = Import::SourceUserPlaceholderReference.from_serialized(item) - reference if reference.model == 'Issue' - end - expect(cached_references.map(&:model)).to eq(['Issue']) - expect(cached_references.map(&:source_user_id)).to eq([import_source_user.id]) - expect(cached_references.map(&:user_reference_column)).to eq(['author_id']) + expect(cached_references).to match_array([ + ['Issue', an_instance_of(Integer), 'author_id', import_source_user.id], + [ + 'IssueAssignee', + a_hash_including('issue_id' => anything, 'user_id' => anything), + 'user_id', + import_source_user.id + ] + ]) end - it 'pushes placeholder references for user references on the issue assignees' do - issue.create! - cached_references = store.get(100).filter_map do |item| - reference = Import::SourceUserPlaceholderReference.from_serialized(item) - reference if reference.model == 'IssueAssignee' - end + context 'when the issue references deleted users in Gitea' do + let(:raw_data) { base_data.merge(user: ghost_user, assignee: ghost_user) } - expect(cached_references.map(&:model)).to match_array(['IssueAssignee']) - expect(cached_references.map(&:source_user_id)).to eq([import_source_user.id]) - expect(cached_references.map(&:user_reference_column)).to match_array(['user_id']) + it 'does not push any placeholder references' do + issue.create! + expect(cached_references).to be_empty + end end - context 'when the issue references deleted users in Gitea' do - let(:raw_data) { base_data.merge(user: ghost_user, assignee: ghost_user) } + context 'when importing into a personal namespace' do + before_all do + project.update!(namespace: create(:namespace)) + end it 'does not push any placeholder references' do issue.create! - expect(store.empty?).to eq(true) + expect(cached_references).to be_empty + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'pushes placeholder references for the issue and assignees' do + issue.create! + + import_source_user = Import::SourceUser.find_source_user( + source_user_identifier: octocat[:id], + namespace: project.root_ancestor, + source_hostname: 'https://gitea.com', + import_type: ::Import::SOURCE_GITEA + ) + + expect(cached_references).to match_array([ + ['Issue', an_instance_of(Integer), 'author_id', import_source_user.id], + [ + 'IssueAssignee', + a_hash_including('issue_id' => anything, 'user_id' => anything), + 'user_id', + import_source_user.id + ] + ]) + end end end context 'when user contribution mapping is disabled' do before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'does not push any placeholder references' do issue.create! - expect(store.empty?).to eq(true) + expect(cached_references).to be_empty end end end diff --git a/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb index 85b62f2b35150326a63e9dd71d6fb35b93f27515..e710299cd8f0ebf5c7ceb63e4d01a707cbc7d774 100644 --- a/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/pull_request_formatter_spec.rb @@ -3,27 +3,20 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::PullRequestFormatter, :clean_gitlab_redis_shared_state, feature_category: :importers do - include Import::GiteaHelper + include Import::UserMappingHelper - let_it_be(:project) do + let_it_be_with_reload(:project) do create( :project, :repository, :with_import_url, :in_group, :import_user_mapping_enabled, + :user_mapping_to_personal_namespace_owner_enabled, import_type: ::Import::SOURCE_GITEA ) end - let_it_be(:source_user_mapper) do - Gitlab::Import::SourceUserMapper.new( - namespace: project.root_ancestor, - import_type: project.import_type, - source_hostname: 'https://gitea.com' - ) - end - let_it_be(:octocat) { { id: 123456, login: 'octocat', email: 'octocat@example.com' } } let_it_be(:import_source_user) do create( @@ -35,6 +28,14 @@ ) end + let(:source_user_mapper) do + Gitlab::Import::SourceUserMapper.new( + namespace: project.root_ancestor, + import_type: project.import_type, + source_hostname: 'https://gitea.com' + ) + end + let(:client) { instance_double(Gitlab::LegacyGithubImport::Client) } let(:ghost_user) { { id: -1, login: 'Ghost' } } let(:source_sha) { create(:commit, project: project).id } @@ -191,11 +192,38 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + let(:raw_data) { base_data.merge(assignee: octocat) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'maps the assignee to the personal namespace owner' do + expect(pull_request.attributes.fetch(:assignee_id)).to eq(user_namespace.owner_id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'maps the assignee to the import user' do + expect(pull_request.attributes.fetch(:assignee_id)).to eq( + user_namespace.namespace_import_user.user_id + ) + end + end + end + context 'and user contribution mapping is disabled' do let(:raw_data) { base_data.merge(assignee: octocat) } before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns nil as assignee_id when is not a GitLab user' do @@ -238,11 +266,38 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + let(:raw_data) { base_data.merge(assignee: octocat) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'maps the author to the personal namespace owner' do + expect(pull_request.attributes.fetch(:author_id)).to eq(user_namespace.owner_id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'maps the assignee to the import user' do + expect(pull_request.attributes.fetch(:author_id)).to eq( + user_namespace.namespace_import_user.user_id + ) + end + end + end + context 'and user contribution mapping is disabled' do let(:raw_data) { base_data.merge(user: octocat) } before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns project creator_id as author_id when is not a GitLab user' do @@ -347,25 +402,18 @@ end context 'when importing a GitHub project' do - let_it_be(:project) do + let_it_be_with_reload(:project) do create( :project, :repository, :with_import_url, :in_group, :import_user_mapping_enabled, + :user_mapping_to_personal_namespace_owner_enabled, import_type: ::Import::SOURCE_GITHUB ) end - let_it_be(:source_user_mapper) do - Gitlab::Import::SourceUserMapper.new( - namespace: project.root_ancestor, - import_type: project.import_type, - source_hostname: 'https://github.com' - ) - end - let_it_be(:import_source_user) do create( :import_source_user, @@ -376,6 +424,14 @@ ) end + let(:source_user_mapper) do + Gitlab::Import::SourceUserMapper.new( + namespace: project.root_ancestor, + import_type: project.import_type, + source_hostname: 'https://github.com' + ) + end + let(:imported_from) { ::Import::SOURCE_GITHUB } it_behaves_like 'Gitlab::LegacyGithubImport::PullRequestFormatter#attributes' @@ -505,7 +561,7 @@ describe '#create!', :aggregate_failures, :clean_gitlab_redis_shared_state do let(:raw_data) { base_data.merge(assignee: octocat) } - let(:store) { project.placeholder_reference_store } + let(:cached_references) { placeholder_user_references(Import::SOURCE_GITEA, project.import_state.id) } it 'saves the pull_request and assignees' do pull_request.create! @@ -515,28 +571,48 @@ expect(created_pull_request&.merge_request_assignees).not_to be_empty end - it 'pushes placeholder references for user references on the pull_request' do + it 'pushes placeholder references for the pull request and assignees' do pull_request.create! - cached_references = store.get(100).filter_map do |item| - reference = Import::SourceUserPlaceholderReference.from_serialized(item) - reference if reference.model == 'MergeRequest' - end - expect(cached_references.map(&:model)).to eq(['MergeRequest']) - expect(cached_references.map(&:source_user_id)).to eq([import_source_user.id]) - expect(cached_references.map(&:user_reference_column)).to eq(['author_id']) + expect(cached_references).to match_array([ + ['MergeRequest', an_instance_of(Integer), 'author_id', import_source_user.id], + ['MergeRequestAssignee', an_instance_of(Integer), 'user_id', import_source_user.id] + ]) end - it 'pushes placeholder references for user references on the pull_request assignees' do - pull_request.create! - cached_references = store.get(100).filter_map do |item| - reference = Import::SourceUserPlaceholderReference.from_serialized(item) - reference if reference.model == 'MergeRequestAssignee' + context 'when importing into a personal namespace' do + before_all do + project.update!(namespace: create(:namespace)) + end + + it 'does not push any placeholder references' do + pull_request.create! + expect(cached_references).to be_empty end - expect(cached_references.map(&:model)).to match_array(['MergeRequestAssignee']) - expect(cached_references.map(&:source_user_id).uniq).to eq([import_source_user.id]) - expect(cached_references.map(&:user_reference_column)).to match_array(['user_id']) + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'pushes placeholder references for the pull request and assignees' do + pull_request.create! + + import_source_user = Import::SourceUser.find_source_user( + source_user_identifier: octocat[:id], + namespace: project.root_ancestor, + source_hostname: 'https://gitea.com', + import_type: ::Import::SOURCE_GITEA + ) + + expect(cached_references).to match_array([ + ['MergeRequest', an_instance_of(Integer), 'author_id', import_source_user.id], + ['MergeRequestAssignee', an_instance_of(Integer), 'user_id', import_source_user.id] + ]) + end + end end context 'when the pull_request references deleted users in Gitea' do @@ -544,18 +620,18 @@ it 'does not push any placeholder references' do pull_request.create! - expect(store.empty?).to eq(true) + expect(cached_references).to be_empty end end context 'when user contribution mapping is disabled' do before do - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'does not push any placeholder references' do pull_request.create! - expect(store.empty?).to eq(true) + expect(cached_references).to be_empty end end end diff --git a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb index e411649ff4a789d714c08abc7b03691304b0106e..cab596f3933aba620a74086ef7e83ee247ee56bb 100644 --- a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb @@ -3,11 +3,18 @@ require 'spec_helper' RSpec.describe Gitlab::LegacyGithubImport::UserFormatter, feature_category: :importers do - include Import::GiteaHelper + let_it_be_with_reload(:project) do + create( + :project, :in_group, + :import_user_mapping_enabled, :user_mapping_to_personal_namespace_owner_enabled, + import_type: 'gitea' + ) + end - let_it_be(:project) { create(:project, :import_user_mapping_enabled, :in_group, import_type: 'gitea') } + # GitLab's system ghost user - used as mapping for Gitea ghost user + let_it_be(:gitlab_ghost_user) { Users::Internal.ghost } - let_it_be(:source_user_mapper) do + let(:source_user_mapper) do Gitlab::Import::SourceUserMapper.new( namespace: project.root_ancestor, import_type: project.import_type, @@ -15,9 +22,6 @@ ) end - # GitLab's system ghost user - used as mapping for Gitea ghost user - let_it_be(:gitlab_ghost_user) { Users::Internal.ghost } - let(:client) { instance_double(Gitlab::LegacyGithubImport::Client) } let(:gitea_user) { { id: 123456, login: 'octocat', full_name: 'Git Tea', email: 'user@email.com' } } let(:ghost_user) { { id: -1, login: 'Ghost' } } @@ -74,10 +78,35 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'returns the personal namespace owner id without creating a source user' do + expect { user_formatter.gitlab_id }.not_to change { Import::SourceUser.count } + expect(user_formatter.gitlab_id).to eq(user_namespace.owner_id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'returns the import user id' do + expect(user_formatter.gitlab_id).to eq(user_namespace.namespace_import_user.user_id) + end + end + end + context 'when user contribution mapping is disabled' do before do allow(client).to receive(:user).and_return(gitea_user) - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns GitLab user id when user confirmed primary email matches Gitea email' do @@ -120,10 +149,35 @@ expect { user_formatter.gitlab_id }.not_to change { User.where(user_type: :placeholder).count }.from(0) end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'returns the gitlab ghost user id without creating a source user' do + expect { user_formatter.gitlab_id }.not_to change { Import::SourceUser.count } + expect(user_formatter.gitlab_id).to eq(gitlab_ghost_user.id) + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'returns the gitlab ghost user id' do + expect(user_formatter.gitlab_id).to eq(gitlab_ghost_user.id) + end + end + end + context 'and improved user mapping is disabled' do before do allow(client).to receive(:user).and_return(ghost_user) - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns nil' do @@ -195,10 +249,36 @@ end end + context 'when importing into a personal namespace' do + let_it_be(:user_namespace) { create(:namespace) } + + before_all do + project.update!(namespace: user_namespace) + end + + it 'returns nil' do + expect(user_formatter.source_user).to be_nil + end + + context 'when user_mapping_to_personal_namespace_owner is disabled' do + before_all do + project.build_or_assign_import_data( + data: { user_mapping_to_personal_namespace_owner_enabled: false } + ).save! + end + + it 'returns a source user' do + allow(client).to receive(:user).and_return(gitea_user) + + expect(user_formatter.source_user).to be_an_instance_of(Import::SourceUser) + end + end + end + context 'when user contribution mapping is disabled' do before do allow(client).to receive(:user).and_return(gitea_user) - stub_user_mapping_chain(project, false) + project.build_or_assign_import_data(data: { user_contribution_mapping_enabled: false }).save! end it 'returns nil' do diff --git a/spec/support/helpers/import/gitea_helper.rb b/spec/support/helpers/import/gitea_helper.rb deleted file mode 100644 index 7528ed83e69fd8f5409be196f1d5e51acdbd83cf..0000000000000000000000000000000000000000 --- a/spec/support/helpers/import/gitea_helper.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -# Helper methods for Gitea user mapping specs -module Import - module GiteaHelper - # This method allows a project to receive two message chains to check the - # user mapping state of a project. - def stub_user_mapping_chain(project, user_mapping_enabled) - allow(project).to receive_message_chain(:import_data, :reset, :user_mapping_enabled?) - .and_return(user_mapping_enabled) - - allow(project).to receive_message_chain(:import_data, :user_mapping_enabled?) - .and_return(user_mapping_enabled) - end - end -end