Change QueryBuilder.find() and callers to be static

The only thing gained by this method being non-static was access to
the instance's type parameter. In most cases this was not even
providing any additional safety, e.g. the ChangeQueryBuilder methods
already specify Predicate<ChangeData> in their signature so the type
can get picked up from there.

This was problematic for callers that might not have access to a
ChangeQueryBuilder instance, since that requires a CurrentUser.

Change-Id: I6b930e9aa4cce66203c7a2352b82bfd0b1cc26cd
This commit is contained in:
Dave Borowitz
2013-08-08 15:40:34 -07:00
parent e2ff4c7092
commit 30b280d309
4 changed files with 71 additions and 68 deletions

View File

@@ -109,6 +109,56 @@ public abstract class QueryBuilder<T> {
}
}
/**
* Locate a predicate in the predicate tree.
*
* @param p the predicate to find.
* @param clazz type of the predicate instance.
* @return the predicate, null if not found.
*/
@SuppressWarnings("unchecked")
public static <T, P extends Predicate<T>> P find(Predicate<T> p, Class<P> clazz) {
if (clazz.isAssignableFrom(p.getClass())) {
return (P) p;
}
for (Predicate<T> c : p.getChildren()) {
P r = find(c, clazz);
if (r != null) {
return r;
}
}
return null;
}
/**
* Locate a predicate in the predicate tree.
*
* @param p the predicate to find.
* @param clazz type of the predicate instance.
* @param name name of the operator.
* @return the predicate, null if not found.
*/
@SuppressWarnings("unchecked")
public static <T, P extends OperatorPredicate<T>> P find(Predicate<T> p,
Class<P> clazz, String name) {
if (p instanceof OperatorPredicate
&& ((OperatorPredicate<?>) p).getOperator().equals(name)
&& clazz.isAssignableFrom(p.getClass())) {
return (P) p;
}
for (Predicate<T> c : p.getChildren()) {
P r = find(c, clazz, name);
if (r != null) {
return r;
}
}
return null;
}
@SuppressWarnings("rawtypes")
private final Map<String, OperatorFactory> opFactories;
@@ -238,56 +288,6 @@ public abstract class QueryBuilder<T> {
throw error("Unsupported query:" + value);
}
/**
* Locate a predicate in the predicate tree.
*
* @param p the predicate to find.
* @param clazz type of the predicate instance.
* @return the predicate, null if not found.
*/
@SuppressWarnings("unchecked")
public <P extends Predicate<T>> P find(Predicate<T> p, Class<P> clazz) {
if (clazz.isAssignableFrom(p.getClass())) {
return (P) p;
}
for (Predicate<T> c : p.getChildren()) {
P r = find(c, clazz);
if (r != null) {
return r;
}
}
return null;
}
/**
* Locate a predicate in the predicate tree.
*
* @param p the predicate to find.
* @param clazz type of the predicate instance.
* @param name name of the operator.
* @return the predicate, null if not found.
*/
@SuppressWarnings("unchecked")
public <P extends OperatorPredicate<T>> P find(Predicate<T> p,
Class<P> clazz, String name) {
if (p instanceof OperatorPredicate
&& ((OperatorPredicate<?>) p).getOperator().equals(name)
&& clazz.isAssignableFrom(p.getClass())) {
return (P) p;
}
for (Predicate<T> c : p.getChildren()) {
P r = find(c, clazz, name);
if (r != null) {
return r;
}
}
return null;
}
@SuppressWarnings("unchecked")
private Predicate<T>[] children(final Tree r) throws QueryParseException,
IllegalArgumentException {

View File

@@ -111,6 +111,21 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
new QueryBuilder.Definition<ChangeData, ChangeQueryBuilder>(
ChangeQueryBuilder.class);
@SuppressWarnings("unchecked")
public static boolean hasLimit(Predicate<ChangeData> p) {
return find(p, IntPredicate.class, FIELD_LIMIT) != null;
}
@SuppressWarnings("unchecked")
public static int getLimit(Predicate<ChangeData> p) {
return ((IntPredicate<?>) find(p, IntPredicate.class, FIELD_LIMIT)).intValue();
}
public static boolean hasSortKey(Predicate<ChangeData> p) {
return find(p, SortKeyPredicate.class, "sortkey_after") != null
|| find(p, SortKeyPredicate.class, "sortkey_before") != null;
}
@VisibleForTesting
public static class Arguments {
final Provider<ReviewDb> dbProvider;
@@ -569,21 +584,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
return sortkey_before(sortKey);
}
@SuppressWarnings("unchecked")
public boolean hasLimit(Predicate<ChangeData> p) {
return find(p, IntPredicate.class, FIELD_LIMIT) != null;
}
@SuppressWarnings("unchecked")
public int getLimit(Predicate<ChangeData> p) {
return ((IntPredicate<?>) find(p, IntPredicate.class, FIELD_LIMIT)).intValue();
}
public boolean hasSortKey(Predicate<ChangeData> p) {
return find(p, SortKeyPredicate.class, "sortkey_after") != null
|| find(p, SortKeyPredicate.class, "sortkey_before") != null;
}
@SuppressWarnings("unchecked")
@Override
protected Predicate<ChangeData> defaultField(String query)

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.query.AndPredicate;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryBuilder;
import com.google.gerrit.server.query.QueryParseException;
import java.util.List;
@@ -53,7 +54,7 @@ class IsWatchedByPredicate extends AndPredicate<ChangeData> {
if (w.getFilter() != null) {
try {
f = builder.parse(w.getFilter());
if (builder.find(f, IsWatchedByPredicate.class) != null) {
if (QueryBuilder.find(f, IsWatchedByPredicate.class) != null) {
// If the query is going to infinite loop, assume it
// will never match and return null. Yes this test
// prevents you from having a filter that matches what

View File

@@ -371,7 +371,9 @@ public class QueryProcessor {
}
private int limit(Predicate<ChangeData> s) {
int n = queryBuilder.hasLimit(s) ? queryBuilder.getLimit(s) : maxLimit;
int n = ChangeQueryBuilder.hasLimit(s)
? ChangeQueryBuilder.getLimit(s)
: maxLimit;
return limit > 0 ? Math.min(n, limit) + 1 : n + 1;
}
@@ -380,7 +382,7 @@ public class QueryProcessor {
final Predicate<ChangeData> visibleToMe) throws QueryParseException {
Predicate<ChangeData> q = queryBuilder.parse(queryString);
if (!queryBuilder.hasSortKey(q)) {
if (!ChangeQueryBuilder.hasSortKey(q)) {
if (sortkeyBefore != null) {
q = Predicate.and(q, queryBuilder.sortkey_before(sortkeyBefore));
} else if (sortkeyAfter != null) {