From e1db6fb1f848465325d94964258681d3f43956aa Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Wed, 2 Apr 2025 15:55:52 +0200
Subject: [PATCH 1/6] Allow specifying the organization for an Internal User

This is optional until all callers support this

Changelog: changed
EE: true
---
 lib/users/internal.rb           | 29 ++++++++++++++++------
 spec/lib/users/internal_spec.rb | 44 ++++++++++++++++++++-------------
 2 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/lib/users/internal.rb b/lib/users/internal.rb
index cc38f6517cd62c..dd02f4baa10b71 100644
--- a/lib/users/internal.rb
+++ b/lib/users/internal.rb
@@ -126,24 +126,37 @@ def admin_bot
         end
       end
 
-      # rubocop:enable CodeReuse/ActiveRecord
-
       def bot_avatar(image:)
         Rails.root.join('lib', 'assets', 'images', 'bot_avatars', image).open
       end
 
+      def for_organization(organization)
+        @organization = organization
+
+        self
+      end
+
       private
 
       # NOTE: This method is patched in spec/spec_helper.rb to allow use of exclusive lease in RSpec's
       # :before_all scope to keep the specs DRY.
       def unique_internal(scope, username, email_pattern, &block)
+        if @organization
+          scope = scope.joins(:organization_users).where(organization_users: { organization: @organization })
+        end
+
         scope.first || create_unique_internal(scope, username, email_pattern, &block)
       end
 
       def create_unique_internal(scope, username, email_pattern, &creation_block)
         # Since we only want a single one of these in an instance, we use an
         # exclusive lease to ensure than this block is never run concurrently.
-        lease_key = "user:unique_internal:#{username}"
+        lease_key = if @organization
+                      "user:unique_internal:#{@organization.id}:#{username}"
+                    else
+                      "user:unique_internal:#{username}"
+                    end
+
         lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.minute.to_i)
 
         uuid = lease.try_obtain
@@ -174,16 +187,18 @@ def create_unique_internal(scope, username, email_pattern, &creation_block)
           &creation_block
         )
 
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/442780
-        organization = ::Organizations::Organization.first
-        user.assign_personal_namespace(organization)
-        user.organizations << organization
+        user_organization = @organization || Organizations::Organization.first
+        user.assign_personal_namespace(user_organization)
+        user.organizations << user_organization
 
         Users::UpdateService.new(user, user: user).execute(validate: false)
         user
       ensure
+        @organization = nil
         Gitlab::ExclusiveLease.cancel(lease_key, uuid)
       end
+
+      # rubocop:enable CodeReuse/ActiveRecord
     end
   end
 end
diff --git a/spec/lib/users/internal_spec.rb b/spec/lib/users/internal_spec.rb
index 0e8ab182a3f0c5..c0e8ee002d17fe 100644
--- a/spec/lib/users/internal_spec.rb
+++ b/spec/lib/users/internal_spec.rb
@@ -3,33 +3,34 @@
 require 'spec_helper'
 
 RSpec.describe Users::Internal, feature_category: :user_profile do
+  let_it_be(:first_organization) { create(:organization) }
   let_it_be(:organization) { create(:organization) }
 
   shared_examples 'bot users' do |bot_type, username, email|
     it 'creates the user if it does not exist' do
       expect do
-        described_class.public_send(bot_type)
+        described_class.for_organization(organization).public_send(bot_type)
       end.to change { User.where(user_type: bot_type).count }.by(1)
     end
 
     it 'creates a route for the namespace of the created user' do
-      bot_user = described_class.public_send(bot_type)
+      bot_user = described_class.for_organization(organization).public_send(bot_type)
 
       expect(bot_user.namespace.route).to be_present
       expect(bot_user.namespace.organization).to eq(organization)
     end
 
-    it 'assigns the first found organization to the created user' do
-      bot_user = described_class.public_send(bot_type)
+    it 'assigns the organization to the created user' do
+      bot_user = described_class.for_organization(organization).public_send(bot_type)
 
-      expect(bot_user.organizations.first).to eq(organization)
+      expect(bot_user.organizations).to eq([organization])
     end
 
     it 'does not create a new user if it already exists' do
-      described_class.public_send(bot_type)
+      described_class.for_organization(organization).public_send(bot_type)
 
       expect do
-        described_class.public_send(bot_type)
+        described_class.for_organization(organization).public_send(bot_type)
       end.not_to change { User.count }
     end
 
@@ -38,7 +39,7 @@
         create(:user, username: username)
 
         expect do
-          described_class.public_send(bot_type)
+          described_class.for_organization(organization).public_send(bot_type)
         end.to change { User.where(user_type: bot_type).count }.by(1)
       end
     end
@@ -48,7 +49,7 @@
         create(:user, email: email)
 
         expect do
-          described_class.public_send(bot_type)
+          described_class.for_organization(organization).public_send(bot_type)
         end.to change { User.where(user_type: bot_type).count }.by(1)
       end
     end
@@ -58,7 +59,7 @@
         create(:group, path: username)
 
         expect do
-          described_class.public_send(bot_type)
+          described_class.for_organization(organization).public_send(bot_type)
         end.to change { User.where(user_type: bot_type).count }.by(1)
       end
     end
@@ -70,7 +71,7 @@
 
       it 'creates the bot user' do
         expect do
-          described_class.public_send(bot_type)
+          described_class.for_organization(organization).public_send(bot_type)
         end.to change { User.where(user_type: bot_type).count }.by(1)
       end
     end
@@ -78,7 +79,7 @@
 
   shared_examples 'bot user avatars' do |bot_type, avatar_filename|
     it 'sets the custom avatar for the created bot' do
-      bot_user = described_class.public_send(bot_type)
+      bot_user = described_class.for_organization(organization).public_send(bot_type)
 
       expect(bot_user.avatar.url).to be_present
       expect(bot_user.avatar.filename).to eq(avatar_filename)
@@ -104,13 +105,13 @@
   it_behaves_like 'bot user avatars', :admin_bot, 'admin-bot.png'
 
   context 'when bot is the support_bot' do
-    subject { described_class.support_bot }
+    subject { described_class.for_organization(organization).support_bot }
 
     it { is_expected.to be_confirmed }
   end
 
   context 'when bot is the admin bot' do
-    subject { described_class.admin_bot }
+    subject { described_class.for_organization(organization).admin_bot }
 
     it { is_expected.to be_admin }
     it { is_expected.to be_confirmed }
@@ -121,11 +122,20 @@
       # Ensure support bot user is created and memoization uses the same id
       # See https://gitlab.com/gitlab-org/gitlab/-/issues/509629
       described_class.clear_memoization(:support_bot_id)
-      described_class.support_bot_id
+      described_class.for_organization(organization).support_bot_id
     end
 
-    subject { described_class.support_bot_id }
+    subject { described_class.for_organization(organization).support_bot_id }
 
-    it { is_expected.to eq(described_class.support_bot.id) }
+    it { is_expected.to eq(described_class.for_organization(organization).support_bot.id) }
+  end
+
+  context 'when organization is not used' do
+    it 'creates user in first organization' do
+      bot = described_class.support_bot
+
+      expect(bot.organizations).to eq([first_organization])
+      expect(bot.namespace.organization).to eq(first_organization)
+    end
   end
 end
-- 
GitLab


From 4f08e8683a2d83afadc956625455d6b3569a5151 Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Tue, 8 Apr 2025 13:50:10 +0200
Subject: [PATCH 2/6] Refactor Users::Internal class: avoid class instance var

---
 ee/lib/ee/users/internal.rb           |  38 ++-
 ee/spec/lib/ee/users/internal_spec.rb |   2 +
 lib/users/internal.rb                 | 322 +++++++++++++-------------
 spec/lib/users/internal_spec.rb       |   7 -
 4 files changed, 183 insertions(+), 186 deletions(-)

diff --git a/ee/lib/ee/users/internal.rb b/ee/lib/ee/users/internal.rb
index d5788c29ec3edd..6fecb930df52d1 100644
--- a/ee/lib/ee/users/internal.rb
+++ b/ee/lib/ee/users/internal.rb
@@ -5,32 +5,30 @@ module Users
     module Internal
       extend ActiveSupport::Concern
 
-      class_methods do
-        # rubocop:disable CodeReuse/ActiveRecord
-        def visual_review_bot
-          email_pattern = "visual_review%s@#{Settings.gitlab.host}"
+      # rubocop:disable CodeReuse/ActiveRecord
+      def visual_review_bot
+        email_pattern = "visual_review%s@#{Settings.gitlab.host}"
 
-          unique_internal(::User.where(user_type: :visual_review_bot), 'visual-review-bot', email_pattern) do |u|
-            u.bio = 'The Gitlab Visual Review feedback bot'
-            u.name = 'Gitlab Visual Review Bot'
-            u.confirmed_at = Time.zone.now
-            u.private_profile = true
-          end
+        unique_internal(::User.where(user_type: :visual_review_bot), 'visual-review-bot', email_pattern) do |u|
+          u.bio = 'The Gitlab Visual Review feedback bot'
+          u.name = 'Gitlab Visual Review Bot'
+          u.confirmed_at = Time.zone.now
+          u.private_profile = true
         end
+      end
 
-        def suggested_reviewers_bot
-          email_pattern = "suggested-reviewers-bot%s@#{Settings.gitlab.host}"
+      def suggested_reviewers_bot
+        email_pattern = "suggested-reviewers-bot%s@#{Settings.gitlab.host}"
 
-          unique_internal(
-            ::User.where(user_type: :suggested_reviewers_bot), 'suggested-reviewers-bot', email_pattern) do |u|
-            u.bio = 'The GitLab suggested reviewers bot used for suggested reviewers'
-            u.name = 'GitLab Suggested Reviewers Bot'
-            u.confirmed_at = Time.zone.now
-            u.private_profile = true
-          end
+        unique_internal(
+          ::User.where(user_type: :suggested_reviewers_bot), 'suggested-reviewers-bot', email_pattern) do |u|
+          u.bio = 'The GitLab suggested reviewers bot used for suggested reviewers'
+          u.name = 'GitLab Suggested Reviewers Bot'
+          u.confirmed_at = Time.zone.now
+          u.private_profile = true
         end
-        # rubocop:enable CodeReuse/ActiveRecord
       end
+      # rubocop:enable CodeReuse/ActiveRecord
     end
   end
 end
diff --git a/ee/spec/lib/ee/users/internal_spec.rb b/ee/spec/lib/ee/users/internal_spec.rb
index f5fade809e8d6e..026d76fef9fb99 100644
--- a/ee/spec/lib/ee/users/internal_spec.rb
+++ b/ee/spec/lib/ee/users/internal_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Users::Internal, feature_category: :user_profile do
+  let_it_be(:first_organization) { create(:organization) }
+
   shared_examples 'bot users' do |bot_type|
     it 'creates the user if it does not exist' do
       expect do
diff --git a/lib/users/internal.rb b/lib/users/internal.rb
index dd02f4baa10b71..c5d89b8c47ff7e 100644
--- a/lib/users/internal.rb
+++ b/lib/users/internal.rb
@@ -4,202 +4,206 @@ module Users
   class Internal
     class << self
       include Gitlab::Utils::StrongMemoize
-      # rubocop:disable CodeReuse/ActiveRecord
-
-      # Return (create if necessary) the ghost user. The ghost user
-      # owns records previously belonging to deleted users.
-      def ghost
-        email = 'ghost%s@example.com'
-        unique_internal(User.where(user_type: :ghost), 'ghost', email) do |u|
-          u.bio = _('This is a "Ghost User", created to hold all issues authored by users that have ' \
-                    'since been deleted. This user cannot be removed.')
-          u.name = 'Ghost User'
-        end
+
+      def for_organization(organization)
+        new(organization: organization)
       end
 
-      def alert_bot
-        email_pattern = "alert%s@#{Settings.gitlab.host}"
+      # rubocop:disable GitlabSecurity/PublicSend -- We need it for method_missing
+      def method_missing(method, *_args)
+        new.public_send(method.to_sym)
+      end
+      # rubocop:enable GitlabSecurity/PublicSend
 
-        unique_internal(User.where(user_type: :alert_bot), 'alert-bot', email_pattern) do |u|
-          u.bio = 'The GitLab alert bot'
-          u.name = 'GitLab Alert Bot'
-          u.avatar = bot_avatar(image: 'alert-bot.png')
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+      def respond_to_missing?(method_name, include_private = false)
+        new.respond_to_missing?(method_name.to_sym) || super
       end
+    end
 
-      def migration_bot
-        email_pattern = "noreply+gitlab-migration-bot%s@#{Settings.gitlab.host}"
+    # rubocop:disable CodeReuse/ActiveRecord
 
-        unique_internal(User.where(user_type: :migration_bot), 'migration-bot', email_pattern) do |u|
-          u.bio = 'The GitLab migration bot'
-          u.name = 'GitLab Migration Bot'
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
-      end
+    def initialize(organization: nil)
+      @organization = organization
+    end
 
-      def security_bot
-        email_pattern = "security-bot%s@#{Settings.gitlab.host}"
-
-        unique_internal(User.where(user_type: :security_bot), 'GitLab-Security-Bot', email_pattern) do |u|
-          u.bio = 'System bot that monitors detected vulnerabilities for solutions ' \
-                  'and creates merge requests with the fixes.'
-          u.name = 'GitLab Security Bot'
-          u.avatar = bot_avatar(image: 'security-bot.png')
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+    # Return (create if necessary) the ghost user. The ghost user
+    # owns records previously belonging to deleted users.
+    def ghost
+      email = 'ghost%s@example.com'
+      unique_internal(User.where(user_type: :ghost), 'ghost', email) do |u|
+        u.bio = _('This is a "Ghost User", created to hold all issues authored by users that have ' \
+                  'since been deleted. This user cannot be removed.')
+        u.name = 'Ghost User'
       end
+    end
 
-      def support_bot
-        email_pattern = "support%s@#{Settings.gitlab.host}"
+    def alert_bot
+      email_pattern = "alert%s@#{Settings.gitlab.host}"
 
-        unique_internal(User.where(user_type: :support_bot), 'support-bot', email_pattern) do |u|
-          u.bio = 'The GitLab support bot used for Service Desk'
-          u.name = 'GitLab Support Bot'
-          u.avatar = bot_avatar(image: 'support-bot.png')
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+      unique_internal(User.where(user_type: :alert_bot), 'alert-bot', email_pattern) do |u|
+        u.bio = 'The GitLab alert bot'
+        u.name = 'GitLab Alert Bot'
+        u.avatar = bot_avatar(image: 'alert-bot.png')
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
+
+    def migration_bot
+      email_pattern = "noreply+gitlab-migration-bot%s@#{Settings.gitlab.host}"
 
-      # Checks against this bot are now included in every issue and work item
-      # detail and list page rendering and in GraphQL queries (especially for determining
-      # the web_url of an issue/work item).
-      # Because the bot never changes once created, we can memoize it for
-      # the lifetime of the application process. It also doesn't matter that
-      # different nodes may have different object instances of the bot.
-      # We only memoize the id because this is the information we check against.
-      def support_bot_id
-        support_bot.id
+      unique_internal(User.where(user_type: :migration_bot), 'migration-bot', email_pattern) do |u|
+        u.bio = 'The GitLab migration bot'
+        u.name = 'GitLab Migration Bot'
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
-      strong_memoize_attr :support_bot_id
-
-      def automation_bot
-        email_pattern = "automation%s@#{Settings.gitlab.host}"
-
-        unique_internal(User.where(user_type: :automation_bot), 'automation-bot', email_pattern) do |u|
-          u.bio = 'The GitLab automation bot used for automated workflows and tasks'
-          u.name = 'GitLab Automation Bot'
-          u.avatar = bot_avatar(image: 'support-bot.png') # todo: add an avatar for automation-bot
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+    end
+
+    def security_bot
+      email_pattern = "security-bot%s@#{Settings.gitlab.host}"
+
+      unique_internal(User.where(user_type: :security_bot), 'GitLab-Security-Bot', email_pattern) do |u|
+        u.bio = 'System bot that monitors detected vulnerabilities for solutions ' \
+                'and creates merge requests with the fixes.'
+        u.name = 'GitLab Security Bot'
+        u.avatar = bot_avatar(image: 'security-bot.png')
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
 
-      def llm_bot
-        email_pattern = "llm-bot%s@#{Settings.gitlab.host}"
+    def support_bot
+      email_pattern = "support%s@#{Settings.gitlab.host}"
 
-        unique_internal(User.where(user_type: :llm_bot), 'GitLab-Llm-Bot', email_pattern) do |u|
-          u.bio = 'The Gitlab LLM bot used for fetching LLM-generated content'
-          u.name = 'GitLab LLM Bot'
-          u.avatar = bot_avatar(image: 'support-bot.png') # todo: add an avatar for llm-bot
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+      unique_internal(User.where(user_type: :support_bot), 'support-bot', email_pattern) do |u|
+        u.bio = 'The GitLab support bot used for Service Desk'
+        u.name = 'GitLab Support Bot'
+        u.avatar = bot_avatar(image: 'support-bot.png')
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
+
+    def support_bot_id
+      support_bot.id
+    end
 
-      def duo_code_review_bot
-        email_pattern = "gitlab-duo%s@#{Settings.gitlab.host}"
+    def automation_bot
+      email_pattern = "automation%s@#{Settings.gitlab.host}"
 
-        unique_internal(User.where(user_type: :duo_code_review_bot), 'GitLabDuo', email_pattern) do |u|
-          u.bio = 'GitLab Duo bot for handling AI tasks'
-          u.name = 'GitLab Duo'
-          u.avatar = bot_avatar(image: 'duo-bot.png')
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+      unique_internal(User.where(user_type: :automation_bot), 'automation-bot', email_pattern) do |u|
+        u.bio = 'The GitLab automation bot used for automated workflows and tasks'
+        u.name = 'GitLab Automation Bot'
+        u.avatar = bot_avatar(image: 'support-bot.png') # todo: add an avatar for automation-bot
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
 
-      def admin_bot
-        email_pattern = "admin-bot%s@#{Settings.gitlab.host}"
-
-        unique_internal(User.where(user_type: :admin_bot), 'GitLab-Admin-Bot', email_pattern) do |u|
-          u.bio = 'Admin bot used for tasks that require admin privileges'
-          u.name = 'GitLab Admin Bot'
-          u.avatar = bot_avatar(image: 'admin-bot.png')
-          u.admin = true
-          u.confirmed_at = Time.zone.now
-          u.private_profile = true
-        end
+    def llm_bot
+      email_pattern = "llm-bot%s@#{Settings.gitlab.host}"
+
+      unique_internal(User.where(user_type: :llm_bot), 'GitLab-Llm-Bot', email_pattern) do |u|
+        u.bio = 'The Gitlab LLM bot used for fetching LLM-generated content'
+        u.name = 'GitLab LLM Bot'
+        u.avatar = bot_avatar(image: 'support-bot.png') # todo: add an avatar for llm-bot
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
+
+    def duo_code_review_bot
+      email_pattern = "gitlab-duo%s@#{Settings.gitlab.host}"
 
-      def bot_avatar(image:)
-        Rails.root.join('lib', 'assets', 'images', 'bot_avatars', image).open
+      unique_internal(User.where(user_type: :duo_code_review_bot), 'GitLabDuo', email_pattern) do |u|
+        u.bio = 'GitLab Duo bot for handling AI tasks'
+        u.name = 'GitLab Duo'
+        u.avatar = bot_avatar(image: 'duo-bot.png')
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
 
-      def for_organization(organization)
-        @organization = organization
+    def admin_bot
+      email_pattern = "admin-bot%s@#{Settings.gitlab.host}"
 
-        self
+      unique_internal(User.where(user_type: :admin_bot), 'GitLab-Admin-Bot', email_pattern) do |u|
+        u.bio = 'Admin bot used for tasks that require admin privileges'
+        u.name = 'GitLab Admin Bot'
+        u.avatar = bot_avatar(image: 'admin-bot.png')
+        u.admin = true
+        u.confirmed_at = Time.zone.now
+        u.private_profile = true
       end
+    end
 
-      private
+    def bot_avatar(image:)
+      Rails.root.join('lib', 'assets', 'images', 'bot_avatars', image).open
+    end
 
-      # NOTE: This method is patched in spec/spec_helper.rb to allow use of exclusive lease in RSpec's
-      # :before_all scope to keep the specs DRY.
-      def unique_internal(scope, username, email_pattern, &block)
-        if @organization
-          scope = scope.joins(:organization_users).where(organization_users: { organization: @organization })
-        end
+    private
 
-        scope.first || create_unique_internal(scope, username, email_pattern, &block)
+    # NOTE: This method is patched in spec/spec_helper.rb to allow use of exclusive lease in RSpec's
+    # :before_all scope to keep the specs DRY.
+    def unique_internal(scope, username, email_pattern, &block)
+      if @organization
+        scope = scope.joins(:organization_users).where(organization_users: { organization: @organization })
       end
 
-      def create_unique_internal(scope, username, email_pattern, &creation_block)
-        # Since we only want a single one of these in an instance, we use an
-        # exclusive lease to ensure than this block is never run concurrently.
-        lease_key = if @organization
-                      "user:unique_internal:#{@organization.id}:#{username}"
-                    else
-                      "user:unique_internal:#{username}"
-                    end
-
-        lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.minute.to_i)
+      scope.first || create_unique_internal(scope, username, email_pattern, &block)
+    end
 
+    def create_unique_internal(scope, username, email_pattern, &creation_block)
+      # Since we only want a single one of these in an instance, we use an
+      # exclusive lease to ensure than this block is never run concurrently.
+      lease_key = if @organization
+                    "user:unique_internal:#{@organization.id}:#{username}"
+                  else
+                    "user:unique_internal:#{username}"
+                  end
+
+      lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.minute.to_i)
+
+      uuid = lease.try_obtain
+      until uuid.present?
+        # Keep trying until we obtain the lease. To prevent hammering Redis too
+        # much we'll wait for a bit between retries.
+        sleep(1)
         uuid = lease.try_obtain
-        until uuid.present?
-          # Keep trying until we obtain the lease. To prevent hammering Redis too
-          # much we'll wait for a bit between retries.
-          sleep(1)
-          uuid = lease.try_obtain
-        end
-
-        # Recheck if the user is already present. One might have been
-        # added between the time we last checked (first line of this method)
-        # and the time we acquired the lock.
-        existing_user = scope.model.uncached { scope.first }
-        return existing_user if existing_user.present?
-
-        uniquify = Gitlab::Utils::Uniquify.new
-
-        username = uniquify.string(username) { |s| Namespace.by_path(s) }
-
-        email = uniquify.string(->(n) { Kernel.sprintf(email_pattern, n) }) do |s|
-          User.find_by_email(s)
-        end
-
-        user = scope.build(
-          username: username,
-          email: email,
-          &creation_block
-        )
-
-        user_organization = @organization || Organizations::Organization.first
-        user.assign_personal_namespace(user_organization)
-        user.organizations << user_organization
-
-        Users::UpdateService.new(user, user: user).execute(validate: false)
-        user
-      ensure
-        @organization = nil
-        Gitlab::ExclusiveLease.cancel(lease_key, uuid)
       end
 
-      # rubocop:enable CodeReuse/ActiveRecord
+      # Recheck if the user is already present. One might have been
+      # added between the time we last checked (first line of this method)
+      # and the time we acquired the lock.
+      existing_user = scope.model.uncached { scope.first }
+      return existing_user if existing_user.present?
+
+      uniquify = Gitlab::Utils::Uniquify.new
+
+      username = uniquify.string(username) { |s| Namespace.by_path(s) }
+
+      email = uniquify.string(->(n) { Kernel.sprintf(email_pattern, n) }) do |s|
+        User.find_by_email(s)
+      end
+
+      user = scope.build(
+        username: username,
+        email: email,
+        &creation_block
+      )
+
+      user_organization = @organization || Organizations::Organization.first
+      user.assign_personal_namespace(user_organization)
+      user.organizations << user_organization
+
+      Users::UpdateService.new(user, user: user).execute(validate: false)
+      user
+    ensure
+      Gitlab::ExclusiveLease.cancel(lease_key, uuid)
     end
+
+    # rubocop:enable CodeReuse/ActiveRecord
   end
 end
 
diff --git a/spec/lib/users/internal_spec.rb b/spec/lib/users/internal_spec.rb
index c0e8ee002d17fe..46273714f0183c 100644
--- a/spec/lib/users/internal_spec.rb
+++ b/spec/lib/users/internal_spec.rb
@@ -118,13 +118,6 @@
   end
 
   describe '.support_bot_id' do
-    before do
-      # Ensure support bot user is created and memoization uses the same id
-      # See https://gitlab.com/gitlab-org/gitlab/-/issues/509629
-      described_class.clear_memoization(:support_bot_id)
-      described_class.for_organization(organization).support_bot_id
-    end
-
     subject { described_class.for_organization(organization).support_bot_id }
 
     it { is_expected.to eq(described_class.for_organization(organization).support_bot.id) }
-- 
GitLab


From b490eaa596d62d082c85eafd022abd65a0464dd9 Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Wed, 9 Apr 2025 14:29:31 +0200
Subject: [PATCH 3/6] Memoize support_bot id if organization is not set

---
 lib/users/internal.rb           | 12 ++++++++++++
 spec/lib/users/internal_spec.rb | 25 +++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/lib/users/internal.rb b/lib/users/internal.rb
index c5d89b8c47ff7e..6e8328a733c223 100644
--- a/lib/users/internal.rb
+++ b/lib/users/internal.rb
@@ -9,6 +9,18 @@ def for_organization(organization)
         new(organization: organization)
       end
 
+      # Checks against this bot are now included in every issue and work item
+      # detail and list page rendering and in GraphQL queries (especially for determining
+      # the web_url of an issue/work item).
+      # Because the bot never changes once created, we can memoize it for
+      # the lifetime of the application process. It also doesn't matter that
+      # different nodes may have different object instances of the bot.
+      # We only memoize the id because this is the information we check against.
+      def support_bot_id
+        new.support_bot.id
+      end
+      strong_memoize_attr :support_bot_id
+
       # rubocop:disable GitlabSecurity/PublicSend -- We need it for method_missing
       def method_missing(method, *_args)
         new.public_send(method.to_sym)
diff --git a/spec/lib/users/internal_spec.rb b/spec/lib/users/internal_spec.rb
index 46273714f0183c..362022869777ec 100644
--- a/spec/lib/users/internal_spec.rb
+++ b/spec/lib/users/internal_spec.rb
@@ -118,9 +118,30 @@
   end
 
   describe '.support_bot_id' do
-    subject { described_class.for_organization(organization).support_bot_id }
+    context 'when organization is not used' do
+      before do
+        # Ensure support bot user is created and memoization uses the same id
+        # See https://gitlab.com/gitlab-org/gitlab/-/issues/509629
+        described_class.clear_memoization(:support_bot_id)
+        described_class.support_bot_id
+      end
+
+      subject(:support_bot_id) { described_class.support_bot_id }
 
-    it { is_expected.to eq(described_class.for_organization(organization).support_bot.id) }
+      it "does not call instance support_bot_id method" do
+        expect_next_instance_of(described_class) do |instance|
+          expect(instance).not_to receive(:support_bot_id)
+        end
+
+        expect(support_bot_id).to eq(described_class.support_bot.id)
+      end
+    end
+
+    context 'when organization is used' do
+      subject { described_class.for_organization(organization).support_bot_id }
+
+      it { is_expected.to eq(described_class.for_organization(organization).support_bot.id) }
+    end
   end
 
   context 'when organization is not used' do
-- 
GitLab


From e428b4837e1d3284b631aa57f426e5a87f109d7c Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Thu, 10 Apr 2025 16:16:40 +0200
Subject: [PATCH 4/6] Use Forwardable module and remove method_missing logic

---
 ee/lib/ee/users/internal.rb |  7 +++++++
 lib/users/internal.rb       | 16 ++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/ee/lib/ee/users/internal.rb b/ee/lib/ee/users/internal.rb
index 6fecb930df52d1..22671e70c2de4a 100644
--- a/ee/lib/ee/users/internal.rb
+++ b/ee/lib/ee/users/internal.rb
@@ -5,6 +5,13 @@ module Users
     module Internal
       extend ActiveSupport::Concern
 
+      class_methods do
+        extend Forwardable
+
+        # Delegate to an instance method of the class
+        def_delegators :new, :visual_review_bot, :suggested_reviewers_bot
+      end
+
       # rubocop:disable CodeReuse/ActiveRecord
       def visual_review_bot
         email_pattern = "visual_review%s@#{Settings.gitlab.host}"
diff --git a/lib/users/internal.rb b/lib/users/internal.rb
index 6e8328a733c223..7bb7b522b66dd7 100644
--- a/lib/users/internal.rb
+++ b/lib/users/internal.rb
@@ -5,6 +5,12 @@ class Internal
     class << self
       include Gitlab::Utils::StrongMemoize
 
+      extend Forwardable
+
+      def_delegators :new, :ghost, :support_bot, :alert_bot, :migration_bot,
+        :security_bot, :automation_bot, :llm_bot, :duo_code_review_bot,
+        :admin_bot
+
       def for_organization(organization)
         new(organization: organization)
       end
@@ -20,16 +26,6 @@ def support_bot_id
         new.support_bot.id
       end
       strong_memoize_attr :support_bot_id
-
-      # rubocop:disable GitlabSecurity/PublicSend -- We need it for method_missing
-      def method_missing(method, *_args)
-        new.public_send(method.to_sym)
-      end
-      # rubocop:enable GitlabSecurity/PublicSend
-
-      def respond_to_missing?(method_name, include_private = false)
-        new.respond_to_missing?(method_name.to_sym) || super
-      end
     end
 
     # rubocop:disable CodeReuse/ActiveRecord
-- 
GitLab


From d61de7d54492c72f0f3ed8fc4cb3edaeae90d94f Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Tue, 15 Apr 2025 14:41:31 +0200
Subject: [PATCH 5/6] Move RSpec exception to instance methods

---
 spec/spec_helper.rb | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 58f0b40b33e6fc..431947ce749558 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -612,28 +612,26 @@ def call(command, redis_config)
 module UsersInternalAllowExclusiveLease
   extend ActiveSupport::Concern
 
-  class_methods do
-    def unique_internal(scope, username, email_pattern, &block)
-      # this lets skip transaction checks when Users::Internal bots are created in
-      # let_it_be blocks during test set-up.
-      #
-      # Users::Internal bot creation within examples are still checked since the RSPec.current_scope is :example
-      if ::RSpec.respond_to?(:current_scope) && ::RSpec.current_scope == :before_all
-        Gitlab::ExclusiveLease.skipping_transaction_check { super }
-      else
-        super
-      end
+  def unique_internal(scope, username, email_pattern, &block)
+    # this lets skip transaction checks when Users::Internal bots are created in
+    # let_it_be blocks during test set-up.
+    #
+    # Users::Internal bot creation within examples are still checked since the RSPec.current_scope is :example
+    if ::RSpec.respond_to?(:current_scope) && ::RSpec.current_scope == :before_all
+      Gitlab::ExclusiveLease.skipping_transaction_check { super }
+    else
+      super
     end
+  end
 
-    # TODO: Until https://gitlab.com/gitlab-org/gitlab/-/issues/442780 is resolved we're creating internal users in the
-    # first organization as a temporary workaround. Many specs lack an organization in the database, causing foreign key
-    # constraint violations when creating internal users. We're not seeding organizations before all specs for
-    # performance.
-    def create_unique_internal(scope, username, email_pattern, &creation_block)
-      Organizations::Organization.first || FactoryBot.create(:organization)
+  # TODO: Until https://gitlab.com/gitlab-org/gitlab/-/issues/442780 is resolved we're creating internal users in the
+  # first organization as a temporary workaround. Many specs lack an organization in the database, causing foreign key
+  # constraint violations when creating internal users. We're not seeding organizations before all specs for
+  # performance.
+  def create_unique_internal(scope, username, email_pattern, &creation_block)
+    Organizations::Organization.first || FactoryBot.create(:organization)
 
-      super
-    end
+    super
   end
 end
 
-- 
GitLab


From a3a129fce4aeb2b3a69e8d501743d7203cc18e7b Mon Sep 17 00:00:00 2001
From: Rutger Wessels <rwessels@gitlab.com>
Date: Thu, 10 Apr 2025 16:16:40 +0200
Subject: [PATCH 6/6] Use Forwardable module and remove method_missing logic

---
 ee/lib/ee/users/internal.rb | 2 +-
 lib/users/internal.rb       | 9 ++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/ee/lib/ee/users/internal.rb b/ee/lib/ee/users/internal.rb
index 22671e70c2de4a..4ca636d1857279 100644
--- a/ee/lib/ee/users/internal.rb
+++ b/ee/lib/ee/users/internal.rb
@@ -12,7 +12,7 @@ module Internal
         def_delegators :new, :visual_review_bot, :suggested_reviewers_bot
       end
 
-      # rubocop:disable CodeReuse/ActiveRecord
+      # rubocop:disable CodeReuse/ActiveRecord -- Need to instantiate a record here
       def visual_review_bot
         email_pattern = "visual_review%s@#{Settings.gitlab.host}"
 
diff --git a/lib/users/internal.rb b/lib/users/internal.rb
index 7bb7b522b66dd7..095c35bbc64c18 100644
--- a/lib/users/internal.rb
+++ b/lib/users/internal.rb
@@ -7,9 +7,9 @@ class << self
 
       extend Forwardable
 
-      def_delegators :new, :ghost, :support_bot, :alert_bot, :migration_bot,
-        :security_bot, :automation_bot, :llm_bot, :duo_code_review_bot,
-        :admin_bot
+      def_delegators :new, :bot_avatar, :ghost, :support_bot, :alert_bot,
+        :migration_bot, :security_bot, :automation_bot, :llm_bot,
+        :duo_code_review_bot, :admin_bot
 
       def for_organization(organization)
         new(organization: organization)
@@ -28,8 +28,7 @@ def support_bot_id
       strong_memoize_attr :support_bot_id
     end
 
-    # rubocop:disable CodeReuse/ActiveRecord
-
+    # rubocop:disable CodeReuse/ActiveRecord -- Need to instantiate a record here
     def initialize(organization: nil)
       @organization = organization
     end
-- 
GitLab