Add a /changes option to include detailed account info
Change-Id: I793e97762231372f223b069cff26c291ad767309
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 27f579d..82f69fa 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -152,6 +152,9 @@
`CURRENT_REVISION` was requested the only that commit's
modified files will be output.
+* `DETAILED_ACCOUNTS`: include `_account_id` and `email` fields when
+ referencing accounts.
+
----
GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES HTTP/1.0
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java b/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java
index a5ab851..5072b76 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/changes/ListChangesOption.java
@@ -30,7 +30,10 @@
/** If a patch set is included, include the files of the patch set. */
CURRENT_FILES(5),
- ALL_FILES(6);
+ ALL_FILES(6),
+
+ /** If accounts are included, include detailed account info. */
+ DETAILED_ACCOUNTS(7);
private final int value;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java
index 056babd..86308a9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java
@@ -20,6 +20,8 @@
public interface AccountCache {
public AccountState get(Account.Id accountId);
+ public AccountState getIfPresent(Account.Id accountId);
+
public AccountState getByUsername(String username);
public void evict(Account.Id accountId);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 4217f9f..b74daa5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -87,6 +87,10 @@
}
}
+ public AccountState getIfPresent(Account.Id accountId) {
+ return byId.getIfPresent(accountId);
+ }
+
@Override
public AccountState getByUsername(String username) {
try {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java
new file mode 100644
index 0000000..56362ce
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java
@@ -0,0 +1,110 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import com.google.common.collect.ArrayListMultimap;
+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.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.assistedinject.Assisted;
+
+import java.util.List;
+import java.util.Map;
+
+public class AccountInfo {
+ static class Loader {
+ interface Factory {
+ Loader create(boolean detailed);
+ }
+
+ private final Provider<ReviewDb> db;
+ private final AccountCache accountCache;
+ private final boolean detailed;
+ private final Map<Account.Id, AccountInfo> created;
+ private final List<AccountInfo> provided;
+
+ @Inject
+ Loader(Provider<ReviewDb> db,
+ AccountCache accountCache,
+ @Assisted boolean detailed) {
+ this.db = db;
+ this.accountCache = accountCache;
+ this.detailed = detailed;
+ created = Maps.newHashMap();
+ provided = Lists.newArrayList();
+ }
+
+ public AccountInfo get(Account.Id id) {
+ if (id == null) {
+ return null;
+ }
+ AccountInfo info = created.get(id);
+ if (info == null) {
+ info = new AccountInfo(id);
+ created.put(id, info);
+ }
+ return info;
+ }
+
+ public void put(AccountInfo info) {
+ provided.add(info);
+ }
+
+ public void fill() throws OrmException {
+ Multimap<Account.Id, AccountInfo> missing = ArrayListMultimap.create();
+ for (AccountInfo info : Iterables.concat(created.values(), provided)) {
+ AccountState state = accountCache.getIfPresent(info._id);
+ if (state != null) {
+ fill(info, state.getAccount());
+ } else {
+ missing.put(info._id, info);
+ }
+ }
+ if (!missing.isEmpty()) {
+ for (Account account : db.get().accounts().get(missing.keySet())) {
+ for (AccountInfo info : missing.get(account.getId())) {
+ fill(info, account);
+ }
+ }
+ }
+ }
+
+ private void fill(AccountInfo info, Account account) {
+ info.name = account.getFullName();
+ if (detailed) {
+ info._account_id = account.getId().get();
+ info.email = account.getPreferredEmail();
+ }
+ }
+ }
+
+ private transient Account.Id _id;
+
+ protected AccountInfo(Account.Id id) {
+ _id = id;
+ }
+
+ public Integer _account_id;
+ public String name;
+ public String email;
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index b004dc6..b0e59390 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.common.changes.ListChangesOption.CURRENT_COMMIT;
import static com.google.gerrit.common.changes.ListChangesOption.CURRENT_FILES;
import static com.google.gerrit.common.changes.ListChangesOption.CURRENT_REVISION;
+import static com.google.gerrit.common.changes.ListChangesOption.DETAILED_ACCOUNTS;
import static com.google.gerrit.common.changes.ListChangesOption.LABELS;
import com.google.common.base.Joiner;
@@ -47,7 +48,6 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.events.AccountAttribute;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -106,13 +106,14 @@
private final ChangeControl.GenericFactory changeControlGenericFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchListCache patchListCache;
+ private final AccountInfo.Loader.Factory accountLoaderFactory;
private final Provider<String> urlProvider;
private final Urls urls;
private ChangeControl.Factory changeControlUserFactory;
private SshInfo sshInfo;
- private Map<Account.Id, AccountAttribute> accounts;
private Map<Change.Id, ChangeControl> controls;
private EnumSet<ListChangesOption> options;
+ private AccountInfo.Loader accountLoader;
@Inject
ChangeJson(
@@ -123,6 +124,7 @@
ChangeControl.GenericFactory gf,
PatchSetInfoFactory psi,
PatchListCache plc,
+ AccountInfo.Loader.Factory ailf,
@CanonicalWebUrl Provider<String> curl,
Urls urls) {
this.db = db;
@@ -132,10 +134,10 @@
this.changeControlGenericFactory = gf;
this.patchSetInfoFactory = psi;
this.patchListCache = plc;
+ this.accountLoaderFactory = ailf;
this.urlProvider = curl;
this.urls = urls;
- accounts = Maps.newHashMap();
controls = Maps.newHashMap();
options = EnumSet.noneOf(ListChangesOption.class);
}
@@ -179,6 +181,8 @@
public List<List<ChangeInfo>> formatList2(List<List<ChangeData>> in)
throws OrmException {
+ accountLoader =
+ accountLoaderFactory.create(options.contains(DETAILED_ACCOUNTS));
List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size());
for (List<ChangeData> changes : in) {
ChangeData.ensureChangeLoaded(db, changes);
@@ -186,12 +190,7 @@
ChangeData.ensureCurrentApprovalsLoaded(db, changes);
res.add(toChangeInfo(changes));
}
- if (!accounts.isEmpty()) {
- for (Account account : db.get().accounts().get(accounts.keySet())) {
- AccountAttribute a = accounts.get(account.getId());
- a.name = Strings.emptyToNull(account.getFullName());
- }
- }
+ accountLoader.fill();
return res;
}
@@ -213,7 +212,7 @@
out.changeId = in.getKey().get();
out.subject = in.getSubject();
out.status = in.getStatus();
- out.owner = asAccountAttribute(in.getOwner());
+ out.owner = accountLoader.get(in.getOwner());
out.created = in.getCreatedOn();
out.updated = in.getLastUpdatedOn();
out._number = in.getId().get();
@@ -236,18 +235,6 @@
return out;
}
- private AccountAttribute asAccountAttribute(Account.Id user) {
- if (user == null) {
- return null;
- }
- AccountAttribute a = accounts.get(user);
- if (a == null) {
- a = new AccountAttribute();
- accounts.put(user, a);
- }
- return a;
- }
-
private ChangeControl control(ChangeData cd) throws OrmException {
ChangeControl ctrl = cd.changeControl();
if (ctrl != null && ctrl.getCurrentUser() == user) {
@@ -295,10 +282,10 @@
n._status = r.status;
switch (r.status) {
case OK:
- n.approved = asAccountAttribute(r.appliedBy);
+ n.approved = accountLoader.get(r.appliedBy);
break;
case REJECT:
- n.rejected = asAccountAttribute(r.appliedBy);
+ n.rejected = accountLoader.get(r.appliedBy);
break;
default:
break;
@@ -336,10 +323,10 @@
if (val != 0 && min < val && val < max
&& psa.getCategoryId().equals(type.getCategory().getId())) {
if (0 < val) {
- e.getValue().recommended = asAccountAttribute(psa.getAccountId());
+ e.getValue().recommended = accountLoader.get(psa.getAccountId());
e.getValue().value = val != 1 ? val : null;
} else {
- e.getValue().disliked = asAccountAttribute(psa.getAccountId());
+ e.getValue().disliked = accountLoader.get(psa.getAccountId());
e.getValue().value = val != -1 ? val : null;
}
}
@@ -542,7 +529,7 @@
String _sortkey;
int _number;
- AccountAttribute owner;
+ AccountInfo owner;
Map<String, LabelInfo> labels;
String current_revision;
Map<String, RevisionInfo> revisions;
@@ -601,11 +588,11 @@
static class LabelInfo {
transient SubmitRecord.Label.Status _status;
- AccountAttribute approved;
- AccountAttribute rejected;
+ AccountInfo approved;
+ AccountInfo rejected;
- AccountAttribute recommended;
- AccountAttribute disliked;
+ AccountInfo recommended;
+ AccountInfo disliked;
Short value;
Boolean optional;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index c824a87..571439d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -70,6 +70,7 @@
install(new FactoryModule() {
@Override
protected void configure() {
+ factory(AccountInfo.Loader.Factory.class);
factory(EmailReviewComments.Factory.class);
}
});
