Merge pull request #17981 from code-dot-org/revert_12745

Revert "Optimize generate_username query (#12745)"

dashboard/test/models/user_helpers_test.rb

@@ -36,38 +36,39 @@ def create_user_with_username(username)
   test 'generate_username for existing username via dart throwing' do
     create_user_with_username 'captain_picard'
 
-    # An existing username attempts the username, fails, and receives '784'.
+    # An existing username attempts the username, fails, and receives '6'.
     srand 0
-    assert_equal 'captain_picard784',
+    assert_equal 'captain_picard6',
       UserHelpers.generate_username(User, 'Captain Picard')
-    create_user_with_username 'captain_picard784'
+    create_user_with_username 'captain_picard6'
 
-    # The next Captain Picard attempts '784', fails, and receives '659'
+    # The next Captain Picard attempts '6', fails, and receives '1'
     srand 0
-    assert_equal 'captain_picard659',
+    assert_equal 'captain_picard1',
       UserHelpers.generate_username(User, 'Captain Picard')
-    create_user_with_username 'captain_picard659'
+    create_user_with_username 'captain_picard1'
 
-    # The next Captain Picard attempts '784' and '659', fails, and
-    # receives '4264'.
+    # The next Captain Picard attempts '6', fails, attempts '1', fails, and
+    # receives '4'.
     srand 0
-    assert_equal 'captain_picard4264',
+    assert_equal 'captain_picard4',
       UserHelpers.generate_username(User, 'Captain Picard')
-    create_user_with_username 'captain_picard4264'
+    create_user_with_username 'captain_picard4'
 
-    # The next Captain Picard attempts the above, fails and receives '5859'.
+    # The next Captain Picard attempts '6', fails, attempts '1', fails,
+    # attempts '4', fails, and receives '77'.
     srand 0
-    assert_equal 'captain_picard5859',
+    assert_equal 'captain_picard77',
       UserHelpers.generate_username(User, 'Captain Picard')
   end
 
   test 'generate_username for existing username via fallback' do
-    ['', 784, 659, 4264, 5859, 51993, 96293, 54824, 47456, 38329, 59306].each do |suffix|
+    ['', '6', '1', '4', '77', '19', '93', '377', '854', '904'].each do |suffix|
       create_user_with_username "captain_picard#{suffix}"
     end
 
     srand 0
-    assert_equal 'captain_picard96294',
+    assert_equal 'captain_picard905',
       UserHelpers.generate_username(User, 'Captain Picard')
   end
 
@@ -97,7 +98,7 @@ def create_user_with_username(username)
   test 'generate_username' do
     default_params = {email: 'foo@bar.com', password: 'foosbars', name: 'tester', user_type: User::TYPE_STUDENT, age: 28}
     names = ['a', 'b', 'Captain Picard', 'Captain Picard', 'Captain Picard', 'this is a really long name blah blah blah blah blah blah']
-    expected_usernames = %w(coder_a coder_b captain_picard captain_picard784 captain_picard659 this_is_a_really)
+    expected_usernames = %w(coder_a coder_b captain_picard captain_picard6 captain_picard1 this_is_a_really)
 
     i = 0
     users = names.map do |name|

lib/cdo/user_helpers.rb

@@ -8,7 +8,7 @@ module UserHelpers
 
   def self.generate_username(queryable, name)
     prefix = name.downcase.
-      gsub(/[^#{USERNAME_ALLOWED_CHARACTERS.source}]+/, ' ')[0..15].
+      gsub(/[^#{USERNAME_ALLOWED_CHARACTERS.source}]+/, ' ')[0..16].
       squish.
       tr(' ', '_')
 
@@ -20,31 +20,27 @@ def self.generate_username(queryable, name)
     return prefix if queryable.where(username: prefix).limit(1).empty?
 
     # Throw darts to find an appropriate suffix, using it if we hit bullseye.
-    (2..6).each do |exponent|
+    (0..2).each do |exponent|
       min_index = 10**exponent
       max_index = 10**(exponent + 1) - 1
-      2.times do |_i|
+      3.times do |_i|
         suffix = Random.rand(min_index..max_index)
-        # Truncate generated username to max allowed length.
-        username = "#{prefix}#{suffix}"[0..18]
-        if queryable.where(username: username).limit(1).empty?
-          return username
+        if queryable.where(username: "#{prefix}#{suffix}").limit(1).empty?
+          return "#{prefix}#{suffix}"
         end
       end
     end
 
-    # Fallback to a range-scan query.
-    # Use a regex to filter integer suffixes from other usernames.
-    last_id = queryable.where(['username LIKE ? and username RLIKE ?', "#{prefix}%", "^#{prefix}[0-9]+$"]).
-      # Find max integer using DB functions to avoid returning all matches to the application.
-      select("MAX(CAST(SUBSTRING(`username`, #{prefix.length + 1}) as unsigned)) as `id`").first
-
-    # ActiveRecord returns a User instance, whereas Sequel returns a hash.
-    last_id = last_id.respond_to?(:id) ? last_id.id : last_id[:id]
+    # The darts missed, so revert to using a slow query.
+    similar_users = queryable.where(["username like ?", prefix + '%']).select(:username).to_a
+    similar_usernames = similar_users.map do |user|
+      # ActiveRecord returns a User instance, whereas Sequel returns a hash.
+      user.respond_to?(:username) ? user.username : user[:username]
+    end
 
     # Increment the current maximum integer suffix. Though it may leave holes,
     # it is guaranteed to be (currently) unique.
-    suffix = last_id.to_i + 1
+    suffix = similar_usernames.map {|n| n[prefix.length..-1]}.map(&:to_i).max + 1
     return "#{prefix}#{suffix}"
   end