From 885fc1de06dbe21f1e329f5e3fe077ba02900d5e Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Mon, 27 Feb 2017 11:54:20 -0800 Subject: generalize the idea of a "unique internal user" Since we will likely be needing this for other features, for example the Gitlab ServiceDesk support bot. --- app/models/user.rb | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 6fb5ac4a4ef..ba3c7991402 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) -- cgit v1.2.1 From 31b633306fec60766c3da97de64760ca132a7b75 Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 28 Feb 2017 10:41:35 -0800 Subject: use a more namespacey key for the exclusive lease --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index ba3c7991402..8443594c055 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1028,7 +1028,7 @@ class User < ActiveRecord::Base 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 = "unique_internal_#{username}" + lease_key = "user:unique_internal:#{username}" lease = Gitlab::ExclusiveLease.new(lease_key, timeout: 1.minute.to_i) until uuid = lease.try_obtain -- cgit v1.2.1