From 9635484dd7d6aee872a36feece7266a12177b02d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 4 Jun 2019 15:55:56 +0200 Subject: [PATCH 1/6] Deprecate and ignore `_field_names` disabling Currently we allow `_field_names` fields to be disabled explicitely, but since the overhead is negligible now we decided to keep it turned on by default and deprecate and ignore the `enable` option on the field type. Closes #27239 --- .../PercolatorFieldMapperTests.java | 15 ++++++----- .../index/mapper/FieldNamesFieldMapper.java | 16 +++++------ .../mapper/FieldNamesFieldMapperTests.java | 25 +++-------------- .../query/QueryStringQueryBuilderTests.java | 27 ------------------- 4 files changed, 20 insertions(+), 63 deletions(-) diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index f5d30a951e68c..9957eb0934d3e 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java @@ -146,7 +146,7 @@ public void init() throws Exception { mapperService = indexService.mapperService(); String mapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("doc") - .startObject("_field_names").field("enabled", false).endObject() // makes testing easier + .startObject("_field_names").endObject() // makes testing easier .startObject("properties") .startObject("field").field("type", "text").endObject() .startObject("field1").field("type", "text").endObject() @@ -322,7 +322,7 @@ public void testExtractTermsAndRanges_partial() throws Exception { ParseContext.Document document = parseContext.doc(); PercolatorFieldMapper.FieldType fieldType = (PercolatorFieldMapper.FieldType) fieldMapper.fieldType(); - assertThat(document.getFields().size(), equalTo(3)); + assertThat(document.getFields().size(), equalTo(4)); assertThat(document.getFields().get(0).binaryValue().utf8ToString(), equalTo("field\u0000term")); assertThat(document.getField(fieldType.extractionResultField.name()).stringValue(), equalTo(EXTRACTION_PARTIAL)); } @@ -590,7 +590,7 @@ public void testAllowNoAdditionalSettings() throws Exception { public void testMultiplePercolatorFields() throws Exception { String typeName = "doc"; String percolatorMapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(typeName) - .startObject("_field_names").field("enabled", false).endObject() // makes testing easier + .startObject("_field_names").endObject() // makes testing easier .startObject("properties") .startObject("query_field1").field("type", "percolator").endObject() .startObject("query_field2").field("type", "percolator").endObject() @@ -605,7 +605,8 @@ public void testMultiplePercolatorFields() throws Exception { .field("query_field2", queryBuilder) .endObject()), XContentType.JSON)); - assertThat(doc.rootDoc().getFields().size(), equalTo(14)); // also includes all other meta fields + System.out.println(doc.rootDoc().getFields()); + assertThat(doc.rootDoc().getFields().size(), equalTo(16)); // also includes all other meta fields BytesRef queryBuilderAsBytes = doc.rootDoc().getField("query_field1.query_builder_field").binaryValue(); assertQueryBuilder(queryBuilderAsBytes, queryBuilder); @@ -617,7 +618,7 @@ public void testMultiplePercolatorFields() throws Exception { public void testNestedPercolatorField() throws Exception { String typeName = "doc"; String percolatorMapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(typeName) - .startObject("_field_names").field("enabled", false).endObject() // makes testing easier + .startObject("_field_names").endObject() // makes testing easier .startObject("properties") .startObject("object_field") .field("type", "object") @@ -635,7 +636,7 @@ public void testNestedPercolatorField() throws Exception { .field("query_field", queryBuilder) .endObject().endObject()), XContentType.JSON)); - assertThat(doc.rootDoc().getFields().size(), equalTo(10)); // also includes all other meta fields + assertThat(doc.rootDoc().getFields().size(), equalTo(12)); // also includes all other meta fields BytesRef queryBuilderAsBytes = doc.rootDoc().getField("object_field.query_field.query_builder_field").binaryValue(); assertQueryBuilder(queryBuilderAsBytes, queryBuilder); @@ -646,7 +647,7 @@ public void testNestedPercolatorField() throws Exception { .endArray() .endObject()), XContentType.JSON)); - assertThat(doc.rootDoc().getFields().size(), equalTo(10)); // also includes all other meta fields + assertThat(doc.rootDoc().getFields().size(), equalTo(12)); // also includes all other meta fields queryBuilderAsBytes = doc.rootDoc().getField("object_field.query_field.query_builder_field").binaryValue(); assertQueryBuilder(queryBuilderAsBytes, queryBuilder); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index 12e53a5f9d4b9..a1ba1eab56dc5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -73,28 +73,26 @@ public static class Defaults { } private static class Builder extends MetadataFieldMapper.Builder { - private boolean enabled = Defaults.ENABLED; private Builder(MappedFieldType existing) { super(Defaults.NAME, existing == null ? Defaults.FIELD_TYPE : existing, Defaults.FIELD_TYPE); } - private Builder enabled(boolean enabled) { - this.enabled = enabled; - return this; - } - @Override public FieldNamesFieldMapper build(BuilderContext context) { setupFieldType(context); fieldType.setHasDocValues(false); FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldType)fieldType; - fieldNamesFieldType.setEnabled(enabled); + fieldNamesFieldType.setEnabled(Defaults.ENABLED); return new FieldNamesFieldMapper(fieldType, context.indexSettings()); } } public static class TypeParser implements MetadataFieldMapper.TypeParser { + public static final String ENABLED_DEPRECATION_MESSAGE = "Changing the `enabled` setting for `_field_names` fields is no " + + "longer necassary. Disabling it has almost no benefits anymore which is why we now ignore this setting and will " + + "remove it in a future major version."; + @Override public MetadataFieldMapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { @@ -105,7 +103,9 @@ public MetadataFieldMapper.Builder parse(String name, Map n String fieldName = entry.getKey(); Object fieldNode = entry.getValue(); if (fieldName.equals("enabled")) { - builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled")); + deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE); + // just read the value and dump it on the floor + XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled"); iterator.remove(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index d0d4a6b1d15b7..d3f28ed17e9b9 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java @@ -114,16 +114,17 @@ public void testExplicitEnabled() throws Exception { XContentType.JSON)); assertFieldNames(set("field"), doc); + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); } - public void testDisabled() throws Exception { + public void testEnabledSettingLogsDeprecation() throws Exception { String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("_field_names").field("enabled", false).endObject() .endObject().endObject()); DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser() .parse("type", new CompressedXContent(mapping)); FieldNamesFieldMapper fieldNamesMapper = docMapper.metadataMapper(FieldNamesFieldMapper.class); - assertFalse(fieldNamesMapper.fieldType().isEnabled()); + assertTrue(fieldNamesMapper.fieldType().isEnabled()); ParsedDocument doc = docMapper.parse(new SourceToParse("test", "type", "1", BytesReference.bytes(XContentFactory.jsonBuilder() @@ -133,24 +134,6 @@ public void testDisabled() throws Exception { XContentType.JSON)); assertNull(doc.rootDoc().get("_field_names")); - } - - public void testMergingMappings() throws Exception { - String enabledMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("_field_names").field("enabled", true).endObject() - .endObject().endObject()); - String disabledMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("_field_names").field("enabled", false).endObject() - .endObject().endObject()); - MapperService mapperService = createIndex("test").mapperService(); - - DocumentMapper mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping), - MapperService.MergeReason.MAPPING_UPDATE); - DocumentMapper mapperDisabled = mapperService.merge("type", new CompressedXContent(disabledMapping), - MapperService.MergeReason.MAPPING_UPDATE); - assertFalse(mapperDisabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled()); - - mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping), MapperService.MergeReason.MAPPING_UPDATE); - assertTrue(mapperEnabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled()); + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); } } 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 ee4e0f9540451..97c0b8482d6ea 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; @@ -1064,32 +1063,6 @@ public void testExistsFieldQuery() throws Exception { assertThat(query, equalTo(expected)); } - public void testDisabledFieldNamesField() throws Exception { - QueryShardContext context = createShardContext(); - context.getMapperService().merge("_doc", - new CompressedXContent( - Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", - "foo", "type=text", - "_field_names", "enabled=false"))), - MapperService.MergeReason.MAPPING_UPDATE); - try { - QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo:*"); - Query query = queryBuilder.toQuery(context); - Query expected = new WildcardQuery(new Term("foo", "*")); - assertThat(query, equalTo(expected)); - } finally { - // restore mappings as they were before - context.getMapperService().merge("_doc", - new CompressedXContent( - Strings.toString(PutMappingRequest.buildFromSimplifiedDef("_doc", - "foo", "type=text", - "_field_names", "enabled=true"))), - MapperService.MergeReason.MAPPING_UPDATE); - } - } - - - public void testFromJson() throws IOException { String json = "{\n" + From 26e8b646657912f1d50b9004a3cf043654cac71a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 5 Jun 2019 11:45:10 +0200 Subject: [PATCH 2/6] iter --- docs/reference/mapping/fields/field-names-field.asciidoc | 1 + .../elasticsearch/percolator/PercolatorFieldMapperTests.java | 1 - .../org/elasticsearch/index/mapper/FieldNamesFieldMapper.java | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/mapping/fields/field-names-field.asciidoc b/docs/reference/mapping/fields/field-names-field.asciidoc index 1ae4ab4c8fc89..2d963f4afa607 100644 --- a/docs/reference/mapping/fields/field-names-field.asciidoc +++ b/docs/reference/mapping/fields/field-names-field.asciidoc @@ -32,3 +32,4 @@ PUT tweets } -------------------------------------------------- // CONSOLE +// TEST[warning:Changing the `enabled` setting for `_field_names` fields is no longer necessary. Disabling it has almost no benefits anymore which is why we now ignore this setting and will remove it in a future major version.] diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index 9957eb0934d3e..d6590fd893c7d 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java @@ -605,7 +605,6 @@ public void testMultiplePercolatorFields() throws Exception { .field("query_field2", queryBuilder) .endObject()), XContentType.JSON)); - System.out.println(doc.rootDoc().getFields()); assertThat(doc.rootDoc().getFields().size(), equalTo(16)); // also includes all other meta fields BytesRef queryBuilderAsBytes = doc.rootDoc().getField("query_field1.query_builder_field").binaryValue(); assertQueryBuilder(queryBuilderAsBytes, queryBuilder); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index a1ba1eab56dc5..0f036b1105dac 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -90,7 +90,7 @@ public FieldNamesFieldMapper build(BuilderContext context) { public static class TypeParser implements MetadataFieldMapper.TypeParser { public static final String ENABLED_DEPRECATION_MESSAGE = "Changing the `enabled` setting for `_field_names` fields is no " - + "longer necassary. Disabling it has almost no benefits anymore which is why we now ignore this setting and will " + + "longer necessary. Disabling it has almost no benefits anymore which is why we now ignore this setting and will " + "remove it in a future major version."; @Override From d46bf47c125e5bfed7e7226912049ffb2cc441e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 5 Aug 2019 21:24:30 +0200 Subject: [PATCH 3/6] Only deprecate and log --- .../mapping/fields/field-names-field.asciidoc | 6 ++-- .../PercolatorFieldMapperTests.java | 3 -- .../index/mapper/FieldNamesFieldMapper.java | 25 +++++++++++---- .../mapper/FieldNamesFieldMapperTests.java | 4 +-- .../query/QueryStringQueryBuilderTests.java | 32 +++++++++++++++++++ 5 files changed, 56 insertions(+), 14 deletions(-) diff --git a/docs/reference/mapping/fields/field-names-field.asciidoc b/docs/reference/mapping/fields/field-names-field.asciidoc index 2d963f4afa607..5048f5b7c0a91 100644 --- a/docs/reference/mapping/fields/field-names-field.asciidoc +++ b/docs/reference/mapping/fields/field-names-field.asciidoc @@ -14,7 +14,9 @@ be available but will not use the `_field_names` field. [[disable-field-names]] ==== Disabling `_field_names` -Disabling `_field_names` is often not necessary because it no longer +NOTE: Disabling `_field_names` has been deprecated and will be removed in a future major version. + +Disabling `_field_names` is usually not necessary because it no longer carries the index overhead it once did. If you have a lot of fields which have `doc_values` and `norms` disabled and you do not need to execute `exists` queries using those fields you might want to disable @@ -32,4 +34,4 @@ PUT tweets } -------------------------------------------------- // CONSOLE -// TEST[warning:Changing the `enabled` setting for `_field_names` fields is no longer necessary. Disabling it has almost no benefits anymore which is why we now ignore this setting and will remove it in a future major version.] +// TEST[warning:Using the `enabled` setting for `_field_names` fields is no longer necessary. Please remove it from your mappings as and templates since it will be removed in the next major version.] diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java index d6590fd893c7d..e563ea9ba2974 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java @@ -146,7 +146,6 @@ public void init() throws Exception { mapperService = indexService.mapperService(); String mapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("doc") - .startObject("_field_names").endObject() // makes testing easier .startObject("properties") .startObject("field").field("type", "text").endObject() .startObject("field1").field("type", "text").endObject() @@ -590,7 +589,6 @@ public void testAllowNoAdditionalSettings() throws Exception { public void testMultiplePercolatorFields() throws Exception { String typeName = "doc"; String percolatorMapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(typeName) - .startObject("_field_names").endObject() // makes testing easier .startObject("properties") .startObject("query_field1").field("type", "percolator").endObject() .startObject("query_field2").field("type", "percolator").endObject() @@ -617,7 +615,6 @@ public void testMultiplePercolatorFields() throws Exception { public void testNestedPercolatorField() throws Exception { String typeName = "doc"; String percolatorMapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(typeName) - .startObject("_field_names").endObject() // makes testing easier .startObject("properties") .startObject("object_field") .field("type", "object") diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index 0f036b1105dac..81b8c0f54e325 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -24,6 +24,7 @@ import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Query; +import org.elasticsearch.Version; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; @@ -73,25 +74,32 @@ public static class Defaults { } private static class Builder extends MetadataFieldMapper.Builder { + private boolean enabled = Defaults.ENABLED; private Builder(MappedFieldType existing) { super(Defaults.NAME, existing == null ? Defaults.FIELD_TYPE : existing, Defaults.FIELD_TYPE); } + private Builder enabled(boolean enabled) { + this.enabled = enabled; + return this; + } + @Override public FieldNamesFieldMapper build(BuilderContext context) { setupFieldType(context); fieldType.setHasDocValues(false); FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldType)fieldType; - fieldNamesFieldType.setEnabled(Defaults.ENABLED); + fieldNamesFieldType.setEnabled(enabled); return new FieldNamesFieldMapper(fieldType, context.indexSettings()); } } public static class TypeParser implements MetadataFieldMapper.TypeParser { - public static final String ENABLED_DEPRECATION_MESSAGE = "Changing the `enabled` setting for `_field_names` fields is no " - + "longer necessary. Disabling it has almost no benefits anymore which is why we now ignore this setting and will " - + "remove it in a future major version."; + + public static final String ENABLED_DEPRECATION_MESSAGE = "Using the `enabled` setting for `_field_names` fields is no " + + "longer necessary. Please remove it from your mappings as and templates since it will be removed in the next " + + "major version."; @Override public MetadataFieldMapper.Builder parse(String name, Map node, @@ -103,9 +111,12 @@ public MetadataFieldMapper.Builder parse(String name, Map n String fieldName = entry.getKey(); Object fieldNode = entry.getValue(); if (fieldName.equals("enabled")) { - deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE); - // just read the value and dump it on the floor - XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled"); + // issue deprecation note if index is V7 + Version indexVersionCreated = parserContext.indexVersionCreated(); + if (indexVersionCreated.onOrAfter(Version.V_7_0_0)) { + deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE); + builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled")); + } iterator.remove(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index d3f28ed17e9b9..00632c34e7442 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java @@ -117,14 +117,14 @@ public void testExplicitEnabled() throws Exception { assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); } - public void testEnabledSettingLogsDeprecation() throws Exception { + public void testDisabled() throws Exception { String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("_field_names").field("enabled", false).endObject() .endObject().endObject()); DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser() .parse("type", new CompressedXContent(mapping)); FieldNamesFieldMapper fieldNamesMapper = docMapper.metadataMapper(FieldNamesFieldMapper.class); - assertTrue(fieldNamesMapper.fieldType().isEnabled()); + assertFalse(fieldNamesMapper.fieldType().isEnabled()); ParsedDocument doc = docMapper.parse(new SourceToParse("test", "type", "1", BytesReference.bytes(XContentFactory.jsonBuilder() 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 7ccb3dffa6228..be29cacea0850 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; @@ -57,6 +58,7 @@ import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.index.mapper.FieldNamesFieldMapper; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.search.QueryStringQueryParser; import org.elasticsearch.search.internal.SearchContext; @@ -1051,6 +1053,36 @@ public void testExistsFieldQuery() throws Exception { assertThat(query, equalTo(expected)); } + public void testDisabledFieldNamesField() throws Exception { + QueryShardContext context = createShardContext(); + context.getMapperService().merge("_doc", + new CompressedXContent(Strings + .toString(PutMappingRequest.buildFromSimplifiedDef("_doc", + "foo", + "type=text", + "_field_names", + "enabled=false"))), + MapperService.MergeReason.MAPPING_UPDATE); + + try { + QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo:*"); + Query query = queryBuilder.toQuery(context); + Query expected = new WildcardQuery(new Term("foo", "*")); + assertThat(query, equalTo(expected)); + } finally { + // restore mappings as they were before + context.getMapperService().merge("_doc", + new CompressedXContent(Strings.toString( + PutMappingRequest.buildFromSimplifiedDef("_doc", + "foo", + "type=text", + "_field_names", + "enabled=true"))), + MapperService.MergeReason.MAPPING_UPDATE); + } + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); + } + public void testFromJson() throws IOException { String json = "{\n" + From 4308e8eae052377f4f428b48ccc775d39f4080f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 9 Aug 2019 16:16:18 +0200 Subject: [PATCH 4/6] Log deprecation on any index version, inlcude name in log --- .../mapping/fields/field-names-field.asciidoc | 2 +- .../index/mapper/FieldNamesFieldMapper.java | 14 +++++--------- .../index/mapper/FieldNamesFieldMapperTests.java | 4 ++-- .../index/query/QueryStringQueryBuilderTests.java | 2 +- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/docs/reference/mapping/fields/field-names-field.asciidoc b/docs/reference/mapping/fields/field-names-field.asciidoc index 5048f5b7c0a91..3b83264086202 100644 --- a/docs/reference/mapping/fields/field-names-field.asciidoc +++ b/docs/reference/mapping/fields/field-names-field.asciidoc @@ -34,4 +34,4 @@ PUT tweets } -------------------------------------------------- // CONSOLE -// TEST[warning:Using the `enabled` setting for `_field_names` fields is no longer necessary. Please remove it from your mappings as and templates since it will be removed in the next major version.] +// TEST[warning:Index [tweets] uses the `enabled` setting for `_field_names`, which is no longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future major version. \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index 81b8c0f54e325..88363c734fb6b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -24,7 +24,6 @@ import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.Query; -import org.elasticsearch.Version; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; @@ -97,8 +96,8 @@ public FieldNamesFieldMapper build(BuilderContext context) { public static class TypeParser implements MetadataFieldMapper.TypeParser { - public static final String ENABLED_DEPRECATION_MESSAGE = "Using the `enabled` setting for `_field_names` fields is no " - + "longer necessary. Please remove it from your mappings as and templates since it will be removed in the next " + public static final String ENABLED_DEPRECATION_MESSAGE = "Index [{}] uses the `enabled` setting for `_field_names`, which is no " + + "longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future " + "major version."; @Override @@ -111,12 +110,9 @@ public MetadataFieldMapper.Builder parse(String name, Map n String fieldName = entry.getKey(); Object fieldNode = entry.getValue(); if (fieldName.equals("enabled")) { - // issue deprecation note if index is V7 - Version indexVersionCreated = parserContext.indexVersionCreated(); - if (indexVersionCreated.onOrAfter(Version.V_7_0_0)) { - deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE); - builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled")); - } + String indexName = parserContext.mapperService().index().getName(); + deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE, indexName); + builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled")); iterator.remove(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index 00632c34e7442..2e56cbc87aa66 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java @@ -114,7 +114,7 @@ public void testExplicitEnabled() throws Exception { XContentType.JSON)); assertFieldNames(set("field"), doc); - assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test")); } public void testDisabled() throws Exception { @@ -134,6 +134,6 @@ public void testDisabled() throws Exception { XContentType.JSON)); assertNull(doc.rootDoc().get("_field_names")); - assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test")); } } 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 be29cacea0850..c1d1869dbbcbf 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -1080,7 +1080,7 @@ public void testDisabledFieldNamesField() throws Exception { "enabled=true"))), MapperService.MergeReason.MAPPING_UPDATE); } - assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE); + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", context.index().getName())); } public void testFromJson() throws IOException { From b653c5d094117622e557d18116f6707aa231d65f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 9 Aug 2019 19:51:08 +0200 Subject: [PATCH 5/6] iter --- docs/reference/mapping/fields/field-names-field.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/mapping/fields/field-names-field.asciidoc b/docs/reference/mapping/fields/field-names-field.asciidoc index 3b83264086202..17a7ed7a48866 100644 --- a/docs/reference/mapping/fields/field-names-field.asciidoc +++ b/docs/reference/mapping/fields/field-names-field.asciidoc @@ -34,4 +34,4 @@ PUT tweets } -------------------------------------------------- // CONSOLE -// TEST[warning:Index [tweets] uses the `enabled` setting for `_field_names`, which is no longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future major version. \ No newline at end of file +// TEST[warning:Index [tweets] uses the `enabled` setting for `_field_names`, which is no longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future major version.] From c55270257360236835299eb22c6e185d75d0d033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 10 Sep 2019 12:38:30 +0200 Subject: [PATCH 6/6] Changes to deprecation message and revert removing test --- .../mapping/fields/field-names-field.asciidoc | 2 +- .../index/mapper/FieldNamesFieldMapper.java | 6 ++--- .../mapper/FieldNamesFieldMapperTests.java | 22 ++++++++++++++++++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/docs/reference/mapping/fields/field-names-field.asciidoc b/docs/reference/mapping/fields/field-names-field.asciidoc index 935e7edea69d5..cec9d8b0d37c3 100644 --- a/docs/reference/mapping/fields/field-names-field.asciidoc +++ b/docs/reference/mapping/fields/field-names-field.asciidoc @@ -34,4 +34,4 @@ PUT tweets } -------------------------------------------------- // CONSOLE -// TEST[warning:Index [tweets] uses the `enabled` setting for `_field_names`, which is no longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future major version.] +// TEST[warning:Index [tweets] uses the deprecated `enabled` setting for `_field_names`. Disabling _field_names is not necessary because it no longer carries a large index overhead. Support for this setting will be removed in a future major version. Please remove it from your mappings and templates.] diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index 88363c734fb6b..84211e69cf420 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -96,9 +96,9 @@ public FieldNamesFieldMapper build(BuilderContext context) { public static class TypeParser implements MetadataFieldMapper.TypeParser { - public static final String ENABLED_DEPRECATION_MESSAGE = "Index [{}] uses the `enabled` setting for `_field_names`, which is no " - + "longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future " - + "major version."; + public static final String ENABLED_DEPRECATION_MESSAGE = "Index [{}] uses the deprecated `enabled` setting for `_field_names`. " + + "Disabling _field_names is not necessary because it no longer carries a large index overhead. Support for this setting " + + "will be removed in a future major version. Please remove it from your mappings and templates."; @Override public MetadataFieldMapper.Builder parse(String name, Map node, diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index 2e56cbc87aa66..a1ad67a0550ae 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java @@ -43,7 +43,7 @@ private static SortedSet extract(String path) { return set; } - private static SortedSet set(T... values) { + private static SortedSet set(String... values) { return new TreeSet<>(Arrays.asList(values)); } @@ -136,4 +136,24 @@ public void testDisabled() throws Exception { assertNull(doc.rootDoc().get("_field_names")); assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test")); } + + public void testMergingMappings() throws Exception { + String enabledMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("_field_names").field("enabled", true).endObject() + .endObject().endObject()); + String disabledMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("_field_names").field("enabled", false).endObject() + .endObject().endObject()); + MapperService mapperService = createIndex("test").mapperService(); + + DocumentMapper mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping), + MapperService.MergeReason.MAPPING_UPDATE); + DocumentMapper mapperDisabled = mapperService.merge("type", new CompressedXContent(disabledMapping), + MapperService.MergeReason.MAPPING_UPDATE); + assertFalse(mapperDisabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled()); + + mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping), MapperService.MergeReason.MAPPING_UPDATE); + assertTrue(mapperEnabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled()); + assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test")); + } }