From 69c2deedc7977516b072be42a783828197704693 Mon Sep 17 00:00:00 2001
From: javanna <cavannaluca@gmail.com>
Date: Wed, 3 Aug 2016 15:11:26 +0200
Subject: [PATCH] Throw parsing error if prefix query contains multiple fields

Prefix Query, like many other queries, used to parse when the query refers to multiple fields and the last one would win. We rather throw an exception now instead.
Also added tests for short prefix quer variant.
---
 .../index/query/PrefixQueryBuilder.java       | 17 +++---
 .../index/query/PrefixQueryBuilderTests.java  | 56 ++++++++++++++++---
 2 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java
index e1f679e0c8d..2ebd0dfc6b1 100644
--- a/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java
+++ b/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java
@@ -64,7 +64,7 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder<PrefixQueryBuilder>
             throw new IllegalArgumentException("field name is null or empty");
         }
         if (value == null) {
-            throw new IllegalArgumentException("value cannot be null.");
+            throw new IllegalArgumentException("value cannot be null");
         }
         this.fieldName = fieldName;
         this.value = value;
@@ -120,7 +120,7 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder<PrefixQueryBuilder>
     public static Optional<PrefixQueryBuilder> fromXContent(QueryParseContext parseContext) throws IOException {
         XContentParser parser = parseContext.parser();
 
-        String fieldName = parser.currentName();
+        String fieldName = null;
         String value = null;
         String rewrite = null;
 
@@ -134,6 +134,10 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder<PrefixQueryBuilder>
             } else if (parseContext.isDeprecatedSetting(currentFieldName)) {
                 // skip
             } else if (token == XContentParser.Token.START_OBJECT) {
+                if (fieldName != null) {
+                    throw new ParsingException(parser.getTokenLocation(), "[prefix] query doesn't support multiple fields, found ["
+                            + fieldName + "] and [" + currentFieldName + "]");
+                }
                 fieldName = currentFieldName;
                 while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
                     if (token == XContentParser.Token.FIELD_NAME) {
@@ -149,19 +153,16 @@ public class PrefixQueryBuilder extends AbstractQueryBuilder<PrefixQueryBuilder>
                             rewrite = parser.textOrNull();
                         } else {
                             throw new ParsingException(parser.getTokenLocation(),
-                                    "[regexp] query does not support [" + currentFieldName + "]");
+                                    "[prefix] query does not support [" + currentFieldName + "]");
                         }
                     }
                 }
             } else {
-                    fieldName = currentFieldName;
-                    value = parser.textOrNull();
+                fieldName = currentFieldName;
+                value = parser.textOrNull();
             }
         }
 
-        if (value == null) {
-            throw new ParsingException(parser.getTokenLocation(), "No value specified for prefix query");
-        }
         return Optional.of(new PrefixQueryBuilder(fieldName, value)
                 .rewrite(rewrite)
                 .boost(boost)
diff --git a/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java
index aba6bec096b..22ee7ef81f3 100644
--- a/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java
+++ b/core/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java
@@ -23,9 +23,12 @@ import org.apache.lucene.index.Term;
 import org.apache.lucene.search.MultiTermQuery;
 import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.Query;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.test.AbstractQueryTestCase;
 
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.elasticsearch.index.query.QueryBuilders.prefixQuery;
 import static org.hamcrest.Matchers.equalTo;
@@ -35,16 +38,32 @@ public class PrefixQueryBuilderTests extends AbstractQueryTestCase<PrefixQueryBu
 
     @Override
     protected PrefixQueryBuilder doCreateTestQueryBuilder() {
-        String fieldName = randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10);
-        String value = randomAsciiOfLengthBetween(1, 10);
-        PrefixQueryBuilder query = new PrefixQueryBuilder(fieldName, value);
-
+        PrefixQueryBuilder query = randomPrefixQuery();
         if (randomBoolean()) {
             query.rewrite(getRandomRewriteMethod());
         }
         return query;
     }
 
+    @Override
+    protected Map<String, PrefixQueryBuilder> getAlternateVersions() {
+        Map<String, PrefixQueryBuilder> alternateVersions = new HashMap<>();
+        PrefixQueryBuilder prefixQuery = randomPrefixQuery();
+        String contentString = "{\n" +
+                "    \"prefix\" : {\n" +
+                "        \"" + prefixQuery.fieldName() + "\" : \"" + prefixQuery.value() + "\"\n" +
+                "    }\n" +
+                "}";
+        alternateVersions.put(contentString, prefixQuery);
+        return alternateVersions;
+    }
+
+    private static PrefixQueryBuilder randomPrefixQuery() {
+        String fieldName = randomBoolean() ? STRING_FIELD_NAME : randomAsciiOfLengthBetween(1, 10);
+        String value = randomAsciiOfLengthBetween(1, 10);
+        return new PrefixQueryBuilder(fieldName, value);
+    }
+
     @Override
     protected void doAssertLuceneQuery(PrefixQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
         assertThat(query, instanceOf(PrefixQuery.class));
@@ -60,16 +79,16 @@ public class PrefixQueryBuilderTests extends AbstractQueryTestCase<PrefixQueryBu
             } else {
                 new PrefixQueryBuilder("", "text");
             }
-            fail("cannot be null or empty");
+            fail("field name is null or empty");
         } catch (IllegalArgumentException e) {
-            // expected
+            assertEquals("field name is null or empty", e.getMessage());
         }
 
         try {
             new PrefixQueryBuilder("field", null);
-            fail("cannot be null or empty");
+            fail("value cannot be null");
         } catch (IllegalArgumentException e) {
-            // expected
+            assertEquals("value cannot be null", e.getMessage());
         }
     }
 
@@ -103,4 +122,25 @@ public class PrefixQueryBuilderTests extends AbstractQueryTestCase<PrefixQueryBu
         assertEquals("Can only use prefix queries on keyword and text fields - not on [mapped_int] which is of type [integer]",
                 e.getMessage());
     }
+
+    public void testParseFailsWithMultipleFields() throws IOException {
+        String json =
+                "{\n" +
+                "    \"prefix\": {\n" +
+                "      \"user1\": {\n" +
+                "        \"value\": \"ki\"\n" +
+                "      },\n" +
+                "      \"user2\": {\n" +
+                "        \"value\": \"ki\"\n" +
+                "      }\n" +
+                "    }\n" +
+                "}";
+
+        try {
+            parseQuery(json);
+            fail("parseQuery should have failed");
+        } catch(ParsingException e) {
+            assertEquals("[prefix] query doesn't support multiple fields, found [user1] and [user2]", e.getMessage());
+        }
+    }
 }