From 57788200ec966446605450cb33cae07b0e269d5d Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Thu, 13 Apr 2017 10:44:26 +0800
Subject: [PATCH] REFACTOR: Add `User.reserved_username?`.

---
 app/controllers/users_controller.rb |  2 +-
 app/models/user.rb                  | 10 +++++++--
 spec/models/user_spec.rb            | 34 ++++++++++++++++++-----------
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index eb69ce7d3ce..a9eaa127e11 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -307,7 +307,7 @@ class UsersController < ApplicationController
       return fail_with("login.email_too_long")
     end
 
-    if SiteSetting.reserved_usernames.split("|").include? params[:username].downcase
+    if User.reserved_username?(params[:username])
       return fail_with("login.reserved_username")
     end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 8c4a0a130a2..8b1d540a1e1 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -162,9 +162,15 @@ class User < ActiveRecord::Base
 
   def self.username_available?(username)
     lower = username.downcase
+    !User.where(username_lower: lower).exists? && !reserved_username?(lower)
+  end
 
-    User.where(username_lower: lower).blank? &&
-      SiteSetting.reserved_usernames.split("|").all? { |reserved| !lower.match('^' + Regexp.escape(reserved).gsub('\*', '.*') + '$') }
+  def self.reserved_username?(username)
+    lower = username.downcase
+
+    SiteSetting.reserved_usernames.split("|").any? do |reserved|
+      !!lower.match("^#{Regexp.escape(reserved).gsub('\*', '.*')}$")
+    end
   end
 
   def self.plugin_staff_user_custom_fields
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9304b1279a4..9409f66ae2c 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -459,7 +459,7 @@ describe User do
     end
   end
 
-  context '.username_available?' do
+  describe '.username_available?' do
     it "returns true for a username that is available" do
       expect(User.username_available?('BruceWayne')).to eq(true)
     end
@@ -471,25 +471,33 @@ describe User do
     it 'returns false when a username is reserved' do
       SiteSetting.reserved_usernames = 'test|donkey'
 
-      expect(User.username_available?('donkey')).to eq(false)
-      expect(User.username_available?('DonKey')).to eq(false)
-      expect(User.username_available?('test')).to eq(false)
+      expect(User.username_available?('tESt')).to eq(false)
+    end
+  end
+
+  describe '.reserved_username?' do
+    it 'returns true when a username is reserved' do
+      SiteSetting.reserved_usernames = 'test|donkey'
+
+      expect(User.reserved_username?('donkey')).to eq(true)
+      expect(User.reserved_username?('DonKey')).to eq(true)
+      expect(User.reserved_username?('test')).to eq(true)
     end
 
     it 'should not allow usernames matched against an expession' do
       SiteSetting.reserved_usernames = 'test)|*admin*|foo*|*bar|abc.def'
 
-      expect(User.username_available?('test')).to eq(true)
-      expect(User.username_available?('abc9def')).to eq(true)
+      expect(User.reserved_username?('test')).to eq(false)
+      expect(User.reserved_username?('abc9def')).to eq(false)
 
-      expect(User.username_available?('admin')).to eq(false)
-      expect(User.username_available?('foo')).to eq(false)
-      expect(User.username_available?('bar')).to eq(false)
+      expect(User.reserved_username?('admin')).to eq(true)
+      expect(User.reserved_username?('foo')).to eq(true)
+      expect(User.reserved_username?('bar')).to eq(true)
 
-      expect(User.username_available?('admi')).to eq(true)
-      expect(User.username_available?('bar.foo')).to eq(true)
-      expect(User.username_available?('foo.bar')).to eq(false)
-      expect(User.username_available?('baz.bar')).to eq(false)
+      expect(User.reserved_username?('admi')).to eq(false)
+      expect(User.reserved_username?('bar.foo')).to eq(false)
+      expect(User.reserved_username?('foo.bar')).to eq(true)
+      expect(User.reserved_username?('baz.bar')).to eq(true)
     end
   end