1
0
Эх сурвалжийг харах

Emit deprecation warnings only for new index or template (#117529) (#117650)

Currently, we emit a deprecation warning in the parser of the source
field when source mode is used in mappings. However, this behavior
causes warnings to be emitted for every mapping update. In tests with
assertions enabled, warnings are also triggered for every change to
index metadata. As a result, deprecation warnings are inadvertently
emitted for index or update requests.

This change relocates the deprecation check to the mapper, limiting it
to cases where a new index is created or a template is created/updated.

Relates to #117524
Nhat Nguyen 10 сар өмнө
parent
commit
a771af3ef1

+ 10 - 0
server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java

@@ -10,8 +10,11 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.logging.DeprecationCategory;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
+import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.xcontent.XContentType;
 
@@ -31,6 +34,7 @@ public final class MappingParser {
     private final Supplier<Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersSupplier;
     private final Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers;
     private final Function<String, String> documentTypeResolver;
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(MappingParser.class);
 
     MappingParser(
         Supplier<MappingParserContext> mappingParserContextSupplier,
@@ -144,6 +148,12 @@ public final class MappingParser {
                 }
                 @SuppressWarnings("unchecked")
                 Map<String, Object> fieldNodeMap = (Map<String, Object>) fieldNode;
+                if (reason == MergeReason.INDEX_TEMPLATE
+                    && SourceFieldMapper.NAME.equals(fieldName)
+                    && fieldNodeMap.containsKey("mode")
+                    && mappingParserContext.indexVersionCreated().onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)) {
+                    deprecationLogger.critical(DeprecationCategory.MAPPINGS, "mapping_source_mode", SourceFieldMapper.DEPRECATION_WARNING);
+                }
                 MetadataFieldMapper metadataFieldMapper = typeParser.parse(fieldName, fieldNodeMap, mappingParserContext).build();
                 metadataMappers.put(metadataFieldMapper.getClass(), metadataFieldMapper);
                 assert fieldNodeMap.isEmpty();

+ 1 - 13
server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java

@@ -18,7 +18,6 @@ import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.CollectionUtils;
@@ -39,7 +38,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
 
 public class SourceFieldMapper extends MetadataFieldMapper {
     public static final NodeFeature SYNTHETIC_SOURCE_FALLBACK = new NodeFeature("mapper.source.synthetic_source_fallback");
@@ -309,17 +307,7 @@ public class SourceFieldMapper extends MetadataFieldMapper {
             c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_LOSSY_PARAMS_CHECK),
             c.indexVersionCreated().before(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER)
         )
-    ) {
-        @Override
-        public MetadataFieldMapper.Builder parse(String name, Map<String, Object> node, MappingParserContext parserContext)
-            throws MapperParsingException {
-            assert name.equals(SourceFieldMapper.NAME) : name;
-            if (parserContext.indexVersionCreated().onOrAfter(IndexVersions.DEPRECATE_SOURCE_MODE_MAPPER) && node.containsKey("mode")) {
-                deprecationLogger.critical(DeprecationCategory.MAPPINGS, "mapping_source_mode", SourceFieldMapper.DEPRECATION_WARNING);
-            }
-            return super.parse(name, node, parserContext);
-        }
-    };
+    );
 
     static final class SourceFieldType extends MappedFieldType {
         private final boolean enabled;

+ 0 - 1
server/src/test/java/org/elasticsearch/index/mapper/DocumentParserContextTests.java

@@ -133,6 +133,5 @@ public class DocumentParserContextTests extends ESTestCase {
         assertEquals(ObjectMapper.Defaults.DYNAMIC, resultFromParserContext.getDynamic());
         assertEquals(MapperService.MergeReason.MAPPING_UPDATE, resultFromParserContext.getMergeReason());
         assertFalse(resultFromParserContext.isInNestedContext());
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
     }
 }

+ 1 - 16
server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java

@@ -65,7 +65,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "synthetic").endObject()),
             dm -> {
                 assertTrue(dm.metadataMapper(SourceFieldMapper.class).isSynthetic());
-                assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
             }
         );
         checker.registerConflictCheck("includes", b -> b.array("includes", "foo*"));
@@ -74,7 +73,7 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             "mode",
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "synthetic").endObject()),
             topMapping(b -> b.startObject(SourceFieldMapper.NAME).field("mode", "stored").endObject()),
-            dm -> assertWarnings(SourceFieldMapper.DEPRECATION_WARNING)
+            d -> {}
         );
     }
 
@@ -211,14 +210,12 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             )
         );
         assertThat(e.getMessage(), containsString("Cannot set both [mode] and [enabled] parameters"));
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
     }
 
     public void testSyntheticUpdates() throws Exception {
         MapperService mapperService = createMapperService("""
             { "_doc" : { "_source" : { "mode" : "synthetic" } } }
             """);
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         SourceFieldMapper mapper = mapperService.documentMapper().sourceMapper();
         assertTrue(mapper.enabled());
         assertTrue(mapper.isSynthetic());
@@ -226,7 +223,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
         merge(mapperService, """
             { "_doc" : { "_source" : { "mode" : "synthetic" } } }
             """);
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         mapper = mapperService.documentMapper().sourceMapper();
         assertTrue(mapper.enabled());
         assertTrue(mapper.isSynthetic());
@@ -239,12 +235,10 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             """));
 
         assertThat(e.getMessage(), containsString("Cannot update parameter [mode] from [synthetic] to [stored]"));
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
 
         merge(mapperService, """
             { "_doc" : { "_source" : { "mode" : "disabled" } } }
             """);
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
 
         mapper = mapperService.documentMapper().sourceMapper();
         assertFalse(mapper.enabled());
@@ -281,7 +275,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
                 topMapping(b -> b.startObject("_source").field("mode", randomBoolean() ? "synthetic" : "stored").endObject())
             ).documentMapper().sourceMapper();
             assertThat(sourceFieldMapper, notNullValue());
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         Exception e = expectThrows(
             MapperParsingException.class,
@@ -313,8 +306,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
                 .documentMapper()
                 .sourceMapper()
         );
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
-
         assertThat(e.getMessage(), containsString("Parameter [mode=disabled] is not allowed in source"));
 
         e = expectThrows(
@@ -423,7 +414,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             ParsedDocument doc = docMapper.parse(source(b -> { b.field("field1", "value1"); }));
             assertNotNull(doc.rootDoc().getField("_recovery_source"));
             assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"field1\":\"value1\"}")));
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         {
             Settings settings = Settings.builder().put(INDICES_RECOVERY_SOURCE_ENABLED_SETTING.getKey(), false).build();
@@ -434,7 +424,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> b.field("field1", "value1")));
             assertNull(doc.rootDoc().getField("_recovery_source"));
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
     }
 
@@ -629,7 +618,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             ParsedDocument doc = docMapper.parse(source(b -> { b.field("@timestamp", "2012-02-13"); }));
             assertNotNull(doc.rootDoc().getField("_recovery_source"));
             assertThat(doc.rootDoc().getField("_recovery_source").binaryValue(), equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\"}")));
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         {
             Settings settings = Settings.builder()
@@ -640,7 +628,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
             DocumentMapper docMapper = mapperService.documentMapper();
             ParsedDocument doc = docMapper.parse(source(b -> b.field("@timestamp", "2012-02-13")));
             assertNull(doc.rootDoc().getField("_recovery_source"));
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
     }
 
@@ -709,7 +696,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
                 doc.rootDoc().getField("_recovery_source").binaryValue(),
                 equalTo(new BytesRef("{\"@timestamp\":\"2012-02-13\",\"field\":\"value1\"}"))
             );
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
         {
             Settings settings = Settings.builder()
@@ -723,7 +709,6 @@ public class SourceFieldMapperTests extends MetadataMapperTestCase {
                 source("123", b -> b.field("@timestamp", "2012-02-13").field("field", randomAlphaOfLength(5)), null)
             );
             assertNull(doc.rootDoc().getField("_recovery_source"));
-            assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
         }
     }
 }

+ 0 - 2
server/src/test/java/org/elasticsearch/index/shard/ShardGetServiceTests.java

@@ -21,7 +21,6 @@ import org.elasticsearch.index.engine.LiveVersionMapTestUtils;
 import org.elasticsearch.index.engine.VersionConflictEngineException;
 import org.elasticsearch.index.get.GetResult;
 import org.elasticsearch.index.mapper.RoutingFieldMapper;
-import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 import org.elasticsearch.xcontent.XContentType;
 
@@ -115,7 +114,6 @@ public class ShardGetServiceTests extends IndexShardTestCase {
             "mode": "synthetic"
             """;
         runGetFromTranslogWithOptions(docToIndex, sourceOptions, expectedFetchedSource, "\"long\"", 7L, true);
-        assertWarnings(SourceFieldMapper.DEPRECATION_WARNING);
     }
 
     public void testGetFromTranslogWithDenseVector() throws IOException {