Move common reviewer addition code out of PostReviewers
Avoids dependencies from one REST API handler on another REST API
handler, and will eventually make it possible to move ReviewerAdder out
of the restapi package entirely.
Keep the structure otherwise the same, with a non-static inner class
managing a single addition. Rename this class from Addition to
ReviewerAddition, which can be imported directly, as opposed to calling
ReviewerAdder.Addition which is a stuttery mouthful.
PostReviewers is now a thin wrapper around ReviewerAdder, containing
only the code necessary to execute the BatchUpdate and return the
result.
Change-Id: Iff578c9e7b56e2bbba1e193524d7cceaa0fcf5fb
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index c7028ab..2bf48ca 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -89,7 +89,6 @@
import com.google.gerrit.server.PublishCommentUtil;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.ChangeResource.Factory;
import com.google.gerrit.server.change.EmailReviewComments;
import com.google.gerrit.server.change.NotifyUtil;
import com.google.gerrit.server.change.RevisionResource;
@@ -112,6 +111,7 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.account.AccountsCollection;
+import com.google.gerrit.server.restapi.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
@@ -170,7 +170,7 @@
private final AccountsCollection accounts;
private final EmailReviewComments.Factory email;
private final CommentAdded commentAdded;
- private final PostReviewers postReviewers;
+ private final ReviewerAdder reviewerAdder;
private final PostReviewersEmail postReviewersEmail;
private final NotesMigration migration;
private final NotifyUtil notifyUtil;
@@ -184,7 +184,7 @@
PostReview(
Provider<ReviewDb> db,
RetryHelper retryHelper,
- Factory changeResourceFactory,
+ ChangeResource.Factory changeResourceFactory,
ChangeData.Factory changeDataFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
@@ -195,7 +195,7 @@
AccountsCollection accounts,
EmailReviewComments.Factory email,
CommentAdded commentAdded,
- PostReviewers postReviewers,
+ ReviewerAdder reviewerAdder,
PostReviewersEmail postReviewersEmail,
NotesMigration migration,
NotifyUtil notifyUtil,
@@ -216,7 +216,7 @@
this.accounts = accounts;
this.email = email;
this.commentAdded = commentAdded;
- this.postReviewers = postReviewers;
+ this.reviewerAdder = reviewerAdder;
this.postReviewersEmail = postReviewersEmail;
this.migration = migration;
this.notifyUtil = notifyUtil;
@@ -273,7 +273,7 @@
notifyUtil.resolveAccounts(input.notifyDetails);
Map<String, AddReviewerResult> reviewerJsonResults = null;
- List<PostReviewers.Addition> reviewerResults = Lists.newArrayList();
+ List<ReviewerAddition> reviewerResults = Lists.newArrayList();
boolean hasError = false;
boolean confirm = false;
if (input.reviewers != null) {
@@ -285,9 +285,8 @@
// specifies explicit accountsToNotify. Unclear whether that's a good thing.
reviewerInput.notify = NotifyHandling.NONE;
- PostReviewers.Addition result =
- postReviewers.prepareApplication(
- revision.getNotes(), revision.getUser(), reviewerInput, true);
+ ReviewerAddition result =
+ reviewerAdder.prepare(revision.getNotes(), revision.getUser(), reviewerInput, true);
reviewerJsonResults.put(reviewerInput.reviewer, result.result);
if (result.result.error != null) {
hasError = true;
@@ -327,7 +326,7 @@
// Apply reviewer changes first. Revision emails should be sent to the
// updated set of reviewers. Also keep track of whether the user added
// themselves as a reviewer or to the CC list.
- for (PostReviewers.Addition reviewerResult : reviewerResults) {
+ for (ReviewerAddition reviewerResult : reviewerResults) {
bu.addOp(revision.getChange().getId(), reviewerResult.op);
if (!ccOrReviewer && reviewerResult.result.reviewers != null) {
for (ReviewerInfo reviewerInfo : reviewerResult.result.reviewers) {
@@ -351,8 +350,7 @@
// User posting this review isn't currently in the reviewer or CC list,
// isn't being explicitly added, and isn't voting on any label.
// Automatically CC them on this change so they receive replies.
- PostReviewers.Addition selfAddition =
- postReviewers.ccCurrentUser(revision.getUser(), revision);
+ ReviewerAddition selfAddition = reviewerAdder.ccCurrentUser(revision.getUser(), revision);
bu.addOp(revision.getChange().getId(), selfAddition.op);
}
@@ -389,7 +387,7 @@
// Re-read change to take into account results of the update.
ChangeData cd =
changeDataFactory.create(db.get(), revision.getProject(), revision.getChange().getId());
- for (PostReviewers.Addition reviewerResult : reviewerResults) {
+ for (ReviewerAddition reviewerResult : reviewerResults) {
reviewerResult.gatherResults(cd);
}
@@ -434,7 +432,7 @@
private void batchEmailReviewers(
CurrentUser user,
Change change,
- List<PostReviewers.Addition> reviewerAdditions,
+ List<ReviewerAddition> reviewerAdditions,
@Nullable NotifyHandling notify,
ListMultimap<RecipientType, Account.Id> accountsToNotify,
boolean readyForReview) {
@@ -442,7 +440,7 @@
List<Account.Id> cc = new ArrayList<>();
List<Address> toByEmail = new ArrayList<>();
List<Address> ccByEmail = new ArrayList<>();
- for (PostReviewers.Addition addition : reviewerAdditions) {
+ for (ReviewerAddition addition : reviewerAdditions) {
if (addition.state == ReviewerState.REVIEWER) {
to.addAll(addition.reviewers);
toByEmail.addAll(addition.reviewersByEmail);
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index 9577fb3..755cb0b 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -14,61 +14,17 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.extensions.client.ReviewerState.CC;
-import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ListMultimap;
-import com.google.common.collect.Lists;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
-import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.api.changes.RecipientType;
-import com.google.gerrit.extensions.api.changes.ReviewerInfo;
-import com.google.gerrit.extensions.client.ReviewerState;
-import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.mail.Address;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
-import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.AnonymousUser;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountLoader;
-import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.NotifyUtil;
import com.google.gerrit.server.change.ReviewerResource;
-import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.group.SystemGroupBackend;
-import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.notedb.NotesMigration;
-import com.google.gerrit.server.permissions.ChangePermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.restapi.account.AccountsCollection;
-import com.google.gerrit.server.restapi.group.GroupsCollection;
+import com.google.gerrit.server.restapi.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestCollectionModifyView;
@@ -79,72 +35,27 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.text.MessageFormat;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
@Singleton
public class PostReviewers
extends RetryingRestCollectionModifyView<
ChangeResource, ReviewerResource, AddReviewerInput, AddReviewerResult> {
- public static final int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
- public static final int DEFAULT_MAX_REVIEWERS = 20;
-
- private final AccountsCollection accounts;
- private final PermissionBackend permissionBackend;
-
- private final GroupsCollection groupsCollection;
- private final GroupMembers groupMembers;
- private final AccountLoader.Factory accountLoaderFactory;
private final Provider<ReviewDb> dbProvider;
private final ChangeData.Factory changeDataFactory;
- private final Config cfg;
- private final ReviewerJson json;
- private final NotesMigration migration;
- private final NotifyUtil notifyUtil;
- private final ProjectCache projectCache;
- private final Provider<AnonymousUser> anonymousProvider;
- private final PostReviewersOp.Factory postReviewersOpFactory;
- private final OutgoingEmailValidator validator;
+ private final ReviewerAdder reviewerAdder;
@Inject
PostReviewers(
- AccountsCollection accounts,
- PermissionBackend permissionBackend,
- GroupsCollection groupsCollection,
- GroupMembers groupMembers,
- AccountLoader.Factory accountLoaderFactory,
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
RetryHelper retryHelper,
- @GerritServerConfig Config cfg,
- ReviewerJson json,
- NotesMigration migration,
- NotifyUtil notifyUtil,
- ProjectCache projectCache,
- Provider<AnonymousUser> anonymousProvider,
- PostReviewersOp.Factory postReviewersOpFactory,
- OutgoingEmailValidator validator) {
+ ReviewerAdder reviewerAdder) {
super(retryHelper);
- this.accounts = accounts;
- this.permissionBackend = permissionBackend;
- this.groupsCollection = groupsCollection;
- this.groupMembers = groupMembers;
- this.accountLoaderFactory = accountLoaderFactory;
this.dbProvider = db;
this.changeDataFactory = changeDataFactory;
- this.cfg = cfg;
- this.json = json;
- this.migration = migration;
- this.notifyUtil = notifyUtil;
- this.projectCache = projectCache;
- this.anonymousProvider = anonymousProvider;
- this.postReviewersOpFactory = postReviewersOpFactory;
- this.validator = validator;
+ this.reviewerAdder = reviewerAdder;
}
@Override
@@ -156,7 +67,7 @@
throw new BadRequestException("missing reviewer field");
}
- Addition addition = prepareApplication(rsrc.getNotes(), rsrc.getUser(), input, true);
+ ReviewerAddition addition = reviewerAdder.prepare(rsrc.getNotes(), rsrc.getUser(), input, true);
if (addition.op == null) {
return addition.result;
}
@@ -173,349 +84,4 @@
changeDataFactory.create(dbProvider.get(), rsrc.getProject(), rsrc.getId()));
return addition.result;
}
-
- /**
- * Prepare application of a single {@link AddReviewerInput}.
- *
- * @param notes change notes.
- * @param user user performing the reviewer addition.
- * @param input input describing user or group to add as a reviewer.
- * @param allowGroup whether to allow
- * @return handle describing the addition operation. If the {@code op} field is present, this
- * operation may be added to a {@code BatchUpdate}. Otherwise, the {@code error} field
- * contains information about an error that occurred
- * @throws OrmException
- * @throws IOException
- * @throws PermissionBackendException
- * @throws ConfigInvalidException
- */
- public Addition prepareApplication(
- ChangeNotes notes, CurrentUser user, AddReviewerInput input, boolean allowGroup)
- throws OrmException, IOException, PermissionBackendException, ConfigInvalidException {
- Branch.NameKey dest = notes.getChange().getDest();
- String reviewer = input.reviewer;
- ReviewerState state = input.state();
- NotifyHandling notify = input.notify;
- ListMultimap<RecipientType, Account.Id> accountsToNotify;
- try {
- accountsToNotify = notifyUtil.resolveAccounts(input.notifyDetails);
- } catch (BadRequestException e) {
- return fail(reviewer, e.getMessage());
- }
- boolean confirmed = input.confirmed();
- boolean allowByEmail =
- projectCache
- .checkedGet(dest.getParentKey())
- .is(BooleanProjectConfig.ENABLE_REVIEWER_BY_EMAIL);
-
- Addition byAccountId =
- addByAccountId(
- reviewer, dest, user, state, notify, accountsToNotify, allowGroup, allowByEmail);
-
- Addition wholeGroup = null;
- if (byAccountId == null || !byAccountId.exactMatchFound) {
- wholeGroup =
- addWholeGroup(
- reviewer,
- dest,
- user,
- state,
- notify,
- accountsToNotify,
- confirmed,
- allowGroup,
- allowByEmail);
- if (wholeGroup != null && wholeGroup.exactMatchFound) {
- return wholeGroup;
- }
- }
-
- if (byAccountId != null) {
- return byAccountId;
- }
- if (wholeGroup != null) {
- return wholeGroup;
- }
-
- return addByEmail(reviewer, notes, user, state, notify, accountsToNotify);
- }
-
- Addition ccCurrentUser(CurrentUser user, RevisionResource revision) {
- return new Addition(
- user.getUserName().orElse(null),
- revision.getUser(),
- ImmutableSet.of(user.getAccountId()),
- null,
- CC,
- NotifyHandling.NONE,
- ImmutableListMultimap.of(),
- true);
- }
-
- @Nullable
- private Addition addByAccountId(
- String reviewer,
- Branch.NameKey dest,
- CurrentUser user,
- ReviewerState state,
- NotifyHandling notify,
- ListMultimap<RecipientType, Account.Id> accountsToNotify,
- boolean allowGroup,
- boolean allowByEmail)
- throws OrmException, PermissionBackendException, IOException, ConfigInvalidException {
- IdentifiedUser reviewerUser;
- boolean exactMatchFound = false;
- try {
- reviewerUser = accounts.parse(reviewer);
- if (reviewer.equalsIgnoreCase(reviewerUser.getName())
- || reviewer.equals(String.valueOf(reviewerUser.getAccountId()))) {
- exactMatchFound = true;
- }
- } catch (UnprocessableEntityException | AuthException e) {
- // AuthException won't occur since the user is authenticated at this point.
- if (!allowGroup && !allowByEmail) {
- // Only return failure if we aren't going to try other interpretations.
- return fail(
- reviewer, MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, reviewer));
- }
- return null;
- }
-
- if (isValidReviewer(dest, reviewerUser.getAccount())) {
- return new Addition(
- reviewer,
- user,
- ImmutableSet.of(reviewerUser.getAccountId()),
- null,
- state,
- notify,
- accountsToNotify,
- exactMatchFound);
- }
- if (!reviewerUser.getAccount().isActive()) {
- if (allowByEmail && state == CC) {
- return null;
- }
- return fail(reviewer, MessageFormat.format(ChangeMessages.get().reviewerInactive, reviewer));
- }
- return fail(
- reviewer, MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, reviewer));
- }
-
- @Nullable
- private Addition addWholeGroup(
- String reviewer,
- Branch.NameKey dest,
- CurrentUser user,
- ReviewerState state,
- NotifyHandling notify,
- ListMultimap<RecipientType, Account.Id> accountsToNotify,
- boolean confirmed,
- boolean allowGroup,
- boolean allowByEmail)
- throws IOException, PermissionBackendException {
- if (!allowGroup) {
- return null;
- }
-
- GroupDescription.Basic group;
- try {
- group = groupsCollection.parseInternal(reviewer);
- } catch (UnprocessableEntityException e) {
- if (!allowByEmail) {
- return fail(
- reviewer,
- MessageFormat.format(ChangeMessages.get().reviewerNotFoundUserOrGroup, reviewer));
- }
- return null;
- }
-
- if (!isLegalReviewerGroup(group.getGroupUUID())) {
- return fail(
- reviewer, MessageFormat.format(ChangeMessages.get().groupIsNotAllowed, group.getName()));
- }
-
- Set<Account.Id> reviewers = new HashSet<>();
- Set<Account> members;
- try {
- members = groupMembers.listAccounts(group.getGroupUUID(), dest.getParentKey());
- } catch (NoSuchProjectException e) {
- return fail(reviewer, e.getMessage());
- }
-
- // if maxAllowed is set to 0, it is allowed to add any number of
- // reviewers
- int maxAllowed = cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
- if (maxAllowed > 0 && members.size() > maxAllowed) {
- return fail(
- reviewer,
- MessageFormat.format(ChangeMessages.get().groupHasTooManyMembers, group.getName()));
- }
-
- // if maxWithoutCheck is set to 0, we never ask for confirmation
- int maxWithoutConfirmation =
- cfg.getInt("addreviewer", "maxWithoutConfirmation", DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
- if (!confirmed && maxWithoutConfirmation > 0 && members.size() > maxWithoutConfirmation) {
- return fail(
- reviewer,
- true,
- MessageFormat.format(
- ChangeMessages.get().groupManyMembersConfirmation, group.getName(), members.size()));
- }
-
- for (Account member : members) {
- if (isValidReviewer(dest, member)) {
- reviewers.add(member.getId());
- }
- }
-
- return new Addition(reviewer, user, reviewers, null, state, notify, accountsToNotify, true);
- }
-
- @Nullable
- private Addition addByEmail(
- String reviewer,
- ChangeNotes notes,
- CurrentUser user,
- ReviewerState state,
- NotifyHandling notify,
- ListMultimap<RecipientType, Account.Id> accountsToNotify)
- throws PermissionBackendException {
- try {
- permissionBackend
- .user(anonymousProvider.get())
- .database(dbProvider)
- .change(notes)
- .check(ChangePermission.READ);
- } catch (AuthException e) {
- return fail(
- reviewer, MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, reviewer));
- }
-
- if (!migration.readChanges()) {
- // addByEmail depends on NoteDb.
- return fail(
- reviewer, MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, reviewer));
- }
- Address adr = Address.tryParse(reviewer);
- if (adr == null || !validator.isValid(adr.getEmail())) {
- return fail(reviewer, MessageFormat.format(ChangeMessages.get().reviewerInvalid, reviewer));
- }
- return new Addition(
- reviewer, user, null, ImmutableList.of(adr), state, notify, accountsToNotify, true);
- }
-
- private boolean isValidReviewer(Branch.NameKey branch, Account member)
- throws PermissionBackendException {
- if (!member.isActive()) {
- return false;
- }
-
- try {
- // Check ref permission instead of change permission, since change permissions take into
- // account the private bit, whereas adding a user as a reviewer is explicitly allowing them to
- // see private changes.
- permissionBackend
- .absentUser(member.getId())
- .database(dbProvider)
- .ref(branch)
- .check(RefPermission.READ);
- return true;
- } catch (AuthException e) {
- return false;
- }
- }
-
- private Addition fail(String reviewer, String error) {
- return fail(reviewer, false, error);
- }
-
- private Addition fail(String reviewer, boolean confirm, String error) {
- Addition addition = new Addition(reviewer);
- addition.result.confirm = confirm ? true : null;
- addition.result.error = error;
- return addition;
- }
-
- public class Addition {
- public final AddReviewerResult result;
- @Nullable public final PostReviewersOp op;
- final Set<Account.Id> reviewers;
- final Collection<Address> reviewersByEmail;
- final ReviewerState state;
- @Nullable final IdentifiedUser caller;
- final boolean exactMatchFound;
-
- Addition(String reviewer) {
- result = new AddReviewerResult(reviewer);
- op = null;
- reviewers = ImmutableSet.of();
- reviewersByEmail = ImmutableSet.of();
- state = REVIEWER;
- caller = null;
- exactMatchFound = false;
- }
-
- Addition(
- String reviewer,
- CurrentUser caller,
- @Nullable Set<Account.Id> reviewers,
- @Nullable Collection<Address> reviewersByEmail,
- ReviewerState state,
- @Nullable NotifyHandling notify,
- ListMultimap<RecipientType, Account.Id> accountsToNotify,
- boolean exactMatchFound) {
- checkArgument(
- reviewers != null || reviewersByEmail != null,
- "must have either reviewers or reviewersByEmail");
-
- result = new AddReviewerResult(reviewer);
- this.reviewers = reviewers == null ? ImmutableSet.of() : reviewers;
- this.reviewersByEmail = reviewersByEmail == null ? ImmutableList.of() : reviewersByEmail;
- this.state = state;
- this.caller = caller.asIdentifiedUser();
- op =
- postReviewersOpFactory.create(
- this.reviewers, this.reviewersByEmail, state, notify, accountsToNotify);
- this.exactMatchFound = exactMatchFound;
- }
-
- void gatherResults(ChangeData cd) throws OrmException, PermissionBackendException {
- checkState(op != null, "addition did not result in an update op");
- checkState(op.getResult() != null, "op did not return a result");
-
- // Generate result details and fill AccountLoader. This occurs outside
- // the Op because the accounts are in a different table.
- PostReviewersOp.Result opResult = op.getResult();
- if (migration.readChanges() && state == CC) {
- result.ccs = Lists.newArrayListWithCapacity(opResult.addedCCs().size());
- for (Account.Id accountId : opResult.addedCCs()) {
- result.ccs.add(json.format(new ReviewerInfo(accountId.get()), accountId, cd));
- }
- accountLoaderFactory.create(true).fill(result.ccs);
- for (Address a : opResult.addedCCsByEmail()) {
- result.ccs.add(new AccountInfo(a.getName(), a.getEmail()));
- }
- } else {
- result.reviewers = Lists.newArrayListWithCapacity(opResult.addedReviewers().size());
- for (PatchSetApproval psa : opResult.addedReviewers()) {
- // New reviewers have value 0, don't bother normalizing.
- result.reviewers.add(
- json.format(
- new ReviewerInfo(psa.getAccountId().get()),
- psa.getAccountId(),
- cd,
- ImmutableList.of(psa)));
- }
- accountLoaderFactory.create(true).fill(result.reviewers);
- for (Address a : opResult.addedReviewersByEmail()) {
- result.reviewers.add(ReviewerInfo.byEmail(a.getName(), a.getEmail()));
- }
- }
- }
- }
-
- public static boolean isLegalReviewerGroup(AccountGroup.UUID groupUUID) {
- return !SystemGroupBackend.isSystemGroup(groupUUID);
- }
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutAssignee.java b/java/com/google/gerrit/server/restapi/change/PutAssignee.java
index 9d626fc..284c977 100644
--- a/java/com/google/gerrit/server/restapi/change/PutAssignee.java
+++ b/java/com/google/gerrit/server/restapi/change/PutAssignee.java
@@ -34,7 +34,7 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.restapi.account.AccountsCollection;
-import com.google.gerrit.server.restapi.change.PostReviewers.Addition;
+import com.google.gerrit.server.restapi.change.ReviewerAdder.ReviewerAddition;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
@@ -54,7 +54,7 @@
private final AccountsCollection accounts;
private final SetAssigneeOp.Factory assigneeFactory;
private final Provider<ReviewDb> db;
- private final PostReviewers postReviewers;
+ private final ReviewerAdder reviewerAdder;
private final AccountLoader.Factory accountLoaderFactory;
private final PermissionBackend permissionBackend;
@@ -64,14 +64,14 @@
SetAssigneeOp.Factory assigneeFactory,
RetryHelper retryHelper,
Provider<ReviewDb> db,
- PostReviewers postReviewers,
+ ReviewerAdder reviewerAdder,
AccountLoader.Factory accountLoaderFactory,
PermissionBackend permissionBackend) {
super(retryHelper);
this.accounts = accounts;
this.assigneeFactory = assigneeFactory;
this.db = db;
- this.postReviewers = postReviewers;
+ this.reviewerAdder = reviewerAdder;
this.accountLoaderFactory = accountLoaderFactory;
this.permissionBackend = permissionBackend;
}
@@ -108,7 +108,7 @@
SetAssigneeOp op = assigneeFactory.create(assignee);
bu.addOp(rsrc.getId(), op);
- PostReviewers.Addition reviewersAddition = addAssigneeAsCC(rsrc, input.assignee);
+ ReviewerAddition reviewersAddition = addAssigneeAsCC(rsrc, input.assignee);
bu.addOp(rsrc.getId(), reviewersAddition.op);
bu.execute();
@@ -116,14 +116,14 @@
}
}
- private Addition addAssigneeAsCC(ChangeResource rsrc, String assignee)
+ private ReviewerAddition addAssigneeAsCC(ChangeResource rsrc, String assignee)
throws OrmException, IOException, PermissionBackendException, ConfigInvalidException {
AddReviewerInput reviewerInput = new AddReviewerInput();
reviewerInput.reviewer = assignee;
reviewerInput.state = ReviewerState.CC;
reviewerInput.confirmed = true;
reviewerInput.notify = NotifyHandling.NONE;
- return postReviewers.prepareApplication(rsrc.getNotes(), rsrc.getUser(), reviewerInput, false);
+ return reviewerAdder.prepare(rsrc.getNotes(), rsrc.getUser(), reviewerInput, false);
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerAdder.java b/java/com/google/gerrit/server/restapi/change/ReviewerAdder.java
new file mode 100644
index 0000000..5c993d1
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerAdder.java
@@ -0,0 +1,476 @@
+// Copyright (C) 2018 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.restapi.change;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.extensions.client.ReviewerState.CC;
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.GroupDescription;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerResult;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.RecipientType;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
+import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.mail.Address;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.account.GroupMembers;
+import com.google.gerrit.server.change.ChangeMessages;
+import com.google.gerrit.server.change.NotifyUtil;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.RefPermission;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.restapi.account.AccountsCollection;
+import com.google.gerrit.server.restapi.group.GroupsCollection;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.text.MessageFormat;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+
+public class ReviewerAdder {
+ public static final int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
+ public static final int DEFAULT_MAX_REVIEWERS = 20;
+
+ private final AccountsCollection accounts;
+ private final PermissionBackend permissionBackend;
+ private final GroupsCollection groupsCollection;
+ private final GroupMembers groupMembers;
+ private final AccountLoader.Factory accountLoaderFactory;
+ private final Provider<ReviewDb> dbProvider;
+ private final Config cfg;
+ private final ReviewerJson json;
+ private final NotesMigration migration;
+ private final NotifyUtil notifyUtil;
+ private final ProjectCache projectCache;
+ private final Provider<AnonymousUser> anonymousProvider;
+ private final PostReviewersOp.Factory postReviewersOpFactory;
+ private final OutgoingEmailValidator validator;
+
+ @Inject
+ ReviewerAdder(
+ AccountsCollection accounts,
+ PermissionBackend permissionBackend,
+ GroupsCollection groupsCollection,
+ GroupMembers groupMembers,
+ AccountLoader.Factory accountLoaderFactory,
+ Provider<ReviewDb> db,
+ @GerritServerConfig Config cfg,
+ ReviewerJson json,
+ NotesMigration migration,
+ NotifyUtil notifyUtil,
+ ProjectCache projectCache,
+ Provider<AnonymousUser> anonymousProvider,
+ PostReviewersOp.Factory postReviewersOpFactory,
+ OutgoingEmailValidator validator) {
+ this.accounts = accounts;
+ this.permissionBackend = permissionBackend;
+ this.groupsCollection = groupsCollection;
+ this.groupMembers = groupMembers;
+ this.accountLoaderFactory = accountLoaderFactory;
+ this.dbProvider = db;
+ this.cfg = cfg;
+ this.json = json;
+ this.migration = migration;
+ this.notifyUtil = notifyUtil;
+ this.projectCache = projectCache;
+ this.anonymousProvider = anonymousProvider;
+ this.postReviewersOpFactory = postReviewersOpFactory;
+ this.validator = validator;
+ }
+
+ /**
+ * Prepare application of a single {@link AddReviewerInput}.
+ *
+ * @param notes change notes.
+ * @param user user performing the reviewer addition.
+ * @param input input describing user or group to add as a reviewer.
+ * @param allowGroup whether to allow
+ * @return handle describing the addition operation. If the {@code op} field is present, this
+ * operation may be added to a {@code BatchUpdate}. Otherwise, the {@code error} field
+ * contains information about an error that occurred
+ * @throws OrmException
+ * @throws IOException
+ * @throws PermissionBackendException
+ * @throws ConfigInvalidException
+ */
+ public ReviewerAddition prepare(
+ ChangeNotes notes, CurrentUser user, AddReviewerInput input, boolean allowGroup)
+ throws OrmException, IOException, PermissionBackendException, ConfigInvalidException {
+ Branch.NameKey dest = notes.getChange().getDest();
+ String reviewer = checkNotNull(input.reviewer);
+ ReviewerState state = input.state();
+ NotifyHandling notify = input.notify;
+ ListMultimap<RecipientType, Account.Id> accountsToNotify;
+ try {
+ accountsToNotify = notifyUtil.resolveAccounts(input.notifyDetails);
+ } catch (BadRequestException e) {
+ return fail(reviewer, e.getMessage());
+ }
+ boolean confirmed = input.confirmed();
+ boolean allowByEmail =
+ projectCache
+ .checkedGet(dest.getParentKey())
+ .is(BooleanProjectConfig.ENABLE_REVIEWER_BY_EMAIL);
+
+ ReviewerAddition byAccountId =
+ addByAccountId(
+ reviewer, dest, user, state, notify, accountsToNotify, allowGroup, allowByEmail);
+
+ ReviewerAddition wholeGroup = null;
+ if (byAccountId == null || !byAccountId.exactMatchFound) {
+ wholeGroup =
+ addWholeGroup(
+ reviewer,
+ dest,
+ user,
+ state,
+ notify,
+ accountsToNotify,
+ confirmed,
+ allowGroup,
+ allowByEmail);
+ if (wholeGroup != null && wholeGroup.exactMatchFound) {
+ return wholeGroup;
+ }
+ }
+
+ if (byAccountId != null) {
+ return byAccountId;
+ }
+ if (wholeGroup != null) {
+ return wholeGroup;
+ }
+
+ return addByEmail(reviewer, notes, user, state, notify, accountsToNotify);
+ }
+
+ ReviewerAddition ccCurrentUser(CurrentUser user, RevisionResource revision) {
+ return new ReviewerAddition(
+ user.getUserName().orElse(null),
+ revision.getUser(),
+ ImmutableSet.of(user.getAccountId()),
+ null,
+ CC,
+ NotifyHandling.NONE,
+ ImmutableListMultimap.of(),
+ true);
+ }
+
+ @Nullable
+ private ReviewerAddition addByAccountId(
+ String reviewer,
+ Branch.NameKey dest,
+ CurrentUser user,
+ ReviewerState state,
+ NotifyHandling notify,
+ ListMultimap<RecipientType, Account.Id> accountsToNotify,
+ boolean allowGroup,
+ boolean allowByEmail)
+ throws OrmException, PermissionBackendException, IOException, ConfigInvalidException {
+ IdentifiedUser reviewerUser;
+ boolean exactMatchFound = false;
+ try {
+ reviewerUser = accounts.parse(reviewer);
+ if (reviewer.equalsIgnoreCase(reviewerUser.getName())
+ || reviewer.equals(String.valueOf(reviewerUser.getAccountId()))) {
+ exactMatchFound = true;
+ }
+ } catch (UnprocessableEntityException | AuthException e) {
+ // AuthException won't occur since the user is authenticated at this point.
+ if (!allowGroup && !allowByEmail) {
+ // Only return failure if we aren't going to try other interpretations.
+ return fail(
+ reviewer, MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, reviewer));
+ }
+ return null;
+ }
+
+ if (isValidReviewer(dest, reviewerUser.getAccount())) {
+ return new ReviewerAddition(
+ reviewer,
+ user,
+ ImmutableSet.of(reviewerUser.getAccountId()),
+ null,
+ state,
+ notify,
+ accountsToNotify,
+ exactMatchFound);
+ }
+ if (!reviewerUser.getAccount().isActive()) {
+ if (allowByEmail && state == CC) {
+ return null;
+ }
+ return fail(reviewer, MessageFormat.format(ChangeMessages.get().reviewerInactive, reviewer));
+ }
+ return fail(
+ reviewer, MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, reviewer));
+ }
+
+ @Nullable
+ private ReviewerAddition addWholeGroup(
+ String reviewer,
+ Branch.NameKey dest,
+ CurrentUser user,
+ ReviewerState state,
+ NotifyHandling notify,
+ ListMultimap<RecipientType, Account.Id> accountsToNotify,
+ boolean confirmed,
+ boolean allowGroup,
+ boolean allowByEmail)
+ throws IOException, PermissionBackendException {
+ if (!allowGroup) {
+ return null;
+ }
+
+ GroupDescription.Basic group;
+ try {
+ group = groupsCollection.parseInternal(reviewer);
+ } catch (UnprocessableEntityException e) {
+ if (!allowByEmail) {
+ return fail(
+ reviewer,
+ MessageFormat.format(ChangeMessages.get().reviewerNotFoundUserOrGroup, reviewer));
+ }
+ return null;
+ }
+
+ if (!isLegalReviewerGroup(group.getGroupUUID())) {
+ return fail(
+ reviewer, MessageFormat.format(ChangeMessages.get().groupIsNotAllowed, group.getName()));
+ }
+
+ Set<Account.Id> reviewers = new HashSet<>();
+ Set<Account> members;
+ try {
+ members = groupMembers.listAccounts(group.getGroupUUID(), dest.getParentKey());
+ } catch (NoSuchProjectException e) {
+ return fail(reviewer, e.getMessage());
+ }
+
+ // if maxAllowed is set to 0, it is allowed to add any number of
+ // reviewers
+ int maxAllowed = cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
+ if (maxAllowed > 0 && members.size() > maxAllowed) {
+ return fail(
+ reviewer,
+ MessageFormat.format(ChangeMessages.get().groupHasTooManyMembers, group.getName()));
+ }
+
+ // if maxWithoutCheck is set to 0, we never ask for confirmation
+ int maxWithoutConfirmation =
+ cfg.getInt("addreviewer", "maxWithoutConfirmation", DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
+ if (!confirmed && maxWithoutConfirmation > 0 && members.size() > maxWithoutConfirmation) {
+ return fail(
+ reviewer,
+ true,
+ MessageFormat.format(
+ ChangeMessages.get().groupManyMembersConfirmation, group.getName(), members.size()));
+ }
+
+ for (Account member : members) {
+ if (isValidReviewer(dest, member)) {
+ reviewers.add(member.getId());
+ }
+ }
+
+ return new ReviewerAddition(
+ reviewer, user, reviewers, null, state, notify, accountsToNotify, true);
+ }
+
+ @Nullable
+ private ReviewerAddition addByEmail(
+ String reviewer,
+ ChangeNotes notes,
+ CurrentUser user,
+ ReviewerState state,
+ NotifyHandling notify,
+ ListMultimap<RecipientType, Account.Id> accountsToNotify)
+ throws PermissionBackendException {
+ try {
+ permissionBackend
+ .user(anonymousProvider.get())
+ .database(dbProvider)
+ .change(notes)
+ .check(ChangePermission.READ);
+ } catch (AuthException e) {
+ return fail(
+ reviewer, MessageFormat.format(ChangeMessages.get().reviewerCantSeeChange, reviewer));
+ }
+
+ if (!migration.readChanges()) {
+ // addByEmail depends on NoteDb.
+ return fail(
+ reviewer, MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, reviewer));
+ }
+ Address adr = Address.tryParse(reviewer);
+ if (adr == null || !validator.isValid(adr.getEmail())) {
+ return fail(reviewer, MessageFormat.format(ChangeMessages.get().reviewerInvalid, reviewer));
+ }
+ return new ReviewerAddition(
+ reviewer, user, null, ImmutableList.of(adr), state, notify, accountsToNotify, true);
+ }
+
+ private boolean isValidReviewer(Branch.NameKey branch, Account member)
+ throws PermissionBackendException {
+ if (!member.isActive()) {
+ return false;
+ }
+
+ try {
+ // Check ref permission instead of change permission, since change permissions take into
+ // account the private bit, whereas adding a user as a reviewer is explicitly allowing them to
+ // see private changes.
+ permissionBackend
+ .absentUser(member.getId())
+ .database(dbProvider)
+ .ref(branch)
+ .check(RefPermission.READ);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
+ }
+
+ private ReviewerAddition fail(String reviewer, String error) {
+ return fail(reviewer, false, error);
+ }
+
+ private ReviewerAddition fail(String reviewer, boolean confirm, String error) {
+ ReviewerAddition addition = new ReviewerAddition(reviewer);
+ addition.result.confirm = confirm ? true : null;
+ addition.result.error = error;
+ return addition;
+ }
+
+ public class ReviewerAddition {
+ public final AddReviewerResult result;
+ @Nullable public final PostReviewersOp op;
+ final Set<Account.Id> reviewers;
+ final Collection<Address> reviewersByEmail;
+ final ReviewerState state;
+ @Nullable final IdentifiedUser caller;
+ final boolean exactMatchFound;
+
+ ReviewerAddition(String reviewer) {
+ result = new AddReviewerResult(reviewer);
+ op = null;
+ reviewers = ImmutableSet.of();
+ reviewersByEmail = ImmutableSet.of();
+ state = REVIEWER;
+ caller = null;
+ exactMatchFound = false;
+ }
+
+ ReviewerAddition(
+ String reviewer,
+ CurrentUser caller,
+ @Nullable Set<Account.Id> reviewers,
+ @Nullable Collection<Address> reviewersByEmail,
+ ReviewerState state,
+ @Nullable NotifyHandling notify,
+ ListMultimap<RecipientType, Account.Id> accountsToNotify,
+ boolean exactMatchFound) {
+ checkArgument(
+ reviewers != null || reviewersByEmail != null,
+ "must have either reviewers or reviewersByEmail");
+
+ result = new AddReviewerResult(reviewer);
+ this.reviewers = reviewers == null ? ImmutableSet.of() : reviewers;
+ this.reviewersByEmail = reviewersByEmail == null ? ImmutableList.of() : reviewersByEmail;
+ this.state = state;
+ this.caller = caller.asIdentifiedUser();
+ op =
+ postReviewersOpFactory.create(
+ this.reviewers, this.reviewersByEmail, state, notify, accountsToNotify);
+ this.exactMatchFound = exactMatchFound;
+ }
+
+ void gatherResults(ChangeData cd) throws OrmException, PermissionBackendException {
+ checkState(op != null, "addition did not result in an update op");
+ checkState(op.getResult() != null, "op did not return a result");
+
+ // Generate result details and fill AccountLoader. This occurs outside
+ // the Op because the accounts are in a different table.
+ PostReviewersOp.Result opResult = op.getResult();
+ if (migration.readChanges() && state == CC) {
+ result.ccs = Lists.newArrayListWithCapacity(opResult.addedCCs().size());
+ for (Account.Id accountId : opResult.addedCCs()) {
+ result.ccs.add(json.format(new ReviewerInfo(accountId.get()), accountId, cd));
+ }
+ accountLoaderFactory.create(true).fill(result.ccs);
+ for (Address a : opResult.addedCCsByEmail()) {
+ result.ccs.add(new AccountInfo(a.getName(), a.getEmail()));
+ }
+ } else {
+ result.reviewers = Lists.newArrayListWithCapacity(opResult.addedReviewers().size());
+ for (PatchSetApproval psa : opResult.addedReviewers()) {
+ // New reviewers have value 0, don't bother normalizing.
+ result.reviewers.add(
+ json.format(
+ new ReviewerInfo(psa.getAccountId().get()),
+ psa.getAccountId(),
+ cd,
+ ImmutableList.of(psa)));
+ }
+ accountLoaderFactory.create(true).fill(result.reviewers);
+ for (Address a : opResult.addedReviewersByEmail()) {
+ result.reviewers.add(ReviewerInfo.byEmail(a.getName(), a.getEmail()));
+ }
+ }
+ }
+ }
+
+ public static boolean isLegalReviewerGroup(AccountGroup.UUID groupUUID) {
+ return !SystemGroupBackend.isSystemGroup(groupUUID);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index 3becf24..beeee948 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -383,7 +383,7 @@
int maxAllowedWithoutConfirmation = suggestReviewers.getMaxAllowedWithoutConfirmation();
logger.atFine().log("maxAllowedWithoutConfirmation: " + maxAllowedWithoutConfirmation);
- if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) {
+ if (!ReviewerAdder.isLegalReviewerGroup(group.getUUID())) {
logger.atFine().log("Ignore group %s that is not legal as reviewer", group.getUUID());
return result;
}
diff --git a/java/com/google/gerrit/server/restapi/change/SuggestReviewers.java b/java/com/google/gerrit/server/restapi/change/SuggestReviewers.java
index b1d49f2..e1f3da2 100644
--- a/java/com/google/gerrit/server/restapi/change/SuggestReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/SuggestReviewers.java
@@ -98,12 +98,12 @@
this.suggestAccounts = (av != AccountVisibility.NONE);
}
- this.maxAllowed = cfg.getInt("addreviewer", "maxAllowed", PostReviewers.DEFAULT_MAX_REVIEWERS);
+ this.maxAllowed = cfg.getInt("addreviewer", "maxAllowed", ReviewerAdder.DEFAULT_MAX_REVIEWERS);
this.maxAllowedWithoutConfirmation =
cfg.getInt(
"addreviewer",
"maxWithoutConfirmation",
- PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
+ ReviewerAdder.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
logger.atFine().log("AccountVisibility: %s", av.name());
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index 098deb5..04ecce7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -49,7 +49,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.restapi.change.PostReviewers;
+import com.google.gerrit.server.restapi.change.ReviewerAdder;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gson.stream.JsonReader;
import java.util.ArrayList;
@@ -69,8 +69,8 @@
String largeGroup = createGroup("largeGroup");
String mediumGroup = createGroup("mediumGroup");
- int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
- int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
+ int largeGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS + 1;
+ int mediumGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
List<TestAccount> users = createAccounts(largeGroupSize, "addGroupAsReviewer");
List<String> largeGroupUsernames = new ArrayList<>(mediumGroupSize);
for (TestAccount u : users) {
@@ -471,8 +471,8 @@
@Test
public void reviewAndAddGroupReviewers() throws Exception {
- int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
- int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
+ int largeGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS + 1;
+ int mediumGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
List<TestAccount> users = createAccounts(largeGroupSize, "reviewAndAddGroupReviewers");
List<String> usernames = new ArrayList<>(largeGroupSize);
for (TestAccount u : users) {
