From 3d217418d415edb8e5ae9ed0cb34e7128a6624bd Mon Sep 17 00:00:00 2001
From: Shawn Pearce <sop@google.com>
Date: Thu, 10 Jan 2013 20:54:17 -0800
Subject: [PATCH] Fix sorting of included groups

Change-Id: I41be5f33c94d3248e6a2c922ebb412f643277050
---
 .../google/gerrit/common/data/GroupInfo.java  |  8 ++++--
 .../server/account/GroupDetailFactory.java    | 18 ++++++------
 .../server/account/GroupInfoCacheFactory.java | 28 ++++++++++---------
 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java
index f01365b81e..c4d92d44d7 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java
@@ -41,10 +41,14 @@ public class GroupInfo {
    *
    * @param a the data store record holding the specific group details.
    */
-  public GroupInfo(final AccountGroup a) {
+  public GroupInfo(GroupDescription.Basic a) {
     uuid = a.getGroupUUID();
     name = a.getName();
-    description = a.getDescription();
+
+    if (a instanceof GroupDescription.Internal) {
+      AccountGroup group = ((GroupDescription.Internal) a).getAccountGroup();
+      description = group.getDescription();
+    }
   }
 
   /** @return the unique local id of the group */
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
index 4d2cc1aacf..8259c121db 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupDetailFactory.java
@@ -136,23 +136,21 @@ public class GroupDetailFactory implements Callable<GroupDetail> {
     Collections.sort(groups, new Comparator<AccountGroupIncludeByUuid>() {
       public int compare(final AccountGroupIncludeByUuid o1,
           final AccountGroupIncludeByUuid o2) {
-        final AccountGroup a = gic.get(o1.getIncludeUUID());
-        final AccountGroup b = gic.get(o2.getIncludeUUID());
+        GroupDescription.Basic a = gic.get(o1.getIncludeUUID());
+        GroupDescription.Basic b = gic.get(o2.getIncludeUUID());
         return n(a).compareTo(n(b));
       }
 
-      private String n(final AccountGroup a) {
+      private String n (GroupDescription.Basic a) {
+        if (a == null) {
+          return "";
+        }
+
         String n = a.getName();
         if (n != null && n.length() > 0) {
           return n;
         }
-
-        n = a.getDescription();
-        if (n != null && n.length() > 0) {
-          return n;
-        }
-
-        return a.getId().toString();
+        return a.getGroupUUID().get();
       }
     });
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java
index 4c399c4476..0dbc0e1c6b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java
@@ -14,13 +14,14 @@
 
 package com.google.gerrit.server.account;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.data.GroupInfo;
 import com.google.gerrit.common.data.GroupInfoCache;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.inject.Inject;
 
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -30,13 +31,13 @@ public class GroupInfoCacheFactory {
     GroupInfoCacheFactory create();
   }
 
-  private final GroupCache groupCache;
-  private final Map<AccountGroup.UUID, AccountGroup> out;
+  private final GroupBackend groupBackend;
+  private final Map<AccountGroup.UUID, GroupDescription.Basic> out;
 
   @Inject
-  GroupInfoCacheFactory(final GroupCache groupCache) {
-    this.groupCache = groupCache;
-    this.out = new HashMap<AccountGroup.UUID, AccountGroup>();
+  GroupInfoCacheFactory(GroupBackend groupBackend) {
+    this.groupBackend = groupBackend;
+    this.out = Maps.newHashMap();
   }
 
   /**
@@ -46,7 +47,7 @@ public class GroupInfoCacheFactory {
    */
   public void want(final AccountGroup.UUID uuid) {
     if (uuid != null && !out.containsKey(uuid)) {
-      out.put(uuid, groupCache.get(uuid));
+      out.put(uuid, groupBackend.get(uuid));
     }
   }
 
@@ -57,7 +58,7 @@ public class GroupInfoCacheFactory {
     }
   }
 
-  public AccountGroup get(final AccountGroup.UUID uuid) {
+  public GroupDescription.Basic get(final AccountGroup.UUID uuid) {
     want(uuid);
     return out.get(uuid);
   }
@@ -66,10 +67,11 @@ public class GroupInfoCacheFactory {
    * Create an GroupInfoCache with the currently loaded AccountGroup entities.
    * */
   public GroupInfoCache create() {
-    final List<GroupInfo> r = new ArrayList<GroupInfo>(out.size());
-    for (final AccountGroup a : out.values()) {
-      if (a == null) continue;
-      r.add(new GroupInfo(a));
+    List<GroupInfo> r = Lists.newArrayListWithCapacity(out.size());
+    for (GroupDescription.Basic a : out.values()) {
+      if (a != null) {
+        r.add(new GroupInfo(a));
+      }
     }
     return new GroupInfoCache(r);
   }