Merge branch 'stable-2.14' into stable-2.15
* stable-2.14:
ReviewersResolver: Log failure to list accounts for group as error
ReviewersResolver: Don't log stack trace when group does not exist
ReviewersResolver: Emit a warning to the log when account is inactive
Update bazlets to latest stable-2.14 and switch to 2.14.8 release API
Change-Id: I36db3315f21d6a38bc3ee8951fc6ed737dc4cfc9
diff --git a/WORKSPACE b/WORKSPACE
index 4306ab1..7eb9aae 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -3,7 +3,7 @@
load("//:bazlets.bzl", "load_bazlets")
load_bazlets(
- commit = "c860a578fa46f0b8cd9ddbd6ef8d96ffa50f9bed",
+ commit = "1419057b24e41cda9a6ee9d9c431b9005faf8190",
# local_path = "/home/<user>/projects/bazlets",
)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
index ec39ffc..3e949d6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -18,7 +18,6 @@
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.config.FactoryModule;
-import com.google.gerrit.extensions.events.DraftPublishedListener;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule;
@@ -66,7 +65,6 @@
});
} else {
DynamicSet.bind(binder(), RevisionCreatedListener.class).to(Reviewers.class);
- DynamicSet.bind(binder(), DraftPublishedListener.class).to(Reviewers.class);
}
factory(AddReviewersByConfiguration.Factory.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
index 9c02c6d..f23e275 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
@@ -22,7 +22,6 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.group.GroupsCollection;
@@ -56,7 +55,6 @@
private final ProjectCache projectCache;
private final AccountResolver accountResolver;
private final Provider<GroupsCollection> groupsCollection;
- private final Provider<ReviewDb> reviewDbProvider;
@Inject
PutReviewers(
@@ -65,15 +63,13 @@
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
ProjectCache projectCache,
AccountResolver accountResolver,
- Provider<GroupsCollection> groupsCollection,
- Provider<ReviewDb> reviewDbProvider) {
+ Provider<GroupsCollection> groupsCollection) {
this.pluginName = pluginName;
this.config = config;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.projectCache = projectCache;
this.accountResolver = accountResolver;
this.groupsCollection = groupsCollection;
- this.reviewDbProvider = reviewDbProvider;
}
@Override
@@ -136,7 +132,7 @@
private void validateReviewer(String reviewer) throws RestApiException {
try {
- Account account = accountResolver.find(reviewDbProvider.get(), reviewer);
+ Account account = accountResolver.find(reviewer);
if (account == null) {
try {
groupsCollection.get().parse(reviewer);
@@ -144,7 +140,7 @@
throw new ResourceNotFoundException("Account or group " + reviewer + " not found");
}
}
- } catch (OrmException e) {
+ } catch (OrmException | IOException | ConfigInvalidException e) {
log.error("Failed to resolve account " + reviewer);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
index 1da75b5..6fc3ebf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -21,25 +21,21 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.events.DraftPublishedListener;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.events.RevisionEvent;
+import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.reviewdb.client.Account;
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.CurrentUser;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.change.SuggestedReviewer;
import com.google.gerrit.server.git.WorkQueue;
-import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -49,13 +45,12 @@
import org.slf4j.LoggerFactory;
@Singleton
-class Reviewers implements RevisionCreatedListener, DraftPublishedListener, ReviewerSuggestion {
+class Reviewers implements RevisionCreatedListener, ReviewerSuggestion {
private static final Logger log = LoggerFactory.getLogger(Reviewers.class);
private final ReviewersResolver resolver;
private final AddReviewersByConfiguration.Factory byConfigFactory;
private final WorkQueue workQueue;
- private final SchemaFactory<ReviewDb> schemaFactory;
private final ReviewersConfig config;
private final Provider<CurrentUser> user;
private final ChangeQueryBuilder queryBuilder;
@@ -66,7 +61,6 @@
ReviewersResolver resolver,
AddReviewersByConfiguration.Factory byConfigFactory,
WorkQueue workQueue,
- SchemaFactory<ReviewDb> schemaFactory,
ReviewersConfig config,
Provider<CurrentUser> user,
ChangeQueryBuilder queryBuilder,
@@ -74,7 +68,6 @@
this.resolver = resolver;
this.byConfigFactory = byConfigFactory;
this.workQueue = workQueue;
- this.schemaFactory = schemaFactory;
this.config = config;
this.user = user;
this.queryBuilder = queryBuilder;
@@ -87,11 +80,6 @@
}
@Override
- public void onDraftPublished(DraftPublishedListener.Event event) {
- onEvent(event);
- }
-
- @Override
public Set<SuggestedReviewer> suggestReviewers(
Project.NameKey projectName,
@Nullable Change.Id changeId,
@@ -103,11 +91,11 @@
return ImmutableSet.of();
}
- try (ReviewDb reviewDb = schemaFactory.open()) {
+ try {
Set<String> reviewers = findReviewers(changeId.get(), sections);
if (!reviewers.isEmpty()) {
return resolver
- .resolve(reviewDb, reviewers, projectName, changeId.get(), null)
+ .resolve(reviewers, projectName, changeId.get(), null)
.stream()
.map(a -> suggestedReviewer(a))
.collect(toSet());
@@ -132,10 +120,6 @@
private void onEvent(RevisionEvent event) {
ChangeInfo c = event.getChange();
- if (config.ignoreDrafts() && c.status == ChangeStatus.DRAFT) {
- log.debug("Ignoring draft change");
- return;
- }
Project.NameKey projectName = new Project.NameKey(c.project);
List<ReviewerFilterSection> sections = getSections(projectName);
@@ -146,14 +130,14 @@
AccountInfo uploader = event.getWho();
int changeNumber = c._number;
- try (ReviewDb reviewDb = schemaFactory.open()) {
+ try {
Set<String> reviewers = findReviewers(changeNumber, sections);
if (reviewers.isEmpty()) {
return;
}
final Runnable task =
byConfigFactory.create(
- c, resolver.resolve(reviewDb, reviewers, projectName, changeNumber, uploader));
+ c, resolver.resolve(reviewers, projectName, changeNumber, uploader));
workQueue.getDefaultQueue().submit(task);
} catch (QueryParseException e) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
index af84789..cd2479a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
@@ -24,7 +24,6 @@
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
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.account.AccountResolver;
import com.google.gerrit.server.account.GroupMembers;
@@ -36,6 +35,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Set;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -65,7 +65,6 @@
* Resolve a set of account names to {@link com.google.gerrit.reviewdb.client.Account.Id}s. Group
* names are resolved to their account members.
*
- * @param reviewDb DB
* @param names the set of account names to convert
* @param project the project name
* @param changeNumber the change Id
@@ -74,7 +73,6 @@
*/
@VisibleForTesting
Set<Account.Id> resolve(
- ReviewDb reviewDb,
Set<String> names,
Project.NameKey project,
int changeNumber,
@@ -82,12 +80,12 @@
Set<Account.Id> reviewers = Sets.newHashSetWithExpectedSize(names.size());
GroupMembers groupMembers = null;
for (String name : names) {
- if (resolveAccount(reviewDb, project, changeNumber, uploader, reviewers, name)) {
+ if (resolveAccount(project, changeNumber, uploader, reviewers, name)) {
continue;
}
if (groupMembers == null && uploader != null) {
- groupMembers = createGroupMembers(reviewDb, project, changeNumber, uploader, name);
+ groupMembers = createGroupMembers(project, changeNumber, uploader, name);
}
if (groupMembers != null) {
@@ -105,14 +103,13 @@
}
private boolean resolveAccount(
- ReviewDb reviewDb,
Project.NameKey project,
int changeNumber,
AccountInfo uploader,
Set<Account.Id> reviewers,
String accountName) {
try {
- Account account = accountResolver.find(reviewDb, accountName);
+ Account account = accountResolver.find(accountName);
if (account != null) {
if (account.isActive()) {
if (uploader == null || uploader._accountId != account.getId().get()) {
@@ -126,7 +123,7 @@
project,
accountName);
}
- } catch (OrmException e) {
+ } catch (OrmException | IOException | ConfigInvalidException e) {
// If the account doesn't exist, find() will return null. We only
// get here if something went wrong accessing the database
log.error(
@@ -172,20 +169,16 @@
}
private GroupMembers createGroupMembers(
- ReviewDb reviewDb,
- Project.NameKey project,
- int changeNumber,
- AccountInfo uploader,
- String group) {
+ Project.NameKey project, int changeNumber, AccountInfo uploader, String group) {
// email is not unique to one account, try to locate the account using
// "Full name <email>" to increase chance of finding only one.
String uploaderNameEmail = String.format("%s <%s>", uploader.name, uploader.email);
try {
- Account uploaderAccount = accountResolver.findByNameOrEmail(reviewDb, uploaderNameEmail);
+ Account uploaderAccount = accountResolver.findByNameOrEmail(uploaderNameEmail);
if (uploaderAccount != null) {
return groupMembersFactory.create(identifiedUserFactory.create(uploaderAccount.getId()));
}
- } catch (OrmException e) {
+ } catch (OrmException | IOException e) {
log.warn(
"For the change {} of project {}: failed to list accounts for group {}, cannot retrieve uploader account {}.",
changeNumber,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/SuggestProjectReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/SuggestProjectReviewers.java
index f71c591..4212e61 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/SuggestProjectReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/SuggestProjectReviewers.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.reviewers;
+import com.google.gerrit.extensions.common.AccountVisibility;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -22,41 +23,50 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewersUtil;
import com.google.gerrit.server.ReviewersUtil.VisibilityControl;
-import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.change.SuggestReviewers;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
import java.util.List;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
public class SuggestProjectReviewers extends SuggestReviewers
implements RestReadView<ProjectResource> {
+ private final PermissionBackend permissionBackend;
+
@Inject
SuggestProjectReviewers(
AccountVisibility av,
IdentifiedUser.GenericFactory identifiedUserFactory,
Provider<ReviewDb> dbProvider,
@GerritServerConfig Config cfg,
- ReviewersUtil reviewersUtil) {
+ ReviewersUtil reviewersUtil,
+ PermissionBackend permissionBackend) {
super(av, identifiedUserFactory, dbProvider, cfg, reviewersUtil);
+ this.permissionBackend = permissionBackend;
}
@Override
public List<SuggestedReviewerInfo> apply(ProjectResource rsrc)
- throws BadRequestException, OrmException, IOException {
- return reviewersUtil.suggestReviewers(null, this, rsrc.getControl(), getVisibility(rsrc), true);
+ throws BadRequestException, OrmException, IOException, ConfigInvalidException {
+ return reviewersUtil.suggestReviewers(
+ null, this, rsrc.getProjectState(), getVisibility(rsrc), true);
}
private VisibilityControl getVisibility(final ProjectResource rsrc) {
return new VisibilityControl() {
@Override
public boolean isVisibleTo(Account.Id account) throws OrmException {
- IdentifiedUser who = identifiedUserFactory.create(account);
- return rsrc.getControl().forUser(who).isVisible();
+ return permissionBackend
+ .user(identifiedUserFactory.create(account))
+ .project(rsrc.getNameKey())
+ .testOrFalse(ProjectPermission.ACCESS);
}
};
}
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index cefe409..eb77417 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -8,7 +8,6 @@
[reviewers]
enableREST = true
enableUI = false
- ignoreDrafts = true
suggestOnly = false
```
@@ -20,11 +19,6 @@
: Enable the UI. When set to false, the 'Reviewers' menu is not displayed
on the project screen. Defaults to true, or false when `enableREST` is false.
-reviewers.ignoreDrafts
-: Ignore draft changes. When set to true draft changes are not considered when
- adding reviewers. Defaults to false. To ignore drafts on a per-project basis
- set this value to false and add "-status:draft" to filter in relevant projects.
-
reviewers.suggestOnly
: Provide the configured reviewers as suggestions in the "Add Reviewer" dialog
instead of automatically adding them to the change. Only supports accounts;
@@ -32,7 +26,6 @@
the suggestions with a weight of 1. To force the suggestions higher in the
list, set a higher value (like 1000) in `addReviewer.@PLUGIN@-reviewer-suggestion.weight`
in `gerrit.config`.
-
Per project configuration of the @PLUGIN@ plugin is done in the
`reviewers.config` file of the project. Missing values are inherited
from the parent projects. This means a global default configuration can
@@ -50,8 +43,6 @@
[filter "branch:stable-2.10"]
reviewer = QAGroup
- [filter "-status:draft"]
- reviewer = DevGroup
```
filter.\<filter\>.reviewer
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
index 2b1f67c..f46ca74 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
@@ -52,7 +52,7 @@
@Test
public void addReviewers() throws Exception {
RevCommit oldHead = getRemoteHead();
- TestAccount user2 = accounts.user2();
+ TestAccount user2 = accountCreator.user2();
Config cfg = new Config();
cfg.setStringList(SECTION_FILTER, "*", KEY_REVIEWER, ImmutableList.of(user.email, user2.email));
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java
index dd8d07b..c5b89ed 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java
@@ -42,7 +42,6 @@
public void testUploaderSkippedAsReviewer() throws Exception {
Set<Account.Id> reviewers =
resolver.resolve(
- db,
Collections.singleton(user.email),
project,
change,
@@ -54,7 +53,6 @@
public void testAccountResolve() throws Exception {
Set<Account.Id> reviewers =
resolver.resolve(
- db,
ImmutableSet.of(user.email, admin.email),
project,
change,
@@ -65,18 +63,17 @@
@Test
public void testAccountGroupResolve() throws Exception {
String group1 = createGroup("group1");
- TestAccount foo = createAccount("foo", group1);
- TestAccount bar = createAccount("bar", group1);
+ TestAccount foo = createTestAccount("foo", group1);
+ TestAccount bar = createTestAccount("bar", group1);
String group2 = createGroup("group2");
- TestAccount baz = createAccount("baz", group2);
- TestAccount qux = createAccount("qux", group2);
+ TestAccount baz = createTestAccount("baz", group2);
+ TestAccount qux = createTestAccount("qux", group2);
- TestAccount system = createAccount("system", "Administrators");
+ TestAccount system = createTestAccount("system", "Administrators");
Set<Account.Id> reviewers =
resolver.resolve(
- db,
ImmutableSet.of(system.email, group1, group2),
project,
change,
@@ -84,8 +81,8 @@
assertThat(reviewers).containsExactly(system.id, foo.id, bar.id, baz.id, qux.id);
}
- private TestAccount createAccount(String name, String group) throws Exception {
+ private TestAccount createTestAccount(String name, String group) throws Exception {
name = name(name);
- return accounts.create(name, name + "@example.com", name + " full name", group);
+ return accountCreator.create(name, name + "@example.com", name + " full name", group);
}
}
