Allow QueryParseException to escape ChangeQueryRewriter
ChangeIndex.getSource throws QueryParseException with the intention of allowing implementations to not support certain operators/features (e.g. regex queries). For this to result in a user-visible error message instead of a 500, we need to not wrap it in an IllegalStateException. Change-Id: I8871df8b806cb48ba976ce00f5864a906cd3e9b3
This commit is contained in:
@@ -140,7 +140,8 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Predicate<ChangeData> rewrite(Predicate<ChangeData> in) {
|
public Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
|
||||||
|
throws QueryParseException {
|
||||||
ChangeIndex index = indexes.getSearchIndex();
|
ChangeIndex index = indexes.getSearchIndex();
|
||||||
if (index == null) {
|
if (index == null) {
|
||||||
return sqlRewriter.rewrite(in);
|
return sqlRewriter.rewrite(in);
|
||||||
@@ -153,7 +154,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
|
|||||||
|
|
||||||
Predicate<ChangeData> out = rewriteImpl(in, index, limit);
|
Predicate<ChangeData> out = rewriteImpl(in, index, limit);
|
||||||
if (in == out || out instanceof IndexPredicate) {
|
if (in == out || out instanceof IndexPredicate) {
|
||||||
return query(out, index, limit);
|
return new IndexedChangeQuery(index, out, limit);
|
||||||
} else if (out == null /* cannot rewrite */) {
|
} else if (out == null /* cannot rewrite */) {
|
||||||
return in;
|
return in;
|
||||||
} else {
|
} else {
|
||||||
@@ -172,9 +173,11 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
|
|||||||
* queried directly in the index. Otherwise, a predicate that is
|
* queried directly in the index. Otherwise, a predicate that is
|
||||||
* semantically equivalent, with some of its subtrees wrapped to query the
|
* semantically equivalent, with some of its subtrees wrapped to query the
|
||||||
* index directly.
|
* index directly.
|
||||||
|
* @throws QueryParseException if the underlying index implementation does not
|
||||||
|
* support this predicate.
|
||||||
*/
|
*/
|
||||||
private Predicate<ChangeData> rewriteImpl(Predicate<ChangeData> in,
|
private Predicate<ChangeData> rewriteImpl(Predicate<ChangeData> in,
|
||||||
ChangeIndex index, int limit) {
|
ChangeIndex index, int limit) throws QueryParseException {
|
||||||
if (isIndexPredicate(in, index)) {
|
if (isIndexPredicate(in, index)) {
|
||||||
return in;
|
return in;
|
||||||
} else if (!isRewritePossible(in)) {
|
} else if (!isRewritePossible(in)) {
|
||||||
@@ -224,10 +227,11 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
|
|||||||
List<Predicate<ChangeData>> newChildren,
|
List<Predicate<ChangeData>> newChildren,
|
||||||
BitSet isIndexed,
|
BitSet isIndexed,
|
||||||
ChangeIndex index,
|
ChangeIndex index,
|
||||||
int limit) {
|
int limit) throws QueryParseException {
|
||||||
if (isIndexed.cardinality() == 1) {
|
if (isIndexed.cardinality() == 1) {
|
||||||
int i = isIndexed.nextSetBit(0);
|
int i = isIndexed.nextSetBit(0);
|
||||||
newChildren.add(0, query(newChildren.remove(i), index, limit));
|
newChildren.add(
|
||||||
|
0, new IndexedChangeQuery(index, newChildren.remove(i), limit));
|
||||||
return copy(in, newChildren);
|
return copy(in, newChildren);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -247,7 +251,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
|
|||||||
all.add(c);
|
all.add(c);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
all.add(0, query(in.copy(indexed), index, limit));
|
all.add(0, new IndexedChangeQuery(index, in.copy(indexed), limit));
|
||||||
return copy(in, all);
|
return copy(in, all);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -262,16 +266,6 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
|
|||||||
return in.copy(all);
|
return in.copy(all);
|
||||||
}
|
}
|
||||||
|
|
||||||
private IndexedChangeQuery query(Predicate<ChangeData> p, ChangeIndex index,
|
|
||||||
int limit) {
|
|
||||||
try {
|
|
||||||
return new IndexedChangeQuery(index, p, limit);
|
|
||||||
} catch (QueryParseException e) {
|
|
||||||
throw new IllegalStateException(
|
|
||||||
"Failed to convert " + p + " to index predicate", e);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
private static boolean isRewritePossible(Predicate<ChangeData> p) {
|
private static boolean isRewritePossible(Predicate<ChangeData> p) {
|
||||||
return p.getChildCount() > 0 && (
|
return p.getChildCount() > 0 && (
|
||||||
p instanceof AndPredicate
|
p instanceof AndPredicate
|
||||||
|
@@ -15,7 +15,9 @@
|
|||||||
package com.google.gerrit.server.query.change;
|
package com.google.gerrit.server.query.change;
|
||||||
|
|
||||||
import com.google.gerrit.server.query.Predicate;
|
import com.google.gerrit.server.query.Predicate;
|
||||||
|
import com.google.gerrit.server.query.QueryParseException;
|
||||||
|
|
||||||
public interface ChangeQueryRewriter {
|
public interface ChangeQueryRewriter {
|
||||||
Predicate<ChangeData> rewrite(Predicate<ChangeData> in);
|
Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
|
||||||
|
throws QueryParseException;
|
||||||
}
|
}
|
||||||
|
@@ -327,7 +327,8 @@ public class IndexRewriteTest extends TestCase {
|
|||||||
return queryBuilder.parse(query);
|
return queryBuilder.parse(query);
|
||||||
}
|
}
|
||||||
|
|
||||||
private Predicate<ChangeData> rewrite(Predicate<ChangeData> in) {
|
private Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
|
||||||
|
throws QueryParseException {
|
||||||
return rewrite.rewrite(in);
|
return rewrite.rewrite(in);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user