Match change predicates without using ChangeIndex
In the reviewers logic, we want to know if a configured filter
applies to a change. If so, we will add reviewers or CCs.
The trigger for this logic is a change update. Hence, we only
want to know if the filter applies to that change. We can do this
by just matching predicates directly and spare the query to the
change index thereby reducing latency.
Change-Id: Icd35b54eb6b0dad6eb357e7af75ab92aec9e3200
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java
index 088d304..e6d467e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewerSuggest.java
@@ -25,6 +25,7 @@
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.change.ReviewerSuggestion;
import com.google.gerrit.server.change.SuggestedReviewer;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.reviewers.config.FiltersFactory;
@@ -38,13 +39,18 @@
private final FiltersFactory filters;
private final ReviewersFilterUtil util;
private final ReviewersResolver resolver;
+ private final ChangeData.Factory changeDataFactory;
@Inject
public ReviewerSuggest(
- FiltersFactory filters, ReviewersFilterUtil filterUtil, ReviewersResolver resolver) {
+ FiltersFactory filters,
+ ReviewersFilterUtil filterUtil,
+ ReviewersResolver resolver,
+ ChangeData.Factory changeDataFactory) {
this.filters = filters;
this.util = filterUtil;
this.resolver = resolver;
+ this.changeDataFactory = changeDataFactory;
}
@Override
@@ -60,7 +66,8 @@
}
try {
- Set<String> reviewers = util.findReviewers(changeId.get(), sections);
+ ChangeData cd = changeDataFactory.create(project, changeId);
+ Set<String> reviewers = util.findReviewers(cd, sections);
if (!reviewers.isEmpty()) {
return resolver.resolve(reviewers, project, changeId.get(), null, false).stream()
.map(a -> suggestedReviewer(a))
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 aaf7c36..188cb77 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/Reviewers.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.reviewers;
import com.google.common.flogger.FluentLogger;
+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;
@@ -24,6 +25,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.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.reviewers.config.FiltersFactory;
@@ -45,6 +47,7 @@
private final GlobalConfig config;
private final FiltersFactory filters;
private final ReviewersFilterUtil filterUtil;
+ private final ChangeData.Factory changeDataFactory;
@Inject
Reviewers(
@@ -53,13 +56,15 @@
ReviewerWorkQueue workQueue,
GlobalConfig config,
FiltersFactory filters,
- ReviewersFilterUtil util) {
+ ReviewersFilterUtil util,
+ ChangeData.Factory changeDataFactory) {
this.resolver = resolver;
this.addReviewersFactory = addReviewersFactory;
this.workQueue = workQueue;
this.config = config;
this.filters = filters;
this.filterUtil = util;
+ this.changeDataFactory = changeDataFactory;
}
@Override
@@ -102,8 +107,9 @@
AccountInfo uploader = event.getWho();
int changeNumber = c._number;
try {
- Set<String> reviewers = filterUtil.findReviewers(changeNumber, filters);
- Set<String> ccs = filterUtil.findCcs(changeNumber, filters);
+ ChangeData cd = changeDataFactory.create(Project.nameKey(c.project), Change.id(c._number));
+ Set<String> reviewers = filterUtil.findReviewers(cd, filters);
+ Set<String> ccs = filterUtil.findCcs(cd, filters);
if (reviewers.isEmpty() && ccs.isEmpty()) {
return;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersFilterUtil.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersFilterUtil.java
index 5f0574c..4871e1e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersFilterUtil.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ReviewersFilterUtil.java
@@ -21,8 +21,8 @@
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.ChangeData;
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;
@@ -30,59 +30,49 @@
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) {
+ public ReviewersFilterUtil(ChangeQueryBuilder queryBuilder, Provider<CurrentUser> user) {
this.queryBuilder = queryBuilder;
- this.queryProvider = queryProvider;
this.user = user;
}
- public Set<String> findReviewers(int change, List<ReviewerFilter> filters)
+ public Set<String> findReviewers(ChangeData cd, List<ReviewerFilter> filters)
throws StorageException, QueryParseException {
Set<String> reviewers = Sets.newHashSet();
- List<ReviewerFilter> found = findReviewerFilters(change, filters);
+ List<ReviewerFilter> found = findReviewerFilters(cd, filters);
for (ReviewerFilter s : found) {
reviewers.addAll(s.getReviewers());
}
return reviewers;
}
- public Set<String> findCcs(int change, List<ReviewerFilter> filters)
+ public Set<String> findCcs(ChangeData cd, List<ReviewerFilter> filters)
throws StorageException, QueryParseException {
Set<String> ccs = Sets.newHashSet();
- List<ReviewerFilter> found = findReviewerFilters(change, filters);
+ List<ReviewerFilter> found = findReviewerFilters(cd, filters);
for (ReviewerFilter s : found) {
ccs.addAll(s.getCcs());
}
return ccs;
}
- private List<ReviewerFilter> findReviewerFilters(int change, List<ReviewerFilter> sections)
+ private List<ReviewerFilter> findReviewerFilters(ChangeData cd, 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())) {
+ } else if (filterMatch(cd, s.getFilter())) {
found.add(s);
}
}
return found.build();
}
- boolean filterMatch(int change, String filter) throws StorageException, QueryParseException {
+ boolean filterMatch(ChangeData cd, 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();
+ return queryBuilder.asUser(user.get()).parse(filter).asMatchable().match(cd);
}
}
