Merge "Merge branch 'stable-2.14' into stable-2.15" into stable-2.15
diff --git a/BUILD b/BUILD
index 6086d5c..1ab91f7 100644
--- a/BUILD
+++ b/BUILD
@@ -1,4 +1,10 @@
-load("//tools/bzl:plugin.bzl", "gerrit_plugin")
+load("//tools/bzl:junit.bzl", "junit_tests")
+load(
+ "//tools/bzl:plugin.bzl",
+ "gerrit_plugin",
+ "PLUGIN_DEPS",
+ "PLUGIN_TEST_DEPS",
+)
gerrit_plugin(
name = "reviewers",
@@ -10,3 +16,21 @@
],
resources = glob(["src/main/**/*"]),
)
+
+junit_tests(
+ name = "reviewers_tests",
+ srcs = glob(["src/test/java/**/*.java"]),
+ tags = ["reviewers"],
+ deps = [
+ ":reviewers__plugin_test_deps",
+ ],
+)
+
+java_library(
+ name = "reviewers__plugin_test_deps",
+ testonly = 1,
+ visibility = ["//visibility:public"],
+ exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":reviewers__plugin",
+ ],
+)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
new file mode 100644
index 0000000..afbb377
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewers.java
@@ -0,0 +1,118 @@
+// 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.googlesource.gerrit.plugins.reviewers;
+
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+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.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Provider;
+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 final ThreadLocalRequestContext tl;
+ protected final GerritApi gApi;
+ protected final IdentifiedUser.GenericFactory identifiedUserFactory;
+ protected final SchemaFactory<ReviewDb> schemaFactory;
+ protected final ChangeInfo changeInfo;
+
+ private ReviewDb db = null;
+
+ AddReviewers(
+ ThreadLocalRequestContext tl,
+ GerritApi gApi,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ SchemaFactory<ReviewDb> schemaFactory,
+ ChangeInfo changeInfo) {
+ this.tl = tl;
+ this.gApi = gApi;
+ this.identifiedUserFactory = identifiedUserFactory;
+ this.schemaFactory = schemaFactory;
+ this.changeInfo = changeInfo;
+ }
+
+ abstract Set<Account.Id> getReviewers();
+
+ @Override
+ public void run() {
+ RequestContext old =
+ tl.setContext(
+ new RequestContext() {
+
+ @Override
+ public CurrentUser getUser() {
+ return identifiedUserFactory.create(new Account.Id(changeInfo.owner._accountId));
+ }
+
+ @Override
+ public Provider<ReviewDb> getReviewDbProvider() {
+ return new Provider<ReviewDb>() {
+ @Override
+ public ReviewDb get() {
+ if (db == null) {
+ try {
+ db = schemaFactory.open();
+ } catch (OrmException e) {
+ throw new ProvisionException("Cannot open ReviewDb", e);
+ }
+ }
+ return db;
+ }
+ };
+ }
+ });
+ try {
+ addReviewers(getReviewers(), changeInfo);
+ } finally {
+ tl.setContext(old);
+ if (db != null) {
+ db.close();
+ db = null;
+ }
+ }
+ }
+
+ private void addReviewers(Set<Account.Id> reviewers, ChangeInfo changeInfo) {
+ try {
+ // 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);
+ }
+ gApi.changes().id(changeInfo._number).current().review(in);
+ } catch (RestApiException e) {
+ log.error("Couldn't add reviewers to the change", e);
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewersByConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewersByConfiguration.java
new file mode 100644
index 0000000..a03c035
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/AddReviewersByConfiguration.java
@@ -0,0 +1,51 @@
+// Copyright (C) 2013 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.googlesource.gerrit.plugins.reviewers;
+
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.common.ChangeInfo;
+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.util.ThreadLocalRequestContext;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import java.util.Set;
+
+class AddReviewersByConfiguration extends AddReviewers {
+ private final Set<Account.Id> reviewers;
+
+ interface Factory {
+ AddReviewersByConfiguration create(ChangeInfo changeInfo, Set<Account.Id> reviewers);
+ }
+
+ @Inject
+ AddReviewersByConfiguration(
+ ThreadLocalRequestContext tl,
+ GerritApi gApi,
+ IdentifiedUser.GenericFactory identifiedUserFactory,
+ SchemaFactory<ReviewDb> schemaFactory,
+ @Assisted ChangeInfo changeInfo,
+ @Assisted Set<Account.Id> reviewers) {
+ super(tl, gApi, identifiedUserFactory, schemaFactory, changeInfo);
+ this.reviewers = reviewers;
+ }
+
+ @Override
+ Set<Account.Id> getReviewers() {
+ return reviewers;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java
deleted file mode 100644
index 47916a0..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java
+++ /dev/null
@@ -1,281 +0,0 @@
-// Copyright (C) 2013 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.googlesource.gerrit.plugins.reviewers;
-
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Sets;
-import com.google.gerrit.common.errors.NoSuchGroupException;
-import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.events.RevisionCreatedListener;
-import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
-import com.google.gerrit.index.query.Predicate;
-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.IdentifiedUser;
-import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.config.PluginConfigFactory;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.WorkQueue;
-import com.google.gerrit.server.group.GroupsCollection;
-import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder;
-import com.google.gerrit.server.util.RequestContext;
-import com.google.gerrit.server.util.ThreadLocalRequestContext;
-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.ProvisionException;
-import com.google.inject.Singleton;
-import java.io.IOException;
-import java.util.List;
-import java.util.Set;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-@Singleton
-class ChangeEventListener implements RevisionCreatedListener {
- private static final Logger log = LoggerFactory.getLogger(ChangeEventListener.class);
-
- private final AccountResolver accountResolver;
- private final Provider<GroupsCollection> groupsCollection;
- private final GroupMembers.Factory groupMembersFactory;
- private final DefaultReviewers.Factory reviewersFactory;
- private final GitRepositoryManager repoManager;
- private final WorkQueue workQueue;
- private final IdentifiedUser.GenericFactory identifiedUserFactory;
- private final ThreadLocalRequestContext tl;
- private final SchemaFactory<ReviewDb> schemaFactory;
- private final ChangeData.Factory changeDataFactory;
- private final ReviewersConfig.Factory configFactory;
- private final Provider<CurrentUser> user;
- private final ChangeQueryBuilder queryBuilder;
-
- @Inject
- ChangeEventListener(
- final AccountResolver accountResolver,
- final Provider<GroupsCollection> groupsCollection,
- final GroupMembers.Factory groupMembersFactory,
- final DefaultReviewers.Factory reviewersFactory,
- final GitRepositoryManager repoManager,
- final WorkQueue workQueue,
- final IdentifiedUser.GenericFactory identifiedUserFactory,
- final ThreadLocalRequestContext tl,
- final SchemaFactory<ReviewDb> schemaFactory,
- final ChangeData.Factory changeDataFactory,
- final ReviewersConfig.Factory configFactory,
- final Provider<CurrentUser> user,
- final ChangeQueryBuilder queryBuilder,
- final PluginConfigFactory cfgFactory,
- @PluginName String pluginName) {
- this.accountResolver = accountResolver;
- this.groupsCollection = groupsCollection;
- this.groupMembersFactory = groupMembersFactory;
- this.reviewersFactory = reviewersFactory;
- this.repoManager = repoManager;
- this.workQueue = workQueue;
- this.identifiedUserFactory = identifiedUserFactory;
- this.tl = tl;
- this.schemaFactory = schemaFactory;
- this.changeDataFactory = changeDataFactory;
- this.configFactory = configFactory;
- this.user = user;
- this.queryBuilder = queryBuilder;
- }
-
- @Override
- public void onRevisionCreated(RevisionCreatedListener.Event event) {
- ChangeInfo c = event.getChange();
- onEvent(new Project.NameKey(c.project), c._number, event.getWho());
- }
-
- private void onEvent(Project.NameKey projectName, int changeNumber, AccountInfo uploader) {
- // TODO(davido): we have to cache per project configuration
- ReviewersConfig config = configFactory.create(projectName);
- List<ReviewerFilterSection> sections = config.getReviewerFilterSections();
-
- if (sections.isEmpty()) {
- return;
- }
-
- try (Repository git = repoManager.openRepository(projectName);
- RevWalk rw = new RevWalk(git);
- ReviewDb reviewDb = schemaFactory.open()) {
- ChangeData changeData =
- changeDataFactory.create(reviewDb, projectName, new Change.Id(changeNumber));
- Set<String> reviewers = findReviewers(sections, changeData);
- if (reviewers.isEmpty()) {
- return;
- }
-
- final Change change = changeData.change();
- final Runnable task =
- reviewersFactory.create(change, toAccounts(reviewers, projectName, uploader));
-
- workQueue
- .getDefaultQueue()
- .submit(
- new Runnable() {
- ReviewDb db = null;
-
- @Override
- public void run() {
- RequestContext old =
- tl.setContext(
- new RequestContext() {
-
- @Override
- public CurrentUser getUser() {
- return identifiedUserFactory.create(change.getOwner());
- }
-
- @Override
- public Provider<ReviewDb> getReviewDbProvider() {
- return new Provider<ReviewDb>() {
- @Override
- public ReviewDb get() {
- if (db == null) {
- try {
- db = schemaFactory.open();
- } catch (OrmException e) {
- throw new ProvisionException("Cannot open ReviewDb", e);
- }
- }
- return db;
- }
- };
- }
- });
- try {
- task.run();
- } finally {
- tl.setContext(old);
- if (db != null) {
- db.close();
- db = null;
- }
- }
- }
- });
- } catch (OrmException | IOException | QueryParseException x) {
- log.error(x.getMessage(), x);
- }
- }
-
- private Set<String> findReviewers(List<ReviewerFilterSection> sections, ChangeData changeData)
- throws OrmException, QueryParseException {
- ImmutableSet.Builder<String> reviewers = ImmutableSet.builder();
- List<ReviewerFilterSection> found = findReviewerSections(sections, changeData);
- for (ReviewerFilterSection s : found) {
- reviewers.addAll(s.getReviewers());
- }
- return reviewers.build();
- }
-
- private List<ReviewerFilterSection> findReviewerSections(
- List<ReviewerFilterSection> sections, ChangeData changeData)
- throws OrmException, QueryParseException {
- ImmutableList.Builder<ReviewerFilterSection> found = ImmutableList.builder();
- for (ReviewerFilterSection s : sections) {
- if (Strings.isNullOrEmpty(s.getFilter()) || s.getFilter().equals("*")) {
- found.add(s);
- } else if (filterMatch(s.getFilter(), changeData)) {
- found.add(s);
- }
- }
- return found.build();
- }
-
- boolean filterMatch(String filter, ChangeData changeData)
- throws OrmException, QueryParseException {
- Preconditions.checkNotNull(filter);
- ChangeQueryBuilder qb = queryBuilder.asUser(user.get());
- Predicate<ChangeData> filterPredicate = qb.parse(filter);
- // TODO(davido): check that the potential review can see this change
- // by adding AND is_visible() predicate? Or is it OK to assume
- // that reviewers always can see it?
- return filterPredicate.asMatchable().match(changeData);
- }
-
- private Set<Account> toAccounts(Set<String> in, Project.NameKey p, AccountInfo uploader) {
- Set<Account> reviewers = Sets.newHashSetWithExpectedSize(in.size());
- GroupMembers groupMembers = null;
- for (String r : in) {
- try {
- Account account = accountResolver.find(r);
- if (account != null) {
- reviewers.add(account);
- continue;
- }
- } 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("Failed to resolve account " + r, e);
- continue;
- }
- if (groupMembers == null) {
- // 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) {
- groupMembers =
- groupMembersFactory.create(identifiedUserFactory.create(uploaderAccount.getId()));
- }
- } catch (OrmException | IOException e) {
- log.warn(
- String.format(
- "Failed to list accounts for group %s, cannot retrieve uploader account %s",
- r, uploaderNameEmail),
- e);
- }
-
- try {
- if (groupMembers != null) {
- reviewers.addAll(
- groupMembers.listAccounts(groupsCollection.get().parse(r).getGroupUUID(), p));
- } else {
- log.warn(
- String.format(
- "Failed to list accounts for group %s; cannot retrieve uploader account for %s",
- r, uploader.email));
- }
- } catch (UnprocessableEntityException | NoSuchGroupException e) {
- log.warn(String.format("Reviewer %s is neither an account nor a group", r));
- } catch (NoSuchProjectException e) {
- log.warn(String.format("Failed to list accounts for group %s and project %s", r, p));
- } catch (IOException | OrmException e) {
- log.warn(String.format("Failed to list accounts for group %s", r), e);
- }
- }
- }
- return reviewers;
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/DefaultReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/DefaultReviewers.java
deleted file mode 100644
index 1d5dac4..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/DefaultReviewers.java
+++ /dev/null
@@ -1,77 +0,0 @@
-// Copyright (C) 2013 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.googlesource.gerrit.plugins.reviewers;
-
-import com.google.gerrit.extensions.api.GerritApi;
-import com.google.gerrit.extensions.api.changes.AddReviewerInput;
-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.Change;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-import java.util.ArrayList;
-import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-class DefaultReviewers implements Runnable {
- private static final Logger log = LoggerFactory.getLogger(DefaultReviewers.class);
-
- private final GerritApi gApi;
- private final Change change;
- private final Set<Account> reviewers;
-
- interface Factory {
- DefaultReviewers create(Change change, Set<Account> reviewers);
- }
-
- @Inject
- DefaultReviewers(GerritApi gApi, @Assisted Change change, @Assisted Set<Account> reviewers) {
- this.gApi = gApi;
- this.change = change;
- this.reviewers = reviewers;
- }
-
- @Override
- public void run() {
- addReviewers(reviewers, change);
- }
-
- /**
- * Append the reviewers to change#{@link Change}
- *
- * @param reviewers Set of reviewers to add
- * @param change {@link Change} to add the reviewers to
- */
- private void addReviewers(Set<Account> reviewers, Change change) {
- try {
- // 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 account : reviewers) {
- AddReviewerInput addReviewerInput = new AddReviewerInput();
- if (account.isActive()) {
- addReviewerInput.reviewer = account.getId().toString();
- in.reviewers.add(addReviewerInput);
- }
- }
- gApi.changes().id(change.getId().get()).current().review(in);
- } catch (RestApiException e) {
- log.error("Couldn't add reviewers to the change", e);
- }
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java
index b0e735f..7ee4c83 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/GetReviewers.java
@@ -14,9 +14,7 @@
package com.googlesource.gerrit.plugins.reviewers;
-import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.project.ProjectResource;
import com.google.inject.Inject;
@@ -25,16 +23,15 @@
@Singleton
class GetReviewers implements RestReadView<ProjectResource> {
- private final ReviewersConfig.Factory configFactory;
+ private final ReviewersConfig config;
@Inject
- GetReviewers(ReviewersConfig.Factory configFactory) {
- this.configFactory = configFactory;
+ GetReviewers(ReviewersConfig config) {
+ this.config = config;
}
@Override
- public List<ReviewerFilterSection> apply(ProjectResource resource)
- throws AuthException, BadRequestException, ResourceConflictException, Exception {
- return configFactory.create(resource.getNameKey()).getReviewerFilterSections();
+ public List<ReviewerFilterSection> apply(ProjectResource resource) throws RestApiException {
+ return config.forProject(resource.getNameKey()).getReviewerFilterSections();
}
}
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 98dad1b..3e949d6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -16,7 +16,7 @@
import static com.google.gerrit.server.project.ProjectResource.PROJECT_KIND;
-import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -24,24 +24,26 @@
import com.google.gerrit.extensions.webui.GwtPlugin;
import com.google.gerrit.extensions.webui.TopMenu;
import com.google.gerrit.extensions.webui.WebUiPlugin;
-import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.change.ReviewerSuggestion;
+import com.google.inject.AbstractModule;
import com.google.inject.Inject;
-import org.eclipse.jgit.lib.Config;
public class Module extends FactoryModule {
private final boolean enableUI;
private final boolean enableREST;
+ private final boolean suggestOnly;
@Inject
- public Module(@PluginName String pluginName, PluginConfigFactory pluginCfgFactory) {
- Config c = pluginCfgFactory.getGlobalPluginConfig(pluginName);
- this.enableREST = c.getBoolean("reviewers", null, "enableREST", true);
- this.enableUI = enableREST ? c.getBoolean("reviewers", null, "enableUI", true) : false;
+ public Module(ReviewersConfig cfg) {
+ this.enableREST = cfg.enableREST();
+ this.enableUI = cfg.enableUI();
+ this.suggestOnly = cfg.suggestOnly();
}
- public Module(boolean enableUI, boolean enableREST) {
+ public Module(boolean enableUI, boolean enableREST, boolean suggestOnly) {
this.enableUI = enableUI;
this.enableREST = enableREST;
+ this.suggestOnly = suggestOnly;
}
@Override
@@ -51,9 +53,21 @@
DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new GwtPlugin("reviewers"));
}
- DynamicSet.bind(binder(), RevisionCreatedListener.class).to(ChangeEventListener.class);
- factory(DefaultReviewers.Factory.class);
- factory(ReviewersConfig.Factory.class);
+ if (suggestOnly) {
+ install(
+ new AbstractModule() {
+ @Override
+ protected void configure() {
+ bind(ReviewerSuggestion.class)
+ .annotatedWith(Exports.named("reviewer-suggest"))
+ .to(Reviewers.class);
+ }
+ });
+ } else {
+ DynamicSet.bind(binder(), RevisionCreatedListener.class).to(Reviewers.class);
+ }
+
+ factory(AddReviewersByConfiguration.Factory.class);
if (enableREST) {
install(
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 69eba6a..856a5cd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/PutReviewers.java
@@ -51,7 +51,7 @@
}
private final String pluginName;
- private final ReviewersConfig.Factory configFactory;
+ private final ReviewersConfig config;
private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final ProjectCache projectCache;
private final AccountResolver accountResolver;
@@ -60,14 +60,14 @@
@Inject
PutReviewers(
@PluginName String pluginName,
- ReviewersConfig.Factory configFactory,
+ ReviewersConfig config,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
ProjectCache projectCache,
AccountResolver accountResolver,
Provider<GroupsCollection> groupsCollection,
Provider<ReviewDb> reviewDbProvider) {
this.pluginName = pluginName;
- this.configFactory = configFactory;
+ this.config = config;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.projectCache = projectCache;
this.accountResolver = accountResolver;
@@ -78,7 +78,7 @@
public List<ReviewerFilterSection> apply(ProjectResource rsrc, Input input)
throws RestApiException {
Project.NameKey projectName = rsrc.getNameKey();
- ReviewersConfig cfg = configFactory.create(projectName);
+ ReviewersConfig.ForProject cfg = config.forProject(projectName);
if (!rsrc.getControl().isOwner() || cfg == null) {
throw new ResourceNotFoundException("Project" + projectName.get() + " not found");
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java
index 1112bdd..06e7387 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerFilterSection.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.reviewers;
+import java.util.Objects;
import java.util.Set;
class ReviewerFilterSection {
@@ -32,4 +33,18 @@
Set<String> getReviewers() {
return reviewers;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (o instanceof ReviewerFilterSection) {
+ ReviewerFilterSection other = ((ReviewerFilterSection) o);
+ return Objects.equals(filter, other.filter) && Objects.equals(reviewers, other.reviewers);
+ }
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(filter, reviewers);
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
new file mode 100644
index 0000000..70c2cec
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -0,0 +1,187 @@
+// Copyright (C) 2013 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.googlesource.gerrit.plugins.reviewers;
+
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+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.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.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+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 ReviewersConfig config;
+ private final Provider<CurrentUser> user;
+ private final ChangeQueryBuilder queryBuilder;
+ private final Provider<InternalChangeQuery> queryProvider;
+
+ @Inject
+ Reviewers(
+ ReviewersResolver resolver,
+ AddReviewersByConfiguration.Factory byConfigFactory,
+ WorkQueue workQueue,
+ ReviewersConfig config,
+ Provider<CurrentUser> user,
+ ChangeQueryBuilder queryBuilder,
+ Provider<InternalChangeQuery> queryProvider) {
+ this.resolver = resolver;
+ this.byConfigFactory = byConfigFactory;
+ this.workQueue = workQueue;
+ this.config = config;
+ this.user = user;
+ this.queryBuilder = queryBuilder;
+ this.queryProvider = queryProvider;
+ }
+
+ @Override
+ public void onRevisionCreated(RevisionCreatedListener.Event event) {
+ onEvent(event);
+ }
+
+ @Override
+ public Set<SuggestedReviewer> suggestReviewers(
+ Project.NameKey projectName,
+ @Nullable Change.Id changeId,
+ @Nullable String query,
+ Set<Account.Id> candidates) {
+ List<ReviewerFilterSection> sections = getSections(projectName);
+
+ if (sections.isEmpty() || changeId == null) {
+ return ImmutableSet.of();
+ }
+
+ try {
+ Set<String> reviewers = findReviewers(changeId.get(), sections);
+ if (!reviewers.isEmpty()) {
+ return resolver
+ .resolve(reviewers, projectName, changeId.get(), null)
+ .stream()
+ .map(a -> suggestedReviewer(a))
+ .collect(toSet());
+ }
+ } catch (OrmException | QueryParseException | IOException x) {
+ log.error(x.getMessage(), x);
+ }
+ return ImmutableSet.of();
+ }
+
+ private SuggestedReviewer suggestedReviewer(Account.Id account) {
+ SuggestedReviewer reviewer = new SuggestedReviewer();
+ reviewer.account = account;
+ reviewer.score = 1;
+ return reviewer;
+ }
+
+ private List<ReviewerFilterSection> getSections(Project.NameKey projectName) {
+ // TODO(davido): we have to cache per project configuration
+ return config.forProject(projectName).getReviewerFilterSections();
+ }
+
+ private void onEvent(RevisionEvent event) {
+ ChangeInfo c = event.getChange();
+ Project.NameKey projectName = new Project.NameKey(c.project);
+
+ List<ReviewerFilterSection> sections = getSections(projectName);
+
+ if (sections.isEmpty()) {
+ return;
+ }
+
+ AccountInfo uploader = event.getWho();
+ int changeNumber = c._number;
+ try {
+ Set<String> reviewers = findReviewers(changeNumber, sections);
+ if (reviewers.isEmpty()) {
+ return;
+ }
+ final Runnable task =
+ byConfigFactory.create(
+ c, resolver.resolve(reviewers, projectName, changeNumber, uploader));
+
+ workQueue.getDefaultQueue().submit(task);
+ } catch (QueryParseException | IOException e) {
+ log.warn(
+ "Could not add default reviewers for change {} of project {}, filter is invalid: {}",
+ changeNumber,
+ projectName.get(),
+ e.getMessage());
+ } catch (OrmException x) {
+ log.error(x.getMessage(), x);
+ }
+ }
+
+ private Set<String> findReviewers(int change, List<ReviewerFilterSection> sections)
+ throws OrmException, QueryParseException {
+ ImmutableSet.Builder<String> reviewers = ImmutableSet.builder();
+ List<ReviewerFilterSection> found = findReviewerSections(change, sections);
+ for (ReviewerFilterSection s : found) {
+ reviewers.addAll(s.getReviewers());
+ }
+ return reviewers.build();
+ }
+
+ private List<ReviewerFilterSection> findReviewerSections(
+ int change, List<ReviewerFilterSection> sections) throws OrmException, QueryParseException {
+ ImmutableList.Builder<ReviewerFilterSection> found = ImmutableList.builder();
+ for (ReviewerFilterSection s : sections) {
+ if (Strings.isNullOrEmpty(s.getFilter()) || s.getFilter().equals("*")) {
+ found.add(s);
+ } else if (filterMatch(change, s.getFilter())) {
+ found.add(s);
+ }
+ }
+ return found.build();
+ }
+
+ boolean filterMatch(int change, String filter) throws OrmException, QueryParseException {
+ Preconditions.checkNotNull(filter);
+ ChangeQueryBuilder qb = queryBuilder.asUser(user.get());
+ return !queryProvider
+ .get()
+ .noFields()
+ .query(qb.parse(String.format("change:%s %s", change, filter)))
+ .isEmpty();
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java
index 6ee15ce..569373c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfig.java
@@ -24,7 +24,7 @@
import com.google.gerrit.server.git.VersionedMetaData;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
+import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
@@ -32,80 +32,129 @@
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;
-class ReviewersConfig extends VersionedMetaData {
- private static final String FILENAME = "reviewers.config";
- private static final String FILTER = "filter";
- private static final String REVIEWER = "reviewer";
- private Config cfg;
+@Singleton
+class ReviewersConfig {
+ private static final Logger log = LoggerFactory.getLogger(ReviewersConfig.class);
- interface Factory {
- ReviewersConfig create(Project.NameKey projectName);
- }
+ static final String FILENAME = "reviewers.config";
+ static final String SECTION_FILTER = "filter";
+ static final String KEY_REVIEWER = "reviewer";
+ private static final String KEY_IGNORE_DRAFTS = "ignoreDrafts";
+ private static final String KEY_ENABLE_REST = "enableREST";
+ private static final String KEY_ENABLE_UI = "enableUI";
+ private static final String KEY_SUGGEST_ONLY = "suggestOnly";
+
+ private final PluginConfigFactory cfgFactory;
+ private final String pluginName;
+
+ private final boolean enableUI;
+ private final boolean enableREST;
+ private final boolean suggestOnly;
+ private final boolean ignoreDrafts;
@Inject
- ReviewersConfig(
- PluginConfigFactory cfgFactory,
- @PluginName String pluginName,
- @Assisted Project.NameKey projectName)
- throws NoSuchProjectException {
- cfg = cfgFactory.getProjectPluginConfigWithInheritance(projectName, pluginName);
+ ReviewersConfig(PluginConfigFactory cfgFactory, @PluginName String pluginName) {
+ this.cfgFactory = cfgFactory;
+ this.pluginName = pluginName;
+ Config cfg = cfgFactory.getGlobalPluginConfig(pluginName);
+ this.ignoreDrafts = cfg.getBoolean(pluginName, null, KEY_IGNORE_DRAFTS, false);
+ this.enableREST = cfg.getBoolean(pluginName, null, KEY_ENABLE_REST, true);
+ this.enableUI = enableREST ? cfg.getBoolean(pluginName, null, KEY_ENABLE_UI, true) : false;
+ this.suggestOnly = cfg.getBoolean(pluginName, null, KEY_SUGGEST_ONLY, false);
}
- List<ReviewerFilterSection> getReviewerFilterSections() {
- ImmutableList.Builder<ReviewerFilterSection> b = ImmutableList.builder();
- for (String f : cfg.getSubsections(FILTER)) {
- b.add(newReviewerFilterSection(f));
+ public ForProject forProject(Project.NameKey projectName) {
+ Config cfg;
+ try {
+ cfg = cfgFactory.getProjectPluginConfigWithMergedInheritance(projectName, pluginName);
+ } catch (NoSuchProjectException e) {
+ log.error("Unable to get config for project {}", projectName.get());
+ cfg = new Config();
}
- return b.build();
+ return new ForProject(cfg);
}
- void addReviewer(String filter, String reviewer) {
- if (!newReviewerFilterSection(filter).getReviewers().contains(reviewer)) {
- List<String> values =
- new ArrayList<>(Arrays.asList(cfg.getStringList(FILTER, filter, REVIEWER)));
- values.add(reviewer);
- cfg.setStringList(FILTER, filter, REVIEWER, values);
+ public boolean ignoreDrafts() {
+ return ignoreDrafts;
+ }
+
+ public boolean enableREST() {
+ return enableREST;
+ }
+
+ public boolean enableUI() {
+ return enableUI;
+ }
+
+ public boolean suggestOnly() {
+ return suggestOnly;
+ }
+
+ static class ForProject extends VersionedMetaData {
+ private Config cfg;
+
+ ForProject(Config cfg) {
+ this.cfg = cfg;
}
- }
- void removeReviewer(String filter, String reviewer) {
- if (newReviewerFilterSection(filter).getReviewers().contains(reviewer)) {
- List<String> values =
- new ArrayList<>(Arrays.asList(cfg.getStringList(FILTER, filter, REVIEWER)));
- values.remove(reviewer);
- if (values.isEmpty()) {
- cfg.unsetSection(FILTER, filter);
- } else {
- cfg.setStringList(FILTER, filter, REVIEWER, values);
+ List<ReviewerFilterSection> getReviewerFilterSections() {
+ ImmutableList.Builder<ReviewerFilterSection> b = ImmutableList.builder();
+ for (String f : cfg.getSubsections(SECTION_FILTER)) {
+ b.add(newReviewerFilterSection(f));
+ }
+ return b.build();
+ }
+
+ void addReviewer(String filter, String reviewer) {
+ if (!newReviewerFilterSection(filter).getReviewers().contains(reviewer)) {
+ List<String> values =
+ new ArrayList<>(Arrays.asList(cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER)));
+ values.add(reviewer);
+ cfg.setStringList(SECTION_FILTER, filter, KEY_REVIEWER, values);
}
}
- }
- private ReviewerFilterSection newReviewerFilterSection(String filter) {
- ImmutableSet.Builder<String> b = ImmutableSet.builder();
- for (String reviewer : cfg.getStringList(FILTER, filter, REVIEWER)) {
- b.add(reviewer);
+ void removeReviewer(String filter, String reviewer) {
+ if (newReviewerFilterSection(filter).getReviewers().contains(reviewer)) {
+ List<String> values =
+ new ArrayList<>(Arrays.asList(cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER)));
+ values.remove(reviewer);
+ if (values.isEmpty()) {
+ cfg.unsetSection(SECTION_FILTER, filter);
+ } else {
+ cfg.setStringList(SECTION_FILTER, filter, KEY_REVIEWER, values);
+ }
+ }
}
- return new ReviewerFilterSection(filter, b.build());
- }
- @Override
- protected String getRefName() {
- return RefNames.REFS_CONFIG;
- }
-
- @Override
- protected void onLoad() throws IOException, ConfigInvalidException {
- cfg = readConfig(FILENAME);
- }
-
- @Override
- protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
- if (Strings.isNullOrEmpty(commit.getMessage())) {
- commit.setMessage("Update reviewers configuration\n");
+ private ReviewerFilterSection newReviewerFilterSection(String filter) {
+ ImmutableSet.Builder<String> b = ImmutableSet.builder();
+ for (String reviewer : cfg.getStringList(SECTION_FILTER, filter, KEY_REVIEWER)) {
+ b.add(reviewer);
+ }
+ return new ReviewerFilterSection(filter, b.build());
}
- saveConfig(FILENAME, cfg);
- return true;
+
+ @Override
+ protected String getRefName() {
+ return RefNames.REFS_CONFIG;
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ cfg = readConfig(FILENAME);
+ }
+
+ @Override
+ protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
+ if (Strings.isNullOrEmpty(commit.getMessage())) {
+ commit.setMessage("Update reviewers configuration\n");
+ }
+ saveConfig(FILENAME, cfg);
+ return true;
+ }
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
new file mode 100644
index 0000000..c3fb00c
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolver.java
@@ -0,0 +1,185 @@
+// 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.googlesource.gerrit.plugins.reviewers;
+
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Sets;
+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.project.NoSuchProjectException;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+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;
+
+/* Resolve account and group names to account ids */
+@Singleton
+class ReviewersResolver {
+ private static final Logger log = LoggerFactory.getLogger(ReviewersResolver.class);
+
+ private final AccountResolver accountResolver;
+ private final Provider<GroupsCollection> groupsCollection;
+ private final GroupMembers.Factory groupMembersFactory;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
+
+ @Inject
+ ReviewersResolver(
+ AccountResolver accountResolver,
+ Provider<GroupsCollection> groupsCollection,
+ GroupMembers.Factory groupMembersFactory,
+ IdentifiedUser.GenericFactory identifiedUserFactory) {
+ this.accountResolver = accountResolver;
+ this.groupsCollection = groupsCollection;
+ this.groupMembersFactory = groupMembersFactory;
+ this.identifiedUserFactory = identifiedUserFactory;
+ }
+
+ /**
+ * 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 names the set of account names to convert
+ * @param project the project name
+ * @param changeNumber the change Id
+ * @param uploader account to use to look up groups, or null if groups are not needed
+ * @return set of {@link com.google.gerrit.reviewdb.client.Account.Id}s.
+ */
+ @VisibleForTesting
+ Set<Account.Id> resolve(
+ Set<String> names, Project.NameKey project, int changeNumber, @Nullable AccountInfo uploader)
+ throws IOException {
+ 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);
+ }
+ }
+ return reviewers;
+ }
+
+ private boolean resolveAccount(
+ Project.NameKey project,
+ int changeNumber,
+ AccountInfo uploader,
+ Set<Account.Id> reviewers,
+ String accountName)
+ throws IOException {
+ try {
+ Account account = accountResolver.find(accountName);
+ if (account != null && account.isActive()) {
+ if (uploader == null || uploader._accountId != account.getId().get()) {
+ reviewers.add(account.getId());
+ }
+ return true;
+ }
+ } 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);
+ return true;
+ }
+ return false;
+ }
+
+ private void resolveGroup(
+ Project.NameKey project,
+ int changeNumber,
+ Set<Account.Id> reviewers,
+ GroupMembers groupMembers,
+ String group) {
+ try {
+ Set<Account.Id> accounts =
+ groupMembers
+ .listAccounts(groupsCollection.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,
+ e);
+ } catch (NoSuchProjectException | IOException | OrmException e) {
+ log.warn(
+ "For the change {} of project {}: failed to list accounts for group {}.",
+ changeNumber,
+ project,
+ group,
+ e);
+ }
+ }
+
+ 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/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index 491e134..cbc531c 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 or Maven.
+This plugin can be 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
@@ -19,6 +19,12 @@
bazel-genfiles/@PLUGIN@.jar
```
+To execute the tests run:
+
+```
+ bazel test //...
+```
+
This project can be imported into the Eclipse IDE:
```
@@ -37,6 +43,12 @@
bazel-genfiles/plugins/@PLUGIN@/@PLUGIN@.jar
```
+To execute the tests run:
+
+```
+ bazel test --test_tag_filters=@PLUGIN@
+```
+
This project can be imported into the Eclipse IDE.
Add the plugin name to the `CUSTOM_PLUGINS` set in
Gerrit core in `tools/bzl/plugins.bzl`, and execute:
@@ -45,20 +57,5 @@
./tools/eclipse/project.py
```
-Maven
------
-
-Note that the Maven build is provided for compatibility reasons, but
-it is considered to be deprecated and will be removed in a future
-version of this plugin.
-
-To build with Maven, run
-
-```
- mvn clean package
-```
-
-When building with Maven, the Gerrit Plugin API must be available.
-
How to build the Gerrit Plugin API is described in the [Gerrit
documentation](../../../Documentation/dev-bazel.html#_extension_and_plugin_api_jar_files).
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index e2112fb..eb77417 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -8,6 +8,7 @@
[reviewers]
enableREST = true
enableUI = false
+ suggestOnly = false
```
reviewers.enableREST
@@ -18,6 +19,13 @@
: 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.suggestOnly
+: Provide the configured reviewers as suggestions in the "Add Reviewer" dialog
+ instead of automatically adding them to the change. Only supports accounts;
+ groups are not suggested. Defaults to false. By default Gerrit will consider
+ 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
@@ -58,4 +66,4 @@
```
1. Push a change for review involving file "build/modules/GLOBAL.pm".
-2. Both john.doe@example.com and jane.doe@example.com get added as reviewers.
+2. Both john.doe@example.com and jane.doe@example.com get added or suggested as reviewers.
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
new file mode 100644
index 0000000..cf3db58
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersConfigIT.java
@@ -0,0 +1,118 @@
+// 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.googlesource.gerrit.plugins.reviewers;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.GitUtil.fetch;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.FILENAME;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.SECTION_FILTER;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+@TestPlugin(name = "reviewers", sysModule = "com.googlesource.gerrit.plugins.reviewers.Module")
+public class ReviewersConfigIT extends LightweightPluginDaemonTest {
+ private static final String BRANCH_MAIN = "branch:main";
+ private static final String NO_FILTER = "*";
+ private static final String JANE_DOE = "jane.doe@example.com";
+ 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");
+ }
+
+ @Test
+ public void reviewersConfigSingle() throws Exception {
+ Config cfg = new Config();
+ cfg.setString(SECTION_FILTER, NO_FILTER, KEY_REVIEWER, JOHN_DOE);
+ cfg.setString(SECTION_FILTER, BRANCH_MAIN, KEY_REVIEWER, JANE_DOE);
+
+ pushFactory
+ .create(db, admin.getIdent(), testRepo, "Add reviewers", FILENAME, cfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+
+ assertThat(reviewersConfig().forProject(project).getReviewerFilterSections())
+ .containsExactlyElementsIn(
+ ImmutableList.of(
+ new ReviewerFilterSection(NO_FILTER, ImmutableSet.of(JOHN_DOE)),
+ new ReviewerFilterSection(BRANCH_MAIN, ImmutableSet.of(JANE_DOE))))
+ .inOrder();
+ }
+
+ @Test
+ public void reviewersConfigWithMergedInheritance() throws Exception {
+ Config parentCfg = new Config();
+ parentCfg.setString(SECTION_FILTER, NO_FILTER, KEY_REVIEWER, JOHN_DOE);
+ parentCfg.setString(SECTION_FILTER, BRANCH_MAIN, KEY_REVIEWER, JOHN_DOE);
+
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ testRepo,
+ "Add reviewers parent project",
+ FILENAME,
+ parentCfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+
+ Project.NameKey childProject = createProject("child", project);
+ TestRepository<?> childTestRepo = cloneProject(childProject);
+ fetch(childTestRepo, RefNames.REFS_CONFIG + ":refs/heads/config");
+ childTestRepo.reset("refs/heads/config");
+
+ Config cfg = new Config();
+ cfg.setString(SECTION_FILTER, NO_FILTER, KEY_REVIEWER, JANE_DOE);
+ cfg.setString(SECTION_FILTER, BRANCH_MAIN, KEY_REVIEWER, JANE_DOE);
+
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(),
+ childTestRepo,
+ "Add reviewers child project",
+ FILENAME,
+ cfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+
+ assertThat(reviewersConfig().forProject(childProject).getReviewerFilterSections())
+ .containsExactlyElementsIn(
+ ImmutableList.of(
+ new ReviewerFilterSection(NO_FILTER, ImmutableSet.of(JOHN_DOE, JANE_DOE)),
+ new ReviewerFilterSection(BRANCH_MAIN, ImmutableSet.of(JOHN_DOE, JANE_DOE))))
+ .inOrder();
+ }
+
+ private ReviewersConfig reviewersConfig() {
+ return plugin.getSysInjector().getInstance(ReviewersConfig.class);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
new file mode 100644
index 0000000..f46ca74
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersIT.java
@@ -0,0 +1,153 @@
+// 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.googlesource.gerrit.plugins.reviewers;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.acceptance.GitUtil.fetch;
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.FILENAME;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.ReviewersConfig.SECTION_FILTER;
+import static java.util.stream.Collectors.toSet;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.RefNames;
+import java.util.Collection;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+@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");
+ }
+
+ @Test
+ public void addReviewers() throws Exception {
+ RevCommit oldHead = getRemoteHead();
+ TestAccount user2 = accountCreator.user2();
+
+ Config cfg = new Config();
+ cfg.setStringList(SECTION_FILTER, "*", KEY_REVIEWER, ImmutableList.of(user.email, 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();
+
+ Config cfg = new Config();
+ cfg.setString(SECTION_FILTER, "branch:master", KEY_REVIEWER, user.email);
+
+ pushFactory
+ .create(db, admin.getIdent(), testRepo, "Add reviewers", FILENAME, cfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+
+ testRepo.reset(oldHead);
+
+ createBranch(new Branch.NameKey(project, "other-branch"));
+
+ // Create a change that matches the filter section.
+ createChange("refs/for/master");
+
+ // The actual change we want to test
+ String changeId = createChange("refs/for/other-branch").getChangeId();
+
+ Collection<AccountInfo> reviewers;
+ long wait = 0;
+ do {
+ Thread.sleep(10);
+ wait += 10;
+ reviewers = gApi.changes().id(changeId).get().reviewers.get(REVIEWER);
+ } while (reviewers == null && wait < 100);
+
+ assertThat(reviewers).isNull();
+ }
+
+ @Test
+ public void addReviewersFromMatchingFilters() throws Exception {
+ RevCommit oldHead = getRemoteHead();
+
+ Config cfg = new Config();
+ cfg.setString(SECTION_FILTER, "branch:other-branch", KEY_REVIEWER, user.email);
+
+ pushFactory
+ .create(db, admin.getIdent(), testRepo, "Add reviewers", FILENAME, cfg.toText())
+ .to(RefNames.REFS_CONFIG)
+ .assertOkStatus();
+
+ testRepo.reset(oldHead);
+
+ createBranch(new Branch.NameKey(project, "other-branch"));
+
+ // Create a change that doesn't match the filter section.
+ createChange("refs/for/master");
+
+ // The actual change we want to test
+ String changeId = createChange("refs/for/other-branch").getChangeId();
+
+ Collection<AccountInfo> reviewers;
+ long wait = 0;
+ do {
+ Thread.sleep(10);
+ wait += 10;
+ reviewers = gApi.changes().id(changeId).get().reviewers.get(REVIEWER);
+ } while (reviewers == null && wait < 100);
+
+ assertThat(reviewers.stream().map(a -> a._accountId).collect(toSet()))
+ .containsExactlyElementsIn(ImmutableSet.of(admin.id.get(), user.id.get()));
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java
new file mode 100644
index 0000000..c5b89ed
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/ReviewersResolverIT.java
@@ -0,0 +1,88 @@
+// 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.googlesource.gerrit.plugins.reviewers;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.inject.Inject;
+import java.util.Collections;
+import java.util.Set;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+public class ReviewersResolverIT extends AbstractDaemonTest {
+
+ @Inject private ReviewersResolver resolver;
+ private int change;
+
+ @Before
+ public void setUp() {
+ change = 1;
+ }
+
+ @Test
+ public void testUploaderSkippedAsReviewer() throws Exception {
+ Set<Account.Id> reviewers =
+ resolver.resolve(
+ Collections.singleton(user.email),
+ project,
+ change,
+ gApi.accounts().id(user.id.get()).get());
+ assertThat(reviewers).isEmpty();
+ }
+
+ @Test
+ public void testAccountResolve() throws Exception {
+ Set<Account.Id> reviewers =
+ resolver.resolve(
+ ImmutableSet.of(user.email, admin.email),
+ project,
+ change,
+ gApi.accounts().id(admin.id.get()).get());
+ assertThat(reviewers).containsExactly(user.id);
+ }
+
+ @Test
+ public void testAccountGroupResolve() throws Exception {
+ String group1 = createGroup("group1");
+ TestAccount foo = createTestAccount("foo", group1);
+ TestAccount bar = createTestAccount("bar", group1);
+
+ String group2 = createGroup("group2");
+ TestAccount baz = createTestAccount("baz", group2);
+ TestAccount qux = createTestAccount("qux", group2);
+
+ TestAccount system = createTestAccount("system", "Administrators");
+
+ Set<Account.Id> reviewers =
+ resolver.resolve(
+ ImmutableSet.of(system.email, group1, group2),
+ project,
+ change,
+ gApi.accounts().id(admin.id.get()).get());
+ assertThat(reviewers).containsExactly(system.id, foo.id, bar.id, baz.id, qux.id);
+ }
+
+ private TestAccount createTestAccount(String name, String group) throws Exception {
+ name = name(name);
+ return accountCreator.create(name, name + "@example.com", name + " full name", group);
+ }
+}
diff --git a/tools/bzl/junit.bzl b/tools/bzl/junit.bzl
new file mode 100644
index 0000000..3af7e58
--- /dev/null
+++ b/tools/bzl/junit.bzl
@@ -0,0 +1,4 @@
+load(
+ "@com_googlesource_gerrit_bazlets//tools:junit.bzl",
+ "junit_tests",
+)
diff --git a/tools/bzl/plugin.bzl b/tools/bzl/plugin.bzl
index 74d4ce1..2ee6681 100644
--- a/tools/bzl/plugin.bzl
+++ b/tools/bzl/plugin.bzl
@@ -2,5 +2,6 @@
"@com_googlesource_gerrit_bazlets//:gerrit_plugin.bzl",
"gerrit_plugin",
"PLUGIN_DEPS",
+ "PLUGIN_TEST_DEPS",
"GWT_PLUGIN_DEPS",
)
diff --git a/tools/eclipse/BUILD b/tools/eclipse/BUILD
index 25b845e..a6cc8cb 100644
--- a/tools/eclipse/BUILD
+++ b/tools/eclipse/BUILD
@@ -1,13 +1,13 @@
load(
"//tools/bzl:plugin.bzl",
- "PLUGIN_DEPS",
"GWT_PLUGIN_DEPS",
)
load("//tools/bzl:classpath.bzl", "classpath_collector")
classpath_collector(
name = "main_classpath_collect",
- deps = PLUGIN_DEPS + GWT_PLUGIN_DEPS + [
- "//:reviewers__plugin",
+ testonly = 1,
+ deps = GWT_PLUGIN_DEPS + [
+ "//:reviewers__plugin_test_deps",
],
)
