Merge "Return list of reviewers as part of ChangeInfo"

This commit is contained in:
Edwin Kempin
2015-11-12 18:08:27 +00:00
committed by Gerrit Code Review
18 changed files with 268 additions and 88 deletions

View File

@@ -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.reviewdb.server.ReviewDb;
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 @@ public class ApprovalsUtil {
* @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 @@ public class ApprovalsUtil {
* 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 @@ public class ApprovalsUtil {
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 @@ public class ApprovalsUtil {
}
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 @@ public class ApprovalsUtil {
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);

View File

@@ -40,6 +40,7 @@ import com.google.common.base.Joiner;
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.LinkedHashMultimap;
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.api.accounts.GpgApiAdapter;
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.sql.Timestamp;
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 @@ public class ChangeJson {
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 @@ public class ChangeJson {
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 {

View File

@@ -17,6 +17,8 @@ package com.google.gerrit.server.change;
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.GroupCollector;
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 @@ public class PatchSetInserter extends BatchUpdate.Op {
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 @@ public class PatchSetInserter extends BatchUpdate.Op {
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 "

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.change;
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.ChangeContext;
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 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
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());
}
}

View File

@@ -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.git.validators.MergeValidators;
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 @@ public class MergeOp {
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);

View File

@@ -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.PatchSetInfo;
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 @@ public abstract class ChangeEmail extends NotificationEmail {
/** 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) {

View File

@@ -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 class MailUtil {
}
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;
}

View File

@@ -116,7 +116,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
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 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
return approvals;
}
public ImmutableSetMultimap<ReviewerState, Account.Id> getReviewers() {
public ImmutableSetMultimap<ReviewerStateInternal, Account.Id> getReviewers() {
return reviewers;
}
@@ -270,9 +270,9 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
} 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());
}

View File

@@ -71,7 +71,7 @@ import java.util.Map;
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 @@ class ChangeNotesParser implements AutoCloseable {
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 @@ class ChangeNotesParser implements AutoCloseable {
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 @@ class ChangeNotesParser implements AutoCloseable {
}
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());

View File

@@ -86,7 +86,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
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 @@ public class ChangeUpdate extends AbstractChangeUpdate {
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 @@ public class ChangeUpdate extends AbstractChangeUpdate {
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())

View File

@@ -14,26 +14,51 @@
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 ReviewerState {
public enum ReviewerStateInternal {
/** The user has contributed at least one nonzero vote on the change. */
REVIEWER(new FooterKey("Reviewer")),
REVIEWER(new FooterKey("Reviewer"), ReviewerState.REVIEWER),
/** The reviewer was added to the change, but has not voted. */
CC(new FooterKey("CC")),
CC(new FooterKey("CC"), ReviewerState.CC),
/** The user was previously a reviewer on the change, but was removed. */
REMOVED(new FooterKey("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 ReviewerState(FooterKey footerKey) {
private ReviewerStateInternal(FooterKey footerKey, ReviewerState state) {
this.footerKey = footerKey;
this.state = state;
}
FooterKey getFooterKey() {
return footerKey;
}
public ReviewerState asReviewerState() {
return state;
}
}

View File

@@ -45,7 +45,7 @@ import com.google.gerrit.server.git.GitRepositoryManager;
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 @@ public class ChangeData {
return allApprovals;
}
public SetMultimap<ReviewerState, Account.Id> reviewers()
public SetMultimap<ReviewerStateInternal, Account.Id> reviewers()
throws OrmException {
return approvalsUtil.getReviewers(notes(), approvals().values());
}

View File

@@ -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;

View File

@@ -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;