Skip to content

Commit

Permalink
Reduce blast radius of MapperMergeContext change
Browse files Browse the repository at this point in the history
  • Loading branch information
@martijnvg
martijnvg committed Jan 26, 2024
1 parent 2a5f175 commit 4c3a1de
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 31 deletions.
3 changes: 2 additions & 1 deletion ...c/main/java/org/elasticsearch/benchmark/compute/operator/ValuesSourceReaderBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ private static BlockLoader blockLoader(String name) {
Lucene.KEYWORD_ANALYZER,
new KeywordFieldMapper.Builder(name, IndexVersion.current()).docValues(ft.docValuesType() != DocValuesType.NONE),
syntheticSource,
false).blockLoader(new MappedFieldType.BlockLoaderContext() {
false
).blockLoader(new MappedFieldType.BlockLoaderContext() {
@Override
public String indexName() {
return "benchmark";
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/MapperMergeContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ public static MapperMergeContext root(boolean isSourceSynthetic, boolean isDataS
return new MapperMergeContext(MapperBuilderContext.root(isSourceSynthetic, isDataStream, isAllFieldEnabled));
}

public static MapperMergeContext root(boolean isSourceSynthetic, boolean isDataStream) {
return new MapperMergeContext(MapperBuilderContext.root(isSourceSynthetic, isDataStream, false));
}

/**
* Creates a new {@link MapperMergeContext} from a {@link MapperBuilderContext}
* @param mapperBuilderContext the {@link MapperBuilderContext} for this {@link MapperMergeContext}
Expand Down
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,7 @@ private static void validateDynamicTemplate(MappingParserContext parserContext,
validate(
template,
dynamicType,
(name, mapping) -> typeParser.parse(name, mapping, parserContext)
.build(MapperBuilderContext.root(false, false))
(name, mapping) -> typeParser.parse(name, mapping, parserContext).build(MapperBuilderContext.root(false, false))
);
}
lastError = null; // ok, the template is valid for at least one type
Expand Down
4 changes: 1 addition & 3 deletions server/src/test/java/org/elasticsearch/index/mapper/IpRangeFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
public class IpRangeFieldTypeTests extends FieldTypeTestCase {

public void testFetchSourceValue() throws IOException {
RangeFieldMapper mapper = new RangeFieldMapper.Builder("field", RangeType.IP, true).build(
MapperBuilderContext.root(false, false)
);
RangeFieldMapper mapper = new RangeFieldMapper.Builder("field", RangeType.IP, true).build(MapperBuilderContext.root(false, false));
Map<String, Object> range = Map.of("gte", "2001:db8:0:0:0:0:2:1");
assertEquals(List.of(Map.of("gte", "2001:db8::2:1")), fetchSourceValue(mapper.fieldType(), range));
assertEquals(List.of("2001:db8::2:1/32"), fetchSourceValue(mapper.fieldType(), "2001:db8:0:0:0:0:2:1/32"));
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1508,14 +1508,14 @@ public void testMergeNested() {

MapperException e = expectThrows(
MapperException.class,
() -> firstMapper.merge(secondMapper, MapperMergeContext.root(false, false, false))
() -> firstMapper.merge(secondMapper, MapperMergeContext.root(false, false))
);
assertThat(e.getMessage(), containsString("[include_in_parent] parameter can't be updated on a nested object mapping"));

NestedObjectMapper result = (NestedObjectMapper) firstMapper.merge(
secondMapper,
MapperService.MergeReason.INDEX_TEMPLATE,
MapperMergeContext.root(false, false, false)
MapperMergeContext.root(false, false)
);
assertFalse(result.isIncludeInParent());
assertTrue(result.isIncludeInRoot());
Expand Down
24 changes: 12 additions & 12 deletions server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperMergeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void testMerge() {
ObjectMapper mergeWith = createMapping(false, true, true, true);

// WHEN merging mappings
final ObjectMapper merged = rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false, false));
final ObjectMapper merged = rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false));

// THEN "baz" new field is added to merged mapping
final ObjectMapper mergedFoo = (ObjectMapper) merged.getMapper("foo");
Expand All @@ -63,7 +63,7 @@ public void testMergeWhenDisablingField() {
// THEN a MapperException is thrown with an excepted message
MapperException e = expectThrows(
MapperException.class,
() -> rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false, false))
() -> rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false))
);
assertEquals("the [enabled] parameter can't be updated for the object mapping [foo]", e.getMessage());
}
Expand All @@ -75,7 +75,7 @@ public void testMergeDisabledField() {
new ObjectMapper.Builder("disabled", Explicit.IMPLICIT_TRUE)
).build(MapperBuilderContext.root(false, false));

RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false, false));
RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false));
assertFalse(((ObjectMapper) merged.getMapper("disabled")).isEnabled());
}

Expand All @@ -84,14 +84,14 @@ public void testMergeEnabled() {

MapperException e = expectThrows(
MapperException.class,
() -> rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false, false))
() -> rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false))
);
assertEquals("the [enabled] parameter can't be updated for the object mapping [disabled]", e.getMessage());

ObjectMapper result = rootObjectMapper.merge(
mergeWith,
MapperService.MergeReason.INDEX_TEMPLATE,
MapperMergeContext.root(false, false, false)
MapperMergeContext.root(false, false)
);
assertTrue(result.isEnabled());
}
Expand All @@ -106,14 +106,14 @@ public void testMergeEnabledForRootMapper() {

MapperException e = expectThrows(
MapperException.class,
() -> firstMapper.merge(secondMapper, MapperMergeContext.root(false, false, false))
() -> firstMapper.merge(secondMapper, MapperMergeContext.root(false, false))
);
assertEquals("the [enabled] parameter can't be updated for the object mapping [" + type + "]", e.getMessage());

ObjectMapper result = firstMapper.merge(
secondMapper,
MapperService.MergeReason.INDEX_TEMPLATE,
MapperMergeContext.root(false, false, false)
MapperMergeContext.root(false, false)
);
assertFalse(result.isEnabled());
}
Expand All @@ -128,7 +128,7 @@ public void testMergeDisabledRootMapper() {
Collections.singletonMap("test", new TestRuntimeField("test", "long"))
).build(MapperBuilderContext.root(false, false));

RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false, false));
RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith, MapperMergeContext.root(false, false));
assertFalse(merged.isEnabled());
assertEquals(1, merged.runtimeFields().size());
assertEquals("test", merged.runtimeFields().iterator().next().name());
Expand All @@ -138,7 +138,7 @@ public void testMergedFieldNamesFieldWithDotsSubobjectsFalseAtRoot() {
RootObjectMapper mergeInto = createRootSubobjectFalseLeafWithDots();
RootObjectMapper mergeWith = createRootSubobjectFalseLeafWithDots();

final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, false));
final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false));

final KeywordFieldMapper keywordFieldMapper = (KeywordFieldMapper) merged.getMapper("host.name");
assertEquals("host.name", keywordFieldMapper.name());
Expand All @@ -153,7 +153,7 @@ public void testMergedFieldNamesFieldWithDotsSubobjectsFalse() {
createObjectSubobjectsFalseLeafWithDots()
).build(MapperBuilderContext.root(false, false));

final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, false));
final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false));

ObjectMapper foo = (ObjectMapper) merged.getMapper("foo");
ObjectMapper metrics = (ObjectMapper) foo.getMapper("metrics");
Expand All @@ -168,7 +168,7 @@ public void testMergedFieldNamesMultiFields() {
RootObjectMapper mergeWith = new RootObjectMapper.Builder("_doc", Explicit.IMPLICIT_TRUE).add(createTextKeywordMultiField("text"))
.build(MapperBuilderContext.root(false, false));

final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, false));
final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false));

TextFieldMapper text = (TextFieldMapper) merged.getMapper("text");
assertEquals("text", text.name());
Expand All @@ -186,7 +186,7 @@ public void testMergedFieldNamesMultiFieldsWithinSubobjectsFalse() {
createObjectSubobjectsFalseLeafWithMultiField()
).build(MapperBuilderContext.root(false, false));

final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false, false));
final ObjectMapper merged = mergeInto.merge(mergeWith, MapperMergeContext.root(false, false));

ObjectMapper foo = (ObjectMapper) merged.getMapper("foo");
ObjectMapper metrics = (ObjectMapper) foo.getMapper("metrics");
Expand Down
16 changes: 8 additions & 8 deletions server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ public void testMerging() {
{"type":"test_mapper","fixed":true,"fixed2":true,"required":"value"}""");
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> mapper.merge(badMerge, MapperMergeContext.root(false, false, false))
() -> mapper.merge(badMerge, MapperMergeContext.root(false, false))
);
String expectedError = """
Mapper for [field] conflicts with existing mapper:
Expand All @@ -358,7 +358,7 @@ public void testMerging() {
// TODO: should we have to include 'fixed' here? Or should updates take as 'defaults' the existing values?
TestMapper goodMerge = fromMapping("""
{"type":"test_mapper","fixed":false,"variable":"updated","required":"value"}""");
TestMapper merged = (TestMapper) mapper.merge(goodMerge, MapperMergeContext.root(false, false, false));
TestMapper merged = (TestMapper) mapper.merge(goodMerge, MapperMergeContext.root(false, false));

assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); // original mapping is unaffected
assertEquals("""
Expand All @@ -376,7 +376,7 @@ public void testMultifields() throws IOException {
String addSubField = """
{"type":"test_mapper","variable":"foo","required":"value","fields":{"sub2":{"type":"keyword"}}}""";
TestMapper toMerge = fromMapping(addSubField);
TestMapper merged = (TestMapper) mapper.merge(toMerge, MapperMergeContext.root(false, false, false));
TestMapper merged = (TestMapper) mapper.merge(toMerge, MapperMergeContext.root(false, false));
assertEquals(XContentHelper.stripWhitespace("""
{
"field": {
Expand All @@ -399,7 +399,7 @@ public void testMultifields() throws IOException {
TestMapper badToMerge = fromMapping(badSubField);
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> merged.merge(badToMerge, MapperMergeContext.root(false, false, false))
() -> merged.merge(badToMerge, MapperMergeContext.root(false, false))
);
assertEquals("mapper [field.sub2] cannot be changed from type [keyword] to [binary]", e.getMessage());
}
Expand All @@ -415,13 +415,13 @@ public void testCopyTo() {

TestMapper toMerge = fromMapping("""
{"type":"test_mapper","variable":"updated","required":"value","copy_to":["foo","bar"]}""");
TestMapper merged = (TestMapper) mapper.merge(toMerge, MapperMergeContext.root(false, false, false));
TestMapper merged = (TestMapper) mapper.merge(toMerge, MapperMergeContext.root(false, false));
assertEquals("""
{"field":{"type":"test_mapper","variable":"updated","required":"value","copy_to":["foo","bar"]}}""", Strings.toString(merged));

TestMapper removeCopyTo = fromMapping("""
{"type":"test_mapper","variable":"updated","required":"value"}""");
TestMapper noCopyTo = (TestMapper) merged.merge(removeCopyTo, MapperMergeContext.root(false, false, false));
TestMapper noCopyTo = (TestMapper) merged.merge(removeCopyTo, MapperMergeContext.root(false, false));
assertEquals("""
{"field":{"type":"test_mapper","variable":"updated","required":"value"}}""", Strings.toString(noCopyTo));
}
Expand Down Expand Up @@ -487,7 +487,7 @@ public void testCustomSerialization() {
TestMapper toMerge = fromMapping(conflict);
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> mapper.merge(toMerge, MapperMergeContext.root(false, false, false))
() -> mapper.merge(toMerge, MapperMergeContext.root(false, false))
);
assertEquals(
"Mapper for [field] conflicts with existing mapper:\n"
Expand Down Expand Up @@ -576,7 +576,7 @@ public void testAnalyzers() {

TestMapper original = mapper;
TestMapper toMerge = fromMapping(mapping);
e = expectThrows(IllegalArgumentException.class, () -> original.merge(toMerge, MapperMergeContext.root(false, false, false)));
e = expectThrows(IllegalArgumentException.class, () -> original.merge(toMerge, MapperMergeContext.root(false, false)));
assertEquals(
"Mapper for [field] conflicts with existing mapper:\n" + "\tCannot update parameter [analyzer] from [default] to [_standard]",
e.getMessage()
Expand Down
4 changes: 1 addition & 3 deletions .../src/test/java/org/elasticsearch/index/mapper/flattened/FlattenedIndexFieldDataTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ public void testGlobalFieldDataCaching() throws IOException {
indicesService.getCircuitBreakerService()
);

FlattenedFieldMapper fieldMapper = new FlattenedFieldMapper.Builder("flattened").build(
MapperBuilderContext.root(false, false)
);
FlattenedFieldMapper fieldMapper = new FlattenedFieldMapper.Builder("flattened").build(MapperBuilderContext.root(false, false));
MappedFieldType fieldType1 = fieldMapper.fieldType().getChildFieldType("key");

AtomicInteger onCacheCalled = new AtomicInteger();
Expand Down

0 comments on commit 4c3a1de

Please sign in to comment.