Return list of reviewers as part of ChangeInfo
ChangeInfo now includes the list of reviewers when DETAILED_LABELS are
requested.
So far the list of reviewers could only indirectly be computed from
the returned approvals, but this relied on having a dummy 0 approval
for users that were added as reviewer, but that haven't voted yet.
When notedb is enabled we no longer store dummy 0 approvals for
reviewers, but we track the list of reviewers explicitly in notedb.
When notedb was enabled the ChangeIT#addReviewerToClosedChange() test
was failing because it relied on the dummy 0 approvals.
With this change, reviewers are now included in ChangeInfo, adapt the
tests to look at this field instead of computing the list of reviewers
from the approvals.
When notedb is enabled the PostReviewers REST endpoint records users
as 'REVIEWER' in notedb. If notedb is disabled the PostReviewers REST
endpoint behaves differently. In this case it adds dummy 0 approvals
for the users, but these translate into 'CC's when the ChangeInfo is
constructed. Hence the tests that check on reviewers must do different
assertions depending on whether notedb is enabled or not.
Change-Id: I5e7b99d85d97abc43ac17da44b7179f4ceb3e268
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 8ae56e5..4679158 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -209,8 +209,8 @@
--
* `DETAILED_LABELS`: detailed label information, including numeric
values of all existing approvals, recognized label values, values
- permitted to be set by the current user, and reviewers that may be
- removed by the current user.
+ permitted to be set by the current user, all reviewers by state, and
+ reviewers that may be removed by the current user.
--
[[current-revision]]
@@ -641,6 +641,22 @@
"username": "jroe"
}
],
+ "reviewers": {
+ "REVIEWER": [
+ {
+ "_account_id": 1000096,
+ "name": "John Doe",
+ "email": "john.doe@example.com",
+ "username": "jdoe"
+ },
+ {
+ "_account_id": 1000097,
+ "name": "Jane Roe",
+ "email": "jane.roe@example.com",
+ "username": "jroe"
+ }
+ ]
+ },
"messages": [
{
"id": "YH-egE",
@@ -1204,6 +1220,13 @@
"_account_id": 1000000
}
],
+ "reviewers": {
+ "REVIEWER": [
+ {
+ "_account_id": 1000000
+ }
+ ]
+ },
"current_revision": "9adb9f4c7b40eeee0646e235de818d09164d7379",
"revisions": {
"9adb9f4c7b40eeee0646e235de818d09164d7379": {
@@ -1315,6 +1338,13 @@
"_account_id": 1000000
}
],
+ "reviewers": {
+ "REVIEWER": [
+ {
+ "_account_id": 1000000
+ }
+ ]
+ },
"current_revision": "1bd7c12a38854a2c6de426feec28800623f492c4",
"revisions": {
"1bd7c12a38854a2c6de426feec28800623f492c4": {
@@ -2447,6 +2477,20 @@
"email": "jane.roe@example.com"
}
],
+ "reviewers": {
+ "REVIEWER": [
+ {
+ "_account_id": 1000096,
+ "name": "John Doe",
+ "email": "john.doe@example.com"
+ },
+ {
+ "_account_id": 1000097,
+ "name": "Jane Roe",
+ "email": "jane.roe@example.com"
+ }
+ ]
+ },
"current_revision": "674ac754f91e64a0efb8087e59a176484bd534d1",
"revisions": {
"674ac754f91e64a0efb8087e59a176484bd534d1": {
@@ -3884,6 +3928,15 @@
The reviewers that can be removed by the calling user as a list of
link:rest-api-accounts.html#account-info[AccountInfo] entities. +
Only set if link:#detailed-labels[detailed labels] are requested.
+|`reviewers` ||
+The reviewers as a map that maps a reviewer state to a list of
+link:rest-api-accounts.html#account-info[AccountInfo] entities.
+Possible reviewer states are `REVIEWER`, `CC` and `REMOVED`. +
+`REVIEWER`: Users with at least one non-zero vote on the change. +
+`CC`: Users that were added to the change, but have not voted. +
+`REMOVED`: Users that were previously reviewers on the change, but have
+been removed. +
+Only set if link:#detailed-labels[detailed labels] are requested.
|`messages`|optional|
Messages associated with the change as a list of
link:#change-message-info[ChangeMessageInfo] entities. +
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 92d7980..1598774 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -17,15 +17,16 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.extensions.client.ReviewerState.CC;
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.Util.blockLabel;
import static com.google.gerrit.server.project.Util.category;
import static com.google.gerrit.server.project.Util.value;
+import static com.google.gerrit.testutil.GerritServerTests.isNoteDbTestEnabled;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -38,6 +39,7 @@
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
@@ -45,7 +47,6 @@
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -58,9 +59,10 @@
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.EnumSet;
+import java.util.Iterator;
import java.util.List;
-import java.util.Set;
@NoHttpd
public class ChangeIT extends AbstractDaemonTest {
@@ -297,17 +299,6 @@
.rebase(ri);
}
- private Set<Account.Id> getReviewers(String changeId) throws Exception {
- ChangeInfo ci = gApi.changes().id(changeId).get();
- Set<Account.Id> result = Sets.newHashSet();
- for (LabelInfo li : ci.labels.values()) {
- for (ApprovalInfo ai : li.all) {
- result.add(new Account.Id(ai._accountId));
- }
- }
- return result;
- }
-
@Test
public void addReviewer() throws Exception {
PushOneCommit.Result r = createChange();
@@ -317,8 +308,20 @@
.id(r.getChangeId())
.addReviewer(in);
- assertThat(getReviewers(r.getChangeId()))
- .containsExactlyElementsIn(ImmutableSet.of(user.id));
+ ChangeInfo c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+
+ // When notedb is enabled adding a reviewer records that user as reviewer
+ // in notedb. When notedb is disabled adding a reviewer results in a dummy 0
+ // approval on the change which is treated as CC when the ChangeInfo is
+ // created.
+ Collection<AccountInfo> reviewers = isNoteDbTestEnabled()
+ ? c.reviewers.get(REVIEWER)
+ : c.reviewers.get(CC);
+ assertThat(reviewers).hasSize(1);
+ assertThat(reviewers.iterator().next()._accountId)
+ .isEqualTo(user.getId().get());
}
@Test
@@ -333,16 +336,46 @@
.revision(r.getCommit().name())
.submit();
- assertThat(getReviewers(r.getChangeId()))
- .containsExactlyElementsIn(ImmutableSet.of(admin.getId()));
+ ChangeInfo c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+ Collection<AccountInfo> reviewers = c.reviewers.get(REVIEWER);
+ assertThat(reviewers).hasSize(1);
+ assertThat(reviewers.iterator().next()._accountId)
+ .isEqualTo(admin.getId().get());
+ assertThat(c.reviewers).doesNotContainKey(CC);
AddReviewerInput in = new AddReviewerInput();
in.reviewer = user.email;
gApi.changes()
.id(r.getChangeId())
.addReviewer(in);
- assertThat(getReviewers(r.getChangeId()))
- .containsExactlyElementsIn(ImmutableSet.of(admin.getId(), user.id));
+
+ c = gApi.changes()
+ .id(r.getChangeId())
+ .get();
+ reviewers = c.reviewers.get(REVIEWER);
+ if (isNoteDbTestEnabled()) {
+ // When notedb is enabled adding a reviewer records that user as reviewer
+ // in notedb.
+ assertThat(reviewers).hasSize(2);
+ Iterator<AccountInfo> reviewerIt = reviewers.iterator();
+ assertThat(reviewerIt.next()._accountId)
+ .isEqualTo(admin.getId().get());
+ assertThat(reviewerIt.next()._accountId)
+ .isEqualTo(user.getId().get());
+ assertThat(c.reviewers).doesNotContainKey(CC);
+ } else {
+ // When notedb is disabled adding a reviewer results in a dummy 0 approval
+ // on the change which is treated as CC when the ChangeInfo is created.
+ assertThat(reviewers).hasSize(1);
+ assertThat(reviewers.iterator().next()._accountId)
+ .isEqualTo(admin.getId().get());
+ Collection<AccountInfo> ccs = c.reviewers.get(CC);
+ assertThat(ccs).hasSize(1);
+ assertThat(ccs.iterator().next()._accountId)
+ .isEqualTo(user.getId().get());
+ }
}
@Test
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerState.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ReviewerState.java
similarity index 63%
rename from gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerState.java
rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ReviewerState.java
index b829a69..a58c959 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerState.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ReviewerState.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2013 The Android Open Source Project
+// Copyright (C) 2015 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.
@@ -12,28 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-package com.google.gerrit.server.notedb;
+package com.google.gerrit.extensions.client;
-import org.eclipse.jgit.revwalk.FooterKey;
-
-/** State of a reviewer on a change. */
public enum ReviewerState {
/** The user has contributed at least one nonzero vote on the change. */
- REVIEWER(new FooterKey("Reviewer")),
+ REVIEWER,
/** The reviewer was added to the change, but has not voted. */
- CC(new FooterKey("CC")),
+ CC,
/** The user was previously a reviewer on the change, but was removed. */
- REMOVED(new FooterKey("Removed"));
-
- private final FooterKey footerKey;
-
- private ReviewerState(FooterKey footerKey) {
- this.footerKey = footerKey;
- }
-
- FooterKey getFooterKey() {
- return footerKey;
- }
+ REMOVED;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
index cdfe0c6..455243a 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -15,6 +15,7 @@
package com.google.gerrit.extensions.common;
import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.ReviewerState;
import java.sql.Timestamp;
import java.util.Collection;
@@ -48,6 +49,7 @@
public Map<String, LabelInfo> labels;
public Map<String, Collection<String>> permittedLabels;
public Collection<AccountInfo> removableReviewers;
+ public Map<ReviewerState, Collection<AccountInfo>> reviewers;
public Collection<ChangeMessageInfo> messages;
public String currentRevision;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index 31058bc..7551d5f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -15,6 +15,8 @@
package com.google.gerrit.server;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
@@ -44,7 +46,7 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.notedb.ReviewerState;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -114,10 +116,10 @@
* @param notes change notes.
* @return multimap of reviewers keyed by state, where each account appears
* exactly once in {@link SetMultimap#values()}, and
- * {@link ReviewerState#REMOVED} is not present.
+ * {@link ReviewerStateInternal#REMOVED} is not present.
* @throws OrmException if reviewers for the change could not be read.
*/
- public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers(
+ public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers(
ReviewDb db, ChangeNotes notes) throws OrmException {
if (!migration.readChanges()) {
return getReviewers(db.patchSetApprovals().byChange(notes.getChangeId()));
@@ -132,9 +134,9 @@
* change.
* @return multimap of reviewers keyed by state, where each account appears
* exactly once in {@link SetMultimap#values()}, and
- * {@link ReviewerState#REMOVED} is not present.
+ * {@link ReviewerStateInternal#REMOVED} is not present.
*/
- public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers(
+ public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers(
ChangeNotes notes, Iterable<PatchSetApproval> allApprovals)
throws OrmException {
if (!migration.readChanges()) {
@@ -143,10 +145,10 @@
return notes.load().getReviewers();
}
- private static ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers(
+ private static ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers(
Iterable<PatchSetApproval> allApprovals) {
PatchSetApproval first = null;
- SetMultimap<ReviewerState, Account.Id> reviewers =
+ SetMultimap<ReviewerStateInternal, Account.Id> reviewers =
LinkedHashMultimap.create();
for (PatchSetApproval psa : allApprovals) {
if (first == null) {
@@ -159,10 +161,10 @@
}
Account.Id id = psa.getAccountId();
if (psa.getValue() != 0) {
- reviewers.put(ReviewerState.REVIEWER, id);
- reviewers.remove(ReviewerState.CC, id);
- } else if (!reviewers.containsEntry(ReviewerState.REVIEWER, id)) {
- reviewers.put(ReviewerState.CC, id);
+ reviewers.put(REVIEWER, id);
+ reviewers.remove(CC, id);
+ } else if (!reviewers.containsEntry(REVIEWER, id)) {
+ reviewers.put(CC, id);
}
}
return ImmutableSetMultimap.copyOf(reviewers);
@@ -215,7 +217,7 @@
cells.add(new PatchSetApproval(
new PatchSetApproval.Key(psId, account, labelId),
(short) 0, TimeUtil.nowTs()));
- update.putReviewer(account, ReviewerState.REVIEWER);
+ update.putReviewer(account, REVIEWER);
}
db.patchSetApprovals().insert(cells);
return Collections.unmodifiableList(cells);
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 733d7a2..a02f0db 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
@@ -40,6 +40,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Optional;
import com.google.common.base.Throwables;
+import com.google.common.collect.ComparisonChain;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.HashMultimap;
@@ -48,6 +49,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
+import com.google.common.collect.Ordering;
import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
import com.google.common.collect.Table;
@@ -94,6 +96,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
@@ -118,7 +121,9 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.EnumSet;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -437,6 +442,13 @@
out.permittedLabels = permittedLabels(ctl, cd);
}
out.removableReviewers = removableReviewers(ctl, out.labels.values());
+
+ out.reviewers = new HashMap<>();
+ for (Map.Entry<ReviewerStateInternal, Collection<Account.Id>> e
+ : cd.reviewers().asMap().entrySet()) {
+ out.reviewers.put(e.getKey().asReviewerState(),
+ toAccountInfo(e.getValue()));
+ }
}
boolean needMessages = has(MESSAGES);
@@ -828,6 +840,27 @@
return result;
}
+ private Collection<AccountInfo> toAccountInfo(
+ Collection<Account.Id> accounts) {
+ return FluentIterable.from(accounts)
+ .transform(new Function<Account.Id, AccountInfo>() {
+ @Override
+ public AccountInfo apply(Account.Id id) {
+ return accountLoader.get(id);
+ }
+ })
+ .toSortedList(new Comparator<AccountInfo>() {
+ @Override
+ public int compare(AccountInfo a, AccountInfo b) {
+ return ComparisonChain.start()
+ .compare(a.name, b.name, Ordering.natural().nullsFirst())
+ .compare(a.email, b.email, Ordering.natural().nullsFirst())
+ .compare(a._accountId, b._accountId,
+ Ordering.natural().nullsFirst()).result();
+ }
+ });
+ }
+
private Map<String, RevisionInfo> revisions(ChangeControl ctl,
Map<PatchSet.Id, PatchSet> map) throws PatchListNotAvailableException,
GpgException, OrmException, IOException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index c34fb43..efe31e3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -17,6 +17,8 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.common.collect.SetMultimap;
import com.google.gerrit.common.ChangeHooks;
@@ -42,7 +44,7 @@
import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
-import com.google.gerrit.server.notedb.ReviewerState;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ChangeModifiedException;
@@ -106,7 +108,7 @@
private PatchSet patchSet;
private PatchSetInfo patchSetInfo;
private ChangeMessage changeMessage;
- private SetMultimap<ReviewerState, Account.Id> oldReviewers;
+ private SetMultimap<ReviewerStateInternal, Account.Id> oldReviewers;
@AssistedInject
public PatchSetInserter(ChangeHooks hooks,
@@ -280,8 +282,8 @@
cm.setFrom(ctx.getUser().getAccountId());
cm.setPatchSet(patchSet, patchSetInfo);
cm.setChangeMessage(changeMessage);
- cm.addReviewers(oldReviewers.get(ReviewerState.REVIEWER));
- cm.addExtraCC(oldReviewers.get(ReviewerState.CC));
+ cm.addReviewers(oldReviewers.get(REVIEWER));
+ cm.addExtraCC(oldReviewers.get(CC));
cm.send();
} catch (Exception err) {
log.error("Cannot send email for new patch set on change "
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 39f7bb4..f2d0ffa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -17,6 +17,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue;
@@ -64,7 +65,6 @@
import com.google.gerrit.server.git.BatchUpdate.Context;
import com.google.gerrit.server.git.UpdateException;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
@@ -549,7 +549,7 @@
ups.add(c);
addLabelDelta(normName, c.getValue());
categories.put(normName, c.getValue());
- update.putReviewer(user.getAccountId(), ReviewerState.REVIEWER);
+ update.putReviewer(user.getAccountId(), REVIEWER);
update.putApproval(ent.getKey(), ent.getValue());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 05e0cec..a2d71fb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.base.Optional;
@@ -61,7 +62,6 @@
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
@@ -1053,7 +1053,7 @@
logDebug("Add approval for " + cd + " from user " + user);
ChangeUpdate update = updateFactory.create(control, timestamp);
- update.putReviewer(user.getAccountId(), ReviewerState.REVIEWER);
+ update.putReviewer(user.getAccountId(), REVIEWER);
List<SubmitRecord> record = records.get(cd.change().getId());
if (record != null) {
update.merge(record);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
index 8948ce3..43d02fb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.mail;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
+
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -26,7 +28,6 @@
import com.google.gerrit.reviewdb.client.StarredChange;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.mail.ProjectWatch.Watchers;
-import com.google.gerrit.server.notedb.ReviewerState;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -329,7 +330,7 @@
/** Users who have non-zero approval codes on the change. */
protected void ccExistingReviewers() {
try {
- for (Account.Id id : changeData.reviewers().get(ReviewerState.REVIEWER)) {
+ for (Account.Id id : changeData.reviewers().get(REVIEWER)) {
add(RecipientType.CC, id);
}
} catch (OrmException err) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
index c6e59eb..a3a048a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java
@@ -14,13 +14,16 @@
package com.google.gerrit.server.mail;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
+
import com.google.common.collect.Multimap;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.notedb.ReviewerState;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.revwalk.FooterKey;
@@ -56,10 +59,10 @@
}
public static MailRecipients getRecipientsFromReviewers(
- Multimap<ReviewerState, Account.Id> reviewers) {
+ Multimap<ReviewerStateInternal, Account.Id> reviewers) {
MailRecipients recipients = new MailRecipients();
- recipients.reviewers.addAll(reviewers.get(ReviewerState.REVIEWER));
- recipients.cc.addAll(reviewers.get(ReviewerState.CC));
+ recipients.reviewers.addAll(reviewers.get(REVIEWER));
+ recipients.cc.addAll(reviewers.get(CC));
return recipients;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index 7331954..801f172 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -116,7 +116,7 @@
private final Change change;
private ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals;
- private ImmutableSetMultimap<ReviewerState, Account.Id> reviewers;
+ private ImmutableSetMultimap<ReviewerStateInternal, Account.Id> reviewers;
private ImmutableList<Account.Id> allPastReviewers;
private ImmutableList<SubmitRecord> submitRecords;
private ImmutableList<ChangeMessage> allChangeMessages;
@@ -144,7 +144,7 @@
return approvals;
}
- public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers() {
+ public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers() {
return reviewers;
}
@@ -270,9 +270,9 @@
} else {
hashtags = ImmutableSet.of();
}
- ImmutableSetMultimap.Builder<ReviewerState, Account.Id> reviewers =
+ ImmutableSetMultimap.Builder<ReviewerStateInternal, Account.Id> reviewers =
ImmutableSetMultimap.builder();
- for (Map.Entry<Account.Id, ReviewerState> e
+ for (Map.Entry<Account.Id, ReviewerStateInternal> e
: parser.reviewers.entrySet()) {
reviewers.put(e.getValue(), e.getKey());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index fd4817f..b58548c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -71,7 +71,7 @@
import java.util.Set;
class ChangeNotesParser implements AutoCloseable {
- final Map<Account.Id, ReviewerState> reviewers;
+ final Map<Account.Id, ReviewerStateInternal> reviewers;
final List<Account.Id> allPastReviewers;
final List<SubmitRecord> submitRecords;
final Multimap<RevId, PatchLineComment> comments;
@@ -168,7 +168,7 @@
parseApproval(psId, accountId, commit, line);
}
- for (ReviewerState state : ReviewerState.values()) {
+ for (ReviewerStateInternal state : ReviewerStateInternal.values()) {
for (String line : commit.getFooterLines(state.getFooterKey())) {
parseReviewer(state, line);
}
@@ -394,7 +394,7 @@
GERRIT_PLACEHOLDER_HOST, email);
}
- private void parseReviewer(ReviewerState state, String line)
+ private void parseReviewer(ReviewerStateInternal state, String line)
throws ConfigInvalidException {
PersonIdent ident = RawParseUtils.parsePersonIdent(line);
if (ident == null) {
@@ -407,11 +407,11 @@
}
private void pruneReviewers() {
- Iterator<Map.Entry<Account.Id, ReviewerState>> rit =
+ Iterator<Map.Entry<Account.Id, ReviewerStateInternal>> rit =
reviewers.entrySet().iterator();
while (rit.hasNext()) {
- Map.Entry<Account.Id, ReviewerState> e = rit.next();
- if (e.getValue() == ReviewerState.REMOVED) {
+ Map.Entry<Account.Id, ReviewerStateInternal> e = rit.next();
+ if (e.getValue() == ReviewerStateInternal.REMOVED) {
rit.remove();
for (Table<Account.Id, ?, ?> curr : approvals.values()) {
curr.rowKeySet().remove(e.getKey());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 92a9856..1d8e507 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -86,7 +86,7 @@
private final AccountCache accountCache;
private final Map<String, Optional<Short>> approvals;
- private final Map<Account.Id, ReviewerState> reviewers;
+ private final Map<Account.Id, ReviewerStateInternal> reviewers;
private Change.Status status;
private String subject;
private List<SubmitRecord> submitRecords;
@@ -320,13 +320,13 @@
this.hashtags = hashtags;
}
- public void putReviewer(Account.Id reviewer, ReviewerState type) {
- checkArgument(type != ReviewerState.REMOVED, "invalid ReviewerType");
+ public void putReviewer(Account.Id reviewer, ReviewerStateInternal type) {
+ checkArgument(type != ReviewerStateInternal.REMOVED, "invalid ReviewerType");
reviewers.put(reviewer, type);
}
public void removeReviewer(Account.Id reviewer) {
- reviewers.put(reviewer, ReviewerState.REMOVED);
+ reviewers.put(reviewer, ReviewerStateInternal.REMOVED);
}
/** @return the tree id for the updated tree */
@@ -422,7 +422,7 @@
addFooter(msg, FOOTER_HASHTAGS, Joiner.on(",").join(hashtags));
}
- for (Map.Entry<Account.Id, ReviewerState> e : reviewers.entrySet()) {
+ for (Map.Entry<Account.Id, ReviewerStateInternal> e : reviewers.entrySet()) {
Account account = accountCache.get(e.getKey()).getAccount();
PersonIdent ident = newIdent(account, when);
addFooter(msg, e.getValue().getFooterKey())
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerStateInternal.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerStateInternal.java
new file mode 100644
index 0000000..3bf2135
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerStateInternal.java
@@ -0,0 +1,64 @@
+// 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.notedb;
+
+import com.google.gerrit.extensions.client.ReviewerState;
+
+import org.eclipse.jgit.revwalk.FooterKey;
+
+import java.util.Arrays;
+
+/** State of a reviewer on a change. */
+public enum ReviewerStateInternal {
+ /** The user has contributed at least one nonzero vote on the change. */
+ REVIEWER(new FooterKey("Reviewer"), ReviewerState.REVIEWER),
+
+ /** The reviewer was added to the change, but has not voted. */
+ CC(new FooterKey("CC"), ReviewerState.CC),
+
+ /** The user was previously a reviewer on the change, but was removed. */
+ REMOVED(new FooterKey("Removed"), ReviewerState.REMOVED);
+
+ static {
+ boolean ok = true;
+ if (ReviewerStateInternal.values().length != ReviewerState.values().length) {
+ ok = false;
+ }
+ for (ReviewerStateInternal s : ReviewerStateInternal.values()) {
+ ok &= s.name().equals(s.state.name());
+ }
+ if (!ok) {
+ throw new IllegalStateException("Mismatched reviewer state mapping: "
+ + Arrays.asList(ReviewerStateInternal.values()) + " != "
+ + Arrays.asList(ReviewerState.values()));
+ }
+ }
+
+ private final FooterKey footerKey;
+ private final ReviewerState state;
+
+ private ReviewerStateInternal(FooterKey footerKey, ReviewerState state) {
+ this.footerKey = footerKey;
+ this.state = state;
+ }
+
+ FooterKey getFooterKey() {
+ return footerKey;
+ }
+
+ public ReviewerState asReviewerState() {
+ return state;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index 1b7713b..c06029b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -45,7 +45,7 @@
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.notedb.ReviewerState;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListEntry;
@@ -707,7 +707,7 @@
return allApprovals;
}
- public SetMultimap<ReviewerState, Account.Id> reviewers()
+ public SetMultimap<ReviewerStateInternal, Account.Id> reviewers()
throws OrmException {
return approvalsUtil.getReviewers(notes(), approvals().values());
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index e74df8f..7debe94 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.notedb;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.server.notedb.ReviewerState.CC;
-import static com.google.gerrit.server.notedb.ReviewerState.REVIEWER;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static com.google.gerrit.testutil.TestChanges.incrementPatchSet;
import static java.nio.charset.StandardCharsets.UTF_8;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index c206902..811cd8a 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.notedb;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.server.notedb.ReviewerState.CC;
-import static com.google.gerrit.server.notedb.ReviewerState.REVIEWER;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.TimeUtil;
