Merge branch 'stable-2.15'
* stable-2.15: (43 commits)
Simplify client module
Split reviewers plugin into GWT and non-GWT variants
Update bazlets to latest revision on stable-2.15
Add modify reviewers config plugin capability
Fix classpath generation for Eclipse project
Update bazlets to latest revision on stable-2.15
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.15 to use 2.15.1 API
Update bazlets to latest stable-2.14 and switch to 2.14.8 release API
Fix compilation warnings flagged by Eclipse
Upgrade bazlets and use 2.15 release API
Reviewers: Restrict index query to the given change
Build with 2.15-SNAPSHOT API
Update bazlets to latest on stable-2.15
Add integration test for adding reviewers
ReviewersConfig: Add unit test for merged project inheritance
ReviewerFilterSection: Add equals and hashCode methods
ReviewersResolver#resolveGroup: Simplify error handling
Remove stale documentation how to build with maven
Add reviewers resolver test
ReviewersResolver#resolve: Extract logic in small methods
Reviewers: Extract account resolver logic to it own class
Add members of all matching groups
ReviewersConfig: merge parent reviewers.config
Update bazlets to latest stable-2.14 to use 2.14.8-SNAPSHOT API
Update bazlets to the latest on stable-2.14 to use 2.14.7 release API
Reviewers: Don't create ChangeData instances
Reviewers: Actually use change index for filter query
Refactor ReviewersConfig to also handle the global config
Rename DefaultReviewers to AddReviewersByConfiguration
Factor addition of reviewers into own Runnable class
Reviewers: Don't add change uploader as reviewer
Reviewers: Only consider active accounts
Reviewers: Pass RevisionEvent to onEvent method
Add project name & change Id to the logs
Reviewers: Use log built-in string formatting
Handle the QueryParseException when querying a group that does not exist
Reviewers#suggestReviewers: Add null check on changeId
Add support for reviewer suggestion
ChangeEventListener: Remove unnecessary final modifiers
ChangeEventListener: Don't unnecessarily open the repository
Change-Id: I74e11c3f770ecf923b78a1e792a59365e8a062a5
diff --git a/BUILD b/BUILD
index 96f7151..fc5a471 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",
+ "PLUGIN_DEPS",
+ "PLUGIN_TEST_DEPS",
+ "gerrit_plugin",
+)
SRC = "src/main/java/com/googlesource/gerrit/plugins/reviewers/"
@@ -32,3 +38,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/ClientModule.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ClientModule.java
index 88c79bd..4aac38f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ClientModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ClientModule.java
@@ -14,38 +14,16 @@
package com.googlesource.gerrit.plugins.reviewers;
-import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.registration.DynamicSet;
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.inject.Inject;
-import org.eclipse.jgit.lib.Config;
+import com.google.inject.AbstractModule;
-public class ClientModule extends FactoryModule {
- private final boolean enableUI;
-
- @Inject
- public ClientModule(@PluginName String pluginName, PluginConfigFactory pluginCfgFactory) {
- this(pluginCfgFactory.getGlobalPluginConfig(pluginName));
- }
-
- public ClientModule(Config c) {
- boolean enableREST = c.getBoolean("reviewers", null, "enableREST", true);
- this.enableUI = enableREST ? c.getBoolean("reviewers", null, "enableUI", true) : false;
- }
-
- public ClientModule(boolean enableUI) {
- this.enableUI = enableUI;
- }
-
+public class ClientModule extends AbstractModule {
@Override
protected void configure() {
- if (enableUI) {
- DynamicSet.bind(binder(), TopMenu.class).to(ReviewersTopMenu.class);
- DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new GwtPlugin("reviewers"));
- }
+ DynamicSet.bind(binder(), TopMenu.class).to(ReviewersTopMenu.class);
+ DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new GwtPlugin("reviewers"));
}
}
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 ea140b5..55b663c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -14,24 +14,22 @@
package com.googlesource.gerrit.plugins.reviewers;
-import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.reviewers.server.BackendModule;
-import org.eclipse.jgit.lib.Config;
+import com.googlesource.gerrit.plugins.reviewers.server.ReviewersConfig;
public class Module extends AbstractModule {
- private final Config c;
+ private final ReviewersConfig cfg;
@Inject
- public Module(@PluginName String pluginName, PluginConfigFactory pluginCfgFactory) {
- c = pluginCfgFactory.getGlobalPluginConfig(pluginName);
+ public Module(ReviewersConfig cfg) {
+ this.cfg = cfg;
}
@Override
protected void configure() {
- install(new BackendModule(c));
- install(new ClientModule(c));
+ install(new BackendModule(cfg.enableREST(), cfg.suggestOnly()));
+ install(new ClientModule());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/AccountCapabilities.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/AccountCapabilities.java
new file mode 100644
index 0000000..397ffcb
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/AccountCapabilities.java
@@ -0,0 +1,31 @@
+// 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.client;
+
+import com.google.gerrit.plugin.client.rpc.RestApi;
+import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.user.client.rpc.AsyncCallback;
+
+public class AccountCapabilities extends JavaScriptObject {
+ static String MODIFY_REVIEWERS_CONFIG = "reviewers-modifyReviewersConfig";
+
+ public static void queryPluginCapability(AsyncCallback<AccountCapabilities> cb) {
+ new RestApi("/accounts/self/capabilities").addParameter("q", MODIFY_REVIEWERS_CONFIG).get(cb);
+ }
+
+ protected AccountCapabilities() {}
+
+ public final native boolean canPerform(String name) /*-{ return this[name] ? true : false; }-*/;
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/ReviewersScreen.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/ReviewersScreen.java
index ba601ae..47bedd7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/ReviewersScreen.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/client/ReviewersScreen.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.reviewers.client;
+import static com.googlesource.gerrit.plugins.reviewers.client.AccountCapabilities.MODIFY_REVIEWERS_CONFIG;
+
import com.google.gerrit.client.rpc.NativeMap;
import com.google.gerrit.client.rpc.Natives;
import com.google.gerrit.plugin.client.rpc.RestApi;
@@ -48,10 +50,11 @@
}
private boolean isOwner;
+ private boolean hasModifyReviewersConfigCapability;
private String projectName;
private Set<ReviewerEntry> rEntries;
- ReviewersScreen(final String projectName) {
+ ReviewersScreen(String projectName) {
setStyleName("reviewers-panel");
this.projectName = projectName;
this.rEntries = new HashSet<>();
@@ -64,7 +67,24 @@
@Override
public void onSuccess(NativeMap<ProjectAccessInfo> result) {
isOwner = result.get(projectName).isOwner();
- display();
+ if (isOwner) {
+ display();
+ } else {
+ // TODO(davido): Find a way to run above and below requests in parallel
+ AccountCapabilities.queryPluginCapability(
+ new AsyncCallback<AccountCapabilities>() {
+
+ @Override
+ public void onSuccess(AccountCapabilities result) {
+ hasModifyReviewersConfigCapability =
+ result.canPerform(MODIFY_REVIEWERS_CONFIG);
+ display();
+ }
+
+ @Override
+ public void onFailure(Throwable caught) {}
+ });
+ }
}
@Override
@@ -124,7 +144,7 @@
doSave(Action.REMOVE, e);
}
});
- removeButton.setVisible(isOwner);
+ removeButton.setVisible(isModifiable());
HorizontalPanel p = new HorizontalPanel();
p.add(l);
@@ -161,9 +181,9 @@
reviewerBox.setText("");
}
});
- filterBox.setEnabled(isOwner);
- reviewerBox.setEnabled(isOwner);
- addButton.setEnabled(isOwner);
+ filterBox.setEnabled(isModifiable());
+ reviewerBox.setEnabled(isModifiable());
+ addButton.setEnabled(isModifiable());
Panel p = new VerticalPanel();
p.setStyleName("reviewers-inputPanel");
@@ -172,6 +192,10 @@
return p;
}
+ boolean isModifiable() {
+ return isOwner || hasModifyReviewersConfigCapability;
+ }
+
void doSave(Action action, ReviewerEntry entry) {
ChangeReviewersInput in = ChangeReviewersInput.create();
in.setAction(action);
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
new file mode 100644
index 0000000..be04ea4
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/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.server;
+
+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/server/AddReviewersByConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/AddReviewersByConfiguration.java
new file mode 100644
index 0000000..1ef93f2
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/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.server;
+
+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/server/BackendModule.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/BackendModule.java
index 08b49e0..c233d2b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/BackendModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/BackendModule.java
@@ -15,37 +15,53 @@
package com.googlesource.gerrit.plugins.reviewers.server;
import static com.google.gerrit.server.project.ProjectResource.PROJECT_KIND;
+import static com.googlesource.gerrit.plugins.reviewers.server.ModifyReviewersConfigCapability.MODIFY_REVIEWERS_CONFIG;
-import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.config.CapabilityDefinition;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule;
-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 BackendModule extends FactoryModule {
- public final boolean enableREST;
+ private final boolean enableREST;
+ private final boolean suggestOnly;
@Inject
- public BackendModule(@PluginName String pluginName, PluginConfigFactory pluginCfgFactory) {
- this(pluginCfgFactory.getGlobalPluginConfig(pluginName));
+ public BackendModule(ReviewersConfig cfg) {
+ this(cfg.enableREST(), cfg.suggestOnly());
}
- public BackendModule(Config c) {
- this.enableREST = c.getBoolean("reviewers", null, "enableREST", true);
- }
-
- public BackendModule(boolean enableREST) {
+ public BackendModule(boolean enableREST, boolean suggestOnly) {
this.enableREST = enableREST;
+ this.suggestOnly = suggestOnly;
}
@Override
protected void configure() {
- DynamicSet.bind(binder(), RevisionCreatedListener.class).to(ChangeEventListener.class);
- factory(DefaultReviewers.Factory.class);
- factory(ReviewersConfig.Factory.class);
+ bind(CapabilityDefinition.class)
+ .annotatedWith(Exports.named(MODIFY_REVIEWERS_CONFIG))
+ .to(ModifyReviewersConfigCapability.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/server/ChangeEventListener.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ChangeEventListener.java
deleted file mode 100644
index 2bf1295..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ChangeEventListener.java
+++ /dev/null
@@ -1,245 +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.server;
-
-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.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.git.GitRepositoryManager;
-import com.google.gerrit.server.git.WorkQueue;
-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.restapi.group.GroupsCollection;
-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.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 groupMembers;
- 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 groupMembers,
- 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) {
- this.accountResolver = accountResolver;
- this.groupsCollection = groupsCollection;
- this.groupMembers = groupMembers;
- 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);
- }
-
- private void onEvent(Project.NameKey projectName, int changeNumber) {
- // 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);
- 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));
-
- 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) {
- Set<Account> reviewers = Sets.newHashSetWithExpectedSize(in.size());
- 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;
- }
- try {
- reviewers.addAll(
- groupMembers.listAccounts(groupsCollection.get().parse(r).getGroupUUID(), p));
- } catch (UnprocessableEntityException 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 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/server/DefaultReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/DefaultReviewers.java
deleted file mode 100644
index 8d4cb30..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/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.server;
-
-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/server/GetReviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/GetReviewers.java
index dccc000..d8ce0ca 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/GetReviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/GetReviewers.java
@@ -14,9 +14,7 @@
package com.googlesource.gerrit.plugins.reviewers.server;
-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/server/ModifyReviewersConfigCapability.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ModifyReviewersConfigCapability.java
new file mode 100644
index 0000000..b97e422
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ModifyReviewersConfigCapability.java
@@ -0,0 +1,26 @@
+// 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.server;
+
+import com.google.gerrit.extensions.config.CapabilityDefinition;
+
+public class ModifyReviewersConfigCapability extends CapabilityDefinition {
+ static final String MODIFY_REVIEWERS_CONFIG = "modifyReviewersConfig";
+
+ @Override
+ public String getDescription() {
+ return "Modify Reviewers Config";
+ }
+}
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 3d17659..1026fbb 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
@@ -14,7 +14,10 @@
package com.googlesource.gerrit.plugins.reviewers.server;
+import static com.googlesource.gerrit.plugins.reviewers.server.ModifyReviewersConfigCapability.MODIFY_REVIEWERS_CONFIG;
+
import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.api.access.PluginPermission;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -55,7 +58,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;
@@ -65,14 +68,14 @@
@Inject
PutReviewers(
@PluginName String pluginName,
- ReviewersConfig.Factory configFactory,
+ ReviewersConfig config,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
ProjectCache projectCache,
AccountResolver accountResolver,
Provider<GroupsCollection> groupsCollection,
PermissionBackend permissionBackend) {
this.pluginName = pluginName;
- this.configFactory = configFactory;
+ this.config = config;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.projectCache = projectCache;
this.accountResolver = accountResolver;
@@ -84,18 +87,15 @@
public List<ReviewerFilterSection> apply(ProjectResource rsrc, Input input)
throws RestApiException, PermissionBackendException {
Project.NameKey projectName = rsrc.getNameKey();
- ReviewersConfig cfg = configFactory.create(projectName);
- try {
- permissionBackend
- .user(rsrc.getUser())
- .project(rsrc.getNameKey())
- .check(ProjectPermission.WRITE_CONFIG);
- } catch (AuthException e) {
+ ReviewersConfig.ForProject cfg = config.forProject(projectName);
+ if (cfg == null) {
throw new ResourceNotFoundException("Project" + projectName.get() + " not found");
}
- if (cfg == null) {
- throw new ResourceNotFoundException("Project" + projectName.get() + " not found");
+ PermissionBackend.WithUser userPermission = permissionBackend.user(rsrc.getUser());
+ 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");
}
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(projectName)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewerFilterSection.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewerFilterSection.java
index 0dd354e..7f0000a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewerFilterSection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewerFilterSection.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.reviewers.server;
+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/server/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/Reviewers.java
new file mode 100644
index 0000000..430a588
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/Reviewers.java
@@ -0,0 +1,186 @@
+// 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.server;
+
+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.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 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 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/server/ReviewersConfig.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfig.java
index b7daadf..a70fd6f 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
@@ -24,7 +24,7 @@
import com.google.gerrit.server.git.meta.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,122 @@
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
+public 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_SUGGEST_ONLY = "suggestOnly";
+
+ private final PluginConfigFactory cfgFactory;
+ private final String pluginName;
+
+ 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.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 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/server/ReviewersResolver.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolver.java
new file mode 100644
index 0000000..82c3139
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolver.java
@@ -0,0 +1,155 @@
+// 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.server;
+
+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.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.project.NoSuchProjectException;
+import com.google.gerrit.server.restapi.group.GroupsCollection;
+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 groupMembers;
+ private final IdentifiedUser.GenericFactory identifiedUserFactory;
+
+ @Inject
+ ReviewersResolver(
+ AccountResolver accountResolver,
+ Provider<GroupsCollection> groupsCollection,
+ GroupMembers groupMembers,
+ IdentifiedUser.GenericFactory identifiedUserFactory) {
+ this.accountResolver = accountResolver;
+ this.groupsCollection = groupsCollection;
+ this.groupMembers = groupMembers;
+ 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) {
+ Set<Account.Id> reviewers = Sets.newHashSetWithExpectedSize(names.size());
+ for (String name : names) {
+ if (resolveAccount(project, changeNumber, uploader, reviewers, name)) {
+ continue;
+ }
+
+ resolveGroup(project, changeNumber, reviewers, groupMembers, name);
+ }
+ return reviewers;
+ }
+
+ private boolean resolveAccount(
+ Project.NameKey project,
+ int changeNumber,
+ AccountInfo uploader,
+ Set<Account.Id> reviewers,
+ String accountName) {
+ try {
+ Account account = accountResolver.find(accountName);
+ if (account != null) {
+ if (account.isActive()) {
+ if (uploader == null || uploader._accountId != account.getId().get()) {
+ reviewers.add(account.getId());
+ }
+ return true;
+ }
+ log.warn(
+ "For the change {} of project {}: account {} 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);
+ 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 e) {
+ log.warn(
+ "For the change {} of project {}: reviewer {} is neither an account nor a group.",
+ changeNumber,
+ project,
+ group);
+ } catch (NoSuchProjectException | IOException e) {
+ log.error(
+ "For the change {} of project {}: failed to list accounts for group {}.",
+ changeNumber,
+ project,
+ group,
+ e);
+ }
+ }
+}
diff --git a/src/main/resources/Documentation/build.md b/src/main/resources/Documentation/build.md
index b2f5da6..1aac8d5 100644
--- a/src/main/resources/Documentation/build.md
+++ b/src/main/resources/Documentation/build.md
@@ -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:
@@ -52,3 +64,6 @@
UI plugin is only compatible with the GWT UI, and does not work with PolyGerrit.
Both build instructions will work with either `reviewers` or
`reviewers-backend`.
+
+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..2c3627a 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -7,17 +7,20 @@
```
[reviewers]
enableREST = true
- enableUI = false
+ suggestOnly = false
```
reviewers.enableREST
: Enable the REST API. When set to false, the REST API is not available.
Defaults to true.
-reviewers.enableUI
-: 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 +61,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/main/resources/Documentation/rest-api.md b/src/main/resources/Documentation/rest-api.md
index 6fe3733..ebd04d4 100644
--- a/src/main/resources/Documentation/rest-api.md
+++ b/src/main/resources/Documentation/rest-api.md
@@ -56,6 +56,9 @@
The change to reviewers must be provided in the request body inside
a [ConfigReviewersInput](#config-reviewers-input) entity.
+Caller must be a member of a group that is granted the 'Modify Reviewers Config'
+capability (provided by this plugin) or be a Project Owner for the project.
+
#### Request
```
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
new file mode 100644
index 0000000..32b2243
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersConfigIT.java
@@ -0,0 +1,116 @@
+// 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.server;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.GitUtil.fetch;
+import static com.googlesource.gerrit.plugins.reviewers.server.ReviewersConfig.FILENAME;
+import static com.googlesource.gerrit.plugins.reviewers.server.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.server.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
+ public void setUp() throws Exception {
+ 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/server/ReviewersIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersIT.java
new file mode 100644
index 0000000..c445e81
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersIT.java
@@ -0,0 +1,151 @@
+// 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.server;
+
+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.server.ReviewersConfig.FILENAME;
+import static com.googlesource.gerrit.plugins.reviewers.server.ReviewersConfig.KEY_REVIEWER;
+import static com.googlesource.gerrit.plugins.reviewers.server.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
+ public void setUp() throws Exception {
+ 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/server/ReviewersResolverIT.java b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/ReviewersResolverIT.java
new file mode 100644
index 0000000..c893a6c
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/reviewers/server/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.server;
+
+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..a06bb85 100644
--- a/tools/eclipse/BUILD
+++ b/tools/eclipse/BUILD
@@ -1,13 +1,15 @@
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 + [
+ "//external:gwt-dev",
+ "//external:gwt-user",
+ "//:reviewers__plugin_test_deps",
],
)
