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)

2 files changed, 4 insertions, 6 deletions

diff --git a/java/com/google/gerrit/index/query/Predicate.java b/java/com/google/gerrit/index/query/Predicate.java
index 53c92c9b37..b5ed82da81 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 @@ public abstract class Predicate<T> {
 
   /** 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 8789b2bfe8..0ea1a70b43 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 @@ public class IsWatchedByPredicate extends AndPredicate<ChangeData> {
       }
     }
     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 @@ public class IsWatchedByPredicate extends AndPredicate<ChangeData> {
     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;