Don't use AccountAccess directly
Accounts are migrated to NoteDb and then ReviewDb#accounts() will be
removed. The new way to read accounts is by using the Accounts class
which makes it transparent for the caller whether accounts are read from
ReviewDb or NoteDb.
FindOwnerIT is adapted to use Accounts to load accounts for
verification.
Since change I363a8ee040 in Gerrit core the account cache is available
to Prolog predicates. Checker is now using the account cache to lookup
accounts. This should also result in better performance than loading the
accounts from the database each time Prolog rules are evaluated.
Change-Id: I92751e401ef3fcb768e315dad4752f35c6a62919
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
index 8cc92ac..57e725c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
@@ -14,12 +14,10 @@
package com.googlesource.gerrit.plugins.findowners;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.server.AccountAccess;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.StoredValues;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.googlecode.prolog_cafe.lang.Prolog;
@@ -50,18 +48,14 @@
}
/** Returns a map from reviewer email to vote value. */
- static Map<String, Integer> getVotes(ChangeData changeData) throws OrmException {
- ReviewDb db = changeData.db();
+ static Map<String, Integer> getVotes(AccountCache accountCache, ChangeData changeData)
+ throws OrmException {
Map<String, Integer> map = new HashMap<>();
- AccountAccess ac = db.accounts();
for (PatchSetApproval p : changeData.currentApprovals()) {
if (p.getValue() != 0) {
- Account.Id id = p.getAccountId();
- try {
- map.put(ac.get(id).getPreferredEmail(), Integer.valueOf(p.getValue()));
- } catch (OrmException e) {
- log.error("Cannot get email address of account id: " + id.get(), e);
- }
+ map.put(
+ accountCache.get(p.getAccountId()).getAccount().getPreferredEmail(),
+ Integer.valueOf(p.getValue()));
}
}
return map;
@@ -86,12 +80,12 @@
}
/** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
- int findApproval(OwnersDb db) throws OrmException {
+ int findApproval(AccountCache accountCache, OwnersDb db) throws OrmException {
Map<String, Set<String>> file2Owners = db.findOwners(changeData.currentFilePaths());
if (file2Owners.size() == 0) { // do not need owner approval
return 0;
}
- Map<String, Integer> votes = getVotes(changeData);
+ Map<String, Integer> votes = getVotes(accountCache, changeData);
for (Set<String> owners : file2Owners.values()) {
if (!findOwnersInVotes(owners, votes)) {
return -1;
@@ -103,9 +97,10 @@
/** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
public static int findApproval(Prolog engine, int minVoteLevel) {
try {
+ AccountCache accountCache = StoredValues.ACCOUNT_CACHE.get(engine);
ChangeData changeData = StoredValues.CHANGE_DATA.get(engine);
Repository repository = StoredValues.REPOSITORY.get(engine);
- return new Checker(repository, changeData, minVoteLevel).findApproval();
+ return new Checker(repository, changeData, minVoteLevel).findApproval(accountCache);
} catch (OrmException e) {
log.error("Exception", e);
return 0; // owner approval may or may not be required.
@@ -128,7 +123,7 @@
return (status == Status.ABANDONED || status == Status.MERGED);
}
- int findApproval() throws OrmException {
+ int findApproval(AccountCache accountCache) throws OrmException {
if (isExemptFromOwnerApproval(changeData)) {
return 0;
}
@@ -142,6 +137,6 @@
minVoteLevel = Config.getMinOwnerVoteLevel(changeData);
}
log.trace("findApproval db key = " + db.key);
- return findApproval(db);
+ return findApproval(accountCache, db);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
index 22d932a..5e73d66 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
@@ -30,6 +30,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
@@ -45,6 +46,7 @@
public class FindOwnersIT extends LightweightPluginDaemonTest {
@Inject private PluginConfigFactory configFactory;
+ @Inject private Accounts accounts;
@Test
public void getOwnersTest() throws Exception {
@@ -175,8 +177,8 @@
Account.Id id = accountCreator.create("User" + email, email, "FullName" + email).getId();
// Action.getReviewers uses accountCache to get email address.
assertThat(accountCache.get(id).getAccount().getPreferredEmail()).isEqualTo(email);
- // Checker.getVotes uses AccountAccess to get email address.
- assertThat(db.accounts().get(id).getPreferredEmail()).isEqualTo(email);
+ // Checker.getVotes uses AccountCache to get email address.
+ assertThat(accounts.get(db, id).getPreferredEmail()).isEqualTo(email);
}
}
