diff --git a/app/models/user.rb b/app/models/user.rb index 6fb5ac4a4ef890d218a2ea8624ab9efaa5867633..ba3c79914027b72f28147fdb05ab2865125d9b98 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -346,7 +346,11 @@ class User < ActiveRecord::Base # Return (create if necessary) the ghost user. The ghost user # owns records previously belonging to deleted users. def ghost - User.find_by_ghost(true) || create_ghost_user + unique_internal(where(ghost: true), 'ghost', 'ghost%s@example.com') 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.state = :blocked + u.name = 'Ghost User' + end end end @@ -1017,10 +1021,14 @@ class User < ActiveRecord::Base end end - def self.create_ghost_user - # Since we only want a single ghost user in an instance, we use an + def self.unique_internal(scope, username, email_pattern, &b) + scope.first || create_unique_internal(scope, username, email_pattern, &b) + end + + def self.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 = "ghost_user_creation" + lease_key = "unique_internal_#{username}" lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.minute.to_i) until uuid = lease.try_obtain @@ -1029,25 +1037,25 @@ class User < ActiveRecord::Base sleep(1) end - # Recheck if a ghost user is already present. One might have been + # 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. - ghost_user = User.find_by_ghost(true) - return ghost_user if ghost_user.present? + existing_user = uncached { scope.first } + return existing_user if existing_user.present? uniquify = Uniquify.new - username = uniquify.string("ghost") { |s| User.find_by_username(s) } + username = uniquify.string(username) { |s| User.find_by_username(s) } - email = uniquify.string(-> (n) { "ghost#{n}@example.com" }) do |s| + email = uniquify.string(-> (n) { Kernel.sprintf(email_pattern, n) }) do |s| User.find_by_email(s) end - bio = 'This is a "Ghost User", created to hold all issues authored by users that have since been deleted. This user cannot be removed.' - - User.create( - username: username, password: Devise.friendly_token, bio: bio, - email: email, name: "Ghost User", state: :blocked, ghost: true + scope.create( + username: username, + password: Devise.friendly_token, + email: email, + &creation_block ) ensure Gitlab::ExclusiveLease.cancel(lease_key, uuid)