Added unit testing support + moved make_unique_username() to module level

Also cleaned up make_unique_username() code to no longer be recursive,
and to correctly handle numerical suffixes ≥ 10.

pillar/application/utils/authentication.py

@@ -1,4 +1,6 @@
 import os
+import random
+
 import requests
 
 from datetime import datetime
@@ -76,28 +78,11 @@ def validate_token():
         users = app.data.driver.db['users']
         email = validation['data']['user']['email']
         db_user = users.find_one({'email': email})
-        # Ensure unique username
+        username = make_unique_username(email)
-        username = email.split('@')[0]
-        def make_unique_username(username, index=1):
-            """Ensure uniqueness of a username by appending an incremental
-            digit at the end of it.
-            """
-            user_from_username = users.find_one({'username': username})
-            if user_from_username:
-                if index > 1:
-                    index += 1
-                    username = username[:-1]
-                username = "{0}{1}".format(username, index)
-                return make_unique_username(username, index=index)
-            return username
-        # Check for min length of username (otherwise validation fails)
-        username = "___{0}".format(username) if len(username) < 3 else username
-        username = make_unique_username(username)
 
-        full_name = username
         if not db_user:
             user_data = {
-                'full_name': full_name,
+                'full_name': username,
                 'username': username,
                 'email': email,
                 'auth': [{
@@ -138,3 +123,23 @@ def validate_token():
 
     g.current_user = current_user
 
+
+def make_unique_username(email):
+    username = email.split('@')[0]
+    # Check for min length of username (otherwise validation fails)
+    username = "___{0}".format(username) if len(username) < 3 else username
+
+    users = app.data.driver.db['users']
+    user_from_username = users.find_one({'username': username})
+
+    if not user_from_username:
+        return username
+
+    # Username exists, make it unique by adding some number after it.
+    suffix = 1
+    while True:
+        unique_name = '%s%i' % (username, suffix)
+        user_from_username = users.find_one({'username': unique_name})
+        if user_from_username is None:
+            return unique_name
+        suffix += 1

pillar/tests/test_auth.py

@@ -0,0 +1,50 @@
+import unittest
+import os
+
+TEST_EMAIL_USER = 'koro'
+TEST_EMAIL_ADDRESS = '%s@testing.blender.org' % TEST_EMAIL_USER
+
+os.environ['MONGO_DBNAME'] = 'unittest'
+os.environ['EVE_SETTINGS'] = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'settings.py')
+
+from application import app
+
+
+class FlaskrTestCase(unittest.TestCase):
+    def setUp(self):
+        self.app = app.test_client()
+
+    def tearDown(self):
+        pass
+
+    def test_make_unique_username(self):
+        from application.utils.authentication import make_unique_username
+
+        with app.test_request_context():
+            # Delete the user we want to test for
+            users = app.data.driver.db['users']
+            users.delete_many({'username': TEST_EMAIL_USER})
+
+            # This user shouldn't exist yet.
+            self.assertEqual(TEST_EMAIL_USER, make_unique_username(TEST_EMAIL_ADDRESS))
+
+            # Add a user, then test again.
+            user_data = {
+                'full_name': 'Coro the Llama',
+                'username': TEST_EMAIL_USER,
+                'email': TEST_EMAIL_ADDRESS,
+                'auth': [{
+                    'provider': 'unit-test',
+                    'user_id': 'test123',
+                    'token': ''}],
+                'settings': {
+                    'email_communications': 0
+                }
+            }
+
+            users.insert_one(user_data)
+            try:
+                self.assertIsNotNone(users.find_one({'username': TEST_EMAIL_USER}))
+                self.assertEqual('%s1' % TEST_EMAIL_USER, make_unique_username(TEST_EMAIL_ADDRESS))
+            finally:
+                users.delete_many({'username': TEST_EMAIL_USER})