From 03ea4bbe6e78a1202fa94521265bfaca494071e0 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 18 Oct 2023 20:27:52 +0200 Subject: [PATCH 1/2] Remove more explicit references to SearchResponse in tests (#101052) Follow up to #100966 introducing new combined assertion `assertSearchHitsWithoutFailures` to combine no-failure, count, and id assertions into one block. --- .../elasticsearch/join/query/InnerHitsIT.java | 25 +- .../PercolatorQuerySearchTests.java | 47 ++- .../index/WaitUntilRefreshIT.java | 6 +- .../index/shard/SearchIdleIT.java | 2 +- .../elasticsearch/recovery/RelocationIT.java | 7 +- .../search/fetch/subphase/InnerHitsIT.java | 24 +- .../search/query/MultiMatchQueryIT.java | 25 +- .../search/query/SearchQueryIT.java | 285 ++++++++---------- .../search/query/SimpleQueryStringIT.java | 216 +++++++------ .../hamcrest/ElasticsearchAssertions.java | 20 ++ .../DocumentLevelSecurityTests.java | 101 ++++--- .../integration/FieldLevelSecurityTests.java | 29 +- .../security/authz/ReadActionsTests.java | 72 +++-- 13 files changed, 411 insertions(+), 448 deletions(-) diff --git a/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java b/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java index 2e647e6ea08e5..0f8bf4e701e09 100644 --- a/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java +++ b/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java @@ -54,7 +54,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHit; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; @@ -612,18 +612,17 @@ public void testInnerHitsWithIgnoreUnmapped() { createIndexRequest("index1", "child_type", "2", "1").get(); client().prepareIndex("index2").setId("3").setSource("key", "value").get(); refresh(); - - SearchResponse response = client().prepareSearch("index1", "index2") - .setQuery( - boolQuery().should( - hasChildQuery("child_type", matchAllQuery(), ScoreMode.None).ignoreUnmapped(true) - .innerHit(new InnerHitBuilder().setIgnoreUnmapped(true)) - ).should(termQuery("key", "value")) - ) - .get(); - assertNoFailures(response); - assertHitCount(response, 2); - assertSearchHits(response, "1", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch("index1", "index2") + .setQuery( + boolQuery().should( + hasChildQuery("child_type", matchAllQuery(), ScoreMode.None).ignoreUnmapped(true) + .innerHit(new InnerHitBuilder().setIgnoreUnmapped(true)) + ).should(termQuery("key", "value")) + ), + "1", + "3" + ); } public void testTooHighResultWindow() { diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchTests.java index bce98ec90d527..05c2c27de40fc 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorQuerySearchTests.java @@ -9,7 +9,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.join.ScoreMode; -import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; @@ -47,7 +46,7 @@ import static org.elasticsearch.index.query.QueryBuilders.scriptQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.equalTo; @@ -87,17 +86,17 @@ public void testPercolateScriptQuery() throws IOException { .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .execute() .actionGet(); - SearchResponse response = client().prepareSearch("index") - .setQuery( - new PercolateQueryBuilder( - "query", - BytesReference.bytes(jsonBuilder().startObject().field("field1", "b").endObject()), - XContentType.JSON - ) - ) - .get(); - assertHitCount(response, 1); - assertSearchHits(response, "1"); + assertSearchHitsWithoutFailures( + client().prepareSearch("index") + .setQuery( + new PercolateQueryBuilder( + "query", + BytesReference.bytes(jsonBuilder().startObject().field("field1", "b").endObject()), + XContentType.JSON + ) + ), + "1" + ); } public void testPercolateQueryWithNestedDocuments_doNotLeakBitsetCacheEntries() throws Exception { @@ -265,17 +264,17 @@ public void testMapUnmappedFieldAsText() throws IOException { .get(); indicesAdmin().prepareRefresh().get(); - SearchResponse response = client().prepareSearch("test") - .setQuery( - new PercolateQueryBuilder( - "query", - BytesReference.bytes(jsonBuilder().startObject().field("field1", "value").endObject()), - XContentType.JSON - ) - ) - .get(); - assertHitCount(response, 1); - assertSearchHits(response, "1"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test") + .setQuery( + new PercolateQueryBuilder( + "query", + BytesReference.bytes(jsonBuilder().startObject().field("field1", "value").endObject()), + XContentType.JSON + ) + ), + "1" + ); } public void testRangeQueriesWithNow() throws Exception { diff --git a/server/src/internalClusterTest/java/org/elasticsearch/index/WaitUntilRefreshIT.java b/server/src/internalClusterTest/java/org/elasticsearch/index/WaitUntilRefreshIT.java index 6970f73e591fc..09290f40d6c29 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/index/WaitUntilRefreshIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/index/WaitUntilRefreshIT.java @@ -75,7 +75,7 @@ public void testDelete() throws InterruptedException, ExecutionException { DeleteResponse delete = client().prepareDelete("test", "1").setRefreshPolicy(RefreshPolicy.WAIT_UNTIL).get(); assertEquals(DocWriteResponse.Result.DELETED, delete.getResult()); assertFalse("request shouldn't have forced a refresh", delete.forcedRefresh()); - assertNoSearchHits(client().prepareSearch("test").setQuery(matchQuery("foo", "bar")).get()); + assertNoSearchHits(client().prepareSearch("test").setQuery(matchQuery("foo", "bar"))); } public void testUpdate() throws InterruptedException, ExecutionException { @@ -109,7 +109,7 @@ public void testUpdate() throws InterruptedException, ExecutionException { .get(); assertEquals(2, update.getVersion()); assertFalse("request shouldn't have forced a refresh", update.forcedRefresh()); - assertNoSearchHits(client().prepareSearch("test").setQuery(matchQuery("foo", "cat")).get()); + assertNoSearchHits(client().prepareSearch("test").setQuery(matchQuery("foo", "cat"))); } public void testBulk() { @@ -129,7 +129,7 @@ public void testBulk() { bulk = client().prepareBulk().setRefreshPolicy(RefreshPolicy.WAIT_UNTIL); bulk.add(client().prepareDelete("test", "1")); assertBulkSuccess(bulk.get()); - assertNoSearchHits(client().prepareSearch("test").setQuery(matchQuery("foo", "bar")).get()); + assertNoSearchHits(client().prepareSearch("test").setQuery(matchQuery("foo", "bar"))); // Update makes a noop bulk = client().prepareBulk().setRefreshPolicy(RefreshPolicy.WAIT_UNTIL); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/index/shard/SearchIdleIT.java b/server/src/internalClusterTest/java/org/elasticsearch/index/shard/SearchIdleIT.java index 3a10539d3c451..22bb5974ad550 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/index/shard/SearchIdleIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/index/shard/SearchIdleIT.java @@ -88,7 +88,7 @@ private void runTestAutomaticRefresh(final IntToLongFunction count) throws Inter assertFalse(indexService.getIndexSettings().isExplicitRefresh()); ensureGreen(); AtomicInteger totalNumDocs = new AtomicInteger(Integer.MAX_VALUE); - assertNoSearchHits(client().prepareSearch().get()); + assertNoSearchHits(client().prepareSearch()); int numDocs = scaledRandomIntBetween(25, 100); totalNumDocs.set(numDocs); CountDownLatch indexingDone = new CountDownLatch(numDocs); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/recovery/RelocationIT.java b/server/src/internalClusterTest/java/org/elasticsearch/recovery/RelocationIT.java index a18015da0737a..6a8de644b00cf 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/recovery/RelocationIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/recovery/RelocationIT.java @@ -27,6 +27,7 @@ import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; @@ -79,7 +80,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.everyItem; import static org.hamcrest.Matchers.in; @@ -522,9 +523,7 @@ public void testIndexSearchAndRelocateConcurrently() throws Exception { final int numIters = randomIntBetween(10, 20); for (int i = 0; i < numIters; i++) { logger.info(" --> checking iteration {}", i); - SearchResponse afterRelocation = client().prepareSearch().setSize(ids.size()).get(); - assertNoFailures(afterRelocation); - assertSearchHits(afterRelocation, ids.toArray(new String[ids.size()])); + assertSearchHitsWithoutFailures(client().prepareSearch().setSize(ids.size()), ids.toArray(Strings.EMPTY_ARRAY)); } stopped.set(true); for (Thread searchThread : searchThreads) { diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java index 4041bbc431f75..183f925ced9d3 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java @@ -52,7 +52,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHit; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; @@ -922,17 +922,17 @@ public void testInnerHitsWithIgnoreUnmapped() throws Exception { client().prepareIndex("index2").setId("3").setSource("key", "value").get(); refresh(); - SearchResponse response = client().prepareSearch("index1", "index2") - .setQuery( - boolQuery().should( - nestedQuery("nested_type", matchAllQuery(), ScoreMode.None).ignoreUnmapped(true) - .innerHit(new InnerHitBuilder().setIgnoreUnmapped(true)) - ).should(termQuery("key", "value")) - ) - .get(); - assertNoFailures(response); - assertHitCount(response, 2); - assertSearchHits(response, "1", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch("index1", "index2") + .setQuery( + boolQuery().should( + nestedQuery("nested_type", matchAllQuery(), ScoreMode.None).ignoreUnmapped(true) + .innerHit(new InnerHitBuilder().setIgnoreUnmapped(true)) + ).should(termQuery("key", "value")) + ), + "1", + "3" + ); } public void testUseMaxDocInsteadOfSize() throws Exception { diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java index 9e55c38336c1b..3af7502cefead 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java @@ -53,7 +53,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFirstHit; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; import static org.hamcrest.Matchers.anyOf; @@ -348,17 +348,18 @@ public void testPhraseType() { .get(); assertThat(searchResponse.getHits().getTotalHits().value, greaterThan(1L)); - searchResponse = client().prepareSearch("test") - .setQuery( - randomizeType( - multiMatchQuery("the Ul", "full_name_phrase", "first_name_phrase", "last_name_phrase", "category_phrase").operator( - Operator.OR - ).type(MatchQueryParser.Type.PHRASE_PREFIX) - ) - ) - .get(); - assertSearchHits(searchResponse, "ultimate2", "ultimate1"); - assertHitCount(searchResponse, 2L); + assertSearchHitsWithoutFailures( + client().prepareSearch("test") + .setQuery( + randomizeType( + multiMatchQuery("the Ul", "full_name_phrase", "first_name_phrase", "last_name_phrase", "category_phrase").operator( + Operator.OR + ).type(MatchQueryParser.Type.PHRASE_PREFIX) + ) + ), + "ultimate2", + "ultimate1" + ); } public void testSingleField() throws NoSuchFieldException, IllegalAccessException { diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/query/SearchQueryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/query/SearchQueryIT.java index 18a5ad78995da..be265cb3985e6 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/query/SearchQueryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/query/SearchQueryIT.java @@ -106,6 +106,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; @@ -414,20 +415,13 @@ public void testIdsQueryTestsIdIndexed() throws Exception { client().prepareIndex("test").setId("3").setSource("field1", "value3") ); - SearchResponse searchResponse = client().prepareSearch().setQuery(constantScoreQuery(idsQuery().addIds("1", "3"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); - - searchResponse = client().prepareSearch().setQuery(idsQuery().addIds("1", "3")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(constantScoreQuery(idsQuery().addIds("1", "3"))), "1", "3"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(idsQuery().addIds("1", "3")), "1", "3"); assertHitCount(client().prepareSearch().setQuery(idsQuery().addIds("7", "10")), 0L); // repeat..., with terms - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(termsQuery("_id", "1", "3"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(constantScoreQuery(termsQuery("_id", "1", "3"))), "1", "3"); } public void testTermIndexQuery() throws Exception { @@ -440,22 +434,22 @@ public void testTermIndexQuery() throws Exception { } for (String indexName : indexNames) { - SearchResponse request = client().prepareSearch().setQuery(constantScoreQuery(termQuery("_index", indexName))).get(); - SearchResponse searchResponse = assertSearchResponse(request); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, indexName + "1"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(constantScoreQuery(termQuery("_index", indexName))), + indexName + "1" + ); } for (String indexName : indexNames) { - SearchResponse request = client().prepareSearch().setQuery(constantScoreQuery(termsQuery("_index", indexName))).get(); - SearchResponse searchResponse = assertSearchResponse(request); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, indexName + "1"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(constantScoreQuery(termsQuery("_index", indexName))), + indexName + "1" + ); } for (String indexName : indexNames) { - SearchResponse request = client().prepareSearch().setQuery(constantScoreQuery(matchQuery("_index", indexName))).get(); - SearchResponse searchResponse = assertSearchResponse(request); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, indexName + "1"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(constantScoreQuery(matchQuery("_index", indexName))), + indexName + "1" + ); } { SearchResponse request = client().prepareSearch().setQuery(constantScoreQuery(termsQuery("_index", indexNames))).get(); @@ -516,35 +510,15 @@ public void testFilterExistsMissing() throws Exception { ) ); - SearchResponse searchResponse = client().prepareSearch().setQuery(existsQuery("field1")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); - - searchResponse = client().prepareSearch().setQuery(constantScoreQuery(existsQuery("field1"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); - - searchResponse = client().prepareSearch().setQuery(queryStringQuery("_exists_:field1")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); - - searchResponse = client().prepareSearch().setQuery(existsQuery("field2")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); - - searchResponse = client().prepareSearch().setQuery(existsQuery("field3")).get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("4")); - + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(existsQuery("field1")), "1", "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(constantScoreQuery(existsQuery("field1"))), "1", "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(queryStringQuery("_exists_:field1")), "1", "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(existsQuery("field2")), "1", "3"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(existsQuery("field3")), "4"); // wildcard check - searchResponse = client().prepareSearch().setQuery(existsQuery("x*")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); - + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(existsQuery("x*")), "1", "2"); // object check - searchResponse = client().prepareSearch().setQuery(existsQuery("obj1")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(existsQuery("obj1")), "1", "2"); } public void testPassQueryOrFilterAsJSONString() throws Exception { @@ -603,21 +577,22 @@ public void testMatchQueryFuzzy() throws Exception { ); assertHitCount(client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.fromEdits(0))), 0L); - SearchResponse searchResponse = client().prepareSearch() - .setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.fromEdits(1))) - .get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); - - searchResponse = client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.fromString("AUTO"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.fromEdits(1))), + "1", + "2" + ); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.fromString("AUTO"))), + "1", + "2" + ); assertHitCount(client().prepareSearch().setQuery(matchQuery("text", "uniy").fuzziness(Fuzziness.fromString("AUTO:5,7"))), 0L); - - searchResponse = client().prepareSearch().setQuery(matchQuery("text", "unify").fuzziness(Fuzziness.fromString("AUTO:5,7"))).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "2"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(matchQuery("text", "unify").fuzziness(Fuzziness.fromString("AUTO:5,7"))), + "2" + ); } public void testMultiMatchQuery() throws Exception { @@ -631,40 +606,31 @@ public void testMultiMatchQuery() throws Exception { ); MultiMatchQueryBuilder builder = multiMatchQuery("value1 value2 value4", "field1", "field2"); - SearchResponse searchResponse = client().prepareSearch() - .setQuery(builder) - .addAggregation(AggregationBuilders.terms("field1").field("field1.keyword")) - .get(); - - assertHitCount(searchResponse, 2L); // this uses dismax so scores are equal and the order can be arbitrary - assertSearchHits(searchResponse, "1", "2"); - - searchResponse = client().prepareSearch().setQuery(builder).get(); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(builder).addAggregation(AggregationBuilders.terms("field1").field("field1.keyword")), + "1", + "2" + ); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(builder), "1", "2"); indicesAdmin().prepareRefresh("test").get(); builder = multiMatchQuery("value1", "field1", "field2").operator(Operator.AND); // Operator only applies on terms inside a field! // Fields are always OR-ed together. - searchResponse = client().prepareSearch().setQuery(builder).get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("1")); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(builder), "1"); refresh(); builder = multiMatchQuery("value1", "field1").field("field3", 1.5f).operator(Operator.AND); // Operator only applies on terms inside // a field! Fields are always OR-ed // together. - searchResponse = client().prepareSearch().setQuery(builder).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "3", "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(builder), "3", "1"); indicesAdmin().prepareRefresh("test").get(); builder = multiMatchQuery("value1").field("field1").field("field3", 1.5f).operator(Operator.AND); // Operator only applies on terms // inside a field! Fields are // always OR-ed together. - searchResponse = client().prepareSearch().setQuery(builder).get(); + SearchResponse searchResponse = client().prepareSearch().setQuery(builder).get(); assertHitCount(searchResponse, 2L); assertSearchHits(searchResponse, "3", "1"); @@ -693,9 +659,7 @@ public void testMultiMatchQuery() throws Exception { } builder.lenient(true); - searchResponse = client().prepareSearch().setQuery(builder).get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("1")); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(builder), "1"); } public void testMatchQueryZeroTermsQuery() { @@ -894,46 +858,39 @@ public void testTermsQuery() throws Exception { client().prepareIndex("test").setId("3").setSource("str", "3", "lng", 3L, "dbl", 3.0d), client().prepareIndex("test").setId("4").setSource("str", "4", "lng", 4L, "dbl", 4.0d) ); - - SearchResponse searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("str", "1", "4"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "4"); - - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new long[] { 2, 3 }))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "2", "3"); - - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new double[] { 2, 3 }))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "2", "3"); - - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new int[] { 1, 3 }))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); - - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new float[] { 2, 4 }))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "2", "4"); - + assertSearchHitsWithoutFailures(client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("str", "1", "4"))), "1", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new long[] { 2, 3 }))), + "2", + "3" + ); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new double[] { 2, 3 }))), + "2", + "3" + ); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new int[] { 1, 3 }))), + "1", + "3" + ); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new float[] { 2, 4 }))), + "2", + "4" + ); // test partial matching - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("str", "2", "5"))).get(); - assertNoFailures(searchResponse); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("2")); - - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new double[] { 2, 5 }))).get(); - assertNoFailures(searchResponse); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("2")); - - searchResponse = client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new long[] { 2, 5 }))).get(); - assertNoFailures(searchResponse); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("2")); - + assertSearchHitsWithoutFailures(client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("str", "2", "5"))), "2"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new double[] { 2, 5 }))), + "2" + ); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new long[] { 2, 5 }))), + "2" + ); // test valid type, but no matching terms assertHitCount(client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("str", "5", "6"))), 0L); - assertHitCount(client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("dbl", new double[] { 5, 6 }))), 0L); assertHitCount(client().prepareSearch("test").setQuery(constantScoreQuery(termsQuery("lng", new long[] { 5, 6 }))), 0L); } @@ -1014,50 +971,55 @@ public void testTermsLookupFilter() throws Exception { client().prepareIndex("test").setId("3").setSource("term", "3"), client().prepareIndex("test").setId("4").setSource("term", "4") ); - - SearchResponse searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup", "1", "terms"))) - .get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "1", "terms"))), + "1", + "3" + ); // same as above, just on the _id... - searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("_id", new TermsLookup("lookup", "1", "terms"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("_id", new TermsLookup("lookup", "1", "terms"))), + "1", + "3" + ); // another search with same parameters... - searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "1", "terms"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "1", "terms"))), + "1", + "3" + ); - searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "2", "terms"))).get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("2")); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "2", "terms"))), + "2" + ); - searchResponse = client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "3", "terms"))).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "2", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "3", "terms"))), + "2", + "4" + ); assertHitCount(client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup", "4", "terms"))), 0L); - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "1", "arr.term"))) - .get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "1", "arr.term"))), + "1", + "3" + ); - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "2", "arr.term"))) - .get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("2")); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "2", "arr.term"))), + "2" + ); - searchResponse = client().prepareSearch("test") - .setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "3", "arr.term"))) - .get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "2", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch("test").setQuery(termsLookupQuery("term", new TermsLookup("lookup2", "3", "arr.term"))), + "2", + "4" + ); assertHitCount( client().prepareSearch("test").setQuery(termsLookupQuery("not_exists", new TermsLookup("lookup2", "3", "arr.term"))), @@ -1737,17 +1699,12 @@ public void testMatchPhrasePrefixQuery() throws ExecutionException, InterruptedE client().prepareIndex("test1").setId("2").setSource("field", "trying out Elasticsearch") ); - SearchResponse searchResponse = client().prepareSearch() - .setQuery(matchPhrasePrefixQuery("field", "Johnnie la").slop(between(2, 5))) - .get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); - searchResponse = client().prepareSearch().setQuery(matchPhrasePrefixQuery("field", "trying")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "2"); - searchResponse = client().prepareSearch().setQuery(matchPhrasePrefixQuery("field", "try")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "2"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(matchPhrasePrefixQuery("field", "Johnnie la").slop(between(2, 5))), + "1" + ); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(matchPhrasePrefixQuery("field", "trying")), "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(matchPhrasePrefixQuery("field", "try")), "2"); } public void testQueryStringParserCache() throws Exception { diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/query/SimpleQueryStringIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/query/SimpleQueryStringIT.java index 930bc565969cc..80c3e73e76bd5 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/query/SimpleQueryStringIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/query/SimpleQueryStringIT.java @@ -51,6 +51,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -84,29 +85,30 @@ public void testSimpleQueryString() throws ExecutionException, InterruptedExcept client().prepareIndex("test").setId("6").setSource("otherbody", "spaghetti") ); - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar")).get(); - assertHitCount(searchResponse, 3L); - assertSearchHits(searchResponse, "1", "2", "3"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar")), "1", "2", "3"); // Tests boost value setting. In this case doc 1 should always be ranked above the other // two matches. - searchResponse = client().prepareSearch() + SearchResponse searchResponse = client().prepareSearch() .setQuery(boolQuery().should(simpleQueryStringQuery("\"foo bar\"").boost(10.0f)).should(termQuery("body", "eggplant"))) .get(); assertHitCount(searchResponse, 2L); assertFirstHit(searchResponse, hasId("3")); - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").defaultOperator(Operator.AND)).get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("3")); - - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("\"quux baz\" +(eggplant | spaghetti)")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "4", "5"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").defaultOperator(Operator.AND)), + "3" + ); - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("eggplants").analyzer("mock_snowball")).get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("4")); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("\"quux baz\" +(eggplant | spaghetti)")), + "4", + "5" + ); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("eggplants").analyzer("mock_snowball")), + "4" + ); searchResponse = client().prepareSearch() .setQuery(simpleQueryStringQuery("spaghetti").field("body", 1000.0f).field("otherbody", 2.0f).queryName("myquery")) @@ -116,9 +118,7 @@ public void testSimpleQueryString() throws ExecutionException, InterruptedExcept assertSearchHits(searchResponse, "5", "6"); assertThat(searchResponse.getHits().getAt(0).getMatchedQueries()[0], equalTo("myquery")); - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("spaghetti").field("*body")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "5", "6"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("spaghetti").field("*body")), "5", "6"); } public void testSimpleQueryStringMinimumShouldMatch() throws Exception { @@ -134,31 +134,35 @@ public void testSimpleQueryStringMinimumShouldMatch() throws Exception { ); logger.info("--> query 1"); - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "3", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")), + "3", + "4" + ); logger.info("--> query 2"); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")) - .get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "3", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")), + "3", + "4" + ); // test case from #13884 logger.info("--> query 3"); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo").field("body").field("body2").field("body3").minimumShouldMatch("-50%")) - .get(); - assertHitCount(searchResponse, 3L); - assertSearchHits(searchResponse, "1", "3", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch() + .setQuery(simpleQueryStringQuery("foo").field("body").field("body2").field("body3").minimumShouldMatch("-50%")), + "1", + "3", + "4" + ); logger.info("--> query 4"); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo bar baz").field("body").field("body2").minimumShouldMatch("70%")) - .get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "3", "4"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body").field("body2").minimumShouldMatch("70%")), + "3", + "4" + ); indexRandom( true, @@ -170,23 +174,32 @@ public void testSimpleQueryStringMinimumShouldMatch() throws Exception { ); logger.info("--> query 5"); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")) - .get(); - assertHitCount(searchResponse, 4L); - assertSearchHits(searchResponse, "3", "4", "7", "8"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").field("body").field("body2").minimumShouldMatch("2")), + "3", + "4", + "7", + "8" + ); logger.info("--> query 6"); - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")).get(); - assertHitCount(searchResponse, 5L); - assertSearchHits(searchResponse, "3", "4", "6", "7", "8"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").minimumShouldMatch("2")), + "3", + "4", + "6", + "7", + "8" + ); logger.info("--> query 7"); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo bar baz").field("body2").field("other").minimumShouldMatch("70%")) - .get(); - assertHitCount(searchResponse, 3L); - assertSearchHits(searchResponse, "6", "7", "8"); + assertSearchHitsWithoutFailures( + client().prepareSearch() + .setQuery(simpleQueryStringQuery("foo bar baz").field("body2").field("other").minimumShouldMatch("70%")), + "6", + "7", + "8" + ); } public void testNestedFieldSimpleQueryString() throws IOException { @@ -211,21 +224,10 @@ public void testNestedFieldSimpleQueryString() throws IOException { client().prepareIndex("test").setId("1").setSource("body", "foo bar baz").get(); refresh(); - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); - - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); - - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body.sub")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); - - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body.sub")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body")), "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body")), "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body.sub")), "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar baz").field("body.sub")), "1"); } public void testSimpleQueryStringFlags() throws ExecutionException, InterruptedException { @@ -240,23 +242,26 @@ public void testSimpleQueryStringFlags() throws ExecutionException, InterruptedE client().prepareIndex("test").setId("6").setSource("otherbody", "spaghetti") ); - SearchResponse searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo bar").flags(SimpleQueryStringFlag.ALL)) - .get(); - assertHitCount(searchResponse, 3L); - assertSearchHits(searchResponse, "1", "2", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch().setQuery(simpleQueryStringQuery("foo bar").flags(SimpleQueryStringFlag.ALL)), + "1", + "2", + "3" + ); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo | bar").defaultOperator(Operator.AND).flags(SimpleQueryStringFlag.OR)) - .get(); - assertHitCount(searchResponse, 3L); - assertSearchHits(searchResponse, "1", "2", "3"); + assertSearchHitsWithoutFailures( + client().prepareSearch() + .setQuery(simpleQueryStringQuery("foo | bar").defaultOperator(Operator.AND).flags(SimpleQueryStringFlag.OR)), + "1", + "2", + "3" + ); - searchResponse = client().prepareSearch() - .setQuery(simpleQueryStringQuery("foo | bar").defaultOperator(Operator.AND).flags(SimpleQueryStringFlag.NONE)) - .get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("3")); + assertSearchHitsWithoutFailures( + client().prepareSearch() + .setQuery(simpleQueryStringQuery("foo | bar").defaultOperator(Operator.AND).flags(SimpleQueryStringFlag.NONE)), + "3" + ); assertHitCount( client().prepareSearch() @@ -274,18 +279,18 @@ public void testSimpleQueryStringFlags() throws ExecutionException, InterruptedE 1L ); - searchResponse = client().prepareSearch() - .setQuery( - simpleQueryStringQuery("quuz~1 + egg*").flags( - SimpleQueryStringFlag.WHITESPACE, - SimpleQueryStringFlag.AND, - SimpleQueryStringFlag.FUZZY, - SimpleQueryStringFlag.PREFIX - ) - ) - .get(); - assertHitCount(searchResponse, 1L); - assertFirstHit(searchResponse, hasId("4")); + assertSearchHitsWithoutFailures( + client().prepareSearch() + .setQuery( + simpleQueryStringQuery("quuz~1 + egg*").flags( + SimpleQueryStringFlag.WHITESPACE, + SimpleQueryStringFlag.AND, + SimpleQueryStringFlag.FUZZY, + SimpleQueryStringFlag.PREFIX + ) + ), + "4" + ); } public void testSimpleQueryStringLenient() throws ExecutionException, InterruptedException { @@ -305,10 +310,7 @@ public void testSimpleQueryStringLenient() throws ExecutionException, Interrupte assertHitCount(searchResponse, 1L); assertSearchHits(searchResponse, "1"); - searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("foo").field("field").lenient(true)).get(); - assertNoFailures(searchResponse); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("foo").field("field").lenient(true)), "1"); } // Issue #7967 @@ -320,12 +322,9 @@ public void testLenientFlagBeingTooLenient() throws Exception { ); BoolQueryBuilder q = boolQuery().should(simpleQueryStringQuery("bar").field("num").field("body").lenient(true)); - SearchResponse resp = client().prepareSearch("test").setQuery(q).get(); - assertNoFailures(resp); // the bug is that this would be parsed into basically a match_all // query and this would match both documents - assertHitCount(resp, 1); - assertSearchHits(resp, "1"); + assertSearchHitsWithoutFailures(client().prepareSearch("test").setQuery(q), "1"); } public void testSimpleQueryStringAnalyzeWildcard() throws ExecutionException, InterruptedException, IOException { @@ -346,32 +345,21 @@ public void testSimpleQueryStringAnalyzeWildcard() throws ExecutionException, In indexRandom(true, client().prepareIndex("test1").setId("1").setSource("location", "Köln")); refresh(); - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("Köln*").field("location")).get(); - assertNoFailures(searchResponse); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("Köln*").field("location")), "1"); } public void testSimpleQueryStringUsesFieldAnalyzer() throws Exception { client().prepareIndex("test").setId("1").setSource("foo", 123, "bar", "abc").get(); client().prepareIndex("test").setId("2").setSource("foo", 234, "bar", "bcd").get(); - refresh(); - - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("123").field("foo").field("bar")).get(); - assertHitCount(searchResponse, 1L); - assertSearchHits(searchResponse, "1"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("123").field("foo").field("bar")), "1"); } public void testSimpleQueryStringOnIndexMetaField() throws Exception { client().prepareIndex("test").setId("1").setSource("foo", 123, "bar", "abc").get(); client().prepareIndex("test").setId("2").setSource("foo", 234, "bar", "bcd").get(); - refresh(); - - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("test").field("_index")).get(); - assertHitCount(searchResponse, 2L); - assertSearchHits(searchResponse, "1", "2"); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("test").field("_index")), "1", "2"); } public void testEmptySimpleQueryStringWithAnalysis() throws Exception { @@ -393,9 +381,7 @@ public void testEmptySimpleQueryStringWithAnalysis() throws Exception { indexRandom(true, client().prepareIndex("test1").setId("1").setSource("body", "Some Text")); refresh(); - SearchResponse searchResponse = client().prepareSearch().setQuery(simpleQueryStringQuery("the*").field("body")).get(); - assertNoFailures(searchResponse); - assertHitCount(searchResponse, 0L); + assertSearchHitsWithoutFailures(client().prepareSearch().setQuery(simpleQueryStringQuery("the*").field("body"))); } public void testBasicAllQuery() throws Exception { diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java index 7868043ac61b3..885a1aafd89b8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java @@ -223,6 +223,15 @@ public static String formatShardStatus(SearchResponse response) { return msg.toString(); } + public static void assertNoSearchHits(SearchRequestBuilder searchRequestBuilder) { + var searchResponse = searchRequestBuilder.get(); + try { + assertNoSearchHits(searchResponse); + } finally { + searchResponse.decRef(); + } + } + public static void assertNoSearchHits(SearchResponse searchResponse) { assertThat(searchResponse.getHits().getHits(), emptyArray()); } @@ -244,6 +253,17 @@ public static void assertSearchHits(SearchResponse searchResponse, String... ids ); } + public static void assertSearchHitsWithoutFailures(SearchRequestBuilder requestBuilder, String... ids) { + var res = requestBuilder.get(); + try { + assertNoFailures(res); + assertHitCount(res, ids.length); + assertSearchHits(res, ids); + } finally { + res.decRef(); + } + } + public static void assertSortValues(SearchRequestBuilder searchRequestBuilder, Object[]... sortValues) { var searchResponse = searchRequestBuilder.get(); try { diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DocumentLevelSecurityTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DocumentLevelSecurityTests.java index 80f2ed881bbad..3baec135d107f 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DocumentLevelSecurityTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DocumentLevelSecurityTests.java @@ -95,6 +95,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER; @@ -208,32 +209,29 @@ public void testSimpleQuery() throws Exception { client().prepareIndex("test").setId("2").setSource("field2", "value2").setRefreshPolicy(IMMEDIATE).get(); client().prepareIndex("test").setId("3").setSource("field3", "value3").setRefreshPolicy(IMMEDIATE).get(); - SearchResponse response = client().filterWithHeader( - Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD)) - ) - .prepareSearch("test") - .setQuery(randomBoolean() ? QueryBuilders.termQuery("field1", "value1") : QueryBuilders.matchAllQuery()) - .get(); - assertHitCount(response, 1); - assertSearchHits(response, "1"); - - response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) - .prepareSearch("test") - .setQuery(randomBoolean() ? QueryBuilders.termQuery("field2", "value2") : QueryBuilders.matchAllQuery()) - .get(); - assertHitCount(response, 1); - assertSearchHits(response, "2"); - + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(randomBoolean() ? QueryBuilders.termQuery("field1", "value1") : QueryBuilders.matchAllQuery()), + "1" + ); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(randomBoolean() ? QueryBuilders.termQuery("field2", "value2") : QueryBuilders.matchAllQuery()), + "2" + ); QueryBuilder combined = QueryBuilders.boolQuery() .should(QueryBuilders.termQuery("field2", "value2")) .should(QueryBuilders.termQuery("field1", "value1")) .minimumShouldMatch(1); - response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) - .prepareSearch("test") - .setQuery(randomBoolean() ? combined : QueryBuilders.matchAllQuery()) - .get(); - assertHitCount(response, 2); - assertSearchHits(response, "1", "2"); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) + .prepareSearch("test") + .setQuery(randomBoolean() ? combined : QueryBuilders.matchAllQuery()), + "1", + "2" + ); } public void testGetApi() throws Exception { @@ -676,11 +674,14 @@ public void testTermsLookupOnIndexWithDLS() { // Lookup doc#1 is: visible to user1 and user3, but hidden from user2 TermsQueryBuilder lookup = QueryBuilders.termsLookupQuery("search_field", new TermsLookup("lookup_index", "1", "lookup_field")); - SearchResponse response = client().filterWithHeader( - Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD)) - ).prepareSearch("search_index").setQuery(lookup).get(); - assertHitCount(response, 3); - assertSearchHits(response, "1", "2", "3"); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1", USERS_PASSWD))) + .prepareSearch("search_index") + .setQuery(lookup), + "1", + "2", + "3" + ); assertHitCount( client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) @@ -688,13 +689,16 @@ public void testTermsLookupOnIndexWithDLS() { .setQuery(lookup), 0 ); - - response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) - .prepareSearch("search_index") - .setQuery(lookup) - .get(); - assertHitCount(response, 5); - assertSearchHits(response, "1", "2", "3", "4", "5"); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) + .prepareSearch("search_index") + .setQuery(lookup), + "1", + "2", + "3", + "4", + "5" + ); // Lookup doc#2 is: hidden from user1, visible to user2 and user3 lookup = QueryBuilders.termsLookupQuery("search_field", new TermsLookup("lookup_index", "2", "lookup_field")); assertHitCount( @@ -704,20 +708,21 @@ public void testTermsLookupOnIndexWithDLS() { .get(), 0 ); - - response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) - .prepareSearch("search_index") - .setQuery(lookup) - .get(); - assertHitCount(response, 2); - assertSearchHits(response, "2", "5"); - - response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) - .prepareSearch("search_index") - .setQuery(lookup) - .get(); - assertHitCount(response, 3); - assertSearchHits(response, "1", "2", "5"); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD))) + .prepareSearch("search_index") + .setQuery(lookup), + "2", + "5" + ); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user3", USERS_PASSWD))) + .prepareSearch("search_index") + .setQuery(lookup), + "1", + "2", + "5" + ); } public void testTVApi() throws Exception { diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FieldLevelSecurityTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FieldLevelSecurityTests.java index 1aee3e445ab94..27b6a10e0619c 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FieldLevelSecurityTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FieldLevelSecurityTests.java @@ -81,7 +81,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; -import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHitsWithoutFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER; import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; @@ -633,20 +633,19 @@ public void testTermsLookupOnIndexWithFLS() { client().prepareIndex("lookup_index").setId("2").setSource("other", "value2", "field", "value2").setRefreshPolicy(IMMEDIATE).get(); // user sees the terms doc field - SearchResponse response = client().filterWithHeader( - Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD)) - ) - .prepareSearch("search_index") - .setQuery(QueryBuilders.termsLookupQuery("field", new TermsLookup("lookup_index", "1", "field"))) - .get(); - assertHitCount(response, 2); - assertSearchHits(response, "1", "2"); - response = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))) - .prepareSearch("search_index") - .setQuery(QueryBuilders.termsLookupQuery("field", new TermsLookup("lookup_index", "2", "field"))) - .get(); - assertHitCount(response, 1); - assertSearchHits(response, "1"); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))) + .prepareSearch("search_index") + .setQuery(QueryBuilders.termsLookupQuery("field", new TermsLookup("lookup_index", "1", "field"))), + "1", + "2" + ); + assertSearchHitsWithoutFailures( + client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))) + .prepareSearch("search_index") + .setQuery(QueryBuilders.termsLookupQuery("field", new TermsLookup("lookup_index", "2", "field"))), + "1" + ); // user does not see the terms doc field assertHitCount( client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user6", USERS_PASSWD))) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java index d90434ea7d9a8..f48a6eaf2655f 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java @@ -13,13 +13,13 @@ import org.elasticsearch.action.search.MultiSearchResponse; import org.elasticsearch.action.search.SearchAction; import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.termvectors.MultiTermVectorsAction; import org.elasticsearch.action.termvectors.MultiTermVectorsResponse; import org.elasticsearch.action.termvectors.TermVectorsAction; import org.elasticsearch.core.Strings; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.search.SearchHit; import org.elasticsearch.test.SecurityIntegTestCase; @@ -54,17 +54,13 @@ protected String configRoles() { public void testSearchForAll() { // index1 is not authorized and referred to through wildcard createIndicesWithRandomAliases("test1", "test2", "test3", "index1"); - - SearchResponse searchResponse = trySearch(); - assertReturnedIndices(searchResponse, "test1", "test2", "test3"); + assertReturnedIndices(trySearch(), "test1", "test2", "test3"); } public void testSearchForWildcard() { // index1 is not authorized and referred to through wildcard createIndicesWithRandomAliases("test1", "test2", "test3", "index1"); - - SearchResponse searchResponse = trySearch("*"); - assertReturnedIndices(searchResponse, "test1", "test2", "test3"); + assertReturnedIndices(trySearch("*"), "test1", "test2", "test3"); } public void testSearchNonAuthorizedWildcard() { @@ -78,7 +74,7 @@ public void testSearchNonAuthorizedWildcardDisallowNoIndices() { createIndicesWithRandomAliases("test1", "test2", "index1", "index2"); IndexNotFoundException e = expectThrows( IndexNotFoundException.class, - () -> trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean()), "index*") + trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean()), "index*") ); assertEquals("no such index [index*]", e.getMessage()); } @@ -90,20 +86,19 @@ public void testEmptyClusterSearchForAll() { public void testEmptyClusterSearchForAllDisallowNoIndices() { IndexNotFoundException e = expectThrows( IndexNotFoundException.class, - () -> trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean())) + trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean())) ); assertEquals("no such index [[]]", e.getMessage()); } public void testEmptyClusterSearchForWildcard() { - SearchResponse searchResponse = trySearch("*"); - assertNoSearchHits(searchResponse); + assertNoSearchHits(trySearch("*")); } public void testEmptyClusterSearchForWildcardDisallowNoIndices() { IndexNotFoundException e = expectThrows( IndexNotFoundException.class, - () -> trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean()), "*") + trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean()), "*") ); assertEquals("no such index [*]", e.getMessage()); } @@ -117,7 +112,7 @@ public void testEmptyAuthorizedIndicesSearchForAllDisallowNoIndices() { createIndicesWithRandomAliases("index1", "index2"); IndexNotFoundException e = expectThrows( IndexNotFoundException.class, - () -> trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean())) + trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean())) ); assertEquals("no such index [[]]", e.getMessage()); } @@ -131,19 +126,19 @@ public void testEmptyAuthorizedIndicesSearchForWildcardDisallowNoIndices() { createIndicesWithRandomAliases("index1", "index2"); IndexNotFoundException e = expectThrows( IndexNotFoundException.class, - () -> trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean()), "*") + trySearch(IndicesOptions.fromOptions(randomBoolean(), false, true, randomBoolean()), "*") ); assertEquals("no such index [*]", e.getMessage()); } public void testExplicitNonAuthorizedIndex() { createIndicesWithRandomAliases("test1", "test2", "index1"); - assertThrowsAuthorizationExceptionDefaultUsers(() -> trySearch("test*", "index1"), SearchAction.NAME); + assertThrowsAuthorizationExceptionDefaultUsers(() -> trySearch("test*", "index1").get(), SearchAction.NAME); } public void testIndexNotFound() { createIndicesWithRandomAliases("test1", "test2", "index1"); - assertThrowsAuthorizationExceptionDefaultUsers(() -> trySearch("missing"), SearchAction.NAME); + assertThrowsAuthorizationExceptionDefaultUsers(() -> trySearch("missing").get(), SearchAction.NAME); } public void testIndexNotFoundIgnoreUnavailable() { @@ -176,46 +171,36 @@ public void testIndexNotFoundIgnoreUnavailable() { public void testExplicitExclusion() { // index1 is not authorized and referred to through wildcard, test2 is excluded createIndicesWithRandomAliases("test1", "test2", "test3", "index1"); - - SearchResponse searchResponse = trySearch("*", "-test2"); - assertReturnedIndices(searchResponse, "test1", "test3"); + assertReturnedIndices(trySearch("*", "-test2"), "test1", "test3"); } public void testWildcardExclusion() { // index1 is not authorized and referred to through wildcard, test2 is excluded createIndicesWithRandomAliases("test1", "test2", "test21", "test3", "index1"); - - SearchResponse searchResponse = trySearch("*", "-test2*"); - assertReturnedIndices(searchResponse, "test1", "test3"); + assertReturnedIndices(trySearch("*", "-test2*"), "test1", "test3"); } public void testInclusionAndWildcardsExclusion() { // index1 is not authorized and referred to through wildcard, test111 and test112 are excluded createIndicesWithRandomAliases("test1", "test10", "test111", "test112", "test2", "index1"); - - SearchResponse searchResponse = trySearch("test1*", "index*", "-test11*"); - assertReturnedIndices(searchResponse, "test1", "test10"); + assertReturnedIndices(trySearch("test1*", "index*", "-test11*"), "test1", "test10"); } public void testExplicitAndWildcardsInclusionAndWildcardExclusion() { // index1 is not authorized and referred to through wildcard, test111 and test112 are excluded createIndicesWithRandomAliases("test1", "test10", "test111", "test112", "test2", "index1"); - - SearchResponse searchResponse = trySearch("test2", "test11*", "index*", "-test2*"); - assertReturnedIndices(searchResponse, "test111", "test112"); + assertReturnedIndices(trySearch("test2", "test11*", "index*", "-test2*"), "test111", "test112"); } public void testExplicitAndWildcardInclusionAndExplicitExclusions() { // index1 is not authorized and referred to through wildcard, test111 and test112 are excluded createIndicesWithRandomAliases("test1", "test10", "test111", "test112", "test2", "index1"); - - SearchResponse searchResponse = trySearch("test10", "test11*", "index*", "-test111", "-test112"); - assertReturnedIndices(searchResponse, "test10"); + assertReturnedIndices(trySearch("test10", "test11*", "index*", "-test111", "-test112"), "test10"); } public void testMissingDateMath() { - expectThrows(ElasticsearchSecurityException.class, () -> trySearch("")); - expectThrows(IndexNotFoundException.class, () -> trySearch("")); + expectThrows(ElasticsearchSecurityException.class, trySearch("")); + expectThrows(IndexNotFoundException.class, trySearch("")); } public void testMultiSearchUnauthorizedIndex() { @@ -424,12 +409,25 @@ public void testMultiTermVectors() { assertThat(response.getResponses()[4].getFailure().getCause(), instanceOf(IndexNotFoundException.class)); } - private SearchResponse trySearch(String... indices) { - return client().prepareSearch(indices).get(TimeValue.timeValueSeconds(20)); + private SearchRequestBuilder trySearch(String... indices) { + return client().prepareSearch(indices); + } + + private SearchRequestBuilder trySearch(IndicesOptions options, String... indices) { + return client().prepareSearch(indices).setIndicesOptions(options); } - private SearchResponse trySearch(IndicesOptions options, String... indices) { - return client().prepareSearch(indices).setIndicesOptions(options).get(TimeValue.timeValueSeconds(20)); + private static T expectThrows(Class expectedType, SearchRequestBuilder searchRequestBuilder) { + return expectThrows(expectedType, searchRequestBuilder::get); + } + + private static void assertReturnedIndices(SearchRequestBuilder searchRequestBuilder, String... indices) { + var searchResponse = searchRequestBuilder.get(); + try { + assertReturnedIndices(searchResponse, indices); + } finally { + searchResponse.decRef(); + } } private static void assertReturnedIndices(SearchResponse searchResponse, String... indices) { From baf251eb63b6c936643701a8fd95404cd9499eb4 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Wed, 18 Oct 2023 21:44:51 +0300 Subject: [PATCH 2/2] ESQL: Alias duplicated aggregations in a stats (#100642) Replace duplicated aggregations in a stat with an alias (through a synthetic eval). Additionally introduce agg normalization which replaces field aliases inside agg and the literals in Count with "*". This improves the query: eval x = salary stats c = count(), m = min(x), m1 = min(salary), c1 = count(1) to stats c = count(*), m = min(x) eval m1 = m, c1 = c keep c, m, m1, c1 Fix #100544 --- docs/changelog/100642.yaml | 6 + .../src/main/resources/stats.csv-spec | 16 ++ .../esql/optimizer/LogicalPlanOptimizer.java | 129 +++++++++++++++- .../LocalPhysicalPlanOptimizerTests.java | 23 ++- .../optimizer/LogicalPlanOptimizerTests.java | 145 +++++++++++++++++- 5 files changed, 314 insertions(+), 5 deletions(-) create mode 100644 docs/changelog/100642.yaml diff --git a/docs/changelog/100642.yaml b/docs/changelog/100642.yaml new file mode 100644 index 0000000000000..805a20174e11d --- /dev/null +++ b/docs/changelog/100642.yaml @@ -0,0 +1,6 @@ +pr: 100642 +summary: "ESQL: Alias duplicated aggregations in a stats" +area: ES|QL +type: enhancement +issues: + - 100544 diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec index d671ba6ec13b1..acf42d908ed66 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec @@ -681,3 +681,19 @@ c:l | job_positions:s 4 |Reporting Analyst 4 |Tech Lead ; + +duplicateAggregationsWithoutGrouping +from employees | eval x = salary | stats c = count(), m = min(x), m1 = min(salary), c1 = count(1); + +c:l | m:i | m1:i | c1:l +100 | 25324 | 25324 | 100 +; + +duplicateAggregationsWithGrouping +from employees | eval x = salary | stats c = count(), m = min(x), m1 = min(salary), c1 = count(1) by gender | sort gender; + +c:l| m:i | m1:i | c1:l| gender:s +33 | 25976 | 25976 | 33 | F +57 | 25945 | 25945 | 57 | M +10 | 25324 | 25324 | 10 | null +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java index 23fa051c1d7a2..ed215efc4c066 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizer.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.esql.optimizer; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.util.Maps; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BlockFactory; import org.elasticsearch.compute.data.BlockUtils; @@ -57,8 +58,10 @@ import org.elasticsearch.xpack.ql.plan.logical.UnaryPlan; import org.elasticsearch.xpack.ql.rule.Rule; import org.elasticsearch.xpack.ql.rule.RuleExecutor; +import org.elasticsearch.xpack.ql.type.DataTypes; import org.elasticsearch.xpack.ql.util.CollectionUtils; import org.elasticsearch.xpack.ql.util.Holder; +import org.elasticsearch.xpack.ql.util.StringUtils; import java.time.ZoneId; import java.util.ArrayList; @@ -95,13 +98,14 @@ protected static List> rules() { new SubstituteSurrogates(), new ReplaceRegexMatch(), new ReplaceAliasingEvalWithProject() - // new ReplaceTextFieldAttributesWithTheKeywordSubfield() + // new NormalizeAggregate(), - waits on https://github.com/elastic/elasticsearch/issues/100634 ); var operators = new Batch<>( "Operator Optimization", new CombineProjections(), new CombineEvals(), + new ReplaceDuplicateAggWithEval(), new PruneEmptyPlans(), new PropagateEmptyRelation(), new ConvertStringToByteRef(), @@ -947,4 +951,127 @@ private LogicalPlan rule(Eval eval) { } } + + /** + * Normalize aggregation functions by: + * 1. replaces reference to field attributes with their source + * 2. in case of Count, aligns the various forms (Count(1), Count(0), Count(), Count(*)) to Count(*) + */ + // TODO waiting on https://github.com/elastic/elasticsearch/issues/100634 + static class NormalizeAggregate extends Rule { + + @Override + public LogicalPlan apply(LogicalPlan plan) { + AttributeMap aliases = new AttributeMap<>(); + + // traverse the tree bottom-up + // 1. if it's Aggregate, normalize the aggregates + // regardless, collect the attributes but only if they refer to an attribute or literal + plan = plan.transformUp(p -> { + if (p instanceof Aggregate agg) { + p = normalize(agg, aliases); + } + p.forEachExpression(Alias.class, a -> { + var child = a.child(); + if (child.foldable() || child instanceof NamedExpression) { + aliases.putIfAbsent(a.toAttribute(), child); + } + }); + + return p; + }); + return plan; + } + + private static LogicalPlan normalize(Aggregate aggregate, AttributeMap aliases) { + var aggs = aggregate.aggregates(); + List newAggs = new ArrayList<>(aggs.size()); + boolean changed = false; + + for (NamedExpression agg : aggs) { + if (agg instanceof Alias as && as.child() instanceof AggregateFunction af) { + // replace field reference + if (af.field() instanceof NamedExpression ne) { + Attribute attr = ne.toAttribute(); + var resolved = aliases.resolve(attr, attr); + if (resolved != attr) { + changed = true; + var newChildren = CollectionUtils.combine(Collections.singletonList(resolved), af.parameters()); + // update the reference so Count can pick it up + af = (AggregateFunction) af.replaceChildren(newChildren); + agg = as.replaceChild(af); + } + } + // handle Count(*) + if (af instanceof Count count) { + var field = af.field(); + if (field.foldable()) { + var fold = field.fold(); + if (fold != null && StringUtils.WILDCARD.equals(fold) == false) { + changed = true; + var source = count.source(); + agg = as.replaceChild(new Count(source, new Literal(source, StringUtils.WILDCARD, DataTypes.KEYWORD))); + } + } + } + } + newAggs.add(agg); + } + return changed ? new Aggregate(aggregate.source(), aggregate.child(), aggregate.groupings(), newAggs) : aggregate; + } + } + + /** + * Replace aggregations that are duplicated inside an Aggregate with an Eval to avoid duplicated compute. + * stats a = min(x), b = min(x), c = count(*), d = count() by g + * becomes + * stats a = min(x), c = count(*) by g + * eval b = a, d = c + * keep a, b, c, d, g + */ + static class ReplaceDuplicateAggWithEval extends OptimizerRules.OptimizerRule { + + ReplaceDuplicateAggWithEval() { + super(TransformDirection.UP); + } + + @Override + protected LogicalPlan rule(Aggregate aggregate) { + LogicalPlan plan = aggregate; + + boolean foundDuplicate = false; + var aggs = aggregate.aggregates(); + Map seenAggs = Maps.newMapWithExpectedSize(aggs.size()); + List projections = new ArrayList<>(); + List keptAggs = new ArrayList<>(aggs.size()); + + for (NamedExpression agg : aggs) { + var attr = agg.toAttribute(); + if (agg instanceof Alias as && as.child() instanceof AggregateFunction af) { + var seen = seenAggs.putIfAbsent(af, attr); + if (seen != null) { + foundDuplicate = true; + projections.add(as.replaceChild(seen)); + } + // otherwise keep the agg in place + else { + keptAggs.add(agg); + projections.add(attr); + } + } else { + keptAggs.add(agg); + projections.add(attr); + } + } + + // at least one duplicate found - add the projection (to keep the output in place) + if (foundDuplicate) { + var source = aggregate.source(); + var newAggregate = new Aggregate(source, aggregate.child(), aggregate.groupings(), keptAggs); + plan = new Project(source, newAggregate, projections); + } + + return plan; + } + } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java index 511a7ee08b5e1..72e12697488df 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java @@ -33,6 +33,7 @@ import org.elasticsearch.xpack.esql.plan.physical.LimitExec; import org.elasticsearch.xpack.esql.plan.physical.LocalSourceExec; import org.elasticsearch.xpack.esql.plan.physical.PhysicalPlan; +import org.elasticsearch.xpack.esql.plan.physical.ProjectExec; import org.elasticsearch.xpack.esql.planner.FilterTests; import org.elasticsearch.xpack.esql.planner.Mapper; import org.elasticsearch.xpack.esql.planner.PlannerUtils; @@ -41,6 +42,7 @@ import org.elasticsearch.xpack.esql.stats.Metrics; import org.elasticsearch.xpack.esql.stats.SearchStats; import org.elasticsearch.xpack.esql.type.EsqlDataTypes; +import org.elasticsearch.xpack.ql.expression.Alias; import org.elasticsearch.xpack.ql.expression.Expressions; import org.elasticsearch.xpack.ql.expression.function.FunctionRegistry; import org.elasticsearch.xpack.ql.index.EsIndex; @@ -299,6 +301,16 @@ public void testAnotherCountAllWithFilter() { assertThat(expected.toString(), is(esStatsQuery.query().toString())); } + /** + * Expected + * ProjectExec[[c{r}#3, c{r}#3 AS call, c_literal{r}#7]] + * \_LimitExec[500[INTEGER]] + * \_AggregateExec[[],[COUNT([2a][KEYWORD]) AS c, COUNT(1[INTEGER]) AS c_literal],FINAL,null] + * \_ExchangeExec[[count{r}#18, seen{r}#19, count{r}#20, seen{r}#21],true] + * \_EsStatsQueryExec[test], stats[Stat[name=*, type=COUNT, query=null], Stat[name=*, type=COUNT, query=null]]], + * query[{"esql_single_value":{"field":"emp_no","next":{"range":{"emp_no":{"gt":10010,"boost":1.0}}}}}] + * [count{r}#23, seen{r}#24, count{r}#25, seen{r}#26], limit[], + */ public void testMultiCountAllWithFilter() { var plan = plan(""" from test @@ -306,14 +318,19 @@ public void testMultiCountAllWithFilter() { | stats c = count(), call = count(*), c_literal = count(1) """, IS_SV_STATS); - var limit = as(plan, LimitExec.class); + var project = as(plan, ProjectExec.class); + var projections = project.projections(); + assertThat(Expressions.names(projections), contains("c", "call", "c_literal")); + var alias = as(projections.get(1), Alias.class); + assertThat(Expressions.name(alias.child()), is("c")); + var limit = as(project.child(), LimitExec.class); var agg = as(limit.child(), AggregateExec.class); assertThat(agg.getMode(), is(FINAL)); - assertThat(Expressions.names(agg.aggregates()), contains("c", "call", "c_literal")); + assertThat(Expressions.names(agg.aggregates()), contains("c", "c_literal")); var exchange = as(agg.child(), ExchangeExec.class); var esStatsQuery = as(exchange.child(), EsStatsQueryExec.class); assertThat(esStatsQuery.limit(), is(nullValue())); - assertThat(Expressions.names(esStatsQuery.output()), contains("count", "seen", "count", "seen", "count", "seen")); + assertThat(Expressions.names(esStatsQuery.output()), contains("count", "seen", "count", "seen")); var expected = wrapWithSingleQuery(QueryBuilders.rangeQuery("emp_no").gt(10010), "emp_no"); assertThat(expected.toString(), is(esStatsQuery.query().toString())); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index a22bb3b91ff0b..285ad7021e83f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.xpack.esql.expression.Order; import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry; import org.elasticsearch.xpack.esql.expression.function.aggregate.Count; +import org.elasticsearch.xpack.esql.expression.function.aggregate.Max; import org.elasticsearch.xpack.esql.expression.function.aggregate.Min; import org.elasticsearch.xpack.esql.expression.function.aggregate.Percentile; import org.elasticsearch.xpack.esql.expression.function.aggregate.Sum; @@ -1923,6 +1924,147 @@ public void testPruneRenameOnAggBy() { var source = as(agg.child(), EsRelation.class); } + /** + * Expects + * Project[[c1{r}#2, c2{r}#4, cs{r}#6, cm{r}#8, cexp{r}#10]] + * \_Eval[[c1{r}#2 AS c2, c1{r}#2 AS cs, c1{r}#2 AS cm, c1{r}#2 AS cexp]] + * \_Limit[500[INTEGER]] + * \_Aggregate[[],[COUNT([2a][KEYWORD]) AS c1]] + * \_EsRelation[test][_meta_field{f}#17, emp_no{f}#11, first_name{f}#12, ..] + */ + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/100634") + public void testEliminateDuplicateAggsCountAll() { + var plan = plan(""" + from test + | stats c1 = count(1), c2 = count(2), cs = count(*), cm = count(), cexp = count("123") + """); + + var project = as(plan, Project.class); + assertThat(Expressions.names(project.projections()), contains("c1", "c2", "cs", "cm", "cexp")); + var eval = as(project.child(), Eval.class); + var fields = eval.fields(); + assertThat(Expressions.names(fields), contains("c2", "cs", "cm", "cexp")); + for (Alias field : fields) { + assertThat(Expressions.name(field.child()), is("c1")); + } + var limit = as(eval.child(), Limit.class); + var agg = as(limit.child(), Aggregate.class); + var aggs = agg.aggregates(); + assertThat(Expressions.names(aggs), contains("c1")); + aggFieldName(aggs.get(0), Count.class, "*"); + var source = as(agg.child(), EsRelation.class); + } + + /** + * Expects + * Project[[c1{r}#7, cx{r}#10, cs{r}#12, cy{r}#15]] + * \_Eval[[c1{r}#7 AS cx, c1{r}#7 AS cs, c1{r}#7 AS cy]] + * \_Limit[500[INTEGER]] + * \_Aggregate[[],[COUNT([2a][KEYWORD]) AS c1]] + * \_EsRelation[test][_meta_field{f}#22, emp_no{f}#16, first_name{f}#17, ..] + */ + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/100634") + public void testEliminateDuplicateAggsWithAliasedFields() { + var plan = plan(""" + from test + | eval x = 1 + | eval y = x + | stats c1 = count(1), cx = count(x), cs = count(*), cy = count(y) + """); + + var project = as(plan, Project.class); + assertThat(Expressions.names(project.projections()), contains("c1", "cx", "cs", "cy")); + var eval = as(project.child(), Eval.class); + var fields = eval.fields(); + assertThat(Expressions.names(fields), contains("cx", "cs", "cy")); + for (Alias field : fields) { + assertThat(Expressions.name(field.child()), is("c1")); + } + var limit = as(eval.child(), Limit.class); + var agg = as(limit.child(), Aggregate.class); + var aggs = agg.aggregates(); + assertThat(Expressions.names(aggs), contains("c1")); + aggFieldName(aggs.get(0), Count.class, "*"); + var source = as(agg.child(), EsRelation.class); + } + + /** + * Expects + * Project[[min{r}#1385, max{r}#1388, min{r}#1385 AS min2, max{r}#1388 AS max2, gender{f}#1398]] + * \_Limit[500[INTEGER]] + * \_Aggregate[[gender{f}#1398],[MIN(salary{f}#1401) AS min, MAX(salary{f}#1401) AS max, gender{f}#1398]] + * \_EsRelation[test][_meta_field{f}#1402, emp_no{f}#1396, first_name{f}#..] + */ + public void testEliminateDuplicateAggsMixed() { + var plan = plan(""" + from test + | stats min = min(salary), max = max(salary), min2 = min(salary), max2 = max(salary) by gender + """); + + var project = as(plan, Project.class); + var projections = project.projections(); + assertThat(Expressions.names(projections), contains("min", "max", "min2", "max2", "gender")); + as(projections.get(0), ReferenceAttribute.class); + as(projections.get(1), ReferenceAttribute.class); + assertThat(Expressions.name(aliased(projections.get(2), ReferenceAttribute.class)), is("min")); + assertThat(Expressions.name(aliased(projections.get(3), ReferenceAttribute.class)), is("max")); + + var limit = as(project.child(), Limit.class); + var agg = as(limit.child(), Aggregate.class); + var aggs = agg.aggregates(); + assertThat(Expressions.names(aggs), contains("min", "max", "gender")); + aggFieldName(aggs.get(0), Min.class, "salary"); + aggFieldName(aggs.get(1), Max.class, "salary"); + var source = as(agg.child(), EsRelation.class); + } + + /** + * Expects + * EsqlProject[[a{r}#5, c{r}#8]] + * \_Eval[[null[INTEGER] AS x]] + * \_EsRelation[test][_meta_field{f}#15, emp_no{f}#9, first_name{f}#10, g..] + */ + @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/100634") + public void testEliminateDuplicateAggWithNull() { + var plan = plan(""" + from test + | eval x = null + 1 + | stats a = avg(x), c = count(x) + """); + fail("Awaits fix"); + } + + /** + * Expects + * Project[[max(x){r}#11, max(x){r}#11 AS max(y), max(x){r}#11 AS max(z)]] + * \_Limit[500[INTEGER]] + * \_Aggregate[[],[MAX(salary{f}#21) AS max(x)]] + * \_EsRelation[test][_meta_field{f}#22, emp_no{f}#16, first_name{f}#17, ..] + */ + public void testEliminateDuplicateAggsNonCount() { + var plan = plan(""" + from test + | eval x = salary + | eval y = x + | eval z = y + | stats max(x), max(y), max(z) + """); + + var project = as(plan, Project.class); + var projections = project.projections(); + assertThat(Expressions.names(projections), contains("max(x)", "max(y)", "max(z)")); + as(projections.get(0), ReferenceAttribute.class); + assertThat(Expressions.name(aliased(projections.get(1), ReferenceAttribute.class)), is("max(x)")); + assertThat(Expressions.name(aliased(projections.get(2), ReferenceAttribute.class)), is("max(x)")); + + var limit = as(project.child(), Limit.class); + var agg = as(limit.child(), Aggregate.class); + var aggs = agg.aggregates(); + assertThat(Expressions.names(aggs), contains("max(x)")); + aggFieldName(aggs.get(0), Max.class, "salary"); + var source = as(agg.child(), EsRelation.class); + } + private T aliased(Expression exp, Class clazz) { var alias = as(exp, Alias.class); return as(alias.child(), clazz); @@ -1932,7 +2074,8 @@ private void aggFieldName(Expression exp, Class var alias = as(exp, Alias.class); var af = as(alias.child(), aggType); var field = af.field(); - assertThat(Expressions.name(field), is(fieldName)); + var name = field.foldable() ? BytesRefs.toString(field.fold()) : Expressions.name(field); + assertThat(name, is(fieldName)); } private LogicalPlan optimizedPlan(String query) {