Predicate: Add safety check against not(any())
It's tempting to use as a way to say "match no results", but it's very
unsafe. This simple check won't catch all misuses, but it would have
prevented Iad564dd47 from causing production problems.
Fix one obvious caller that now trips this check. This was a real
problem: a bare [is:watched] query on a large site for a user with no
project watches required iterating all changes to return an empty
result. We have real instances of this causing extremely long requests
in our server logs.
Release-Notes: skip
Change-Id: Ib43f7466db5db315bf87558a37f18a1832ec641f
(cherry picked from commit 02bab2161cd7ce7f682a28dad496a4465cb40844)
diff --git a/java/com/google/gerrit/index/query/Predicate.java b/java/com/google/gerrit/index/query/Predicate.java
index 53c92c9..b5ed82d 100644
--- a/java/com/google/gerrit/index/query/Predicate.java
+++ b/java/com/google/gerrit/index/query/Predicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.index.query;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import com.google.common.collect.Iterables;
@@ -82,6 +83,8 @@
/** Invert the passed node. */
public static <T> Predicate<T> not(Predicate<T> that) {
+ checkArgument(
+ !(that instanceof Any), "negating any() is unsafe because it post-filters all results");
if (that instanceof NotPredicate) {
// Negate of a negate is the original predicate.
//
diff --git a/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java b/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
index 8789b2b..0ea1a70 100644
--- a/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
+++ b/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
@@ -81,7 +81,7 @@
}
}
if (r.isEmpty()) {
- return none();
+ return ImmutableList.of(ChangeIndexPredicate.none());
} else if (checkIsVisible) {
return ImmutableList.of(or(r), builder.isVisible());
} else {
@@ -98,11 +98,6 @@
return Collections.<ProjectWatchKey>emptySet();
}
- protected static List<Predicate<ChangeData>> none() {
- Predicate<ChangeData> any = any();
- return ImmutableList.of(not(any));
- }
-
@Override
public int getCost() {
return 1;
