From 5b2c024975f0027f3abc93c877e8094dabe51d5c Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Thu, 22 Aug 2013 11:41:28 -0700
Subject: [PATCH] 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
---
 .../gerrit/server/index/IndexRewriteImpl.java | 26 +++++++------------
 .../query/change/ChangeQueryRewriter.java     |  4 ++-
 .../gerrit/server/index/IndexRewriteTest.java |  3 ++-
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
index a782d345ae..aab6c64f38 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexRewriteImpl.java
@@ -140,7 +140,8 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
   }
 
   @Override
-  public Predicate<ChangeData> rewrite(Predicate<ChangeData> in) {
+  public Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
+      throws QueryParseException {
     ChangeIndex index = indexes.getSearchIndex();
     if (index == null) {
       return sqlRewriter.rewrite(in);
@@ -153,7 +154,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
 
     Predicate<ChangeData> out = rewriteImpl(in, index, limit);
     if (in == out || out instanceof IndexPredicate) {
-      return query(out, index, limit);
+      return new IndexedChangeQuery(index, out, limit);
     } else if (out == null /* cannot rewrite */) {
       return in;
     } else {
@@ -172,9 +173,11 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
    *     queried directly in the index. Otherwise, a predicate that is
    *     semantically equivalent, with some of its subtrees wrapped to query the
    *     index directly.
+   * @throws QueryParseException if the underlying index implementation does not
+   *     support this predicate.
    */
   private Predicate<ChangeData> rewriteImpl(Predicate<ChangeData> in,
-      ChangeIndex index, int limit) {
+      ChangeIndex index, int limit) throws QueryParseException {
     if (isIndexPredicate(in, index)) {
       return in;
     } else if (!isRewritePossible(in)) {
@@ -224,10 +227,11 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
       List<Predicate<ChangeData>> newChildren,
       BitSet isIndexed,
       ChangeIndex index,
-      int limit) {
+      int limit) throws QueryParseException {
     if (isIndexed.cardinality() == 1) {
       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);
     }
 
@@ -247,7 +251,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
         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);
   }
 
@@ -262,16 +266,6 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
     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) {
     return p.getChildCount() > 0 && (
            p instanceof AndPredicate
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java
index 1d6de6bc3c..bd186c7402 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryRewriter.java
@@ -15,7 +15,9 @@
 package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.QueryParseException;
 
 public interface ChangeQueryRewriter {
-  Predicate<ChangeData> rewrite(Predicate<ChangeData> in);
+  Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
+      throws QueryParseException;
 }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
index 64f7965a29..bc96b449b7 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
@@ -327,7 +327,8 @@ public class IndexRewriteTest extends TestCase {
     return queryBuilder.parse(query);
   }
 
-  private Predicate<ChangeData> rewrite(Predicate<ChangeData> in) {
+  private Predicate<ChangeData> rewrite(Predicate<ChangeData> in)
+      throws QueryParseException {
     return rewrite.rewrite(in);
   }