Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
Documentation: Fix the in-tree bazel test command
Change-Id: I76b72cd23269d4fca93ae065ce638129ec65dc2e
diff --git a/WORKSPACE b/WORKSPACE
index ba28115..83dd4e9 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -3,7 +3,7 @@
load("//:bazlets.bzl", "load_bazlets")
load_bazlets(
- commit = "90314da56cc057c2c8201dab43dfa98e84235957",
+ commit = "b084bd74743f4303971a2a057bcea4f3352ddf4e",
#local_path = "/home/<user>/projects/bazlets",
)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/AddReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/AddReviewers.java
index be04ea4..54d1552 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/AddReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/AddReviewers.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.reviewers.server;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -31,11 +32,9 @@
import com.google.inject.ProvisionException;
import java.util.ArrayList;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
abstract class AddReviewers implements Runnable {
- private static final Logger log = LoggerFactory.getLogger(AddReviewers.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ThreadLocalRequestContext tl;
protected final GerritApi gApi;
@@ -112,7 +111,7 @@
}
gApi.changes().id(changeInfo._number).current().review(in);
} catch (RestApiException e) {
- log.error("Couldn't add reviewers to the change", e);
+ logger.atSevere().withCause(e).log("Couldn't add reviewers to the change");
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/PutReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/PutReviewers.java
index b90b350..cc985df 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/PutReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/PutReviewers.java
@@ -16,6 +16,7 @@
import static com.googlesource.gerrit.plugins.reviewers.server.ModifyReviewersConfigCapability.MODIFY_REVIEWERS_CONFIG;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.access.PluginPermission;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -27,9 +28,11 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.group.GroupsCollection;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gwtorm.server.OrmException;
@@ -42,12 +45,10 @@
import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
class PutReviewers implements RestModifyView<ProjectResource, Input> {
- private static final Logger log = LoggerFactory.getLogger(PutReviewers.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public static class Input {
public Action action;
@@ -60,7 +61,7 @@
private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final ProjectCache projectCache;
private final AccountResolver accountResolver;
- private final Provider<GroupsCollection> groupsCollection;
+ private final Provider<GroupResolver> groupResolver;
private final PermissionBackend permissionBackend;
@Inject
@@ -70,20 +71,20 @@
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
ProjectCache projectCache,
AccountResolver accountResolver,
- Provider<GroupsCollection> groupsCollection,
+ Provider<GroupResolver> groupResolver,
PermissionBackend permissionBackend) {
this.pluginName = pluginName;
this.config = config;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.projectCache = projectCache;
this.accountResolver = accountResolver;
- this.groupsCollection = groupsCollection;
+ this.groupResolver = groupResolver;
this.permissionBackend = permissionBackend;
}
@Override
public List<ReviewerFilterSection> apply(ProjectResource rsrc, Input input)
- throws RestApiException {
+ throws RestApiException, PermissionBackendException {
Project.NameKey projectName = rsrc.getNameKey();
ReviewersConfig.ForProject cfg = config.forProject(projectName);
if (cfg == null) {
@@ -91,7 +92,7 @@
}
PermissionBackend.WithUser userPermission = permissionBackend.user(rsrc.getUser());
- if (!rsrc.getControl().isOwner()
+ if (!userPermission.project(rsrc.getNameKey()).testOrFalse(ProjectPermission.WRITE_CONFIG)
&& !userPermission.testOrFalse(new PluginPermission(pluginName, MODIFY_REVIEWERS_CONFIG))) {
throw new AuthException("not allowed to modify reviewers config");
}
@@ -150,13 +151,13 @@
Account account = accountResolver.find(reviewer);
if (account == null) {
try {
- groupsCollection.get().parse(reviewer);
+ groupResolver.get().parse(reviewer);
} catch (UnprocessableEntityException e) {
throw new ResourceNotFoundException("Account or group " + reviewer + " not found");
}
}
} catch (OrmException | IOException | ConfigInvalidException e) {
- log.error("Failed to resolve account " + reviewer);
+ logger.atSevere().log("Failed to resolve account %s", reviewer);
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/Reviewers.java
index faa5088..31c18d9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/Reviewers.java
@@ -20,6 +20,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -43,8 +44,6 @@
import com.google.inject.Singleton;
import java.util.List;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
class Reviewers
@@ -52,7 +51,7 @@
PrivateStateChangedListener,
WorkInProgressStateChangedListener,
ReviewerSuggestion {
- private static final Logger log = LoggerFactory.getLogger(Reviewers.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ReviewersResolver resolver;
private final AddReviewersByConfiguration.Factory byConfigFactory;
@@ -117,7 +116,7 @@
.collect(toSet());
}
} catch (OrmException | QueryParseException x) {
- log.error(x.getMessage(), x);
+ logger.atSevere().withCause(x).log(x.getMessage());
}
return ImmutableSet.of();
}
@@ -163,13 +162,11 @@
workQueue.getDefaultQueue().submit(task);
} catch (QueryParseException e) {
- log.warn(
- "Could not add default reviewers for change {} of project {}, filter is invalid: {}",
- changeNumber,
- projectName.get(),
- e.getMessage());
+ logger.atWarning().log(
+ "Could not add default reviewers for change %d of project %s, filter is invalid: %s",
+ changeNumber, projectName.get(), e.getMessage());
} catch (OrmException x) {
- log.error(x.getMessage(), x);
+ logger.atSevere().withCause(x).log(x.getMessage());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfig.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfig.java
index b3e2ed2..fe06aa3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfig.java
@@ -17,11 +17,12 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.config.PluginConfigFactory;
-import com.google.gerrit.server.git.VersionedMetaData;
+import com.google.gerrit.server.git.meta.VersionedMetaData;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -32,12 +33,10 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class ReviewersConfig {
- private static final Logger log = LoggerFactory.getLogger(ReviewersConfig.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
static final String FILENAME = "reviewers.config";
static final String SECTION_FILTER = "filter";
@@ -71,7 +70,7 @@
try {
cfg = cfgFactory.getProjectPluginConfigWithMergedInheritance(projectName, pluginName);
} catch (NoSuchProjectException e) {
- log.error("Unable to get config for project {}", projectName.get());
+ logger.atSevere().log("Unable to get config for project %s", projectName.get());
cfg = new Config();
}
return new ForProject(cfg);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolver.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolver.java
index 6f5c202..e362891 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolver.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolver.java
@@ -18,16 +18,15 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Sets;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.common.AccountInfo;
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.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.group.GroupsCollection;
+import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -36,29 +35,24 @@
import java.io.IOException;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/* Resolve account and group names to account ids */
@Singleton
class ReviewersResolver {
- private static final Logger log = LoggerFactory.getLogger(ReviewersResolver.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final AccountResolver accountResolver;
- private final Provider<GroupsCollection> groupsCollection;
- private final GroupMembers.Factory groupMembersFactory;
- private final IdentifiedUser.GenericFactory identifiedUserFactory;
+ private final Provider<GroupResolver> groupResolver;
+ private final GroupMembers groupMembers;
@Inject
ReviewersResolver(
AccountResolver accountResolver,
- Provider<GroupsCollection> groupsCollection,
- GroupMembers.Factory groupMembersFactory,
- IdentifiedUser.GenericFactory identifiedUserFactory) {
+ Provider<GroupResolver> groupResolver,
+ GroupMembers groupMembers) {
this.accountResolver = accountResolver;
- this.groupsCollection = groupsCollection;
- this.groupMembersFactory = groupMembersFactory;
- this.identifiedUserFactory = identifiedUserFactory;
+ this.groupResolver = groupResolver;
+ this.groupMembers = groupMembers;
}
/**
@@ -78,26 +72,12 @@
int changeNumber,
@Nullable AccountInfo uploader) {
Set<Account.Id> reviewers = Sets.newHashSetWithExpectedSize(names.size());
- GroupMembers groupMembers = null;
for (String name : names) {
if (resolveAccount(project, changeNumber, uploader, reviewers, name)) {
continue;
}
- if (groupMembers == null && uploader != null) {
- groupMembers = createGroupMembers(project, changeNumber, uploader, name);
- }
-
- if (groupMembers != null) {
- resolveGroup(project, changeNumber, reviewers, groupMembers, name);
- } else {
- log.warn(
- "For the change {} of project {}: failed to list accounts for group {}; cannot retrieve uploader account for {}.",
- changeNumber,
- project,
- name,
- uploader.email);
- }
+ resolveGroup(project, changeNumber, reviewers, groupMembers, name);
}
return reviewers;
}
@@ -117,21 +97,16 @@
}
return true;
}
- log.warn(
- "For the change {} of project {}: account {} is inactive.",
- changeNumber,
- project,
- accountName);
+ logger.atWarning().log(
+ "For the change %d of project %s: account %s is inactive.",
+ changeNumber, project, accountName);
}
} 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(
- "For the change {} of project {}: failed to resolve account {}.",
- changeNumber,
- project,
- accountName,
- e);
+ logger.atSevere().withCause(e).log(
+ "For the change %d of project %s: failed to resolve account %s.",
+ changeNumber, project, accountName);
return true;
}
return false;
@@ -146,47 +121,20 @@
try {
Set<Account.Id> accounts =
groupMembers
- .listAccounts(groupsCollection.get().parse(group).getGroupUUID(), project)
+ .listAccounts(groupResolver.get().parse(group).getGroupUUID(), project)
.stream()
.filter(Account::isActive)
.map(Account::getId)
.collect(toSet());
reviewers.addAll(accounts);
- } catch (UnprocessableEntityException | NoSuchGroupException e) {
- log.warn(
- "For the change {} of project {}: reviewer {} is neither an account nor a group.",
- changeNumber,
- project,
- group);
- } catch (NoSuchProjectException | IOException | OrmException e) {
- log.error(
- "For the change {} of project {}: failed to list accounts for group {}.",
- changeNumber,
- project,
- group,
- e);
+ } catch (UnprocessableEntityException e) {
+ logger.atWarning().log(
+ "For the change %d of project %s: reviewer %s is neither an account nor a group.",
+ changeNumber, project, group);
+ } catch (NoSuchProjectException | IOException e) {
+ logger.atSevere().withCause(e).log(
+ "For the change %d of project %s: failed to list accounts for group %s.",
+ changeNumber, project, group);
}
}
-
- private GroupMembers createGroupMembers(
- 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(uploaderNameEmail);
- if (uploaderAccount != null) {
- return groupMembersFactory.create(identifiedUserFactory.create(uploaderAccount.getId()));
- }
- } catch (OrmException | IOException e) {
- log.warn(
- "For the change {} of project {}: failed to list accounts for group {}, cannot retrieve uploader account {}.",
- changeNumber,
- project,
- group,
- uploaderNameEmail,
- e);
- }
- return null;
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/SuggestProjectReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/SuggestProjectReviewers.java
index b8b076e..35619c1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/SuggestProjectReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/SuggestProjectReviewers.java
@@ -20,14 +20,14 @@
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.IdentifiedUser;
-import com.google.gerrit.server.ReviewersUtil;
-import com.google.gerrit.server.ReviewersUtil.VisibilityControl;
-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.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.restapi.change.ReviewersUtil;
+import com.google.gerrit.server.restapi.change.ReviewersUtil.VisibilityControl;
+import com.google.gerrit.server.restapi.change.SuggestReviewers;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -43,18 +43,18 @@
@Inject
SuggestProjectReviewers(
AccountVisibility av,
- IdentifiedUser.GenericFactory identifiedUserFactory,
Provider<ReviewDb> dbProvider,
@GerritServerConfig Config cfg,
ReviewersUtil reviewersUtil,
PermissionBackend permissionBackend) {
- super(av, identifiedUserFactory, dbProvider, cfg, reviewersUtil);
+ super(av, dbProvider, cfg, reviewersUtil);
this.permissionBackend = permissionBackend;
}
@Override
public List<SuggestedReviewerInfo> apply(ProjectResource rsrc)
- throws BadRequestException, OrmException, IOException, ConfigInvalidException {
+ throws BadRequestException, OrmException, IOException, ConfigInvalidException,
+ PermissionBackendException {
return reviewersUtil.suggestReviewers(
null, this, rsrc.getProjectState(), getVisibility(rsrc), true);
}
@@ -62,9 +62,9 @@
private VisibilityControl getVisibility(final ProjectResource rsrc) {
return new VisibilityControl() {
@Override
- public boolean isVisibleTo(Account.Id account) throws OrmException {
+ public boolean isVisibleTo(Account.Id account) {
return permissionBackend
- .user(identifiedUserFactory.create(account))
+ .absentUser(account)
.project(rsrc.getNameKey())
.testOrFalse(ProjectPermission.ACCESS);
}
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index 1c80901..864485c 100644
--- a/src/main/resources/Documentation/build.md
+++ b/src/main/resources/Documentation/build.md
@@ -1,7 +1,7 @@
Build
=====
-This plugin can be built with Bazel.
+This plugin is built with Bazel.
Two build modes are supported: Standalone and in Gerrit tree.
The standalone build mode is recommended, as this mode doesn't require
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfigIT.java
index c966a6f..32b2243 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfigIT.java
@@ -41,9 +41,7 @@
private static final String JOHN_DOE = "john.doe@example.com";
@Before
- @Override
public void setUp() throws Exception {
- super.setUp();
fetch(testRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
testRepo.reset("refs/heads/config");
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersIT.java
index 6036865..5c5f478 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersIT.java
@@ -42,9 +42,7 @@
@TestPlugin(name = "reviewers", sysModule = "com.googlesource.gerrit.plugins.reviewers.Module")
public class ReviewersIT extends LightweightPluginDaemonTest {
@Before
- @Override
public void setUp() throws Exception {
- super.setUp();
fetch(testRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
testRepo.reset("refs/heads/config");
}
@@ -85,6 +83,42 @@
}
@Test
+ public void addReviewersMatchMultipleSections() throws Exception {
+ RevCommit oldHead = getRemoteHead();
+ TestAccount user2 = accountCreator.user2();
+
+ Config cfg = new Config();
+ cfg.setStringList(SECTION_FILTER, "*", KEY_REVIEWER, ImmutableList.of(user.email));
+ cfg.setStringList(SECTION_FILTER, "^a.txt", KEY_REVIEWER, ImmutableList.of(user2.email));
+
+ pushFactory
+ .create(db, admin.getIdent(), testRepo, "Add reviewers", FILENAME, cfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+
+ testRepo.reset(oldHead);
+ String changeId = createChange().getChangeId();
+
+ Collection<AccountInfo> reviewers;
+ // Wait for 100 ms until the create patch set event
+ // is processed by the reviewers plugin
+ long wait = 0;
+ do {
+ reviewers = gApi.changes().id(changeId).get().reviewers.get(REVIEWER);
+ if (reviewers == null) {
+ Thread.sleep(10);
+ wait += 10;
+ if (wait > 100) {
+ assert_().fail("Timeout of 100 ms exceeded");
+ }
+ }
+ } while (reviewers == null);
+
+ assertThat(reviewers.stream().map(a -> a._accountId).collect(toSet()))
+ .containsExactlyElementsIn(ImmutableSet.of(admin.id.get(), user.id.get(), user2.id.get()));
+ }
+
+ @Test
public void doNotAddReviewersFromNonMatchingFilters() throws Exception {
RevCommit oldHead = getRemoteHead();
