summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTimothy Andrew <mail@timothyandrew.net>2017-02-03 18:25:50 +0530
committerTimothy Andrew <mail@timothyandrew.net>2017-02-24 16:50:19 +0530
commitca16c3734b7b89f71bdc9e1c18152aa1599c4f89 (patch)
treec00010e64f210675c3a6c777f080aae51455934b
parentff19bbd3b40621ae94632b9aa68fd12645b6ed41 (diff)
downloadgitlab-ce-ca16c3734b7b89f71bdc9e1c18152aa1599c4f89.tar.gz
Extract code from `Namespace#clean_path` for ghost user generation.
1. Create a `Uniquify` class, which generalizes the process of generating unique strings, by accepting a function that defines what "uniqueness" means in a given context. 2. WIP: Make sure tests for `Namespace` pass, add more if necessary. 3. WIP: Add tests for `Uniquify`
-rw-r--r--app/models/concerns/uniquify.rb25
-rw-r--r--app/models/namespace.rb10
-rw-r--r--app/models/user.rb27
-rw-r--r--spec/models/user_spec.rb4
4 files changed, 42 insertions, 24 deletions
diff --git a/app/models/concerns/uniquify.rb b/app/models/concerns/uniquify.rb
new file mode 100644
index 00000000000..1485ab6ae99
--- /dev/null
+++ b/app/models/concerns/uniquify.rb
@@ -0,0 +1,25 @@
+class Uniquify
+ # Return a version of the given 'base' string that is unique
+ # by appending a counter to it. Uniqueness is determined by
+ # repeated calls to `exists_fn`.
+ #
+ # If `base` is a function/proc, we expect that calling it with a
+ # candidate counter returns a string to test/return.
+ def string(base, exists_fn)
+ @counter = nil
+
+ if base.respond_to?(:call)
+ increment_counter! while exists_fn[base.call(@counter)]
+ base.call(@counter)
+ else
+ increment_counter! while exists_fn["#{base}#{@counter}"]
+ "#{base}#{@counter}"
+ end
+ end
+
+ private
+
+ def increment_counter!
+ @counter = @counter ? @counter.next : 1
+ end
+end
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index bd0336c984a..8cc3c1473e2 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -98,14 +98,8 @@ class Namespace < ActiveRecord::Base
# Work around that by setting their username to "blank", followed by a counter.
path = "blank" if path.blank?
- counter = 0
- base = path
- while Namespace.find_by_path_or_name(path)
- counter += 1
- path = "#{base}#{counter}"
- end
-
- path
+ uniquify = Uniquify.new
+ uniquify.string(path, -> (s) { Namespace.find_by_path_or_name(s) })
end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index 23beaa6c7c8..13529c9997a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -351,20 +351,19 @@ class User < ActiveRecord::Base
ghost_user ||
begin
- users = Enumerator.new do |y|
- n = nil
- loop do
- user = User.new(
- username: "ghost#{n}", password: Devise.friendly_token,
- email: "ghost#{n}@example.com", name: "Ghost User", state: :ghost
- )
-
- y.yield(user)
- n = n ? n.next : 0
- end
- end
-
- users.lazy.select { |user| user.valid? }.first.tap(&:save!)
+ uniquify = Uniquify.new
+
+ username = uniquify.string("ghost", -> (s) { User.find_by_username(s) })
+
+ email = uniquify.string(
+ -> (n) { "ghost#{n}@example.com" },
+ -> (s) { User.find_by_email(s) }
+ )
+
+ User.create(
+ username: username, password: Devise.friendly_token,
+ email: email, name: "Ghost User", state: :ghost
+ )
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 00e7927bf90..861d338ef95 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1513,7 +1513,7 @@ describe User, models: true do
ghost = User.ghost
expect(ghost).to be_persisted
- expect(ghost.username).to eq('ghost0')
+ expect(ghost.username).to eq('ghost1')
end
end
@@ -1523,7 +1523,7 @@ describe User, models: true do
ghost = User.ghost
expect(ghost).to be_persisted
- expect(ghost.email).to eq('ghost0@example.com')
+ expect(ghost.email).to eq('ghost1@example.com')
end
end
end