Merge branch 'stable-3.3' into stable-3.4 * stable-3.3: Don't call 'review' with empty ReviewInput Change-Id: I9b1a0a7444f504f1b2e24c17ab18642b59a7e2e0
diff --git a/BUILD b/BUILD index b0ef90f..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,38 +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", - plugin_name = "rv-reviewers", -) - -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(); + } +}