Merge branch 'stable-2.16' into stable-3.0
* stable-2.16:
Upgrade bazlets to latest stable-2.16
Add reviewers from OWNERS all at once
Implement visibility using permission backend
Do not assign reviewers that cannot see the change
Fix NPE when target branch doesn't exist
Change-Id: Ie7d6439bf2723696cf2920fbbe8a2ca7049a8e2d
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
index 4a16316..3634cbe 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
@@ -23,6 +23,14 @@
import com.google.gerrit.extensions.common.ChangeInfo;
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.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
@@ -38,11 +46,22 @@
private final OneOffRequestContext requestContext;
private final GerritApi gApi;
+ private final IdentifiedUser.GenericFactory userFactory;
+ private final ChangeData.Factory changeDataFactory;
+ private final PermissionBackend permissionBackend;
@Inject
- public ReviewerManager(OneOffRequestContext requestContext, GerritApi gApi) {
+ public ReviewerManager(
+ OneOffRequestContext requestContext,
+ GerritApi gApi,
+ IdentifiedUser.GenericFactory userFactory,
+ ChangeData.Factory changeDataFactory,
+ PermissionBackend permissionBackend) {
this.requestContext = requestContext;
this.gApi = gApi;
+ this.userFactory = userFactory;
+ this.changeDataFactory = changeDataFactory;
+ this.permissionBackend = permissionBackend;
}
public void addReviewers(ChangeApi cApi, Collection<Account.Id> reviewers)
@@ -56,9 +75,16 @@
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(ctx.getReviewDbProvider().get(), changeInfo, 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,
+ changeInfo._number);
+ }
}
gApi.changes().id(changeInfo.id).current().review(in);
}
@@ -67,4 +93,14 @@
throw new ReviewerManagerException(e);
}
}
+
+ private boolean isVisibleTo(ReviewDb reviewDb, ChangeInfo changeInfo, Id account) {
+ ChangeData changeData =
+ changeDataFactory.create(
+ reviewDb, new Project.NameKey(changeInfo.project), new Change.Id(changeInfo._number));
+ return permissionBackend
+ .user(userFactory.create(account))
+ .change(changeData)
+ .testOrFalse(ChangePermission.READ);
+ }
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java
index 9668af8..c1a7368 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/JgitWrapper.java
@@ -34,9 +34,14 @@
public static Optional<byte[]> getBlobAsBytes(Repository repository, String revision, String path)
throws IOException {
+ ObjectId objectId = repository.resolve(revision);
+ if (objectId == null) {
+ return Optional.empty();
+ }
+
try (final TreeWalk w =
TreeWalk.forPath(
- repository, path, parseCommit(repository, repository.resolve(revision)).getTree())) {
+ repository, path, parseCommit(repository, objectId).getTree())) {
return Optional.ofNullable(w)
.filter(walk -> (walk.getRawMode(0) & TYPE_MASK) == TYPE_FILE)
