diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2017-02-03 18:25:50 +0530 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2017-02-24 16:50:19 +0530 |
commit | ca16c3734b7b89f71bdc9e1c18152aa1599c4f89 (patch) | |
tree | c00010e64f210675c3a6c777f080aae51455934b | |
parent | ff19bbd3b40621ae94632b9aa68fd12645b6ed41 (diff) | |
download | gitlab-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.rb | 25 | ||||
-rw-r--r-- | app/models/namespace.rb | 10 | ||||
-rw-r--r-- | app/models/user.rb | 27 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 4 |
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 |