Skip to content

Commit 2923fb5

Browse files
cbismuthChristoph Büscher
authored andcommitted
Disallow "enabled" attribute change for types in mapping update (#33933)
This commit adds a check for "enabled" attribute change for types when a RestPutMappingAction is received. A MappingException is thrown when such a change is detected. Change are prevented in both ways: "false -> true" and "true -> false". Closes #33566
1 parent d12a64e commit 2923fb5

File tree

2 files changed

+139
-0
lines changed

2 files changed

+139
-0
lines changed

server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,8 @@ protected void doMerge(final ObjectMapper mergeWith) {
458458

459459
for (Mapper mergeWithMapper : mergeWith) {
460460
Mapper mergeIntoMapper = mappers.get(mergeWithMapper.simpleName());
461+
checkEnabledFieldChange(mergeWith, mergeWithMapper, mergeIntoMapper);
462+
461463
Mapper merged;
462464
if (mergeIntoMapper == null) {
463465
// no mapping, simply add it
@@ -470,6 +472,18 @@ protected void doMerge(final ObjectMapper mergeWith) {
470472
}
471473
}
472474

475+
private static void checkEnabledFieldChange(ObjectMapper mergeWith, Mapper mergeWithMapper, Mapper mergeIntoMapper) {
476+
if (mergeIntoMapper instanceof ObjectMapper && mergeWithMapper instanceof ObjectMapper) {
477+
final ObjectMapper mergeIntoObjectMapper = (ObjectMapper) mergeIntoMapper;
478+
final ObjectMapper mergeWithObjectMapper = (ObjectMapper) mergeWithMapper;
479+
480+
if (mergeIntoObjectMapper.isEnabled() != mergeWithObjectMapper.isEnabled()) {
481+
final String path = mergeWith.fullPath() + "." + mergeWithObjectMapper.simpleName() + ".enabled";
482+
throw new MapperException("Can't update attribute for type [" + path + "] in index mapping");
483+
}
484+
}
485+
}
486+
473487
@Override
474488
public ObjectMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
475489
List<Mapper> updatedMappers = null;
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.index.mapper;
20+
21+
import com.google.common.collect.ImmutableMap;
22+
import org.elasticsearch.Version;
23+
import org.elasticsearch.common.settings.Settings;
24+
import org.elasticsearch.index.mapper.FieldMapper.CopyTo;
25+
import org.elasticsearch.index.mapper.FieldMapper.MultiFields;
26+
import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType;
27+
import org.elasticsearch.test.ESTestCase;
28+
import org.junit.AfterClass;
29+
30+
import java.util.Map;
31+
32+
import static java.util.Collections.emptyMap;
33+
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED;
34+
import static org.hamcrest.Matchers.notNullValue;
35+
36+
public class ObjectMapperMergeTests extends ESTestCase {
37+
38+
private static FieldMapper barFieldMapper = createTextFieldMapper("bar");
39+
private static FieldMapper bazFieldMapper = createTextFieldMapper("baz");
40+
41+
private static RootObjectMapper rootObjectMapper = createRootObjectMapper(
42+
"type1", true, ImmutableMap.of(
43+
"disabled", createObjectMapper("disabled", false, emptyMap()),
44+
"foo", createObjectMapper("foo", true, ImmutableMap.of(
45+
"bar", barFieldMapper))));
46+
47+
@AfterClass
48+
public static void cleanupReferences() {
49+
barFieldMapper = null;
50+
bazFieldMapper = null;
51+
rootObjectMapper = null;
52+
}
53+
54+
public void testMerge() {
55+
// GIVEN an enriched mapping with "baz" new field
56+
ObjectMapper mergeWith = createRootObjectMapper(
57+
"type1", true, ImmutableMap.of(
58+
"disabled", createObjectMapper("disabled", false, emptyMap()),
59+
"foo", createObjectMapper("foo", true, ImmutableMap.of(
60+
"bar", barFieldMapper,
61+
"baz", bazFieldMapper))));
62+
63+
// WHEN merging mappings
64+
final ObjectMapper merged = rootObjectMapper.merge(mergeWith);
65+
66+
// THEN "baz" new field is added to merged mapping
67+
final ObjectMapper mergedFoo = (ObjectMapper) merged.getMapper("foo");
68+
assertThat(mergedFoo.getMapper("bar"), notNullValue());
69+
assertThat(mergedFoo.getMapper("baz"), notNullValue());
70+
}
71+
72+
public void testMergeWhenDisablingField() {
73+
// GIVEN a mapping with "foo" field disabled
74+
ObjectMapper mergeWith = createRootObjectMapper(
75+
"type1", true, ImmutableMap.of(
76+
"disabled", createObjectMapper("disabled", false, emptyMap()),
77+
"foo", createObjectMapper("foo", false, emptyMap())));
78+
79+
// WHEN merging mappings
80+
// THEN a MapperException is thrown with an excepted message
81+
MapperException e = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith));
82+
assertEquals("Can't update attribute for type [type1.foo.enabled] in index mapping", e.getMessage());
83+
}
84+
85+
public void testMergeWhenEnablingField() {
86+
// GIVEN a mapping with "disabled" field enabled
87+
ObjectMapper mergeWith = createRootObjectMapper(
88+
"type1", true, ImmutableMap.of(
89+
"disabled", createObjectMapper("disabled", true, emptyMap()),
90+
"foo", createObjectMapper("foo", true, ImmutableMap.of(
91+
"bar", barFieldMapper))));
92+
93+
// WHEN merging mappings
94+
// THEN a MapperException is thrown with an excepted message
95+
MapperException e = expectThrows(MapperException.class, () -> rootObjectMapper.merge(mergeWith));
96+
assertEquals("Can't update attribute for type [type1.disabled.enabled] in index mapping", e.getMessage());
97+
}
98+
99+
private static RootObjectMapper createRootObjectMapper(String name, boolean enabled, Map<String, Mapper> mappers) {
100+
final Settings indexSettings = Settings.builder().put(SETTING_VERSION_CREATED, Version.CURRENT).build();
101+
final Mapper.BuilderContext context = new Mapper.BuilderContext(indexSettings, new ContentPath());
102+
final RootObjectMapper rootObjectMapper = new RootObjectMapper.Builder(name).enabled(enabled).build(context);
103+
104+
mappers.values().forEach(rootObjectMapper::putMapper);
105+
106+
return rootObjectMapper;
107+
}
108+
109+
private static ObjectMapper createObjectMapper(String name, boolean enabled, Map<String, Mapper> mappers) {
110+
final Settings indexSettings = Settings.builder().put(SETTING_VERSION_CREATED, Version.CURRENT).build();
111+
final Mapper.BuilderContext context = new Mapper.BuilderContext(indexSettings, new ContentPath());
112+
final ObjectMapper mapper = new ObjectMapper.Builder(name).enabled(enabled).build(context);
113+
114+
mappers.values().forEach(mapper::putMapper);
115+
116+
return mapper;
117+
}
118+
119+
private static TextFieldMapper createTextFieldMapper(String name) {
120+
final TextFieldType fieldType = new TextFieldType();
121+
final Settings indexSettings = Settings.builder().put(SETTING_VERSION_CREATED, Version.CURRENT).build();
122+
123+
return new TextFieldMapper(name, fieldType, fieldType, -1, null, indexSettings, MultiFields.empty(), CopyTo.empty());
124+
}
125+
}

0 commit comments

Comments
 (0)