diff --git a/app/models/concerns/has_user_type.rb b/app/models/concerns/has_user_type.rb index 871ffda3d90e3253a632e86cf5daca515787a8f6..6c272921351cba9bd52e5c86e8796e89345527b3 100644 --- a/app/models/concerns/has_user_type.rb +++ b/app/models/concerns/has_user_type.rb @@ -18,7 +18,8 @@ module HasUserType admin_bot: 11, suggested_reviewers_bot: 12, service_account: 13, - llm_bot: 14 + llm_bot: 14, + placeholder: 15 }.with_indifferent_access.freeze BOT_USER_TYPES = %w[ diff --git a/app/models/import/source_user.rb b/app/models/import/source_user.rb new file mode 100644 index 0000000000000000000000000000000000000000..fc53b7b9ef97a08ca5189996f453409585ea399a --- /dev/null +++ b/app/models/import/source_user.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Import + class SourceUser < ApplicationRecord + self.table_name = 'import_source_users' + + belongs_to :placeholder_user, class_name: 'User', optional: true + belongs_to :reassign_to_user, class_name: 'User', optional: true + belongs_to :namespace + + validates :namespace_id, :import_type, :source_hostname, :source_user_identifier, :status, presence: true + + scope :for_namespace, ->(namespace_id) { where(namespace_id: namespace_id) } + + state_machine :status, initial: :pending do + state :pending_assignment, value: 0 + state :awaiting_approval, value: 1 + state :rejected, value: 2 + state :failed, value: 3 + state :completed, value: 4 + end + + def self.find_source_user(source_user_identifier:, namespace:, source_hostname:, import_type:) + return unless namespace + + find_by( + source_user_identifier: source_user_identifier, + namespace_id: namespace.id, + source_hostname: source_hostname, + import_type: import_type + ) + end + end +end diff --git a/app/models/user.rb b/app/models/user.rb index acda8cd88e741bc779619477a11ac9b50f4572f7..721dca9d5a56d53f7cd2efd6423114382b579605 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1015,6 +1015,10 @@ def get_ids_by_ids_or_usernames(ids, usernames) def generate_incoming_mail_token "#{INCOMING_MAIL_TOKEN_PREFIX}#{SecureRandom.hex.to_i(16).to_s(36)}" end + + def username_exists?(username) + exists?(username: username) + end end # diff --git a/db/docs/import_source_users.yml b/db/docs/import_source_users.yml new file mode 100644 index 0000000000000000000000000000000000000000..5c4c8d2ef13f2a39e49167f2c6f554f0105cee8e --- /dev/null +++ b/db/docs/import_source_users.yml @@ -0,0 +1,18 @@ +--- +table_name: import_source_users +classes: +- Import::SourceUser +feature_categories: +- importers +description: Used to map imported user contributions to internal placeholder users or real users +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/147488 +milestone: '17.0' +gitlab_schema: gitlab_main_cell +allow_cross_joins: +- gitlab_main_clusterwide +allow_cross_transactions: +- gitlab_main_clusterwide +allow_cross_foreign_keys: +- gitlab_main_clusterwide +sharding_key: + namespace_id: namespaces diff --git a/db/migrate/20240321163104_create_import_source_users.rb b/db/migrate/20240321163104_create_import_source_users.rb new file mode 100644 index 0000000000000000000000000000000000000000..ef78b75ca9d8ad9711096c910ef089acfb9f559a --- /dev/null +++ b/db/migrate/20240321163104_create_import_source_users.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class CreateImportSourceUsers < Gitlab::Database::Migration[2.2] + milestone '17.0' + + def change + create_table :import_source_users do |t| + t.references :placeholder_user, index: true, foreign_key: { to_table: :users, on_delete: :nullify } + t.references :reassign_to_user, index: true, foreign_key: { to_table: :users, on_delete: :nullify } + t.references :namespace, null: false, index: true, foreign_key: { on_delete: :cascade } + t.timestamps_with_timezone null: false + t.integer :status, null: false, limit: 2, default: 0 + t.text :source_username, limit: 255 + t.text :source_name, limit: 255 + t.text :source_user_identifier, null: false, limit: 255 + t.text :source_hostname, null: false, limit: 255 + t.text :import_type, null: false, limit: 255 + end + + add_index(:import_source_users, + %i[source_user_identifier namespace_id source_hostname import_type], + unique: true, + name: 'unique_import_source_users_source_identifier_and_import_source' + ) + end +end diff --git a/db/schema_migrations/20240321163104 b/db/schema_migrations/20240321163104 new file mode 100644 index 0000000000000000000000000000000000000000..55308c4bd4b0c5823015145a3eb342338d88355a --- /dev/null +++ b/db/schema_migrations/20240321163104 @@ -0,0 +1 @@ +b527c507041a3cf1f1bd65208c73bc5d20f6b0b35233a90313347b59931aa74d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 4ef1aa64a9330833042d09d017b7e987215a0a7e..3b411348806a4b7b864058ceb2a08f6cf63bc3c8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9797,6 +9797,35 @@ CREATE SEQUENCE import_failures_id_seq ALTER SEQUENCE import_failures_id_seq OWNED BY import_failures.id; +CREATE TABLE import_source_users ( + id bigint NOT NULL, + placeholder_user_id bigint, + reassign_to_user_id bigint, + namespace_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + status smallint DEFAULT 0 NOT NULL, + source_username text, + source_name text, + source_user_identifier text NOT NULL, + source_hostname text NOT NULL, + import_type text NOT NULL, + CONSTRAINT check_0d7295a307 CHECK ((char_length(import_type) <= 255)), + CONSTRAINT check_199c28ec54 CHECK ((char_length(source_username) <= 255)), + CONSTRAINT check_562655155f CHECK ((char_length(source_name) <= 255)), + CONSTRAINT check_cc9d4093b5 CHECK ((char_length(source_user_identifier) <= 255)), + CONSTRAINT check_e2039840c5 CHECK ((char_length(source_hostname) <= 255)) +); + +CREATE SEQUENCE import_source_users_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE import_source_users_id_seq OWNED BY import_source_users.id; + CREATE TABLE incident_management_escalation_policies ( id bigint NOT NULL, project_id bigint NOT NULL, @@ -19241,6 +19270,8 @@ ALTER TABLE ONLY import_export_uploads ALTER COLUMN id SET DEFAULT nextval('impo ALTER TABLE ONLY import_failures ALTER COLUMN id SET DEFAULT nextval('import_failures_id_seq'::regclass); +ALTER TABLE ONLY import_source_users ALTER COLUMN id SET DEFAULT nextval('import_source_users_id_seq'::regclass); + ALTER TABLE ONLY incident_management_escalation_policies ALTER COLUMN id SET DEFAULT nextval('incident_management_escalation_policies_id_seq'::regclass); ALTER TABLE ONLY incident_management_escalation_rules ALTER COLUMN id SET DEFAULT nextval('incident_management_escalation_rules_id_seq'::regclass); @@ -21395,6 +21426,9 @@ ALTER TABLE ONLY import_export_uploads ALTER TABLE ONLY import_failures ADD CONSTRAINT import_failures_pkey PRIMARY KEY (id); +ALTER TABLE ONLY import_source_users + ADD CONSTRAINT import_source_users_pkey PRIMARY KEY (id); + ALTER TABLE ONLY incident_management_oncall_shifts ADD CONSTRAINT inc_mgmnt_no_overlapping_oncall_shifts EXCLUDE USING gist (rotation_id WITH =, tstzrange(starts_at, ends_at, '[)'::text) WITH &&); @@ -25633,6 +25667,12 @@ CREATE INDEX index_import_failures_on_project_id_not_null ON import_failures USI CREATE INDEX index_import_failures_on_user_id_not_null ON import_failures USING btree (user_id) WHERE (user_id IS NOT NULL); +CREATE INDEX index_import_source_users_on_namespace_id ON import_source_users USING btree (namespace_id); + +CREATE INDEX index_import_source_users_on_placeholder_user_id ON import_source_users USING btree (placeholder_user_id); + +CREATE INDEX index_import_source_users_on_reassign_to_user_id ON import_source_users USING btree (reassign_to_user_id); + CREATE INDEX index_imported_projects_on_import_type_creator_id_created_at ON projects USING btree (import_type, creator_id, created_at) WHERE (import_type IS NOT NULL); CREATE INDEX index_imported_projects_on_import_type_id ON projects USING btree (import_type, id) WHERE (import_type IS NOT NULL); @@ -28003,6 +28043,8 @@ CREATE UNIQUE INDEX unique_idx_member_approvals_on_pending_status ON member_appr CREATE UNIQUE INDEX unique_idx_namespaces_storage_limit_exclusions_on_namespace_id ON namespaces_storage_limit_exclusions USING btree (namespace_id); +CREATE UNIQUE INDEX unique_import_source_users_source_identifier_and_import_source ON import_source_users USING btree (source_user_identifier, namespace_id, source_hostname, import_type); + CREATE UNIQUE INDEX unique_index_ci_build_pending_states_on_partition_id_build_id ON ci_build_pending_states USING btree (partition_id, build_id); CREATE UNIQUE INDEX unique_index_for_credit_card_validation_payment_method_xid ON user_credit_card_validations USING btree (zuora_payment_method_xid) WHERE (zuora_payment_method_xid IS NOT NULL); @@ -31097,6 +31139,9 @@ ALTER TABLE ONLY namespaces_storage_limit_exclusions ALTER TABLE ONLY users_security_dashboard_projects ADD CONSTRAINT fk_rails_150cd5682c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY import_source_users + ADD CONSTRAINT fk_rails_167f82fd95 FOREIGN KEY (reassign_to_user_id) REFERENCES users(id) ON DELETE SET NULL; + ALTER TABLE ONLY ci_build_report_results ADD CONSTRAINT fk_rails_16cb1ff064_p FOREIGN KEY (partition_id, build_id) REFERENCES p_ci_builds(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE; @@ -31730,6 +31775,9 @@ ALTER TABLE ONLY ci_cost_settings ALTER TABLE ONLY operations_feature_flags_issues ADD CONSTRAINT fk_rails_6a8856ca4f FOREIGN KEY (feature_flag_id) REFERENCES operations_feature_flags(id) ON DELETE CASCADE; +ALTER TABLE ONLY import_source_users + ADD CONSTRAINT fk_rails_6aee6cd676 FOREIGN KEY (placeholder_user_id) REFERENCES users(id) ON DELETE SET NULL; + ALTER TABLE ONLY ml_experiment_metadata ADD CONSTRAINT fk_rails_6b39844d44 FOREIGN KEY (experiment_id) REFERENCES ml_experiments(id) ON DELETE CASCADE; @@ -32690,6 +32738,9 @@ ALTER TABLE ONLY ci_daily_build_group_report_results ALTER TABLE ONLY ci_daily_build_group_report_results ADD CONSTRAINT fk_rails_ee072d13b3_tmp FOREIGN KEY (last_pipeline_id) REFERENCES ci_pipelines(id_convert_to_bigint) ON DELETE CASCADE NOT VALID; +ALTER TABLE ONLY import_source_users + ADD CONSTRAINT fk_rails_ee30e569be FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY audit_events_group_streaming_event_type_filters ADD CONSTRAINT fk_rails_ee6950967f FOREIGN KEY (external_streaming_destination_id) REFERENCES audit_events_group_external_streaming_destinations(id) ON DELETE CASCADE; diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index b090c95ac22e49eb9fa7041ab41b9338b5914c92..2eb1a0e5ea90de9cf85aff47bb1a09a168c6d012 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -1206,7 +1206,7 @@ AND "users"."user_type" IN \(0, 6, 4, 13\) AND - "users"."user_type" IN \(0, 4, 5\)'.squish + "users"."user_type" IN \(0, 4, 5, 15\)'.squish end it 'validates the sql matches the specific index we have' do @@ -1232,7 +1232,7 @@ AND "users"."user_type" IN \(0, 6, 4, 13\) AND - "users"."user_type" IN \(0, 4, 5\) + "users"."user_type" IN \(0, 4, 5, 15\) AND \(EXISTS \(SELECT 1 FROM "members" LEFT OUTER JOIN "member_roles" ON "member_roles"."id" = "members"."member_role_id" diff --git a/lib/gitlab/import/placeholder_user_creator.rb b/lib/gitlab/import/placeholder_user_creator.rb new file mode 100644 index 0000000000000000000000000000000000000000..011c4d49e1a946da35a450748f1bfddf0be898cf --- /dev/null +++ b/lib/gitlab/import/placeholder_user_creator.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Gitlab + module Import + class PlaceholderUserCreator + def initialize(import_type:, source_hostname:, source_name:, source_username:) + @import_type = import_type + @source_hostname = source_hostname + @source_name = source_name + @source_username = source_username + end + + def execute + user = User.new( + user_type: :placeholder, + name: placeholder_name, + username: placeholder_username, + email: placeholder_email + ) + + user.assign_personal_namespace(Organizations::Organization.default_organization) + user.save! + user + end + + private + + attr_reader :import_type, :source_hostname, :source_name, :source_username + + def placeholder_name + # Some APIs don't expose users' names, so set a default if it's nil + name = source_name || "#{import_type} Source User" + "Placeholder #{name}" + end + + def placeholder_username + # Some APIs don't expose users' usernames, so set a default if it's nil + username_pattern = "#{valid_source_username}_placeholder_user_%s" + lambda_for_unique_username = ->(username) { User.username_exists?(username) } + uniquify_string(username_pattern, lambda_for_unique_username) + end + + def placeholder_email + email_pattern = "#{valid_source_username}_placeholder_user_%s@#{Settings.gitlab.host}" + lambda_for_unique_email = ->(email) { User.find_by_email(email) } + uniquify_string(email_pattern, lambda_for_unique_email) + end + + def valid_source_username + return "#{import_type}_source_username" unless source_username + + source_username.slice(0, User::MAX_USERNAME_LENGTH - 55) + end + + def uniquify_string(base_pattern, lambda_for_uniqueness) + uniquify = Gitlab::Utils::Uniquify.new(1) + + uniquify.string(->(unique_number) { format(base_pattern, unique_number) }) do |str| + lambda_for_uniqueness.call(str) + end + end + end + end +end diff --git a/lib/gitlab/import/source_user_mapper.rb b/lib/gitlab/import/source_user_mapper.rb new file mode 100644 index 0000000000000000000000000000000000000000..b4a041bdf965601a95f8743df04ecd392bf038f8 --- /dev/null +++ b/lib/gitlab/import/source_user_mapper.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Gitlab + module Import + class SourceUserMapper + include Gitlab::ExclusiveLeaseHelpers + + def initialize(namespace:, import_type:, source_hostname:) + @namespace = namespace + @import_type = import_type + @source_hostname = source_hostname + end + + def find_or_create_internal_user(source_name:, source_username:, source_user_identifier:) + @source_name = source_name + @source_username = source_username + @source_user_identifier = source_user_identifier + + internal_user = find_internal_user + return internal_user if internal_user + + in_lock(lock_key(source_user_identifier), sleep_sec: 2.seconds) do |retried| + if retried + internal_user = find_internal_user + next internal_user if internal_user + end + + create_source_user_mapping + end + end + + private + + attr_reader :namespace, :import_type, :source_hostname, :source_name, :source_username, :source_user_identifier + + def find_internal_user + source_user = ::Import::SourceUser.find_source_user( + source_user_identifier: source_user_identifier, + namespace: namespace, + source_hostname: source_hostname, + import_type: import_type + ) + + return unless source_user + + source_user.reassign_to_user || source_user.placeholder_user + end + + def create_source_user_mapping + ::Import::SourceUser.transaction do + import_source_user = ::Import::SourceUser.new( + namespace: namespace, + import_type: import_type, + source_username: source_username, + source_name: source_name, + source_user_identifier: source_user_identifier, + source_hostname: source_hostname + ) + + internal_user = create_placeholder_user + import_source_user.placeholder_user = internal_user + + import_source_user.save! + import_source_user + end + end + + def create_placeholder_user + # If limit is reached, get import user instead, but that's not implemented yet + Gitlab::Import::PlaceholderUserCreator.new( + import_type: import_type, + source_hostname: source_hostname, + source_name: source_name, + source_username: source_username + ).execute + end + + def lock_key(source_user_identifier) + "import:source_user_mapper:#{namespace.id}:#{import_type}:#{source_hostname}:#{source_user_identifier}" + end + end + end +end diff --git a/spec/factories/import_source_users.rb b/spec/factories/import_source_users.rb new file mode 100644 index 0000000000000000000000000000000000000000..f0ed58d8489454bbf167a8ec5a5bdb566cc3f7e6 --- /dev/null +++ b/spec/factories/import_source_users.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :import_source_user, class: 'Import::SourceUser' do + namespace + source_user_identifier { SecureRandom.uuid } + source_hostname { 'github.com' } + import_type { 'github' } + + trait :with_placeholder_user do + placeholder_user factory: [:user, :placeholder] + end + + trait :with_reassign_to_user do + reassign_to_user factory: :user + end + end +end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index c100e3d938f4bddfb64e3586a2b1eb6952ebce60..b49c48b89b96c00a0b333973b2b84ac99e94ab22 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -115,6 +115,10 @@ user_type { :llm_bot } end + trait :placeholder do + user_type { :placeholder } + end + trait :external do external { true } end diff --git a/spec/lib/gitlab/import/placeholder_user_creator_spec.rb b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..62b62c4f66c7e184806f37351c1e3425078e80d9 --- /dev/null +++ b/spec/lib/gitlab/import/placeholder_user_creator_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Import::PlaceholderUserCreator, feature_category: :importers do + describe '#execute' do + let(:import_type) { 'github' } + let(:source_hostname) { 'github.com' } + let(:source_name) { 'Pry Contributor' } + let(:source_username) { 'a_pry_contributor' } + + subject(:service) do + described_class.new( + import_type: import_type, + source_hostname: source_hostname, + source_name: source_name, + source_username: source_username + ) + end + + it 'creates one new placeholder user with a unique email and username' do + expect { service.execute }.to change { User.where(user_type: :placeholder).count }.from(0).to(1) + + new_placeholder_user = User.where(user_type: :placeholder).last + + expect(new_placeholder_user.name).to eq("Placeholder #{source_name}") + expect(new_placeholder_user.username).to match(/^#{source_username}_placeholder_user_\d+$/) + expect(new_placeholder_user.email).to match(/^#{source_username}_placeholder_user_\d+@#{Settings.gitlab.host}$/) + end + + context 'when there are non-unique usernames on the same import source' do + it 'creates two unique users with different usernames and emails' do + placeholder_user1 = service.execute + placeholder_user2 = service.execute + + expect(placeholder_user1.username).not_to eq(placeholder_user2.username) + expect(placeholder_user1.email).not_to eq(placeholder_user2.email) + end + end + + context 'and the incoming source_user attributes are invalid' do + context 'when source_name is nil' do + let(:source_name) { nil } + + it 'assigns a default name' do + placeholder_user = service.execute + + expect(placeholder_user.name).to eq("Placeholder #{import_type} Source User") + end + end + + context 'when source_username is nil' do + let(:source_username) { nil } + + it 'assigns a default username' do + placeholder_user = service.execute + + expect(placeholder_user.username).to match(/^#{import_type}_source_username_placeholder_user_\d+$/) + end + end + + context 'when source_username is too long' do + let(:source_username) { 'a' * 500 } + + it 'truncates the original username to 200 characters' do + placeholder_user = service.execute + + expect(placeholder_user.username).to match(/^#{'a' * 200}_placeholder_user_\d+$/) + end + end + end + end +end diff --git a/spec/lib/gitlab/import/source_user_mapper_spec.rb b/spec/lib/gitlab/import/source_user_mapper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6e2f8d65d6f600d5bee36f3665d31f8ef52c2596 --- /dev/null +++ b/spec/lib/gitlab/import/source_user_mapper_spec.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Import::SourceUserMapper, feature_category: :importers do + describe '#find_or_create_internal_user' do + let_it_be(:namespace) { create(:namespace) } + + let(:import_type) { 'github' } + let(:source_hostname) { 'github.com' } + let(:source_name) { 'Pry Contributor' } + let(:source_username) { 'a_pry_contributor' } + let(:source_user_identifier) { '123456' } + + subject(:find_or_create_internal_user) do + described_class.new( + namespace: namespace, + import_type: import_type, + source_hostname: source_hostname + ).find_or_create_internal_user( + source_name: source_name, + source_username: source_username, + source_user_identifier: source_user_identifier + ) + end + + shared_examples 'creates an import_source_user and a unique placeholder user' do + it 'creates a import_source_user with an internal placeholder user' do + expect { find_or_create_internal_user }.to change { Import::SourceUser.count }.from(2).to(3) + + new_import_source_user = Import::SourceUser.last + + expect(new_import_source_user.placeholder_user.user_type).to eq('placeholder') + expect(new_import_source_user.attributes).to include({ + 'namespace_id' => namespace.id, + 'import_type' => import_type, + 'source_hostname' => source_hostname, + 'source_name' => source_name, + 'source_username' => source_username, + 'source_user_identifier' => source_user_identifier + }) + end + + it 'creates a new placeholder user with a unique email and username' do + expect { find_or_create_internal_user }.to change { User.where(user_type: :placeholder).count }.from(0).to(1) + + new_placeholder_user = User.where(user_type: :placeholder).last + + expect(new_placeholder_user.name).to eq("Placeholder #{source_name}") + expect(new_placeholder_user.username).to match(/^#{source_username}_placeholder_user_\d+$/) + expect(new_placeholder_user.email).to match(/^#{source_username}_placeholder_user_\d+@#{Settings.gitlab.host}$/) + end + end + + shared_examples 'it does not create an import_source_user or placeholder user' do + it 'does not create a import_source_user' do + expect { find_or_create_internal_user }.not_to change { Import::SourceUser.count } + end + + it 'does not create any internal users' do + expect { find_or_create_internal_user }.not_to change { User.count } + end + end + + context 'when the placeholder user limit has not been reached' do + let!(:import_source_user_from_another_import) { create(:import_source_user) } + let!(:different_source_user_from_same_import) do + create(:import_source_user, + namespace_id: namespace.id, + import_type: import_type, + source_hostname: source_hostname, + source_user_identifier: '999999' + ) + end + + it_behaves_like 'creates an import_source_user and a unique placeholder user' + + context 'when retried and another placeholder user is not created while waiting' do + before do + allow_next_instance_of(described_class) do |source_user_mapper| + allow(source_user_mapper).to receive(:in_lock).and_yield(true) + end + end + + it_behaves_like 'creates an import_source_user and a unique placeholder user' + end + + context 'when retried and another placeholder user was made while waiting' do + let!(:existing_import_source_user) do + create( + :import_source_user, + :with_placeholder_user, + namespace: namespace, + import_type: import_type, + source_hostname: source_hostname, + source_user_identifier: '123456') + end + + before do + allow_next_instance_of(described_class) do |source_user_mapper| + allow(source_user_mapper).to receive(:in_lock).and_yield(true) + end + + allow(Import::SourceUser).to receive(:find_source_user).and_return(nil, existing_import_source_user) + end + + it 'returns the existing placeholder user' do + expect(find_or_create_internal_user).to eq(existing_import_source_user.placeholder_user) + end + + it_behaves_like 'it does not create an import_source_user or placeholder user' + end + + context 'and an import source user exists for current import source' do + context 'and the source user maps to a placeholder user' do + let!(:existing_import_source_user) do + create( + :import_source_user, + :with_placeholder_user, + namespace: namespace, + import_type: import_type, + source_hostname: source_hostname, + source_user_identifier: '123456') + end + + it 'returns the existing placeholder user' do + expect(find_or_create_internal_user).to eq(existing_import_source_user.placeholder_user) + end + + it_behaves_like 'it does not create an import_source_user or placeholder user' + end + + context 'and the source_user maps to a reassigned user' do + let!(:existing_import_source_user) do + create( + :import_source_user, + :with_reassign_to_user, + namespace: namespace, + import_type: import_type, + source_hostname: source_hostname, + source_user_identifier: '123456') + end + + it 'returns the existing placeholder user' do + expect(find_or_create_internal_user).to eq(existing_import_source_user.reassign_to_user) + end + + it_behaves_like 'it does not create an import_source_user or placeholder user' + end + end + end + end +end diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index effb588e55ff7a8eb034e8dd5db6fe0a62f00228..e9f8e0a487cbbd59a2c1b09960a24b582711d4a5 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -14,7 +14,7 @@ expect(described_class::USER_TYPES.keys) .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot automation_bot security_policy_bot admin_bot suggested_reviewers_bot - service_account llm_bot]) + service_account llm_bot placeholder]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) diff --git a/spec/models/import/source_user_spec.rb b/spec/models/import/source_user_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..43464715203b56737ba3c004bbe8a63c210db446 --- /dev/null +++ b/spec/models/import/source_user_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::SourceUser, type: :model, feature_category: :importers do + describe 'associations' do + it { is_expected.to belong_to(:placeholder_user).class_name('User') } + it { is_expected.to belong_to(:reassign_to_user).class_name('User') } + it { is_expected.to belong_to(:namespace) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:namespace_id) } + it { is_expected.to validate_presence_of(:import_type) } + end + + describe 'scopes' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:source_user_1) { create(:import_source_user, namespace: namespace) } + let_it_be(:source_user_2) { create(:import_source_user, namespace: namespace) } + let_it_be(:source_user_3) { create(:import_source_user) } + + describe '.for_namespace' do + it 'only returns source users for the given namespace_id' do + expect(described_class.for_namespace(namespace.id).to_a).to match_array( + [source_user_1, source_user_2] + ) + end + end + end + + describe 'state machine' do + it 'begins in pending state' do + expect(described_class.new.pending_assignment?).to eq(true) + end + end + + describe '.find_source_user' do + let_it_be(:namespace_1) { create(:namespace) } + let_it_be(:namespace_2) { create(:namespace) } + let_it_be(:source_user_1) { create(:import_source_user, source_user_identifier: '1', namespace: namespace_1) } + let_it_be(:source_user_2) { create(:import_source_user, source_user_identifier: '2', namespace: namespace_1) } + let_it_be(:source_user_3) { create(:import_source_user, source_user_identifier: '1', namespace: namespace_2) } + let_it_be(:source_user_4) do + create(:import_source_user, + source_user_identifier: '1', + namespace: namespace_1, + import_type: 'bitbucket', + source_hostname: 'bitbucket.org' + ) + end + + let_it_be(:source_user_5) do + create(:import_source_user, + source_user_identifier: '1', + namespace: namespace_1, + source_hostname: 'bitbucket-server-domain.com', + import_type: 'bitbucket_server' + ) + end + + it 'returns the first source_user that matches the source_user_identifier for the import source attributes' do + expect(described_class.find_source_user( + source_user_identifier: '1', + namespace: namespace_1, + source_hostname: 'github.com', + import_type: 'github' + )).to eq(source_user_1) + end + + it 'does not throw an error when any attributes are nil' do + expect do + described_class.find_source_user( + source_user_identifier: nil, + namespace: nil, + source_hostname: nil, + import_type: nil + ) + end.not_to raise_error + end + + it 'returns nil if no namespace is provided' do + expect(described_class.find_source_user( + source_user_identifier: '1', + namespace: nil, + source_hostname: 'github.com', + import_type: 'github' + )).to be_nil + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 707ee900236c72460a265cafd37279e22a905659..25d33bbd7a362026ce4168a9027a9412be51fbd0 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -8478,6 +8478,18 @@ def owner_class_attribute_default; end end end + describe '.username_exists?' do + let_it_be(:user) { create(:user, username: 'user_1') } + + it 'returns true if a user with the given username exists' do + expect(described_class.username_exists?('user_1')).to be(true) + end + + it 'returns false if a username with the username does not exist' do + expect(described_class.username_exists?('second_user')).to be(false) + end + end + context 'when email is not unique' do let_it_be(:existing_user) { create(:user) }