FEATURE: allow a huge number of users to share common prefix

Previously username suggester would give up after 100 attempts at getting
a username and fallback to random string.

This amends the logic so we do all the work of figuring out a good username
in SQL and avoids a large amount of queries in cases where a lot of usernames
were used up.

This corrects an issue on sites with large numbers of anon users
This commit is contained in:
Sam Saffron 2019-05-16 17:15:03 +10:00
parent ea214b2b0c
commit a8fbb19e7c
2 changed files with 42 additions and 1 deletions

View File

@ -23,14 +23,46 @@ module UserNameSuggester
def self.find_available_username_based_on(name, allowed_username = nil)
name = fix_username(name)
offset = nil
i = 1
attempt = name
until attempt == allowed_username || User.username_available?(attempt) || i > 100
suffix = i.to_s
if offset.nil?
normalized = User.normalize_username(name)
similar = "#{normalized}(0|1|2|3|4|5|6|7|8|9)+"
count = DB.query_single(<<~SQL, like: "#{normalized}%", similar: similar).first
SELECT count(*) FROM users
WHERE username_lower LIKE :like AND
username_lower SIMILAR TO :similar
SQL
if count > 0
available = DB.query_single(<<~SQL, count: count, name: normalized).first
WITH numbers AS (SELECT generate_series(1, :count) AS n)
SELECT n FROM numbers
LEFT JOIN users ON username_lower = :name || n::varchar
WHERE users.id IS NULL
ORDER by n ASC
LIMIT 1
SQL
# we start at 1
offset = available - 1
else
offset = 0
end
end
suffix = (i + offset).to_s
max_length = User.username_length.end - suffix.length
attempt = "#{truncate(name, max_length)}#{suffix}"
i += 1
end
until attempt == allowed_username || User.username_available?(attempt) || i > 200
attempt = SecureRandom.hex[1..SiteSetting.max_username_length]
i += 1

View File

@ -10,6 +10,15 @@ describe UserNameSuggester do
SiteSetting.max_username_length = 15
end
it "keeps adding numbers to the username" do
Fabricate(:user, username: 'sam')
Fabricate(:user, username: 'sAm1')
Fabricate(:user, username: 'sam2')
Fabricate(:user, username: 'sam4')
expect(UserNameSuggester.suggest('saM')).to eq('saM3')
end
it "doesn't raise an error on nil username" do
expect(UserNameSuggester.suggest(nil)).to eq(nil)
end