From 81b1256a0ceb68e9cd5a9ac4bfb1e635da653cf8 Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Wed, 20 Nov 2019 23:23:04 -0800 Subject: [PATCH 1/7] Exclude unmapped fileds during max clause limit checking for querying --- .../index/search/QueryParserHelper.java | 22 +++++-- .../query/QueryStringQueryBuilderTests.java | 64 +++++++++++++++++++ .../test/resources/config/elasticsearch.yml | 1 - .../test/AbstractBuilderTestCase.java | 27 +++----- 4 files changed, 90 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java index e4b397c9a538b..2a6dcca401bb1 100644 --- a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java +++ b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java @@ -28,6 +28,7 @@ import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -87,8 +88,8 @@ public static Map resolveMappingFields(QueryShardContext context, boolean allField = Regex.isMatchAllPattern(fieldEntry.getKey()); boolean multiField = Regex.isSimpleMatchPattern(fieldEntry.getKey()); float weight = fieldEntry.getValue() == null ? 1.0f : fieldEntry.getValue(); - Map fieldMap = resolveMappingField(context, fieldEntry.getKey(), weight, - !multiField, !allField, fieldSuffix); + Map fieldMap = resolveMappingField(context, fieldEntry.getKey(), weight, !multiField, !allField, fieldSuffix); + for (Map.Entry field : fieldMap.entrySet()) { float boost = field.getValue(); if (resolvedFields.containsKey(field.getKey())) { @@ -97,7 +98,14 @@ public static Map resolveMappingFields(QueryShardContext context, resolvedFields.put(field.getKey(), boost); } } - checkForTooManyFields(resolvedFields, context); + + Set unmappedFields = new HashSet<>(); + resolvedFields.keySet().stream().forEach(k -> { + if (context.getMapperService().fullName(k) == null) { + unmappedFields.add(k); + } + }); + checkForTooManyFields(resolvedFields, unmappedFields, context); return resolvedFields; } @@ -133,6 +141,7 @@ public static Map resolveMappingField(QueryShardContext context, boolean acceptAllTypes, boolean acceptMetadataField, String fieldSuffix) { Set allFields = context.simpleMatchToIndexNames(fieldOrPattern); Map fields = new HashMap<>(); + Set unmappedFields = new HashSet<>(); for (String fieldName : allFields) { if (fieldSuffix != null && context.fieldMapper(fieldName + fieldSuffix) != null) { @@ -143,6 +152,7 @@ public static Map resolveMappingField(QueryShardContext context, if (fieldType == null) { // Note that we don't ignore unmapped fields. fields.put(fieldName, weight); + unmappedFields.add(fieldName); continue; } @@ -172,13 +182,13 @@ public static Map resolveMappingField(QueryShardContext context, fields.put(fieldName, w * weight); } - checkForTooManyFields(fields, context); + checkForTooManyFields(fields, unmappedFields, context); return fields; } - private static void checkForTooManyFields(Map fields, QueryShardContext context) { + private static void checkForTooManyFields(Map fields, Set unmappedFields, QueryShardContext context) { Integer limit = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings()); - if (fields.size() > limit) { + if (fields.size() - unmappedFields.size() > limit) { throw new IllegalArgumentException("field expansion matches too many fields, limit: " + limit + ", got: " + fields.size()); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index 85453e774fe6e..3bbea6828f1de 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -50,6 +50,7 @@ import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.Operations; import org.apache.lucene.util.automaton.TooComplexToDeterminizeException; +import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; @@ -399,6 +400,14 @@ protected void doAssertLuceneQuery(QueryStringQueryBuilder queryBuilder, // nothing yet, put additional assertions here. } + @Override + protected Settings createTestIndexSettings() { + return Settings.builder() + .put(super.createTestIndexSettings()) + .put("indices.query.bool.max_clause_count", "13") + .build(); + } + // Tests fix for https://github.com/elastic/elasticsearch/issues/29403 public void testTimezoneEquals() { QueryStringQueryBuilder builder1 = new QueryStringQueryBuilder("bar"); @@ -1290,6 +1299,61 @@ public void testQuoteAnalyzer() throws Exception { assertEquals(expectedQuery, query); } + // The only expectation for this test is to not throw exception + public void testToQueryExceedingMaxClauseCountWithUnmappedFields() throws Exception { + QueryShardContext shardContext = createShardContext(); + + new QueryStringQueryBuilder("\"bar\"") + .field("*") + .field("unmappedField1") + .field("unmappedField2") + .field("unmappedField3") + .field("unmappedField4") + .toQuery(shardContext); + } + + public void testToQueryExceedingMaxCaluseCountWithMappedFeilds() throws IOException { + QueryShardContext shardContext = createShardContext(); + shardContext.getMapperService() + .merge("_doc", new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", + "additionalMappedField1", "type=text", + "additionalMappedField2", "type=text", + "additionalMappedField3", "type=text", + "additionalMappedField4", "type=text" + ))), MapperService.MergeReason.MAPPING_UPDATE); + + try { + new QueryStringQueryBuilder("\"bar\"") + .field("*") + .toQuery(shardContext); + fail(); + } + catch (IllegalArgumentException e) { + assertEquals("field expansion matches too many fields, limit: 13, got: 17", e.getMessage()); + } + + } + + public void testToQueryExceedingMaxCaluseCountWithMappedFeildsWithoutWeightAndBoost() throws IOException { + QueryShardContext shardContext = createShardContext(); + shardContext.getMapperService() + .merge("_doc", new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", + "additionalMappedField1", "type=text", + "additionalMappedField2", "type=text", + "additionalMappedField3", "type=text", + "additionalMappedField4", "type=text" + ))), MapperService.MergeReason.MAPPING_UPDATE); + + try { + new QueryStringQueryBuilder("\"bar\"") + .toQuery(shardContext); + fail(); + } + catch (IllegalArgumentException e) { + assertEquals("field expansion matches too many fields, limit: 13, got: 17", e.getMessage()); + } + } + public void testQuoteFieldSuffix() throws IOException { QueryShardContext context = createShardContext(); assertEquals(new TermQuery(new Term(STRING_FIELD_NAME, "bar")), diff --git a/server/src/test/resources/config/elasticsearch.yml b/server/src/test/resources/config/elasticsearch.yml index b6ebc6bd10576..21f4f7b1b933a 100644 --- a/server/src/test/resources/config/elasticsearch.yml +++ b/server/src/test/resources/config/elasticsearch.yml @@ -1,3 +1,2 @@ yaml.config.exists: true - diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index e726b4dc15094..44ed1963f30f2 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -212,21 +212,14 @@ public static void afterClass() throws Exception { @Before public void beforeTest() throws Exception { - if (serviceHolder == null) { - assert serviceHolderWithNoType == null; - // we initialize the serviceHolder and serviceHolderWithNoType just once, but need some - // calls to the randomness source during its setup. In order to not mix these calls with - // the randomness source that is later used in the test method, we use the master seed during - // this setup - long masterSeed = SeedUtils.parseSeed(RandomizedTest.getContext().getRunnerSeedAsString()); - RandomizedTest.getContext().runWithPrivateRandomness(masterSeed, (Callable) () -> { - serviceHolder = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, - AbstractBuilderTestCase.this, true); - serviceHolderWithNoType = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, - AbstractBuilderTestCase.this, false); - return null; - }); - } + long masterSeed = SeedUtils.parseSeed(RandomizedTest.getContext().getRunnerSeedAsString()); + RandomizedTest.getContext().runWithPrivateRandomness(masterSeed, (Callable) () -> { + serviceHolder = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, + AbstractBuilderTestCase.this, true); + serviceHolderWithNoType = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, + AbstractBuilderTestCase.this, false); + return null; + }); serviceHolder.clientInvocationHandler.delegate = this; serviceHolderWithNoType.clientInvocationHandler.delegate = this; @@ -400,11 +393,11 @@ public void onRemoval(ShardId shardId, Accountable accountable) { testCase.initializeAdditionalMappings(mapperService); } } - + public static Predicate indexNameMatcher() { // Simplistic index name matcher used for testing return pattern -> Regex.simpleMatch(pattern, index.getName()); - } + } @Override public void close() throws IOException { From 79df51c6f9c9f914a4e6db60291a44b730abcdff Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Mon, 25 Nov 2019 20:12:44 -0800 Subject: [PATCH 2/7] Address comments --- .../index/search/QueryParserHelper.java | 19 ++++--------------- .../query/QueryStringQueryBuilderTests.java | 4 ++-- .../test/AbstractBuilderTestCase.java | 3 +++ 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java index 2a6dcca401bb1..768d017974e2f 100644 --- a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java +++ b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java @@ -28,7 +28,6 @@ import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -99,13 +98,7 @@ public static Map resolveMappingFields(QueryShardContext context, } } - Set unmappedFields = new HashSet<>(); - resolvedFields.keySet().stream().forEach(k -> { - if (context.getMapperService().fullName(k) == null) { - unmappedFields.add(k); - } - }); - checkForTooManyFields(resolvedFields, unmappedFields, context); + checkForTooManyFields(resolvedFields, context); return resolvedFields; } @@ -141,7 +134,6 @@ public static Map resolveMappingField(QueryShardContext context, boolean acceptAllTypes, boolean acceptMetadataField, String fieldSuffix) { Set allFields = context.simpleMatchToIndexNames(fieldOrPattern); Map fields = new HashMap<>(); - Set unmappedFields = new HashSet<>(); for (String fieldName : allFields) { if (fieldSuffix != null && context.fieldMapper(fieldName + fieldSuffix) != null) { @@ -150,9 +142,6 @@ public static Map resolveMappingField(QueryShardContext context, MappedFieldType fieldType = context.getMapperService().fullName(fieldName); if (fieldType == null) { - // Note that we don't ignore unmapped fields. - fields.put(fieldName, weight); - unmappedFields.add(fieldName); continue; } @@ -182,13 +171,13 @@ public static Map resolveMappingField(QueryShardContext context, fields.put(fieldName, w * weight); } - checkForTooManyFields(fields, unmappedFields, context); + checkForTooManyFields(fields, context); return fields; } - private static void checkForTooManyFields(Map fields, Set unmappedFields, QueryShardContext context) { + private static void checkForTooManyFields(Map fields, QueryShardContext context) { Integer limit = SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING.get(context.getIndexSettings().getSettings()); - if (fields.size() - unmappedFields.size() > limit) { + if (fields.size() > limit) { throw new IllegalArgumentException("field expansion matches too many fields, limit: " + limit + ", got: " + fields.size()); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index 3bbea6828f1de..d39d9b8299a1e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -1312,7 +1312,7 @@ public void testToQueryExceedingMaxClauseCountWithUnmappedFields() throws Except .toQuery(shardContext); } - public void testToQueryExceedingMaxCaluseCountWithMappedFeilds() throws IOException { + public void testToQueryMaxClauseWithWeight() throws IOException { QueryShardContext shardContext = createShardContext(); shardContext.getMapperService() .merge("_doc", new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", @@ -1334,7 +1334,7 @@ public void testToQueryExceedingMaxCaluseCountWithMappedFeilds() throws IOExcept } - public void testToQueryExceedingMaxCaluseCountWithMappedFeildsWithoutWeightAndBoost() throws IOException { + public void testToQueryMaxClauseNoWeight() throws IOException { QueryShardContext shardContext = createShardContext(); shardContext.getMapperService() .merge("_doc", new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index 44ed1963f30f2..7da48b2d6458a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -212,6 +212,9 @@ public static void afterClass() throws Exception { @Before public void beforeTest() throws Exception { + // we re-initiate serviceHolder for every test as some test cases may alter the state of serviceHolder + // and its inner objects, and reusing the same one may cause failure of other test cases. + // E.g. some test cases may alter field mapping information in serviceHolder.mapperService long masterSeed = SeedUtils.parseSeed(RandomizedTest.getContext().getRunnerSeedAsString()); RandomizedTest.getContext().runWithPrivateRandomness(masterSeed, (Callable) () -> { serviceHolder = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, From 42687458e35c8b09bc1c656810350ac5fcbe78a8 Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Thu, 28 Nov 2019 10:52:45 -0800 Subject: [PATCH 3/7] Remove unneeded changes in elasticsearch.yml --- server/src/test/resources/config/elasticsearch.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/resources/config/elasticsearch.yml b/server/src/test/resources/config/elasticsearch.yml index 21f4f7b1b933a..b6ebc6bd10576 100644 --- a/server/src/test/resources/config/elasticsearch.yml +++ b/server/src/test/resources/config/elasticsearch.yml @@ -1,2 +1,3 @@ yaml.config.exists: true + From b4279d1ed77f06a60d8108a418bdc9045c590585 Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Thu, 9 Jan 2020 22:10:26 -0800 Subject: [PATCH 4/7] Add integration test for igoring unmapping fields --- .../search/query/QueryStringIT.java | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java index 764b434e774e5..9d88c59fb0256 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java @@ -25,7 +25,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryStringQueryBuilder; import org.elasticsearch.search.SearchHit; @@ -275,10 +274,7 @@ public void testLimitOnExpandedFields() throws Exception { builder.endObject(); // type1 builder.endObject(); - assertAcked(prepareCreate("toomanyfields") - .setSettings(Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), - CLUSTER_MAX_CLAUSE_COUNT + 100)) - .addMapping("type1", builder)); + assertAcked(prepareCreate("toomanyfields").addMapping("type1", builder)); client().prepareIndex("toomanyfields").setId("1").setSource("field1", "foo bar baz").get(); refresh(); @@ -295,6 +291,35 @@ public void testLimitOnExpandedFields() throws Exception { + (CLUSTER_MAX_CLAUSE_COUNT + 1))); } + // The only expectation for this test is to not throw exception + public void testLimitOnExpandedFieldsButIgnoreUnmappedFields() throws Exception { + XContentBuilder builder = jsonBuilder(); + builder.startObject(); + builder.startObject("type1"); + builder.startObject("properties"); + for (int i = 0; i < CLUSTER_MAX_CLAUSE_COUNT; i++) { + builder.startObject("field" + i).field("type", "text").endObject(); + } + builder.endObject(); // properties + builder.endObject(); // type1 + builder.endObject(); + + assertAcked(prepareCreate("ignoreunmappedfields").addMapping("type1", builder)); + + client().prepareIndex("ignoreunmappedfields").setId("1").setSource("field1", "foo bar baz").get(); + refresh(); + + QueryStringQueryBuilder qb = queryStringQuery("bar"); + if (randomBoolean()) { + qb.field("*") + .field("unmappedField1") + .field("unmappedField2") + .field("unmappedField3") + .field("unmappedField4"); + } + client().prepareSearch("ignoreunmappedfields").setQuery(qb).get(); + } + public void testFieldAlias() throws Exception { List indexRequests = new ArrayList<>(); indexRequests.add(client().prepareIndex("test").setId("1").setSource("f3", "text", "f2", "one")); From 60efd2cfcbf6b049cd8437437f8a5dfd5d296525 Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Mon, 13 Jan 2020 20:11:36 -0800 Subject: [PATCH 5/7] Remove unit tests and unneeded changes to integration test --- .../query/QueryStringQueryBuilderTests.java | 64 ------------------- .../search/query/QueryStringIT.java | 6 +- .../test/AbstractBuilderTestCase.java | 30 +++++---- 3 files changed, 22 insertions(+), 78 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index d39d9b8299a1e..85453e774fe6e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -50,7 +50,6 @@ import org.apache.lucene.util.automaton.Automaton; import org.apache.lucene.util.automaton.Operations; import org.apache.lucene.util.automaton.TooComplexToDeterminizeException; -import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; @@ -400,14 +399,6 @@ protected void doAssertLuceneQuery(QueryStringQueryBuilder queryBuilder, // nothing yet, put additional assertions here. } - @Override - protected Settings createTestIndexSettings() { - return Settings.builder() - .put(super.createTestIndexSettings()) - .put("indices.query.bool.max_clause_count", "13") - .build(); - } - // Tests fix for https://github.com/elastic/elasticsearch/issues/29403 public void testTimezoneEquals() { QueryStringQueryBuilder builder1 = new QueryStringQueryBuilder("bar"); @@ -1299,61 +1290,6 @@ public void testQuoteAnalyzer() throws Exception { assertEquals(expectedQuery, query); } - // The only expectation for this test is to not throw exception - public void testToQueryExceedingMaxClauseCountWithUnmappedFields() throws Exception { - QueryShardContext shardContext = createShardContext(); - - new QueryStringQueryBuilder("\"bar\"") - .field("*") - .field("unmappedField1") - .field("unmappedField2") - .field("unmappedField3") - .field("unmappedField4") - .toQuery(shardContext); - } - - public void testToQueryMaxClauseWithWeight() throws IOException { - QueryShardContext shardContext = createShardContext(); - shardContext.getMapperService() - .merge("_doc", new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", - "additionalMappedField1", "type=text", - "additionalMappedField2", "type=text", - "additionalMappedField3", "type=text", - "additionalMappedField4", "type=text" - ))), MapperService.MergeReason.MAPPING_UPDATE); - - try { - new QueryStringQueryBuilder("\"bar\"") - .field("*") - .toQuery(shardContext); - fail(); - } - catch (IllegalArgumentException e) { - assertEquals("field expansion matches too many fields, limit: 13, got: 17", e.getMessage()); - } - - } - - public void testToQueryMaxClauseNoWeight() throws IOException { - QueryShardContext shardContext = createShardContext(); - shardContext.getMapperService() - .merge("_doc", new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", - "additionalMappedField1", "type=text", - "additionalMappedField2", "type=text", - "additionalMappedField3", "type=text", - "additionalMappedField4", "type=text" - ))), MapperService.MergeReason.MAPPING_UPDATE); - - try { - new QueryStringQueryBuilder("\"bar\"") - .toQuery(shardContext); - fail(); - } - catch (IllegalArgumentException e) { - assertEquals("field expansion matches too many fields, limit: 13, got: 17", e.getMessage()); - } - } - public void testQuoteFieldSuffix() throws IOException { QueryShardContext context = createShardContext(); assertEquals(new TermQuery(new Term(STRING_FIELD_NAME, "bar")), diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java index 9d88c59fb0256..bbda2207b5641 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryStringQueryBuilder; import org.elasticsearch.search.SearchHit; @@ -274,7 +275,10 @@ public void testLimitOnExpandedFields() throws Exception { builder.endObject(); // type1 builder.endObject(); - assertAcked(prepareCreate("toomanyfields").addMapping("type1", builder)); + assertAcked(prepareCreate("toomanyfields") + .setSettings(Settings.builder().put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), + CLUSTER_MAX_CLAUSE_COUNT + 100)) + .addMapping("type1", builder)); client().prepareIndex("toomanyfields").setId("1").setSource("field1", "foo bar baz").get(); refresh(); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index 7da48b2d6458a..e726b4dc15094 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -212,17 +212,21 @@ public static void afterClass() throws Exception { @Before public void beforeTest() throws Exception { - // we re-initiate serviceHolder for every test as some test cases may alter the state of serviceHolder - // and its inner objects, and reusing the same one may cause failure of other test cases. - // E.g. some test cases may alter field mapping information in serviceHolder.mapperService - long masterSeed = SeedUtils.parseSeed(RandomizedTest.getContext().getRunnerSeedAsString()); - RandomizedTest.getContext().runWithPrivateRandomness(masterSeed, (Callable) () -> { - serviceHolder = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, - AbstractBuilderTestCase.this, true); - serviceHolderWithNoType = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, - AbstractBuilderTestCase.this, false); - return null; - }); + if (serviceHolder == null) { + assert serviceHolderWithNoType == null; + // we initialize the serviceHolder and serviceHolderWithNoType just once, but need some + // calls to the randomness source during its setup. In order to not mix these calls with + // the randomness source that is later used in the test method, we use the master seed during + // this setup + long masterSeed = SeedUtils.parseSeed(RandomizedTest.getContext().getRunnerSeedAsString()); + RandomizedTest.getContext().runWithPrivateRandomness(masterSeed, (Callable) () -> { + serviceHolder = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, + AbstractBuilderTestCase.this, true); + serviceHolderWithNoType = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis, + AbstractBuilderTestCase.this, false); + return null; + }); + } serviceHolder.clientInvocationHandler.delegate = this; serviceHolderWithNoType.clientInvocationHandler.delegate = this; @@ -396,11 +400,11 @@ public void onRemoval(ShardId shardId, Accountable accountable) { testCase.initializeAdditionalMappings(mapperService); } } - + public static Predicate indexNameMatcher() { // Simplistic index name matcher used for testing return pattern -> Regex.simpleMatch(pattern, index.getName()); - } + } @Override public void close() throws IOException { From fe53f2dd261250f6895a3598b4b7cde4e0d3270b Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Wed, 15 Jan 2020 19:09:16 -0800 Subject: [PATCH 6/7] Update test to use CreateIndexRequestBuilder.setMapping after merge from latest --- .../test/java/org/elasticsearch/search/query/QueryStringIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java index 3f4ccfe7887c6..85ae7ad18dd60 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java @@ -308,7 +308,7 @@ public void testLimitOnExpandedFieldsButIgnoreUnmappedFields() throws Exception builder.endObject(); // type1 builder.endObject(); - assertAcked(prepareCreate("ignoreunmappedfields").addMapping("type1", builder)); + assertAcked(prepareCreate("ignoreunmappedfields").setMapping(builder)); client().prepareIndex("ignoreunmappedfields").setId("1").setSource("field1", "foo bar baz").get(); refresh(); From a6ef57de7e1d134c009db3b31442d231634675c8 Mon Sep 17 00:00:00 2001 From: Xi Chen Date: Thu, 16 Jan 2020 08:04:00 -0800 Subject: [PATCH 7/7] ix test failure as type has been removed in CreateIndexRequest --- .../test/java/org/elasticsearch/search/query/QueryStringIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java index 85ae7ad18dd60..e47c330bb9fb5 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java @@ -299,7 +299,7 @@ public void testLimitOnExpandedFields() throws Exception { public void testLimitOnExpandedFieldsButIgnoreUnmappedFields() throws Exception { XContentBuilder builder = jsonBuilder(); builder.startObject(); - builder.startObject("type1"); + builder.startObject("_doc"); builder.startObject("properties"); for (int i = 0; i < CLUSTER_MAX_CLAUSE_COUNT; i++) { builder.startObject("field" + i).field("type", "text").endObject();