[#1117] Refactor and add docstring to user_name_validator

- Add docstring

- Use model.User.get() to find whether a user exists, instead of a lot
  of SQLAlchemy in ckan.logic.

  This should make it easier to unit test user_name_validator() in
  isolation, because the tests will only have to mock one method
  ckan.model.User.get() instead of having to mock several things.

  Also SQLAlchemy should just be in the model anyway, not in the logic.

- Refactor and add code comments to clarify the obscure thing that
  user_name_validator() does with context['user_obj'] on user_update()s.

  This was completely obscure before, now hopefully it's clearer.
  (But it's a bad design anyway, user_create and user_update shouldn't
  be sharing the same user_name_validator function.)

I don't *think* I broke anything by refactoring this (tests
are still passing).

ckan/logic/validators.py

@@ -493,23 +493,37 @@ def ignore_not_group_admin(key, data, errors, context):
     data.pop(key)
 
 def user_name_validator(key, data, errors, context):
-    model = context["model"]
-    session = context["session"]
-    user = context.get("user_obj")
+    '''Validate a new user name.
+
+    Append an error message to ``errors[key]`` if a user named ``data[key]``
+    already exists. Otherwise, do nothing.
+
+    :raises ckan.lib.navl.dictization_functions.Invalid: if ``data[key]`` is
+        not a string
+    :rtype: None
+
+    '''
+    model = context['model']
+    new_user_name = data[key]
 
-    if not isinstance(data[key], basestring):
+    if not isinstance(new_user_name, basestring):
         raise Invalid(_('User names must be strings'))
 
-    query = session.query(model.User.name).filter_by(name=data[key])
-    if user:
-        user_id = user.id
-    else:
-        user_id = data.get(key[:-1] + ("id",))
-    if user_id and user_id is not missing:
-        query = query.filter(model.User.id <> user_id)
-    result = query.first()
-    if result:
-        errors[key].append(_('That login name is not available.'))
+    user = model.User.get(new_user_name)
+    if user is not None:
+        # A user with new_user_name already exists in the database.
+
+        user_obj_from_context = context.get('user_obj')
+        if user_obj_from_context and user_obj_from_context.id == user.id:
+            # If there's a user_obj in context with the same id as the user
+            # found in the db, then we must be doing a user_update and not
+            # updating the user name, so don't return an error.
+            return
+        else:
+            # Otherwise return an error: there's already another user with that
+            # name, so you can create a new user with that name or update an
+            # existing user's name to that name.
+            errors[key].append(_('That login name is not available.'))
 
 def user_both_passwords_entered(key, data, errors, context):