1 files changed, 19 insertions, 26 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
index 6ce02fbf30..5364a66316 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java
@@ -14,39 +14,41 @@
 
 package com.google.gerrit.server.query.change;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.stream.Collectors.toList;
 
+import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
-import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
 import com.google.gwtorm.server.OrmException;
 import java.util.stream.Stream;
 
-class ReviewerPredicate extends ChangeIndexPredicate {
-  static Predicate<ChangeData> reviewer(Arguments args, Account.Id id) {
-    Predicate<ChangeData> p;
+public class ReviewerPredicate extends ChangeIndexPredicate {
+  protected static Predicate<ChangeData> forState(Account.Id id, ReviewerStateInternal state) {
+    checkArgument(state != ReviewerStateInternal.REMOVED, "can't query by removed reviewer");
+    return new ReviewerPredicate(state, id);
+  }
+
+  protected static Predicate<ChangeData> reviewer(Arguments args, Account.Id id) {
     if (args.notesMigration.readChanges()) {
       // With NoteDb, Reviewer/CC are clearly distinct states, so only choose reviewer.
-      p = new ReviewerPredicate(ReviewerStateInternal.REVIEWER, id);
-    } else {
-      // Without NoteDb, Reviewer/CC are a bit unpredictable; maintain the old behavior of matching
-      // any reviewer state.
-      p = anyReviewerState(id);
+      return new ReviewerPredicate(ReviewerStateInternal.REVIEWER, id);
     }
-    return create(args, p);
+    // Without NoteDb, Reviewer/CC are a bit unpredictable; maintain the old behavior of matching
+    // any reviewer state.
+    return anyReviewerState(id);
   }
 
-  static Predicate<ChangeData> cc(Arguments args, Account.Id id) {
+  protected static Predicate<ChangeData> cc(Account.Id id) {
     // As noted above, CC is nebulous without NoteDb, but it certainly doesn't make sense to return
     // Reviewers for cc:foo. Most likely this will just not match anything, but let the index sort
     // it out.
-    return create(args, new ReviewerPredicate(ReviewerStateInternal.CC, id));
+    return new ReviewerPredicate(ReviewerStateInternal.CC, id);
   }
 
-  private static Predicate<ChangeData> anyReviewerState(Account.Id id) {
+  protected static Predicate<ChangeData> anyReviewerState(Account.Id id) {
     return Predicate.or(
         Stream.of(ReviewerStateInternal.values())
             .filter(s -> s != ReviewerStateInternal.REMOVED)
@@ -54,17 +56,8 @@ class ReviewerPredicate extends ChangeIndexPredicate {
             .collect(toList()));
   }
 
-  private static Predicate<ChangeData> create(Arguments args, Predicate<ChangeData> p) {
-    if (!args.allowsDrafts) {
-      // TODO(dborowitz): This really belongs much higher up e.g. QueryProcessor. Also, why are we
-      // even doing this?
-      return Predicate.and(p, Predicate.not(new ChangeStatusPredicate(Change.Status.DRAFT)));
-    }
-    return p;
-  }
-
-  private final ReviewerStateInternal state;
-  private final Account.Id id;
+  protected final ReviewerStateInternal state;
+  protected final Account.Id id;
 
   private ReviewerPredicate(ReviewerStateInternal state, Account.Id id) {
     super(ChangeField.REVIEWER, ChangeField.getReviewerFieldValue(state, id));
@@ -72,7 +65,7 @@ class ReviewerPredicate extends ChangeIndexPredicate {
     this.id = id;
   }
 
-  Account.Id getAccountId() {
+  protected Account.Id getAccountId() {
     return id;
   }