From fb4c87450953b16b5235e063453e95044de5b626 Mon Sep 17 00:00:00 2001
From: David Pursehouse <david.pursehouse@sonymobile.com>
Date: Wed, 20 Aug 2014 14:56:16 +0900
Subject: [PATCH] Simplify error handling in account username setting

Move the check for valid account pattern down into AccountSecurityImpl
and raise InvalidUserNameException from there.

Remove the invalidUserName utility method which is no longer needed
as we now only need to generate the error dialog at one place.

Check for the exception being an instance of InvalidUserNameException
rather than doing a string comparison of the message.

Change-Id: Ia4a9df88318f8459e8dc4e63368d896791dd9a18
---
 .../gerrit/client/account/UsernameField.java   | 18 +++++-------------
 .../httpd/rpc/account/AccountSecurityImpl.java |  8 ++++++--
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
index bf4d75c09b..945ae18d6b 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
@@ -106,18 +106,14 @@ class UsernameField extends Composite {
       return;
     }
 
+    enableUI(false);
+
     String newName = userNameTxt.getText();
     if ("".equals(newName)) {
       newName = null;
     }
-    if (newName != null && !newName.matches(Account.USER_NAME_PATTERN)) {
-      invalidUserName();
-      return;
-    }
-
-    enableUI(false);
-
     final String newUserName = newName;
+
     Util.ACCOUNT_SEC.changeUserName(newUserName,
         new GerritCallback<VoidResult>() {
           public void onSuccess(final VoidResult result) {
@@ -131,8 +127,8 @@ class UsernameField extends Composite {
           @Override
           public void onFailure(final Throwable caught) {
             enableUI(true);
-            if (InvalidUserNameException.MESSAGE.equals(caught.getMessage())) {
-              invalidUserName();
+            if (caught instanceof InvalidUserNameException) {
+              new ErrorDialog(Util.C.invalidUserName()).center();
             } else {
               super.onFailure(caught);
             }
@@ -140,10 +136,6 @@ class UsernameField extends Composite {
         });
   }
 
-  private void invalidUserName() {
-    new ErrorDialog(Util.C.invalidUserName()).center();
-  }
-
   private void enableUI(final boolean on) {
     userNameTxt.setEnabled(on);
     setUserName.setEnabled(on);
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountSecurityImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountSecurityImpl.java
index 622ba6c171..dfdfe9ddad 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountSecurityImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountSecurityImpl.java
@@ -20,6 +20,7 @@ import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.AccountSecurity;
 import com.google.gerrit.common.data.ContributorAgreement;
 import com.google.gerrit.common.errors.ContactInformationStoreException;
+import com.google.gerrit.common.errors.InvalidUserNameException;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.common.errors.PermissionDeniedException;
 import com.google.gerrit.httpd.rpc.BaseServiceImplementation;
@@ -109,10 +110,13 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
   public void changeUserName(final String newName,
       final AsyncCallback<VoidResult> callback) {
     if (realm.allowsEdit(Account.FieldName.USER_NAME)) {
+      if (newName == null || !newName.matches(Account.USER_NAME_PATTERN)) {
+        callback.onFailure(new InvalidUserNameException());
+      }
       Handler.wrap(changeUserNameFactory.create(newName)).to(callback);
     } else {
-      callback.onFailure(new PermissionDeniedException("Not allowed to change"
-          + " username"));
+      callback.onFailure(
+          new PermissionDeniedException("Not allowed to change username"));
     }
   }