From 9f83deadf40441e43f6f8fefd56141fad9e6626f Mon Sep 17 00:00:00 2001 From: Suraj Singh <79435743+dreamer-89@users.noreply.github.com> Date: Thu, 17 Mar 2022 18:38:10 -0700 Subject: [PATCH] [Remove] Type from Percolate query API (#2490) * [Remove] Type from Percolator query API Signed-off-by: Suraj Singh * Address review comment Signed-off-by: Suraj Singh --- .../percolator/PercolatorQuerySearchIT.java | 14 +- .../percolator/PercolateQueryBuilder.java | 169 +++++------------- .../PercolateQueryBuilderTests.java | 62 ++----- 3 files changed, 62 insertions(+), 183 deletions(-) diff --git a/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java index f78b74e272e..8d3c37bc9b0 100644 --- a/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/internalClusterTest/java/org/opensearch/percolator/PercolatorQuerySearchIT.java @@ -397,14 +397,14 @@ public class PercolatorQuerySearchIT extends OpenSearchIntegTestCase { logger.info("percolating empty doc"); SearchResponse response = client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "1", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "1", null, null, null)) .get(); assertHitCount(response, 1); assertThat(response.getHits().getAt(0).getId(), equalTo("1")); logger.info("percolating doc with 1 field"); response = client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "5", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "5", null, null, null)) .addSort("id", SortOrder.ASC) .get(); assertHitCount(response, 2); @@ -413,7 +413,7 @@ public class PercolatorQuerySearchIT extends OpenSearchIntegTestCase { logger.info("percolating doc with 2 fields"); response = client().prepareSearch() - .setQuery(new PercolateQueryBuilder("query", "test", "type", "6", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "6", null, null, null)) .addSort("id", SortOrder.ASC) .get(); assertHitCount(response, 3); @@ -438,7 +438,7 @@ public class PercolatorQuerySearchIT extends OpenSearchIntegTestCase { logger.info("percolating empty doc with source disabled"); IllegalArgumentException e = expectThrows( IllegalArgumentException.class, - () -> { client().prepareSearch().setQuery(new PercolateQueryBuilder("query", "test", "type", "1", null, null, null)).get(); } + () -> { client().prepareSearch().setQuery(new PercolateQueryBuilder("query", "test", "1", null, null, null)).get(); } ); assertThat(e.getMessage(), containsString("source disabled")); } @@ -1193,10 +1193,10 @@ public class PercolatorQuerySearchIT extends OpenSearchIntegTestCase { ) ) ) - .add(client().prepareSearch("test").setQuery(new PercolateQueryBuilder("query", "test", "type", "5", null, null, null))) + .add(client().prepareSearch("test").setQuery(new PercolateQueryBuilder("query", "test", "5", null, null, null))) .add( client().prepareSearch("test") // non existing doc, so error element - .setQuery(new PercolateQueryBuilder("query", "test", "type", "6", null, null, null)) + .setQuery(new PercolateQueryBuilder("query", "test", "6", null, null, null)) ) .get(); @@ -1228,7 +1228,7 @@ public class PercolatorQuerySearchIT extends OpenSearchIntegTestCase { item = response.getResponses()[5]; assertThat(item.getResponse(), nullValue()); assertThat(item.getFailureMessage(), notNullValue()); - assertThat(item.getFailureMessage(), containsString("[test/type/6] couldn't be found")); + assertThat(item.getFailureMessage(), containsString("[test/6] couldn't be found")); } public void testDisallowExpensiveQueries() throws IOException { diff --git a/modules/percolator/src/main/java/org/opensearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/opensearch/percolator/PercolateQueryBuilder.java index 87f08e2ff50..b2130eca3bb 100644 --- a/modules/percolator/src/main/java/org/opensearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/opensearch/percolator/PercolateQueryBuilder.java @@ -67,7 +67,6 @@ import org.opensearch.common.io.stream.NamedWriteableAwareStreamInput; import org.opensearch.common.io.stream.NamedWriteableRegistry; import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.logging.DeprecationLogger; import org.opensearch.common.xcontent.ConstructingObjectParser; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.NamedXContentRegistry; @@ -111,19 +110,11 @@ import static org.opensearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES; public class PercolateQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "percolate"; - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ParseField.class); - static final String DOCUMENT_TYPE_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [percolate] queries. " - + "The [document_type] should no longer be specified."; - static final String TYPE_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [percolate] queries. " - + "The [type] of the indexed document should no longer be specified."; - static final ParseField DOCUMENT_FIELD = new ParseField("document"); static final ParseField DOCUMENTS_FIELD = new ParseField("documents"); private static final ParseField NAME_FIELD = new ParseField("name"); private static final ParseField QUERY_FIELD = new ParseField("field"); - private static final ParseField DOCUMENT_TYPE_FIELD = new ParseField("document_type"); private static final ParseField INDEXED_DOCUMENT_FIELD_INDEX = new ParseField("index"); - private static final ParseField INDEXED_DOCUMENT_FIELD_TYPE = new ParseField("type"); private static final ParseField INDEXED_DOCUMENT_FIELD_ID = new ParseField("id"); private static final ParseField INDEXED_DOCUMENT_FIELD_ROUTING = new ParseField("routing"); private static final ParseField INDEXED_DOCUMENT_FIELD_PREFERENCE = new ParseField("preference"); @@ -131,29 +122,16 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder documents; private final XContentType documentXContentType; private final String indexedDocumentIndex; - @Deprecated - private final String indexedDocumentType; private final String indexedDocumentId; private final String indexedDocumentRouting; private final String indexedDocumentPreference; private final Long indexedDocumentVersion; private final Supplier documentSupplier; - /** - * @deprecated use {@link #PercolateQueryBuilder(String, BytesReference, XContentType)} with the document content - * type to avoid autodetection. - */ - @Deprecated - public PercolateQueryBuilder(String field, String documentType, BytesReference document) { - this(field, documentType, Collections.singletonList(document), XContentHelper.xContentType(document)); - } - /** * Creates a percolator query builder instance for percolating a provided document. * @@ -162,7 +140,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder documents, XContentType documentXContentType) { - this(field, null, documents, documentXContentType); - } - - @Deprecated - public PercolateQueryBuilder(String field, String documentType, List documents, XContentType documentXContentType) { if (field == null) { throw new IllegalArgumentException("[field] is a required argument"); } @@ -185,11 +158,9 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder documentSupplier) { - if (field == null) { - throw new IllegalArgumentException("[field] is a required argument"); - } - this.field = field; - this.documentType = documentType; - this.documents = Collections.emptyList(); - this.documentXContentType = null; - this.documentSupplier = documentSupplier; - indexedDocumentIndex = null; - indexedDocumentType = null; - indexedDocumentId = null; - indexedDocumentRouting = null; - indexedDocumentPreference = null; - indexedDocumentVersion = null; - } - /** * Creates a percolator query builder instance for percolating a document in a remote index. * * @param field The field that contains the percolator query * @param indexedDocumentIndex The index containing the document to percolate - * @param indexedDocumentType The type containing the document to percolate * @param indexedDocumentId The id of the document to percolate * @param indexedDocumentRouting The routing value for the document to percolate * @param indexedDocumentPreference The preference to use when fetching the document to percolate @@ -228,30 +181,6 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder documentSupplier) { + if (field == null) { + throw new IllegalArgumentException("[field] is a required argument"); + } + this.field = field; + this.documents = Collections.emptyList(); + this.documentXContentType = null; + this.documentSupplier = documentSupplier; + indexedDocumentIndex = null; + indexedDocumentId = null; + indexedDocumentRouting = null; + indexedDocumentPreference = null; + indexedDocumentVersion = null; + } + /** * Read from a stream. */ @@ -286,9 +228,20 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder { if (getResponse.isExists() == false) { throw new ResourceNotFoundException( - "indexed document [{}{}/{}] couldn't be found", + "indexed document [{}/{}] couldn't be found", indexedDocumentIndex, - indexedDocumentType == null ? "" : "/" + indexedDocumentType, indexedDocumentId ); } if (getResponse.isSourceEmpty()) { throw new IllegalArgumentException( - "indexed document [" - + indexedDocumentIndex - + (indexedDocumentType == null ? "" : "/" + indexedDocumentType) - + "/" - + indexedDocumentId - + "] source disabled" + "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentId + "] source disabled" ); } documentSupplier.set(getResponse.getSourceAsBytesRef()); @@ -534,7 +467,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder getDocuments() { return documents; } diff --git a/modules/percolator/src/test/java/org/opensearch/percolator/PercolateQueryBuilderTests.java b/modules/percolator/src/test/java/org/opensearch/percolator/PercolateQueryBuilderTests.java index 44d8d640860..87aa28a3346 100644 --- a/modules/percolator/src/test/java/org/opensearch/percolator/PercolateQueryBuilderTests.java +++ b/modules/percolator/src/test/java/org/opensearch/percolator/PercolateQueryBuilderTests.java @@ -148,16 +148,14 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase pqb.toQuery(createShardContext())); assertThat(e.getMessage(), equalTo("query builder must be rewritten first")); QueryBuilder rewrite = rewriteAndFetch(pqb, createShardContext()); - PercolateQueryBuilder geoShapeQueryBuilder = new PercolateQueryBuilder( - pqb.getField(), - pqb.getDocumentType(), - documentSource, - XContentType.JSON - ); + PercolateQueryBuilder geoShapeQueryBuilder = new PercolateQueryBuilder(pqb.getField(), documentSource, XContentType.JSON); assertEquals(geoShapeQueryBuilder, rewrite); } @@ -259,25 +251,19 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase new PercolateQueryBuilder("_field", "_document_type", null, null)); + e = expectThrows( + IllegalArgumentException.class, + () -> new PercolateQueryBuilder("_field", (List) null, XContentType.JSON) + ); assertThat(e.getMessage(), equalTo("[document] is a required argument")); - e = expectThrows( - IllegalArgumentException.class, - () -> { new PercolateQueryBuilder(null, null, "_index", "_type", "_id", null, null, null); } - ); + e = expectThrows(IllegalArgumentException.class, () -> { new PercolateQueryBuilder(null, "_index", "_id", null, null, null); }); assertThat(e.getMessage(), equalTo("[field] is a required argument")); - e = expectThrows( - IllegalArgumentException.class, - () -> { new PercolateQueryBuilder("_field", "_document_type", null, "_type", "_id", null, null, null); } - ); + e = expectThrows(IllegalArgumentException.class, () -> { new PercolateQueryBuilder("_field", null, "_id", null, null, null); }); assertThat(e.getMessage(), equalTo("[index] is a required argument")); - e = expectThrows( - IllegalArgumentException.class, - () -> { new PercolateQueryBuilder("_field", "_document_type", "_index", "_type", null, null, null, null); } - ); + e = expectThrows(IllegalArgumentException.class, () -> { new PercolateQueryBuilder("_field", "_index", null, null, null, null); }); assertThat(e.getMessage(), equalTo("[id] is a required argument")); } @@ -287,15 +273,6 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase())); - - QueryShardContext queryShardContext = createShardContext(); - QueryBuilder queryBuilder = parseQuery( - "{\"percolate\" : { \"index\": \"" - + indexedDocumentIndex - + "\", \"type\": \"_doc\", \"id\": \"" - + indexedDocumentId - + "\", \"field\":\"" - + queryField - + "\"}}" - ); - rewriteAndFetch(queryBuilder, queryShardContext).toQuery(queryShardContext); - } - public void testBothDocumentAndDocumentsSpecified() { IllegalArgumentException e = expectThrows( IllegalArgumentException.class, @@ -426,7 +384,7 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase supplier = () -> new BytesArray("{\"test\": \"test\"}"); String testName = "name1"; QueryShardContext shardContext = createShardContext(); - PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, null, supplier); + PercolateQueryBuilder percolateQueryBuilder = new PercolateQueryBuilder(queryField, supplier); percolateQueryBuilder.setName(testName); QueryBuilder rewrittenQueryBuilder = percolateQueryBuilder.doRewrite(shardContext);