Merge branch 'stable-3.3' into stable-3.4
* stable-3.3:
Count uploader as a resolved reviewer
UI: Don't suggest unsupported system groups
Document that configured groups must be visible to uploader
Bazel: Remove unnecessary plugin_name parameter
Change-Id: I7090c8b547deebd094a679b1d5a3c1a6ca1b2c49
diff --git a/BUILD b/BUILD
index 6f5f9dc..52f43b1 100644
--- a/BUILD
+++ b/BUILD
@@ -1,5 +1,4 @@
load("@rules_java//java:defs.bzl", "java_library")
-load("@npm//@bazel/rollup:index.bzl", "rollup_bundle")
load("//tools/bzl:junit.bzl", "junit_tests")
load("//tools/js:eslint.bzl", "eslint")
load(
@@ -8,8 +7,7 @@
"PLUGIN_TEST_DEPS",
"gerrit_plugin",
)
-load("//tools/bzl:genrule2.bzl", "genrule2")
-load("//tools/bzl:js.bzl", "polygerrit_plugin")
+load("//tools/bzl:js.bzl", "gerrit_js_bundle")
gerrit_plugin(
name = "reviewers",
@@ -18,37 +16,14 @@
"Gerrit-PluginName: reviewers",
"Gerrit-Module: com.googlesource.gerrit.plugins.reviewers.Module",
],
- resource_jars = [":rv-reviewers-static"],
+ resource_jars = [":rv-reviewers"],
resources = glob(["src/main/resources/**/*"]),
)
-genrule2(
- name = "rv-reviewers-static",
- srcs = [":rv-reviewers"],
- outs = ["rv-reviewers-static.jar"],
- cmd = " && ".join([
- "mkdir $$TMP/static",
- "cp -r $(locations :rv-reviewers) $$TMP/static",
- "cd $$TMP",
- "zip -Drq $$ROOT/$@ -g .",
- ]),
-)
-
-polygerrit_plugin(
+gerrit_js_bundle(
name = "rv-reviewers",
- app = "reviewers-bundle.js",
-)
-
-rollup_bundle(
- name = "reviewers-bundle",
srcs = glob(["rv-reviewers/*.js"]),
entry_point = "rv-reviewers/plugin.js",
- format = "iife",
- rollup_bin = "//tools/node_tools:rollup-bin",
- sourcemap = "hidden",
- deps = [
- "@tools_npm//rollup-plugin-node-resolve",
- ],
)
junit_tests(
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 8de2cc4..80b9365 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Module.java
@@ -60,7 +60,7 @@
protected void configure() {
bind(ReviewerSuggestion.class)
.annotatedWith(Exports.named("reviewer-suggest"))
- .to(Reviewers.class);
+ .to(ReviewerSuggest.class);
}
});
} else {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java
new file mode 100644
index 0000000..3751548
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java
@@ -0,0 +1,81 @@
+// Copyright (C) 2020 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.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change.Id;
+import com.google.gerrit.entities.Project.NameKey;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.change.ReviewerSuggestion;
+import com.google.gerrit.server.change.SuggestedReviewer;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig;
+import java.util.List;
+import java.util.Set;
+
+@Singleton
+public class ReviewerSuggest implements ReviewerSuggestion {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final ReviewersConfig config;
+ private final ReviewersFilterUtil util;
+ private final ReviewersResolver resolver;
+
+ @Inject
+ public ReviewerSuggest(
+ ReviewersConfig config, ReviewersFilterUtil filterUtil, ReviewersResolver resolver) {
+ this.config = config;
+ this.util = filterUtil;
+ this.resolver = resolver;
+ }
+
+ @Override
+ public Set<SuggestedReviewer> suggestReviewers(
+ NameKey project,
+ Id changeId,
+ String query,
+ Set<com.google.gerrit.entities.Account.Id> candidates) {
+ List<ReviewerFilter> sections = config.filtersWithInheritance(project);
+
+ if (sections.isEmpty() || changeId == null) {
+ return ImmutableSet.of();
+ }
+
+ try {
+ Set<String> reviewers = util.findReviewers(changeId.get(), sections);
+ if (!reviewers.isEmpty()) {
+ return resolver.resolve(reviewers, project, changeId.get(), null).stream()
+ .map(a -> suggestedReviewer(a))
+ .collect(toSet());
+ }
+ } catch (StorageException | QueryParseException x) {
+ logger.atSevere().withCause(x).log(x.getMessage());
+ }
+ return ImmutableSet.of();
+ }
+
+ private SuggestedReviewer suggestedReviewer(Account.Id account) {
+ SuggestedReviewer reviewer = new SuggestedReviewer();
+ reviewer.account = account;
+ reviewer.score = 1;
+ return reviewer;
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
index 348e856..a3b96ab 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -14,17 +14,7 @@
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.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -34,13 +24,7 @@
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
import com.google.gerrit.index.query.QueryParseException;
-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.query.change.ChangeQueryBuilder;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.reviewers.config.ReviewersConfig;
import java.util.List;
@@ -51,17 +35,14 @@
class Reviewers
implements RevisionCreatedListener,
PrivateStateChangedListener,
- WorkInProgressStateChangedListener,
- ReviewerSuggestion {
+ WorkInProgressStateChangedListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ReviewersResolver resolver;
private final AddReviewers.Factory addReviewersFactory;
private final ReviewerWorkQueue workQueue;
private final ReviewersConfig config;
- private final Provider<CurrentUser> user;
- private final ChangeQueryBuilder queryBuilder;
- private final Provider<InternalChangeQuery> queryProvider;
+ private final ReviewersFilterUtil filterUtil;
@Inject
Reviewers(
@@ -69,16 +50,12 @@
AddReviewers.Factory addReviewersFactory,
ReviewerWorkQueue workQueue,
ReviewersConfig config,
- Provider<CurrentUser> user,
- ChangeQueryBuilder queryBuilder,
- Provider<InternalChangeQuery> queryProvider) {
+ ReviewersFilterUtil util) {
this.resolver = resolver;
this.addReviewersFactory = addReviewersFactory;
this.workQueue = workQueue;
this.config = config;
- this.user = user;
- this.queryBuilder = queryBuilder;
- this.queryProvider = queryProvider;
+ this.filterUtil = util;
}
@Override
@@ -96,38 +73,6 @@
onEvent(event);
}
- @Override
- public Set<SuggestedReviewer> suggestReviewers(
- Project.NameKey projectName,
- @Nullable Change.Id changeId,
- @Nullable String query,
- Set<Account.Id> candidates) {
- List<ReviewerFilter> filters = getFilters(projectName);
-
- if (filters.isEmpty() || changeId == null) {
- return ImmutableSet.of();
- }
-
- try {
- Set<String> reviewers = findReviewers(changeId.get(), filters);
- if (!reviewers.isEmpty()) {
- return resolver.resolve(reviewers, projectName, changeId.get(), null).stream()
- .map(a -> suggestedReviewer(a))
- .collect(toSet());
- }
- } catch (StorageException | QueryParseException x) {
- logger.atSevere().withCause(x).log(x.getMessage());
- }
- return ImmutableSet.of();
- }
-
- private SuggestedReviewer suggestedReviewer(Account.Id account) {
- SuggestedReviewer reviewer = new SuggestedReviewer();
- reviewer.account = account;
- reviewer.score = 1;
- return reviewer;
- }
-
private List<ReviewerFilter> getFilters(Project.NameKey projectName) {
// TODO(davido): we have to cache per project configuration
return config.filtersWithInheritance(projectName);
@@ -153,9 +98,8 @@
AccountInfo uploader = event.getWho();
int changeNumber = c._number;
try {
- List<ReviewerFilter> matching = findMatchingFilters(changeNumber, filters);
- Set<String> reviewers = getReviewersFrom(matching);
- Set<String> ccs = getCcsFrom(matching);
+ Set<String> reviewers = filterUtil.findReviewers(changeNumber, filters);
+ Set<String> ccs = filterUtil.findCcs(changeNumber, filters);
if (reviewers.isEmpty() && ccs.isEmpty()) {
return;
}
@@ -176,48 +120,4 @@
logger.atSevere().withCause(x).log(x.getMessage());
}
}
-
- private Set<String> findReviewers(int change, List<ReviewerFilter> filters)
- throws StorageException, QueryParseException {
- return getReviewersFrom(findMatchingFilters(change, filters));
- }
-
- private Set<String> getReviewersFrom(List<ReviewerFilter> filters) throws StorageException {
- ImmutableSet.Builder<String> reviewers = ImmutableSet.builder();
- for (ReviewerFilter f : filters) {
- reviewers.addAll(f.getReviewers());
- }
- return reviewers.build();
- }
-
- private Set<String> getCcsFrom(List<ReviewerFilter> filters) throws StorageException {
- Set<String> ccs = Sets.newHashSet();
- for (ReviewerFilter f : filters) {
- ccs.addAll(f.getCcs());
- }
- return ccs;
- }
-
- private List<ReviewerFilter> findMatchingFilters(int change, List<ReviewerFilter> filters)
- throws StorageException, QueryParseException {
- ImmutableList.Builder<ReviewerFilter> found = ImmutableList.builder();
- for (ReviewerFilter s : filters) {
- 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 StorageException, 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/ReviewersFilterUtil.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersFilterUtil.java
new file mode 100644
index 0000000..5f0574c
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersFilterUtil.java
@@ -0,0 +1,88 @@
+// Copyright (C) 2020 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.Sets;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.List;
+import java.util.Set;
+
+public class ReviewersFilterUtil {
+ private final ChangeQueryBuilder queryBuilder;
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final Provider<CurrentUser> user;
+
+ @Inject
+ public ReviewersFilterUtil(
+ ChangeQueryBuilder queryBuilder,
+ Provider<InternalChangeQuery> queryProvider,
+ Provider<CurrentUser> user) {
+ this.queryBuilder = queryBuilder;
+ this.queryProvider = queryProvider;
+ this.user = user;
+ }
+
+ public Set<String> findReviewers(int change, List<ReviewerFilter> filters)
+ throws StorageException, QueryParseException {
+ Set<String> reviewers = Sets.newHashSet();
+ List<ReviewerFilter> found = findReviewerFilters(change, filters);
+ for (ReviewerFilter s : found) {
+ reviewers.addAll(s.getReviewers());
+ }
+ return reviewers;
+ }
+
+ public Set<String> findCcs(int change, List<ReviewerFilter> filters)
+ throws StorageException, QueryParseException {
+ Set<String> ccs = Sets.newHashSet();
+ List<ReviewerFilter> found = findReviewerFilters(change, filters);
+ for (ReviewerFilter s : found) {
+ ccs.addAll(s.getCcs());
+ }
+ return ccs;
+ }
+
+ private List<ReviewerFilter> findReviewerFilters(int change, List<ReviewerFilter> sections)
+ throws StorageException, QueryParseException {
+ ImmutableList.Builder<ReviewerFilter> found = ImmutableList.builder();
+ for (ReviewerFilter 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 StorageException, 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();
+ }
+}
