honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Validate that fields are defined only once. 

There are two ways that a field can be defined twice:
 - by reusing the name of a meta mapper in the root object (`_id`, `_routing`,
   etc.)
 - by defining a sub-field both explicitly in the mapping and through the code
   in a field mapper (like ExternalMapper does)

This commit adds new checks in order to make sure this never happens.

Close #15057
This commit is contained in:
Adrien Grand 2015-12-04 16:40:37 +01:00
parent 5f7b863067
commit 5d5c6591aa
5 changed files with 98 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
core/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View File

@ -32,7 +32,6 @@ import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchGenerationException;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.lucene.search.Queries;
@ -92,7 +91,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
private final ReleasableLock mappingWriteLock = new ReleasableLock(mappingLock.writeLock());
private volatile FieldTypeLookup fieldTypes;
private volatile ImmutableOpenMap<String, ObjectMapper> fullPathObjectMappers = ImmutableOpenMap.of();
private volatile Map<String, ObjectMapper> fullPathObjectMappers = new HashMap<>();
private boolean hasNested = false; // updated dynamically to true when a nested object is added
private final DocumentMapperParser documentParser;
@ -300,8 +299,41 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
return true;
}
private void checkFieldUniqueness(String type, Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
final Set<String> objectFullNames = new HashSet<>();
for (ObjectMapper objectMapper : objectMappers) {
final String fullPath = objectMapper.fullPath();
if (objectFullNames.add(fullPath) == false) {
throw new IllegalArgumentException("Object mapper [" + fullPath + "] is defined twice in mapping for type [" + type + "]");
}
}
if (indexSettings.getIndexVersionCreated().before(Version.V_3_0_0)) {
// Before 3.0 some metadata mappers are also registered under the root object mapper
// So we avoid false positives by deduplicating mappers
// given that we check exact equality, this would still catch the case that a mapper
// is defined under the root object
Collection<FieldMapper> uniqueFieldMappers = Collections.newSetFromMap(new IdentityHashMap<>());
uniqueFieldMappers.addAll(fieldMappers);
fieldMappers = uniqueFieldMappers;
}
final Set<String> fieldNames = new HashSet<>();
for (FieldMapper fieldMapper : fieldMappers) {
final String name = fieldMapper.name();
if (objectFullNames.contains(name)) {
throw new IllegalArgumentException("Field [" + name + "] is defined both as an object and a field in [" + type + "]");
} else if (fieldNames.add(name) == false) {
throw new IllegalArgumentException("Field [" + name + "] is defined twice in [" + type + "]");
}
}
}
protected void checkMappersCompatibility(String type, Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers, boolean updateAllTypes) {
assert mappingLock.isWriteLockedByCurrentThread();
checkFieldUniqueness(type, objectMappers, fieldMappers);
for (ObjectMapper newObjectMapper : objectMappers) {
ObjectMapper existingObjectMapper = fullPathObjectMappers.get(newObjectMapper.fullPath());
if (existingObjectMapper != null) {
@ -313,6 +345,13 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
}
}
for (FieldMapper fieldMapper : fieldMappers) {
if (fullPathObjectMappers.containsKey(fieldMapper.name())) {
throw new IllegalArgumentException("Field [{}] is defined as a field in mapping [" + fieldMapper.name() + "] but this name is already used for an object in other types");
}
}
fieldTypes.checkCompatibility(type, fieldMappers, updateAllTypes);
}
@ -330,14 +369,14 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
protected void addMappers(String type, Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
assert mappingLock.isWriteLockedByCurrentThread();
ImmutableOpenMap.Builder<String, ObjectMapper> fullPathObjectMappers = ImmutableOpenMap.builder(this.fullPathObjectMappers);
Map<String, ObjectMapper> fullPathObjectMappers = new HashMap<>(this.fullPathObjectMappers);
for (ObjectMapper objectMapper : objectMappers) {
fullPathObjectMappers.put(objectMapper.fullPath(), objectMapper);
if (objectMapper.nested().isNested()) {
hasNested = true;
}
}
this.fullPathObjectMappers = fullPathObjectMappers.build();
this.fullPathObjectMappers = Collections.unmodifiableMap(fullPathObjectMappers);
this.fieldTypes = this.fieldTypes.copyAndAddAll(type, fieldMappers);
}

8
core/src/main/java/org/elasticsearch/index/mapper/Mapping.java
View File

@ -27,10 +27,12 @@ import org.elasticsearch.index.mapper.object.RootObjectMapper;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
@ -41,7 +43,9 @@ import static java.util.Collections.unmodifiableMap;
*/
public final class Mapping implements ToXContent {
public static final List<String> LEGACY_INCLUDE_IN_OBJECT = Arrays.asList("_all", "_id", "_parent", "_routing", "_timestamp", "_ttl");
// Set of fields that were included into the root object mapper before 2.0
public static final Set<String> LEGACY_INCLUDE_IN_OBJECT = Collections.unmodifiableSet(new HashSet<>(
Arrays.asList("_all", "_id", "_parent", "_routing", "_timestamp", "_ttl")));
final Version indexCreated;
final RootObjectMapper root;

4
core/src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalValuesMapperIntegrationIT.java
View File

@ -87,7 +87,7 @@ public class ExternalValuesMapperIntegrationIT extends ESIntegTestCase {
.startObject("f")
.field("type", ExternalMapperPlugin.EXTERNAL_UPPER)
.startObject("fields")
.startObject("f")
.startObject("g")
.field("type", "string")
.field("store", "yes")
.startObject("fields")
@ -107,7 +107,7 @@ public class ExternalValuesMapperIntegrationIT extends ESIntegTestCase {
refresh();
SearchResponse response = client().prepareSearch("test-idx")
.setQuery(QueryBuilders.termQuery("f.f.raw", "FOO BAR"))
.setQuery(QueryBuilders.termQuery("f.g.raw", "FOO BAR"))
.execute().actionGet();
assertThat(response.getHits().totalHits(), equalTo((long) 1));

45
core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java
View File

@ -202,6 +202,51 @@ public class UpdateMappingTests extends ESSingleNodeTestCase {
assertNull(mapperService.documentMapper("type2").mapping().root().getMapper("foo"));
}
public void testReuseMetaField() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("_id").field("type", "string").endObject()
.endObject().endObject().endObject();
MapperService mapperService = createIndex("test", Settings.settingsBuilder().build()).mapperService();
try {
mapperService.merge("type", new CompressedXContent(mapping.string()), false, false);
fail();
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Field [_id] is defined twice in [type]"));
}
try {
mapperService.merge("type", new CompressedXContent(mapping.string()), false, false);
fail();
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Field [_id] is defined twice in [type]"));
}
}
public void testReuseMetaFieldBackCompat() throws IOException {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties").startObject("_id").field("type", "string").endObject()
.endObject().endObject().endObject();
// the logic is different for 2.x indices since they record some meta mappers (including _id)
// in the root object
Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_2_1_0).build();
MapperService mapperService = createIndex("test", settings).mapperService();
try {
mapperService.merge("type", new CompressedXContent(mapping.string()), false, false);
fail();
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Field [_id] is defined twice in [type]"));
}
try {
mapperService.merge("type", new CompressedXContent(mapping.string()), false, false);
fail();
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("Field [_id] is defined twice in [type]"));
}
}
public void testIndexFieldParsingBackcompat() throws IOException {
IndexService indexService = createIndex("test", Settings.settingsBuilder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build());
XContentBuilder indexMapping = XContentFactory.jsonBuilder();

6
modules/lang-groovy/src/test/java/org/elasticsearch/messy/tests/SearchFieldsTests.java
View File

@ -392,8 +392,7 @@ public class SearchFieldsTests extends ESIntegTestCase {
createIndex("test");
client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForYellowStatus().execute().actionGet();
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("properties")
.startObject("_source").field("enabled", false).endObject()
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("_source").field("enabled", false).endObject().startObject("properties")
.startObject("byte_field").field("type", "byte").field("store", "yes").endObject()
.startObject("short_field").field("type", "short").field("store", "yes").endObject()
.startObject("integer_field").field("type", "integer").field("store", "yes").endObject()
@ -556,8 +555,7 @@ public class SearchFieldsTests extends ESIntegTestCase {
createIndex("test");
client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForYellowStatus().execute().actionGet();
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("properties")
.startObject("_source").field("enabled", false).endObject()
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1").startObject("_source").field("enabled", false).endObject().startObject("properties")
.startObject("string_field").field("type", "string").endObject()
.startObject("byte_field").field("type", "byte").endObject()
.startObject("short_field").field("type", "short").endObject()