Merge "Introduce a new submit requirement predicate that can be used in SRs"
diff --git a/java/com/google/gerrit/index/query/Predicate.java b/java/com/google/gerrit/index/query/Predicate.java
index 2791f2c..9dc7689 100644
--- a/java/com/google/gerrit/index/query/Predicate.java
+++ b/java/com/google/gerrit/index/query/Predicate.java
@@ -18,9 +18,12 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.common.collect.Iterables;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.LinkedList;
import java.util.List;
+import java.util.Queue;
/**
* An abstract predicate tree for any form of query.
@@ -146,6 +149,19 @@
return leafCount;
}
+ /** Returns a list of this predicate and all its descendants. */
+ public List<Predicate<T>> getFlattenedPredicateList() {
+ List<Predicate<T>> result = new ArrayList<>();
+ Queue<Predicate<T>> queue = new LinkedList<>();
+ queue.add(this);
+ while (!queue.isEmpty()) {
+ Predicate<T> current = queue.poll();
+ result.add(current);
+ current.getChildren().forEach(p -> queue.add(p));
+ }
+ return result;
+ }
+
/** Create a copy of this predicate, with new children. */
public abstract Predicate<T> copy(Collection<? extends Predicate<T>> children);
@@ -153,6 +169,11 @@
return this instanceof Matchable;
}
+ /** Whether this predicate can be used in search queries. */
+ public boolean supportedForQueries() {
+ return true;
+ }
+
@SuppressWarnings("unchecked")
public Matchable<T> asMatchable() {
checkState(isMatchable(), "not matchable");
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index c05516b..ea23d91 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -227,6 +227,7 @@
List<DataSource<T>> sources = new ArrayList<>(cnt);
int queryCount = 0;
for (Predicate<T> q : queries) {
+ checkSupportedForQueries(q);
int limit = getEffectiveLimit(q);
limits.add(limit);
@@ -294,6 +295,15 @@
return out;
}
+ private void checkSupportedForQueries(Predicate<T> predicate) throws QueryParseException {
+ List<Predicate<T>> descendants = predicate.getFlattenedPredicateList();
+ for (Predicate<T> p : descendants) {
+ if (!p.supportedForQueries()) {
+ throw new QueryParseException(String.format("Operator '%s' cannot be used in queries", p));
+ }
+ }
+ }
+
private static <T> ImmutableList<QueryResult<T>> disabledResults(
List<String> queryStrings, List<Predicate<T>> queries) {
return IntStream.range(0, queries.size())
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index de637b4..2caa9dd 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -27,7 +27,7 @@
import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.SubmitRequirementChangeQueryBuilder;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Module;
@@ -40,7 +40,7 @@
/** Evaluates submit requirements for different change data. */
public class SubmitRequirementsEvaluatorImpl implements SubmitRequirementsEvaluator {
- private final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
+ private final Provider<SubmitRequirementChangeQueryBuilder> queryBuilder;
private final ProjectCache projectCache;
private final SubmitRuleEvaluator.Factory legacyEvaluator;
private final ExperimentFeatures experimentFeatures;
@@ -58,11 +58,11 @@
@Inject
private SubmitRequirementsEvaluatorImpl(
- Provider<ChangeQueryBuilder> changeQueryBuilderProvider,
+ Provider<SubmitRequirementChangeQueryBuilder> queryBuilder,
ProjectCache projectCache,
SubmitRuleEvaluator.Factory legacyEvaluator,
ExperimentFeatures experimentFeatures) {
- this.changeQueryBuilderProvider = changeQueryBuilderProvider;
+ this.queryBuilder = queryBuilder;
this.projectCache = projectCache;
this.legacyEvaluator = legacyEvaluator;
this.experimentFeatures = experimentFeatures;
@@ -71,7 +71,7 @@
@Override
public void validateExpression(SubmitRequirementExpression expression)
throws QueryParseException {
- changeQueryBuilderProvider.get().parse(expression.expressionString());
+ queryBuilder.get().parse(expression.expressionString());
}
@Override
@@ -121,8 +121,7 @@
public SubmitRequirementExpressionResult evaluateExpression(
SubmitRequirementExpression expression, ChangeData changeData) {
try {
- Predicate<ChangeData> predicate =
- changeQueryBuilderProvider.get().parse(expression.expressionString());
+ Predicate<ChangeData> predicate = queryBuilder.get().parse(expression.expressionString());
PredicateResult predicateResult = evaluatePredicateTree(predicate, changeData);
return SubmitRequirementExpressionResult.create(expression, predicateResult);
} catch (QueryParseException e) {
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
new file mode 100644
index 0000000..e63714f
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementChangeQueryBuilder.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2021 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.google.gerrit.server.query.change;
+
+import com.google.gerrit.index.query.QueryBuilder;
+import com.google.inject.Inject;
+
+/**
+ * A query builder for submit requirement expressions that includes all {@link ChangeQueryBuilder}
+ * operators, in addition to extra operators contributed by this class.
+ *
+ * <p>Operators defined in this class cannot be used in change queries.
+ */
+public class SubmitRequirementChangeQueryBuilder extends ChangeQueryBuilder {
+
+ private static final QueryBuilder.Definition<ChangeData, ChangeQueryBuilder> def =
+ new QueryBuilder.Definition<>(SubmitRequirementChangeQueryBuilder.class);
+
+ @Inject
+ SubmitRequirementChangeQueryBuilder(Arguments args) {
+ super(def, args);
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/SubmitRequirementPredicate.java b/java/com/google/gerrit/server/query/change/SubmitRequirementPredicate.java
new file mode 100644
index 0000000..184ede3
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/SubmitRequirementPredicate.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2021 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.google.gerrit.server.query.change;
+
+import com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.OperatorPredicate;
+
+/**
+ * Predicate that can be used in submit requirement expressions. Subclasses extended by this
+ * predicate cannot be used in search queries.
+ */
+public abstract class SubmitRequirementPredicate extends OperatorPredicate<ChangeData>
+ implements Matchable<ChangeData> {
+
+ public SubmitRequirementPredicate(String name, String value) {
+ super(name, value);
+ }
+
+ @Override
+ public boolean supportedForQueries() {
+ return false;
+ }
+}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 65b725b..2c6dd66 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -3691,6 +3691,32 @@
}
@Test
+ public void bySubmitRequirement_notAllowed() throws Exception {
+ Exception thrown =
+ assertThrows(
+ QueryParseException.class,
+ () ->
+ queryProcessorProvider
+ .get()
+ .query(
+ new SubmitRequirementPredicate("submit-requirement", "value") {
+ @Override
+ public boolean match(ChangeData object) {
+ return false;
+ }
+
+ @Override
+ public int getCost() {
+ return 0;
+ }
+ }));
+
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("Operator 'submit-requirement:value' cannot be used in queries");
+ }
+
+ @Test
public void selfFailsForAnonymousUser() throws Exception {
for (String query : ImmutableList.of("assignee:self", "has:star", "is:starred", "star:star")) {
assertQuery(query);
