Merge "Fix IAE when listing reviewers of change that has reviewers by email"
diff --git a/java/com/google/gerrit/server/change/ReviewerJson.java b/java/com/google/gerrit/server/change/ReviewerJson.java
index 6502569..ef2c926 100644
--- a/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -23,6 +23,7 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -77,12 +78,15 @@
if (cd == null || !cd.getId().equals(rsrc.getChangeId())) {
cd = changeDataFactory.create(db.get(), rsrc.getChangeResource().getNotes());
}
- ReviewerInfo info =
- format(
- new ReviewerInfo(rsrc.getReviewerUser().getAccountId().get()),
- rsrc.getReviewerUser().getAccountId(),
- cd);
- loader.put(info);
+ ReviewerInfo info;
+ if (rsrc.isByEmail()) {
+ Address address = rsrc.getReviewerByEmail();
+ info = ReviewerInfo.byEmail(address.getName(), address.getEmail());
+ } else {
+ Account.Id reviewerAccountId = rsrc.getReviewerUser().getAccountId();
+ info = format(new ReviewerInfo(reviewerAccountId.get()), reviewerAccountId, cd);
+ loader.put(info);
+ }
infos.add(info);
}
loader.fill();
@@ -94,19 +98,21 @@
return format(ImmutableList.<ReviewerResource>of(rsrc));
}
- public ReviewerInfo format(ReviewerInfo out, Account.Id reviewer, ChangeData cd)
+ public ReviewerInfo format(ReviewerInfo out, Account.Id reviewerAccountId, ChangeData cd)
throws OrmException, PermissionBackendException {
PatchSet.Id psId = cd.change().currentPatchSetId();
return format(
out,
- reviewer,
+ reviewerAccountId,
cd,
- approvalsUtil.byPatchSetUser(
- db.get(), cd.notes(), psId, new Account.Id(out._accountId), null, null));
+ approvalsUtil.byPatchSetUser(db.get(), cd.notes(), psId, reviewerAccountId, null, null));
}
public ReviewerInfo format(
- ReviewerInfo out, Account.Id reviewer, ChangeData cd, Iterable<PatchSetApproval> approvals)
+ ReviewerInfo out,
+ Account.Id reviewerAccountId,
+ ChangeData cd,
+ Iterable<PatchSetApproval> approvals)
throws OrmException, PermissionBackendException {
LabelTypes labelTypes = cd.getLabelTypes();
@@ -123,7 +129,7 @@
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
PermissionBackend.ForChange perm =
- permissionBackend.absentUser(reviewer).database(db).change(cd);
+ permissionBackend.absentUser(reviewerAccountId).database(db).change(cd);
for (SubmitRecord rec : submitRuleEvaluator.evaluate(cd)) {
if (rec.labels == null) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index 257c88b..dc71c1f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -22,8 +22,8 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -35,11 +35,12 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.mail.Address;
import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gson.reflect.TypeToken;
+import java.lang.reflect.Type;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
-@NoHttpd
public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
@Before
@@ -96,6 +97,34 @@
}
@Test
+ public void listReviewersByEmail() throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+ for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+ PushOneCommit.Result r = createChange();
+
+ AddReviewerInput input = new AddReviewerInput();
+ input.reviewer = toRfcAddressString(acc);
+ input.state = state;
+ gApi.changes().id(r.getChangeId()).addReviewer(input);
+
+ RestResponse restResponse =
+ adminRestSession.get("/changes/" + r.getChangeId() + "/reviewers/");
+ restResponse.assertOK();
+ Type type = new TypeToken<List<ReviewerInfo>>() {}.getType();
+ List<ReviewerInfo> reviewers = newGson().fromJson(restResponse.getReader(), type);
+ restResponse.consume();
+
+ assertThat(reviewers).hasSize(1);
+ ReviewerInfo reviewerInfo = Iterables.getOnlyElement(reviewers);
+ assertThat(reviewerInfo._accountId).isNull();
+ assertThat(reviewerInfo.name).isEqualTo(acc.name);
+ assertThat(reviewerInfo.email).isEqualTo(acc.email);
+ }
+ }
+
+ @Test
public void removeByEmail() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
