Emit deprecation warning when TermsLookup contains a type

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/51_filter_with_types.yml

@@ -35,8 +35,13 @@ setup:
 ---
 "Filter aggs with terms lookup and ensure it's cached":
   # Because the filter agg rewrites the terms lookup in the rewrite phase the request can be cached
-
+  - skip:
+      version: " - 7.7.0"
+      reason: "Deprecation warnings added in 7.7.0"
+      features: warnings
   - do:
+      warnings:
+        - "Deprecated field [type] used, which has been removed entirely"
       search:
         rest_total_hits_as_int: true
         size: 0

rest-api-spec/src/main/resources/rest-api-spec/test/search/150_rewrite_on_coordinator.yml

@@ -1,4 +1,7 @@
 "Ensure that we fetch the document only once":
+  - skip:
+      version: "- 7.0.0"
+      reason: "TermsLookups in 6.0 require a _type, which emits warnings beyond 7.7"
   - do:
       indices.create:
           index: search_index
@@ -35,7 +38,7 @@
       search:
         rest_total_hits_as_int: true
         index: "search_index"
-        body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "type": "_doc", "id": "1", "path": "followers"} } } }
+        body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "id": "1", "path": "followers"} } } }
   - do:
       indices.create:
           index: lookup_index
@@ -58,7 +61,7 @@
       search:
         rest_total_hits_as_int: true
         index: "search_index"
-        body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "type": "_doc", "id": "1", "path": "followers"} } } }
+        body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "id": "1", "path": "followers"} } } }
 
   - match: { _shards.total: 5 }
   - match: { _shards.successful: 5 }

rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_query_with_types.yml

@@ -1,8 +1,9 @@
 ---
 "Terms Query with No.of terms exceeding index.max_terms_count should FAIL":
   - skip:
-      version: " - 6.99.99"
-      reason: index.max_terms_count setting has been added in 7.0.0
+      version: " - 7.7.0"
+      reason: deprecation warnings added in 7.7.0
+      features: warnings
   - do:
       indices.create:
           include_type_name: true
@@ -46,6 +47,8 @@
           body: {"query" : {"terms" : {"user" : ["u1", "u2", "u3"]}}}
 
   - do:
+      warnings:
+        - "Deprecated field [type] used, which has been removed entirely"
       search:
           rest_total_hits_as_int: true
           index: test_index

server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
@@ -35,13 +34,12 @@
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.IndexSettings;
-import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.ConstantFieldType;
+import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.indices.TermsLookup;
 
 import java.io.IOException;
@@ -64,8 +62,6 @@
 public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
     public static final String NAME = "terms";
 
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
-        LogManager.getLogger(TermsQueryBuilder.class));
     static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated " +
         "in [terms] lookup queries.";
 
@@ -402,15 +398,9 @@ public static TermsQueryBuilder fromXContent(XContentParser parser) throws IOExc
                     "followed by array of terms or a document lookup specification");
         }
 
-        TermsQueryBuilder builder = new TermsQueryBuilder(fieldName, values, termsLookup)
+        return new TermsQueryBuilder(fieldName, values, termsLookup)
             .boost(boost)
             .queryName(queryName);
-
-        if (builder.isTypeless() == false) {
-            deprecationLogger.deprecatedAndMaybeLog("terms_lookup_with_types", TYPES_DEPRECATION_MESSAGE);
-        }
-
-        return builder;
     }
 
     static List<Object> parseValues(XContentParser parser) throws IOException {

server/src/main/java/org/elasticsearch/indices/TermsLookup.java

@@ -21,10 +21,11 @@
 
 import org.elasticsearch.Version;
 import org.elasticsearch.common.Nullable;
-import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -33,10 +34,14 @@
 import java.io.IOException;
 import java.util.Objects;
 
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
+import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
+
 /**
  * Encapsulates the parameters needed to fetch terms.
  */
 public class TermsLookup implements Writeable, ToXContentFragment {
+
     private final String index;
     private @Nullable String type;
     private final String id;
@@ -142,48 +147,24 @@ public TermsLookup routing(String routing) {
         return this;
     }
 
+    private static final ConstructingObjectParser<TermsLookup, Void> PARSER = new ConstructingObjectParser<>("terms_lookup",
+        args -> {
+            String index = (String) args[0];
+            String type = (String) args[1];
+            String id = (String) args[2];
+            String path = (String) args[3];
+            return new TermsLookup(index, type, id, path);
+        });
+    static {
+        PARSER.declareString(constructorArg(), new ParseField("index"));
+        PARSER.declareString(optionalConstructorArg(), new ParseField("type").withAllDeprecated());
+        PARSER.declareString(constructorArg(), new ParseField("id"));
+        PARSER.declareString(constructorArg(), new ParseField("path"));
+        PARSER.declareString(TermsLookup::routing, new ParseField("routing"));
+    }
+
     public static TermsLookup parseTermsLookup(XContentParser parser) throws IOException {
-        String index = null;
-        String type = null;
-        String id = null;
-        String path = null;
-        String routing = null;
-        XContentParser.Token token;
-        String currentFieldName = "";
-        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
-            if (token == XContentParser.Token.FIELD_NAME) {
-                currentFieldName = parser.currentName();
-            } else if (token.isValue()) {
-                switch (currentFieldName) {
-                case "index":
-                    index = parser.text();
-                    break;
-                case "type":
-                    type = parser.text();
-                    break;
-                case "id":
-                    id = parser.text();
-                    break;
-                case "routing":
-                    routing = parser.textOrNull();
-                    break;
-                case "path":
-                    path = parser.text();
-                    break;
-                default:
-                    throw new ParsingException(parser.getTokenLocation(), "[" + TermsQueryBuilder.NAME +
-                        "] query does not support [" + currentFieldName + "] within lookup element");
-                }
-            } else {
-                throw new ParsingException(parser.getTokenLocation(), "[" + TermsQueryBuilder.NAME + "] unknown token ["
-                    + token + "] after [" + currentFieldName + "]");
-            }
-        }
-        if (type == null) {
-            return new TermsLookup(index, id, path).routing(routing);
-        } else {
-            return new TermsLookup(index, type, id, path).routing(routing);
-        }
+        return PARSER.parse(parser, null);
     }
 
     @Override

server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java

@@ -326,30 +326,30 @@ public void testTypeField() throws IOException {
         builder.doToQuery(createShardContext());
         assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE);
     }
-    
+
     public void testRewriteIndexQueryToMatchNone() throws IOException {
         TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", "also_does_not_exist");
         QueryShardContext queryShardContext = createShardContext();
         QueryBuilder rewritten = query.rewrite(queryShardContext);
         assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
-    }      
-    
+    }
+
     public void testRewriteIndexQueryToNotMatchNone() throws IOException {
         // At least one name is good
         TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", getIndex().getName());
         QueryShardContext queryShardContext = createShardContext();
         QueryBuilder rewritten = query.rewrite(queryShardContext);
         assertThat(rewritten, instanceOf(MatchAllQueryBuilder.class));
-    }      
-    
+    }
+
     @Override
     protected QueryBuilder parseQuery(XContentParser parser) throws IOException {
         QueryBuilder query = super.parseQuery(parser);
         assertThat(query, CoreMatchers.instanceOf(TermsQueryBuilder.class));
 
         TermsQueryBuilder termsQuery = (TermsQueryBuilder) query;
         if (termsQuery.isTypeless() == false) {
-            assertWarnings(TermsQueryBuilder.TYPES_DEPRECATION_MESSAGE);
+            assertWarnings("Deprecated field [type] used, which has been removed entirely");
         }
         return query;
     }

server/src/test/java/org/elasticsearch/indices/TermsLookupTests.java

@@ -22,6 +22,8 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -105,6 +107,32 @@ public void testSerializationWithTypes() throws IOException {
         }
     }
 
+    public void testXContentParsingWithType() throws IOException {
+        XContentParser parser = createParser(JsonXContent.jsonXContent,
+            "{ \"index\" : \"index\", \"id\" : \"id\", \"type\" : \"type\", \"path\" : \"path\", \"routing\" : \"routing\" }");
+
+        TermsLookup tl = TermsLookup.parseTermsLookup(parser);
+        assertEquals("index", tl.index());
+        assertEquals("type", tl.type());
+        assertEquals("id", tl.id());
+        assertEquals("path", tl.path());
+        assertEquals("routing", tl.routing());
+
+        assertWarnings("Deprecated field [type] used, which has been removed entirely");
+    }
+
+    public void testXContentParsing() throws IOException {
+        XContentParser parser = createParser(JsonXContent.jsonXContent,
+            "{ \"index\" : \"index\", \"id\" : \"id\", \"path\" : \"path\", \"routing\" : \"routing\" }");
+
+        TermsLookup tl = TermsLookup.parseTermsLookup(parser);
+        assertEquals("index", tl.index());
+        assertNull(tl.type());
+        assertEquals("id", tl.id());
+        assertEquals("path", tl.path());
+        assertEquals("routing", tl.routing());
+    }
+
     public static TermsLookup randomTermsLookup() {
         return new TermsLookup(
             randomAlphaOfLength(10),