CheckerQuery: Handle QueryParseException from query execution
QueryParseException may be thrown not just while parsing the query, but
also from ChangeQueryProcessor#query. Ensure this exception is converted
to ConfigInvalidException rather than wrapped in an opaque OrmException.
Also factor out a method to use consistent messages in
ConfigInvalidExceptions thrown from CheckerQuery.
Change-Id: Idd08b98dde7b26e82f644a6c3d88b3cfd9af0555
diff --git a/java/com/google/gerrit/plugins/checks/CheckerQuery.java b/java/com/google/gerrit/plugins/checks/CheckerQuery.java
index 0830d8b..8a9dc84 100644
--- a/java/com/google/gerrit/plugins/checks/CheckerQuery.java
+++ b/java/com/google/gerrit/plugins/checks/CheckerQuery.java
@@ -25,6 +25,7 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.index.query.IndexPredicate;
import com.google.gerrit.index.query.Predicate;
@@ -207,7 +208,11 @@
public List<ChangeData> queryMatchingChanges(Checker checker)
throws ConfigInvalidException, OrmException {
- return executeIndexQueryWithRetry(createQueryPredicate(checker));
+ try {
+ return executeIndexQueryWithRetry(createQueryPredicate(checker));
+ } catch (QueryParseException e) {
+ throw invalidQueryException(checker, e);
+ }
}
private Predicate<ChangeData> createQueryPredicate(Checker checker)
@@ -220,17 +225,16 @@
try {
predicateForQuery = queryBuilder.parse(query);
} catch (QueryParseException e) {
- throw new ConfigInvalidException(
- String.format("change query of checker %s is invalid: %s", checker.getUuid(), query),
- e);
+ throw invalidQueryException(checker, e);
}
if (!predicateForQuery.isMatchable()) {
// Assuming nobody modified the query behind Gerrit's back, this is programmer error:
// CheckerQuery should not be able to produce non-matchable queries.
- throw new ConfigInvalidException(
- String.format(
- "change query of checker %s is non-matchable: %s", checker.getUuid(), query));
+ logger.atWarning().log(
+ "change query of checker %s is not matchable: %s",
+ checker.getUuid(), checker.getQuery().get());
+ throw invalidQueryException(checker, null);
}
predicate = Predicate.and(predicate, predicateForQuery);
@@ -255,7 +259,7 @@
// TODO(ekempin): Retrying the query should be done by ChangeQueryProcessor.
private List<ChangeData> executeIndexQueryWithRetry(Predicate<ChangeData> predicate)
- throws OrmException {
+ throws OrmException, QueryParseException {
try {
return retryHelper.execute(
ActionType.INDEX_QUERY,
@@ -263,8 +267,21 @@
OrmException.class::isInstance);
} catch (Exception e) {
Throwables.throwIfUnchecked(e);
+ Throwables.throwIfInstanceOf(e, QueryParseException.class);
Throwables.throwIfInstanceOf(e, OrmException.class);
throw new OrmException(e);
}
}
+
+ private static ConfigInvalidException invalidQueryException(
+ Checker checker, @Nullable QueryParseException parseException) {
+ String msg =
+ String.format(
+ "change query of checker %s is invalid: %s",
+ checker.getUuid(), checker.getQuery().orElse(""));
+ if (parseException != null) {
+ msg += " (" + parseException.getMessage() + ")";
+ }
+ return new ConfigInvalidException(msg, parseException);
+ }
}
diff --git a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
index 57f7be0..e880244 100644
--- a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
@@ -80,7 +80,8 @@
}
public List<PendingChecksInfo> apply()
- throws RestApiException, IOException, ConfigInvalidException, OrmException {
+ throws RestApiException, IOException, ConfigInvalidException, OrmException,
+ QueryParseException {
return apply(TopLevelResource.INSTANCE);
}
