Include AvatarInfo as part of AccountInfo

Instead of the browser sending many RPCs to the server to ask for
each /accounts/{id}/avatar URL, include avatar information in the
JSON response as part of any REST API producing AccountInfo.

If avatars are not enabled on the server every AccountInfo now has
an empty avatar list ("avatars:[]") making a positive assertion
that no avatars exist for the descibed account. This avoids a large
number of requests on busy change pages.

If avatars are enabled the server automatically includes the URL for
the "size 26 pixel" image. Currently we assume this is 26px along
the height dimension. The avatar plugin API needs to be reworked
to permit better description of the image to be carried around.

Change-Id: I3f6fe0458feef10441c8ed8467c71b2ef7b00acf

gerrit-gwtui/src/main/java/com/google/gerrit/client/AvatarImage.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.client;
 
 import com.google.gerrit.client.account.AccountInfo;
+import com.google.gerrit.client.account.AccountInfo.AvatarInfo;
 import com.google.gerrit.client.changes.Util;
 import com.google.gerrit.client.rpc.RestApi;
 import com.google.gwt.event.dom.client.LoadEvent;
@@ -27,14 +28,15 @@ import com.google.gwt.user.client.Timer;
 import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.UIObject;
 
-public class AvatarImage extends Image {
+public class AvatarImage extends Image implements LoadHandler {
-
   public AvatarImage() {
+    setVisible(false);
+    addLoadHandler(this);
   }
 
   /** A default sized avatar image. */
   public AvatarImage(AccountInfo account) {
-    this(account, 0);
+    this(account, 26, true);
   }
 
   /**
@@ -60,26 +62,46 @@ public class AvatarImage extends Image {
    *        avatar image
    */
   public AvatarImage(AccountInfo account, int size, boolean addPopup) {
+    addLoadHandler(this);
     setAccount(account, size, addPopup);
   }
 
   public void setAccount(AccountInfo account, int size, boolean addPopup) {
-    setUrl(isGerritServer(account) ? getGerritServerAvatarUrl() :
+    if (account == null) {
-      url(account.email(), size));
+      setVisible(false);
-    setVisible(false);
+    } else if (isGerritServer(account)) {
-
+      setVisible(true);
-    if (size > 0) {
+      setResource(Gerrit.RESOURCES.gerritAvatar());
-      // If the provider does not resize the image, force it in the browser.
+    } else if (account.has_avatar_info()) {
-      setSize(size + "px", size + "px");
+      setVisible(false);
-    }
+      AvatarInfo info = account.avatar(size);
-
+      if (info != null) {
-    addLoadHandler(new LoadHandler() {
+        setWidth(info.width() > 0 ? info.width() + "px" : "");
-      @Override
+        setHeight(info.height() > 0 ? info.height() + "px" : "");
-      public void onLoad(LoadEvent event) {
+        setUrl(info.url());
-        setVisible(true);
+        popup(account, addPopup);
       }
-    });
+    } else if (account.email() != null) {
+      // TODO Kill /accounts/*/avatar URL.
+      String u = account.email();
+      if (Gerrit.isSignedIn()
+          && u.equals(Gerrit.getUserAccount().getPreferredEmail())) {
+        u = "self";
+      }
+      RestApi api = new RestApi("/accounts/").id(u).view("avatar");
+      if (size > 0) {
+        api.addParameter("s", size);
+        setSize("", size + "px");
+      }
+      setVisible(false);
+      setUrl(api.url());
+      popup(account, addPopup);
+    } else {
+      setVisible(false);
+    }
+  }
 
+  private void popup(AccountInfo account, boolean addPopup) {
     if (addPopup) {
       PopupHandler popupHandler = new PopupHandler(account, this);
       addMouseOverHandler(popupHandler);
@@ -87,33 +109,17 @@ public class AvatarImage extends Image {
     }
   }
 
+  @Override
+  public void onLoad(LoadEvent event) {
+    setVisible(true);
+  }
+
   private static boolean isGerritServer(AccountInfo account) {
     return account._account_id() == 0
         && Util.C.messageNoAuthor().equals(account.name());
   }
 
-  private static String getGerritServerAvatarUrl() {
+  private static class PopupHandler implements MouseOverHandler, MouseOutHandler {
-    return Gerrit.RESOURCES.gerritAvatar().getSafeUri().asString();
-  }
-
-  private static String url(String email, int size) {
-    if (email == null) {
-      return "";
-    }
-    String u;
-    if (Gerrit.isSignedIn() && email.equals(Gerrit.getUserAccount().getPreferredEmail())) {
-      u = "self";
-    } else {
-      u = email;
-    }
-    RestApi api = new RestApi("/accounts/").id(u).view("avatar");
-    if (size > 0) {
-      api.addParameter("s", size);
-    }
-    return api.url();
-  }
-
-  private class PopupHandler implements MouseOverHandler, MouseOutHandler {
     private final AccountInfo account;
     private final UIObject target;
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountInfo.java

@@ -15,12 +15,36 @@
 package com.google.gerrit.client.account;
 
 import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.core.client.JsArray;
 
 public class AccountInfo extends JavaScriptObject {
   public final native int _account_id() /*-{ return this._account_id || 0; }-*/;
   public final native String name() /*-{ return this.name; }-*/;
   public final native String email() /*-{ return this.email; }-*/;
 
+  /**
+   * @return true if the server supplied avatar information about this account.
+   *         The information may be an empty list, indicating no avatars are
+   *         available, such as when no plugin is installed. This method returns
+   *         false if the server did not check on avatars for the account.
+   */
+  public final native boolean has_avatar_info()
+  /*-{ return this.hasOwnProperty('avatars') }-*/;
+
+  public final AvatarInfo avatar(int sz) {
+    JsArray<AvatarInfo> a = avatars();
+    for (int i = 0; a != null && i < a.length(); i++) {
+      AvatarInfo r = a.get(i);
+      if (r.height() == sz) {
+        return r;
+      }
+    }
+    return null;
+  }
+
+  private final native JsArray<AvatarInfo> avatars()
+  /*-{ return this.avatars }-*/;
+
   public static native AccountInfo create(int id, String name,
       String email) /*-{
     return {'_account_id': id, 'name': name, 'email': email};
@@ -28,4 +52,13 @@ public class AccountInfo extends JavaScriptObject {
 
   protected AccountInfo() {
   }
+
+  public static class AvatarInfo extends JavaScriptObject {
+    public final native String url() /*-{ return this.url }-*/;
+    public final native int height() /*-{ return this.height || 0 }-*/;
+    public final native int width() /*-{ return this.width || 0 }-*/;
+
+    protected AvatarInfo() {
+    }
+  }
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Message.java

@@ -51,7 +51,7 @@ class Message extends Composite {
 
   Message(CommentLinkProcessor clp, MessageInfo info) {
     if (info.author() != null) {
-      avatar = new AvatarImage(info.author(), 26);
+      avatar = new AvatarImage(info.author());
       avatar.setSize("", "");
     } else {
       avatar = new AvatarImage();

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java

@@ -73,7 +73,7 @@ class PublishedBox extends CommentBox {
     this.comment = info;
 
     if (info.author() != null) {
-      avatar = new AvatarImage(info.author(), 26);
+      avatar = new AvatarImage(info.author());
       avatar.setSize("", "");
     } else {
       avatar = new AvatarImage();

gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CommentPanel.java

@@ -128,7 +128,7 @@ public class CommentPanel extends Composite implements HasDoubleClickHandlers,
   }
 
   public void setAuthorNameText(final AccountInfo author, final String nameText) {
-    header.setWidget(0, 0, new AvatarImage(author, 26));
+    header.setWidget(0, 0, new AvatarImage(author));
     header.setText(0, 1, nameText);
     body.getElement().setAttribute("email", author.email());
     body.getElement().setAttribute("name", author.name());

gerrit-server/src/main/java/com/google/gerrit/server/account/AccountInfo.java

@@ -20,8 +20,11 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
+import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.avatar.AvatarProvider;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -39,6 +42,8 @@ public class AccountInfo {
 
     private final Provider<ReviewDb> db;
     private final AccountCache accountCache;
+    private final DynamicItem<AvatarProvider> avatar;
+    private final IdentifiedUser.GenericFactory userFactory;
     private final boolean detailed;
     private final Map<Account.Id, AccountInfo> created;
     private final List<AccountInfo> provided;
@@ -46,9 +51,13 @@ public class AccountInfo {
     @Inject
     Loader(Provider<ReviewDb> db,
         AccountCache accountCache,
+        DynamicItem<AvatarProvider> avatar,
+        IdentifiedUser.GenericFactory userFactory,
         @Assisted boolean detailed) {
       this.db = db;
       this.accountCache = accountCache;
+      this.avatar = avatar;
+      this.userFactory = userFactory;
       this.detailed = detailed;
       created = Maps.newHashMap();
       provided = Lists.newArrayList();
@@ -75,7 +84,7 @@ public class AccountInfo {
       for (AccountInfo info : Iterables.concat(created.values(), provided)) {
         AccountState state = accountCache.getIfPresent(info._id);
         if (state != null) {
-          info.fill(state.getAccount(), detailed);
+          fill(info, state.getAccount());
         } else {
           missing.put(info._id, info);
         }
@@ -83,7 +92,7 @@ public class AccountInfo {
       if (!missing.isEmpty()) {
         for (Account account : db.get().accounts().get(missing.keySet())) {
           for (AccountInfo info : missing.get(account.getId())) {
-            info.fill(account, detailed);
+            fill(info, account);
           }
         }
       }
@@ -96,12 +105,28 @@ public class AccountInfo {
       }
       fill();
     }
-  }
 
-  public static AccountInfo parse(Account a, boolean detailed) {
+    private void fill(AccountInfo info, Account account) {
-    AccountInfo ai = new AccountInfo(a.getId());
+      info.name = Strings.emptyToNull(account.getFullName());
-    ai.fill(a, detailed);
+      if (info.name == null) {
-    return ai;
+        info.name = account.getUserName();
+      }
+
+      if (detailed) {
+        info._account_id = account.getId().get();
+        info.email = account.getPreferredEmail();
+        info.username = account.getUserName();
+      }
+
+      info.avatars = Lists.newArrayListWithCapacity(1);
+      AvatarProvider ap = avatar.get();
+      if (ap != null) {
+        String u = ap.getUrl(userFactory.create(account.getId()), 26);
+        if (u != null) {
+          info.avatars.add(new AvatarInfo(u, 26));
+        }
+      }
+    }
   }
 
   public transient Account.Id _id;
@@ -114,17 +139,15 @@ public class AccountInfo {
   public String name;
   public String email;
   public String username;
+  public List<AvatarInfo> avatars;
 
-  private void fill(Account account, boolean detailed) {
+  public static class AvatarInfo {
-    name = Strings.emptyToNull(account.getFullName());
+    String url;
-    if (name == null) {
+    int height;
-      name = account.getUserName();
-    }
 
-    if (detailed) {
+    AvatarInfo(String url, int height) {
-      _account_id = account.getId().get();
+      this.url = url;
-      email = account.getPreferredEmail();
+      this.height = height;
-      username = account.getUserName();
     }
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java

@@ -68,12 +68,14 @@ public class CreateAccount implements RestModifyView<TopLevelResource, Input> {
   private final SshKeyCache sshKeyCache;
   private final AccountCache accountCache;
   private final AccountByEmailCache byEmailCache;
+  private final AccountInfo.Loader.Factory infoLoader;
   private final String username;
 
   @Inject
   CreateAccount(ReviewDb db, IdentifiedUser currentUser,
       GroupsCollection groupsCollection, SshKeyCache sshKeyCache,
       AccountCache accountCache, AccountByEmailCache byEmailCache,
+      AccountInfo.Loader.Factory infoLoader,
       @Assisted String username) {
     this.db = db;
     this.currentUser = currentUser;
@@ -81,6 +83,7 @@ public class CreateAccount implements RestModifyView<TopLevelResource, Input> {
     this.sshKeyCache = sshKeyCache;
     this.accountCache = accountCache;
     this.byEmailCache = byEmailCache;
+    this.infoLoader = infoLoader;
     this.username = username;
   }
 
@@ -167,7 +170,10 @@ public class CreateAccount implements RestModifyView<TopLevelResource, Input> {
     accountCache.evictByUsername(username);
     byEmailCache.evict(input.email);
 
-    return Response.created(AccountInfo.parse(a, true));
+    AccountInfo.Loader loader = infoLoader.create(true);
+    AccountInfo info = loader.get(id);
+    loader.fill();
+    return Response.created(info);
   }
 
   private Set<AccountGroup.Id> parseGroups(List<String> groups)

gerrit-server/src/main/java/com/google/gerrit/server/account/GetAccount.java

@@ -15,10 +15,22 @@
 package com.google.gerrit.server.account;
 
 import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
 
 public class GetAccount implements RestReadView<AccountResource> {
+  private final AccountInfo.Loader.Factory infoFactory;
+
+  @Inject
+  GetAccount(AccountInfo.Loader.Factory infoFactory) {
+    this.infoFactory = infoFactory;
+  }
+
   @Override
-  public AccountInfo apply(AccountResource rsrc) {
+  public AccountInfo apply(AccountResource rsrc) throws OrmException {
-    return AccountInfo.parse(rsrc.getUser().getAccount(), true);
+    AccountInfo.Loader loader = infoFactory.create(true);
+    AccountInfo info = loader.get(rsrc.getUser().getAccountId());
+    loader.fill();
+    return info;
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/group/AddMembers.java

@@ -77,6 +77,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
   private final Provider<AccountsCollection> accounts;
   private final AccountResolver accountResolver;
   private final AccountCache accountCache;
+  private final AccountInfo.Loader.Factory infoFactory;
   private final ReviewDb db;
 
   @Inject
@@ -85,12 +86,14 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
       Provider<AccountsCollection> accounts,
       AccountResolver accountResolver,
       AccountCache accountCache,
+      AccountInfo.Loader.Factory infoFactory,
       ReviewDb db) {
     this.accountManager = accountManager;
     this.authType = authConfig.getAuthType();
     this.accounts = accounts;
     this.accountResolver = accountResolver;
     this.accountCache = accountCache;
+    this.infoFactory = infoFactory;
     this.db = db;
   }
 
@@ -109,6 +112,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
     List<AccountGroupMemberAudit> newAccountGroupMemberAudits = Lists.newLinkedList();
     List<AccountInfo> result = Lists.newLinkedList();
     Account.Id me = ((IdentifiedUser) control.getCurrentUser()).getAccountId();
+    AccountInfo.Loader loader = infoFactory.create(true);
 
     for (String nameOrEmail : input.members) {
       Account a = findAccount(nameOrEmail);
@@ -131,7 +135,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
           newAccountGroupMemberAudits.add(new AccountGroupMemberAudit(m, me));
         }
       }
-      result.add(AccountInfo.parse(a, true));
+      result.add(loader.get(a.getId()));
     }
 
     db.accountGroupMembersAudit().insert(newAccountGroupMemberAudits);
@@ -140,6 +144,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
       accountCache.evict(m.getAccountId());
     }
 
+    loader.fill();
     return result;
   }
 
@@ -218,7 +223,8 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
     }
 
     @Override
-    public Object apply(MemberResource resource, PutMember.Input input) {
+    public Object apply(MemberResource resource, PutMember.Input input)
+        throws OrmException {
       // Do nothing, the user is already a member.
       return get.get().apply(resource);
     }

gerrit-server/src/main/java/com/google/gerrit/server/group/GetMember.java

@@ -16,10 +16,22 @@ package com.google.gerrit.server.group;
 
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.server.account.AccountInfo;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
 
 public class GetMember implements RestReadView<MemberResource> {
+  private final AccountInfo.Loader.Factory infoFactory;
+
+  @Inject
+  GetMember(AccountInfo.Loader.Factory infoFactory) {
+    this.infoFactory = infoFactory;
+  }
+
   @Override
-  public AccountInfo apply(MemberResource resource) {
+  public AccountInfo apply(MemberResource rsrc) throws OrmException {
-    return AccountInfo.parse(resource.getMember().getAccount(), true);
+    AccountInfo.Loader loader = infoFactory.create(true);
+    AccountInfo info = loader.get(rsrc.getMember().getAccountId());
+    loader.fill();
+    return info;
   }
 }