Move PostReviewers result classes into extensions

Details about the results returned by the server when adding
reviewers were previously unknown to users of the extensions
API package. This change moves these details into new classes
(AddReviewerResult and ReviewerInfo) so we can more thoroughly
test this API.

Change-Id: I3bb0d6b5becc487cc66c3e5660f9fe3c9c916432

Documentation/rest-api-changes.txt

@@ -2296,6 +2296,7 @@ returned that describes the newly added reviewers.
   {
     "reviewers": [
       {
+        "input": "john.doe@example.com",
         "approvals": {
           "Verified": " 0",
           "Code-Review": " 0"
@@ -2333,6 +2334,7 @@ required.
 
   )]}'
   {
+    "input": "MyProjectVerifiers",
     "error": "The group My Group has 15 members. Do you want to add them all as reviewers?",
     "confirm": true
   }
@@ -2347,7 +2349,7 @@ To confirm the addition of the reviewers, resend the request with the
   Content-Type: application/json; charset=UTF-8
 
   {
-    "reviewer": "MyProjectVerifiers",
+    "input": "MyProjectVerifiers",
     "confirmed": true
   }
 ----
@@ -4058,6 +4060,9 @@ reviewer to a change.
 [options="header",cols="1,^1,5"]
 |===========================
 |Field Name    ||Description
+|`input`    ||
+Value of the `reviewer` field from link:#reviewer-input[ReviewerInput]
+set while adding the reviewer.
 |`reviewers`   |optional|
 The newly added reviewers as a list of link:#reviewer-info[
 ReviewerInfo] entities.

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java

@@ -0,0 +1,110 @@
+// Copyright (C) 2016 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.acceptance.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerResult;
+import com.google.gerrit.server.change.PostReviewers;
+import com.google.gson.stream.JsonReader;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class ChangeReviewersIT extends AbstractDaemonTest {
+  @Test
+  public void addGroupAsReviewer() throws Exception {
+    // Set up two groups, one that is too large too add as reviewer, and one
+    // that is too large to add without confirmation.
+    String largeGroup = createGroup("largeGroup");
+    String mediumGroup = createGroup("mediumGroup");
+
+    int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
+    int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
+    List<TestAccount> users = new ArrayList<>(largeGroupSize);
+    List<String> largeGroupUsernames = new ArrayList<>(mediumGroupSize);
+    List<String> mediumGroupUsernames = new ArrayList<>(mediumGroupSize);
+    for (int i = 0; i < largeGroupSize; i++) {
+      users.add(accounts.create(name("u" + i), name("u" + i + "@example.com"),
+          "Full Name " + i));
+      largeGroupUsernames.add(users.get(i).username);
+      if (i < mediumGroupSize) {
+        mediumGroupUsernames.add(users.get(i).username);
+      }
+    }
+    gApi.groups().id(largeGroup).addMembers(
+        largeGroupUsernames.toArray(new String[largeGroupSize]));
+    gApi.groups().id(mediumGroup).addMembers(
+        mediumGroupUsernames.toArray(new String[mediumGroupSize]));
+
+    // Attempt to add overly large group as reviewers.
+    PushOneCommit.Result r = createChange();
+    String changeId = r.getChangeId();
+    AddReviewerResult result = addReviewer(changeId, largeGroup);
+    assertThat(result.input).isEqualTo(largeGroup);
+    assertThat(result.confirm).isNull();
+    assertThat(result.error)
+        .contains("has too many members to add them all as reviewers");
+    assertThat(result.reviewers).isNull();
+
+    // Attempt to add medium group without confirmation.
+    result = addReviewer(changeId, mediumGroup);
+    assertThat(result.input).isEqualTo(mediumGroup);
+    assertThat(result.confirm).isTrue();
+    assertThat(result.error)
+        .contains("has " + mediumGroupSize + " members. Do you want to add them"
+            + " all as reviewers?");
+    assertThat(result.reviewers).isNull();
+
+    // Add medium group with confirmation.
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = mediumGroup;
+    in.confirmed = true;
+    result = addReviewer(changeId, in);
+    assertThat(result.input).isEqualTo(mediumGroup);
+    assertThat(result.confirm).isNull();
+    assertThat(result.error).isNull();
+    assertThat(result.reviewers).hasSize(mediumGroupSize);
+  }
+
+  private AddReviewerResult addReviewer(String changeId, String reviewer)
+      throws Exception {
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = reviewer;
+    return addReviewer(changeId, in);
+  }
+
+  private AddReviewerResult addReviewer(String changeId, AddReviewerInput in)
+      throws Exception {
+    RestResponse resp =
+        adminRestSession.post("/changes/" + changeId + "/reviewers", in);
+    return readContentFromJson(resp, AddReviewerResult.class);
+  }
+
+  private static <T> T readContentFromJson(RestResponse r, Class<T> clazz)
+      throws Exception {
+    r.assertOK();
+    JsonReader jsonReader = new JsonReader(r.getReader());
+    jsonReader.setLenient(true);
+    return newGson().fromJson(jsonReader, clazz);
+  }
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerResult.java

@@ -0,0 +1,80 @@
+// Copyright (C) 2016 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.extensions.api.changes;
+
+import com.google.gerrit.common.Nullable;
+
+import java.util.List;
+
+/**
+ * Result object representing the outcome of a request to add a reviewer.
+ */
+public class AddReviewerResult {
+  /**
+   * The identifier of an account or group that was to be added as a reviewer.
+   */
+  public String input;
+
+  /**
+   * If non-null, a string describing why the reviewer could not be added.
+   */
+  @Nullable
+  public String error;
+
+  /**
+   * Non-null and true if the reviewer cannot be added without explicit
+   * confirmation. This may be the case for groups of a certain size.
+   */
+  @Nullable
+  public Boolean confirm;
+
+  /**
+   * @{List} of individual reviewers added to the change. The size of this list
+   * may be greater than one (e.g. when a group is added). Null if no reviewers
+   * were added.
+   */
+  @Nullable
+  public List<ReviewerInfo> reviewers;
+
+  /**
+   * Constructs a partially initialized result for the given reviewer.
+   *
+   * @param input String identifier of an account or group, from user request
+   */
+  public AddReviewerResult(String input) {
+    this.input = input;
+  }
+
+  /**
+   * Constructs an error result for the given account.
+   *
+   * @param reviewer String identifier of an account or group
+   * @param error Error message
+   */
+  public AddReviewerResult(String reviewer, String error) {
+    this(reviewer);
+    this.error = error;
+  }
+
+  /**
+   * Constructs a needs-confirmation result for the given account.
+   *
+   * @param confirm Whether confirmation is needed.
+   */
+  public AddReviewerResult(String reviewer, boolean confirm) {
+    this(reviewer);
+    this.confirm = confirm;
+  }
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerInfo.java

@@ -0,0 +1,41 @@
+// Copyright (C) 2016 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.extensions.api.changes;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.common.AccountInfo;
+
+import java.util.Map;
+
+/**
+ * Account and approval details for an added reviewer.
+ */
+public class ReviewerInfo extends AccountInfo {
+  /**
+   * {@link Map} of label name to initial value for each approval the reviewer
+   * is responsible for.
+   */
+  @Nullable
+  public Map<String, String> approvals;
+
+  public ReviewerInfo(Integer id) {
+    super(id);
+  }
+
+  @Override
+  public String toString() {
+    return username;
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java

@@ -14,8 +14,8 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;

gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java

@@ -14,11 +14,11 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
-import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java

@@ -21,6 +21,8 @@ import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerResult;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -38,8 +40,6 @@ import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountLoader;
 import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.change.ReviewerJson.PostResult;
-import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.extensions.events.ReviewerAdded;
 import com.google.gerrit.server.git.BatchUpdate;
@@ -128,7 +128,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
   }
 
   @Override
-  public PostResult apply(ChangeResource rsrc, AddReviewerInput input)
+  public AddReviewerResult apply(ChangeResource rsrc, AddReviewerInput input)
       throws UpdateException, OrmException, RestApiException, IOException {
     if (input.reviewer == null) {
       throw new BadRequestException("missing reviewer field");
@@ -136,7 +136,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
 
     try {
       Account.Id accountId = accounts.parse(input.reviewer).getAccountId();
-      return putAccount(reviewerFactory.create(rsrc, accountId));
+      return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId));
     } catch (UnprocessableEntityException e) {
       try {
         return putGroup(rsrc, input);
@@ -148,11 +148,11 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     }
   }
 
-  private PostResult putAccount(ReviewerResource rsrc)
+  private AddReviewerResult putAccount(String reviewer, ReviewerResource rsrc)
       throws OrmException, UpdateException, RestApiException {
     Account member = rsrc.getReviewerUser().getAccount();
     ChangeControl control = rsrc.getReviewerControl();
-    PostResult result = new PostResult();
+    AddReviewerResult result = new AddReviewerResult(reviewer);
     if (isValidReviewer(member, control)) {
       addReviewers(rsrc.getChangeResource(), result,
           ImmutableMap.of(member.getId(), control));
@@ -160,10 +160,10 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     return result;
   }
 
-  private PostResult putGroup(ChangeResource rsrc, AddReviewerInput input)
+  private AddReviewerResult putGroup(ChangeResource rsrc, AddReviewerInput input)
       throws UpdateException, RestApiException, OrmException, IOException {
     GroupDescription.Basic group = groupsCollection.parseInternal(input.reviewer);
-    PostResult result = new PostResult();
+    AddReviewerResult result = new AddReviewerResult(input.reviewer);
     if (!isLegalReviewerGroup(group.getGroupUUID())) {
       result.error = MessageFormat.format(
           ChangeMessages.get().groupIsNotAllowed, group.getName());
@@ -227,7 +227,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
 
 
   private void addReviewers(
-      ChangeResource rsrc, PostResult result, Map<Account.Id, ChangeControl> reviewers)
+      ChangeResource rsrc, AddReviewerResult result, Map<Account.Id, ChangeControl> reviewers)
       throws OrmException, RestApiException, UpdateException {
     try (BatchUpdate bu = batchUpdateFactory.create(
             dbProvider.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
@@ -240,8 +240,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       for (PatchSetApproval psa : op.added) {
         // New reviewers have value 0, don't bother normalizing.
         result.reviewers.add(
-          json.format(new ReviewerInfo(
-              psa.getAccountId()), reviewers.get(psa.getAccountId()),
+          json.format(new ReviewerInfo(psa.getAccountId().get()),
+              reviewers.get(psa.getAccountId()),
               ImmutableList.of(psa)));
       }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java

@@ -23,7 +23,7 @@ import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -40,7 +40,6 @@ import com.google.inject.Singleton;
 
 import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 import java.util.TreeMap;
 
 @Singleton
@@ -67,7 +66,7 @@ public class ReviewerJson {
     AccountLoader loader = accountLoaderFactory.create(true);
     for (ReviewerResource rsrc : rsrcs) {
       ReviewerInfo info = format(new ReviewerInfo(
-          rsrc.getReviewerUser().getAccountId()),
+          rsrc.getReviewerUser().getAccountId().get()),
           rsrc.getReviewerControl());
       loader.put(info);
       infos.add(info);
@@ -132,18 +131,4 @@ public class ReviewerJson {
 
     return out;
   }
-
-  public static class ReviewerInfo extends AccountInfo {
-    Map<String, String> approvals;
-
-    protected ReviewerInfo(Account.Id id) {
-      super(id.get());
-    }
-  }
-
-  public static class PostResult {
-    public List<ReviewerInfo> reviewers;
-    public String error;
-    Boolean confirm;
-  }
 }