Do not assign reviewers that cannot see the change
When assigning reviewers to a change automatically, do not
include the ones that have no read visibility to the change
ref. Trying to assign users that cannot see the change would
result in throwing an exception and would thus break the entire
functionality of auto-assigning reviewers to a change.
Keep the message of the users not assigned to a change
as a warning so that the Gerrit admin could understand the problem
and adjust the ACLs if needed.
Bug: Issue 12118
Change-Id: Id84d10d13cb3a5291d7275b2ad24e4686464380d
diff --git a/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/ReviewerManager.java b/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/ReviewerManager.java
index 5b0abe4..e35842d 100644
--- a/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/ReviewerManager.java
+++ b/owners-autoassign/src/main/java/com/vmware/gerrit/owners/common/ReviewerManager.java
@@ -21,7 +21,11 @@
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Account.Id;
import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.change.ChangesCollection;
+import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gwtorm.server.OrmException;
@@ -38,24 +42,42 @@
private final GerritApi gApi;
private final OneOffRequestContext requestContext;
+ private final IdentifiedUser.GenericFactory userFactory;
+
+ private final ChangesCollection changes;
@Inject
- public ReviewerManager(GerritApi gApi, OneOffRequestContext requestContext) {
+ public ReviewerManager(
+ GerritApi gApi,
+ OneOffRequestContext requestContext,
+ IdentifiedUser.GenericFactory userFactory,
+ ChangesCollection changes) {
this.gApi = gApi;
this.requestContext = requestContext;
+ this.userFactory = userFactory;
+ this.changes = changes;
}
public void addReviewers(Change change, Collection<Account.Id> reviewers)
throws ReviewerManagerException {
try (ManualRequestContext ctx = requestContext.openAs(change.getOwner())) {
+ ChangeControl changeControl = changes.parse(change.getId()).getControl();
+
// TODO(davido): Switch back to using changes API again,
// when it supports batch mode for adding reviewers
ReviewInput in = new ReviewInput();
in.reviewers = new ArrayList<>(reviewers.size());
for (Account.Id account : reviewers) {
- AddReviewerInput addReviewerInput = new AddReviewerInput();
- addReviewerInput.reviewer = account.toString();
- in.reviewers.add(addReviewerInput);
+ if (isVisibleTo(changeControl, account)) {
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.reviewer = account.toString();
+ in.reviewers.add(addReviewerInput);
+ } else {
+ log.warn(
+ "Not adding account {} as reviewer to change {} because the associated ref is not visible",
+ account,
+ change.getId());
+ }
}
gApi.changes().id(change.getId().get()).current().review(in);
} catch (RestApiException | OrmException e) {
@@ -63,4 +85,8 @@
throw new ReviewerManagerException(e);
}
}
+
+ private boolean isVisibleTo(ChangeControl changeControl, Id account) {
+ return changeControl.forUser(userFactory.create(account)).isRefVisible();
+ }
}
