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

Move validation from FieldTypeLookup to MapperMergeValidator. (#39814) 

This commit consolidates more mapping validation logic into the same class.
`FieldTypeLookup` is now a bit simpler, and has the sole responsibility of quickly
resolving field names to their types.

I have a broader refactor planned around mapping merge validation, but this
change should at least be a step in the right direction.
This commit is contained in:
Julie Tibshirani 2019-03-08 15:58:32 -08:00
parent 993182e426
commit 8454cfc1b2
5 changed files with 189 additions and 179 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java
View File

@ -22,11 +22,9 @@ package org.elasticsearch.index.mapper;
import org.elasticsearch.common.collect.CopyOnWriteHashMap;
import org.elasticsearch.common.regex.Regex;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Set;
@ -71,7 +69,6 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
MappedFieldType fullNameFieldType = fullName.get(fieldType.name());
if (!Objects.equals(fieldType, fullNameFieldType)) {
validateField(fullNameFieldType, fieldType, aliases);
fullName = fullName.copyAndPut(fieldType.name(), fieldType);
}
}
@ -79,66 +76,12 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
for (FieldAliasMapper fieldAliasMapper : fieldAliasMappers) {
String aliasName = fieldAliasMapper.name();
String path = fieldAliasMapper.path();
validateAlias(aliasName, path, aliases, fullName);
aliases = aliases.copyAndPut(aliasName, path);
}
return new FieldTypeLookup(fullName, aliases);
}
/**
* Checks that the new field type is valid.
*/
private void validateField(MappedFieldType existingFieldType,
MappedFieldType newFieldType,
CopyOnWriteHashMap<String, String> aliasToConcreteName) {
String fieldName = newFieldType.name();
if (aliasToConcreteName.containsKey(fieldName)) {
throw new IllegalArgumentException("The name for field [" + fieldName + "] has already" +
" been used to define a field alias.");
}
if (existingFieldType != null) {
List<String> conflicts = new ArrayList<>();
existingFieldType.checkCompatibility(newFieldType, conflicts);
if (conflicts.isEmpty() == false) {
throw new IllegalArgumentException("Mapper for [" + fieldName +
"] conflicts with existing mapping:\n" + conflicts.toString());
}
}
}
/**
* Checks that the new field alias is valid.
*
* Note that this method assumes that new concrete fields have already been processed, so that it
* can verify that an alias refers to an existing concrete field.
*/
private void validateAlias(String aliasName,
String path,
CopyOnWriteHashMap<String, String> aliasToConcreteName,
CopyOnWriteHashMap<String, MappedFieldType> fullNameToFieldType) {
if (fullNameToFieldType.containsKey(aliasName)) {
throw new IllegalArgumentException("The name for field alias [" + aliasName + "] has already" +
" been used to define a concrete field.");
}
if (path.equals(aliasName)) {
throw new IllegalArgumentException("Invalid [path] value [" + path + "] for field alias [" +
aliasName + "]: an alias cannot refer to itself.");
}
if (aliasToConcreteName.containsKey(path)) {
throw new IllegalArgumentException("Invalid [path] value [" + path + "] for field alias [" +
aliasName + "]: an alias cannot refer to another alias.");
}
if (!fullNameToFieldType.containsKey(path)) {
throw new IllegalArgumentException("Invalid [path] value [" + path + "] for field alias [" +
aliasName + "]: an alias must refer to an existing field in the mappings.");
}
}
/** Returns the field for the given field */
public MappedFieldType get(String field) {

91
server/src/main/java/org/elasticsearch/index/mapper/MapperMergeValidator.java
View File

@ -19,13 +19,13 @@
package org.elasticsearch.index.mapper;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
/**
* A utility class that helps validate certain aspects of a mappings update.
@ -33,17 +33,18 @@ import java.util.stream.Stream;
class MapperMergeValidator {
/**
* Validates the overall structure of the mapping addition, including whether
* duplicate fields are present, and if the provided fields have already been
* defined with a different data type.
* Validates the new mapping addition, checking whether duplicate entries are present and if the
* provided fields are compatible with the mappings that are already defined.
*
* @param objectMappers The newly added object mappers.
* @param fieldMappers The newly added field mappers.
* @param fieldAliasMappers The newly added field alias mappers.
* @param fieldTypes Any existing field and field alias mappers, collected into a lookup structure.
*/
public static void validateMapperStructure(Collection<ObjectMapper> objectMappers,
Collection<FieldMapper> fieldMappers,
Collection<FieldAliasMapper> fieldAliasMappers) {
public static void validateNewMappers(Collection<ObjectMapper> objectMappers,
Collection<FieldMapper> fieldMappers,
Collection<FieldAliasMapper> fieldAliasMappers,
FieldTypeLookup fieldTypes) {
Set<String> objectFullNames = new HashSet<>();
for (ObjectMapper objectMapper : objectMappers) {
String fullPath = objectMapper.fullPath();
@ -53,17 +54,75 @@ class MapperMergeValidator {
}
Set<String> fieldNames = new HashSet<>();
Stream.concat(fieldMappers.stream(), fieldAliasMappers.stream())
.forEach(mapper -> {
String name = mapper.name();
if (objectFullNames.contains(name)) {
throw new IllegalArgumentException("Field [" + name + "] is defined both as an object and a field.");
} else if (fieldNames.add(name) == false) {
throw new IllegalArgumentException("Field [" + name + "] is defined twice.");
}
});
for (FieldMapper fieldMapper : fieldMappers) {
String name = fieldMapper.name();
if (objectFullNames.contains(name)) {
throw new IllegalArgumentException("Field [" + name + "] is defined both as an object and a field.");
} else if (fieldNames.add(name) == false) {
throw new IllegalArgumentException("Field [" + name + "] is defined twice.");
}
validateFieldMapper(fieldMapper, fieldTypes);
}
Set<String> fieldAliasNames = new HashSet<>();
for (FieldAliasMapper fieldAliasMapper : fieldAliasMappers) {
String name = fieldAliasMapper.name();
if (objectFullNames.contains(name)) {
throw new IllegalArgumentException("Field [" + name + "] is defined both as an object and a field.");
} else if (fieldNames.contains(name)) {
throw new IllegalArgumentException("Field [" + name + "] is defined both as an alias and a concrete field.");
} else if (fieldAliasNames.add(name) == false) {
throw new IllegalArgumentException("Field [" + name + "] is defined twice.");
}
validateFieldAliasMapper(name, fieldAliasMapper.path(), fieldNames, fieldAliasNames);
}
}
/**
* Checks that the new field mapper does not conflict with existing mappings.
*/
private static void validateFieldMapper(FieldMapper fieldMapper,
FieldTypeLookup fieldTypes) {
MappedFieldType newFieldType = fieldMapper.fieldType();
MappedFieldType existingFieldType = fieldTypes.get(newFieldType.name());
if (existingFieldType != null && Objects.equals(newFieldType, existingFieldType) == false) {
List<String> conflicts = new ArrayList<>();
existingFieldType.checkCompatibility(newFieldType, conflicts);
if (conflicts.isEmpty() == false) {
throw new IllegalArgumentException("Mapper for [" + newFieldType.name() +
"] conflicts with existing mapping:\n" + conflicts.toString());
}
}
}
/**
* Checks that the new field alias is valid.
*
* Note that this method assumes that new concrete fields have already been processed, so that it
* can verify that an alias refers to an existing concrete field.
*/
private static void validateFieldAliasMapper(String aliasName,
String path,
Set<String> fieldMappers,
Set<String> fieldAliasMappers) {
if (path.equals(aliasName)) {
throw new IllegalArgumentException("Invalid [path] value [" + path + "] for field alias [" +
aliasName + "]: an alias cannot refer to itself.");
}
if (fieldAliasMappers.contains(path)) {
throw new IllegalArgumentException("Invalid [path] value [" + path + "] for field alias [" +
aliasName + "]: an alias cannot refer to another alias.");
}
if (fieldMappers.contains(path) == false) {
throw new IllegalArgumentException("Invalid [path] value [" + path + "] for field alias [" +
aliasName + "]: an alias must refer to an existing field in the mappings.");
}
}
/**
* Verifies that each field reference, e.g. the value of copy_to or the target
* of a field alias, corresponds to a valid part of the mapping.

3
server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View File

@ -471,11 +471,10 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
Collections.addAll(fieldMappers, metadataMappers);
MapperUtils.collect(newMapper.mapping().root(), objectMappers, fieldMappers, fieldAliasMappers);
MapperMergeValidator.validateMapperStructure(objectMappers, fieldMappers, fieldAliasMappers);
MapperMergeValidator.validateNewMappers(objectMappers, fieldMappers, fieldAliasMappers, fieldTypes);
checkPartitionedIndexConstraints(newMapper);
// update lookup data-structures
// this will in particular make sure that the merged fields are compatible with other types
fieldTypes = fieldTypes.copyAndAddAll(newMapper.type(), fieldMappers, fieldAliasMappers);
for (ObjectMapper objectMapper : objectMappers) {

102
server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java
View File

@ -79,46 +79,6 @@ public class FieldTypeLookupTests extends ESTestCase {
assertEquals(f2.fieldType(), lookup2.get("foo"));
}
public void testMismatchedFieldTypes() {
FieldMapper f1 = new MockFieldMapper("foo");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll("type", newList(f1), emptyList());
OtherFakeFieldType ft2 = new OtherFakeFieldType();
ft2.setName("foo");
FieldMapper f2 = new MockFieldMapper("foo", ft2);
try {
lookup.copyAndAddAll("type2", newList(f2), emptyList());
fail("expected type mismatch");
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("cannot be changed from type [faketype] to [otherfaketype]"));
}
}
public void testConflictingFieldTypes() {
FieldMapper f1 = new MockFieldMapper("foo");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll("type", newList(f1), emptyList());
MappedFieldType ft2 = new MockFieldMapper.FakeFieldType();
ft2.setName("foo");
ft2.setBoost(2.0f);
FieldMapper f2 = new MockFieldMapper("foo", ft2);
lookup.copyAndAddAll("type", newList(f2), emptyList()); // boost is updateable, so ok since we are implicitly updating all types
lookup.copyAndAddAll("type2", newList(f2), emptyList()); // boost is updateable, so ok if forcing
// now with a non changeable setting
MappedFieldType ft3 = new MockFieldMapper.FakeFieldType();
ft3.setName("foo");
ft3.setStored(true);
FieldMapper f3 = new MockFieldMapper("foo", ft3);
try {
lookup.copyAndAddAll("type2", newList(f3), emptyList());
fail("expected conflict");
} catch (IllegalArgumentException e) {
assertTrue(e.getMessage().contains("has different [store] values"));
}
}
public void testAddFieldAlias() {
MockFieldMapper field = new MockFieldMapper("foo");
FieldAliasMapper alias = new FieldAliasMapper("alias", "alias", "foo");
@ -181,68 +141,6 @@ public class FieldTypeLookupTests extends ESTestCase {
assertEquals(fieldType2, aliasType2);
}
public void testAliasThatRefersToAlias() {
MockFieldMapper field = new MockFieldMapper("foo");
FieldAliasMapper alias = new FieldAliasMapper("alias", "alias", "foo");
FieldTypeLookup lookup = new FieldTypeLookup()
.copyAndAddAll("type", newList(field), newList(alias));
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid-alias", "invalid-alias", "alias");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> lookup.copyAndAddAll("type", emptyList(), newList(invalidAlias)));
assertEquals("Invalid [path] value [alias] for field alias [invalid-alias]: an alias" +
" cannot refer to another alias.", e.getMessage());
}
public void testAliasThatRefersToItself() {
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid-alias", "invalid-alias", "invalid-alias");
FieldTypeLookup lookup = new FieldTypeLookup();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> lookup.copyAndAddAll("type", emptyList(), newList(invalidAlias)));
assertEquals("Invalid [path] value [invalid-alias] for field alias [invalid-alias]: an alias" +
" cannot refer to itself.", e.getMessage());
}
public void testAliasWithNonExistentPath() {
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid-alias", "invalid-alias", "non-existent");
FieldTypeLookup lookup = new FieldTypeLookup();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> lookup.copyAndAddAll("type", emptyList(), newList(invalidAlias)));
assertEquals("Invalid [path] value [non-existent] for field alias [invalid-alias]: an alias" +
" must refer to an existing field in the mappings.", e.getMessage());
}
public void testAddAliasWithPreexistingField() {
MockFieldMapper field = new MockFieldMapper("field");
FieldTypeLookup lookup = new FieldTypeLookup()
.copyAndAddAll("type", newList(field), emptyList());
MockFieldMapper invalidField = new MockFieldMapper("invalid");
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid", "invalid", "field");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> lookup.copyAndAddAll("type", newList(invalidField), newList(invalidAlias)));
assertEquals("The name for field alias [invalid] has already been used to define a concrete field.",
e.getMessage());
}
public void testAddFieldWithPreexistingAlias() {
MockFieldMapper field = new MockFieldMapper("field");
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid", "invalid", "field");
FieldTypeLookup lookup = new FieldTypeLookup()
.copyAndAddAll("type", newList(field), newList(invalidAlias));
MockFieldMapper invalidField = new MockFieldMapper("invalid");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> lookup.copyAndAddAll("type", newList(invalidField), emptyList()));
assertEquals("The name for field [invalid] has already been used to define a field alias.",
e.getMessage());
}
public void testSimpleMatchToFullName() {
MockFieldMapper field1 = new MockFieldMapper("foo");
MockFieldMapper field2 = new MockFieldMapper("bar");

115
server/src/test/java/org/elasticsearch/index/mapper/MapperMergeValidatorTests.java
View File

@ -21,6 +21,7 @@ package org.elasticsearch.index.mapper;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESTestCase;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@ -31,18 +32,128 @@ import static java.util.Collections.singletonList;
public class MapperMergeValidatorTests extends ESTestCase {
public void testMismatchedFieldTypes() {
FieldMapper existingField = new MockFieldMapper("foo");
FieldTypeLookup lookup = new FieldTypeLookup()
.copyAndAddAll("type", singletonList(existingField), emptyList());
FieldTypeLookupTests.OtherFakeFieldType newFieldType = new FieldTypeLookupTests.OtherFakeFieldType();
newFieldType.setName("foo");
FieldMapper invalidField = new MockFieldMapper("foo", newFieldType);
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateNewMappers(
emptyList(),
singletonList(invalidField),
emptyList(),
lookup));
assertTrue(e.getMessage().contains("cannot be changed from type [faketype] to [otherfaketype]"));
}
public void testConflictingFieldTypes() {
FieldMapper existingField = new MockFieldMapper("foo");
FieldTypeLookup lookup = new FieldTypeLookup()
.copyAndAddAll("type", singletonList(existingField), emptyList());
MappedFieldType newFieldType = new MockFieldMapper.FakeFieldType();
newFieldType.setName("foo");
newFieldType.setBoost(2.0f);
FieldMapper validField = new MockFieldMapper("foo", newFieldType);
// Boost is updateable, so no exception should be thrown.
MapperMergeValidator.validateNewMappers(
emptyList(),
singletonList(validField),
emptyList(),
lookup);
MappedFieldType invalidFieldType = new MockFieldMapper.FakeFieldType();
invalidFieldType.setName("foo");
invalidFieldType.setStored(true);
FieldMapper invalidField = new MockFieldMapper("foo", invalidFieldType);
// Store is not updateable, so we expect an exception.
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateNewMappers(
emptyList(),
singletonList(invalidField),
emptyList(),
lookup));
assertTrue(e.getMessage().contains("has different [store] values"));
}
public void testDuplicateFieldAliasAndObject() {
ObjectMapper objectMapper = createObjectMapper("some.path");
FieldAliasMapper aliasMapper = new FieldAliasMapper("path", "some.path", "field");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateMapperStructure(
MapperMergeValidator.validateNewMappers(
singletonList(objectMapper),
emptyList(),
singletonList(aliasMapper)));
singletonList(aliasMapper),
new FieldTypeLookup()));
assertEquals("Field [some.path] is defined both as an object and a field.", e.getMessage());
}
public void testDuplicateFieldAliasAndConcreteField() {
FieldMapper field = new MockFieldMapper("field");
FieldMapper invalidField = new MockFieldMapper("invalid");
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid", "invalid", "field");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateNewMappers(
emptyList(),
Arrays.asList(field, invalidField),
singletonList(invalidAlias),
new FieldTypeLookup()));
assertEquals("Field [invalid] is defined both as an alias and a concrete field.", e.getMessage());
}
public void testAliasThatRefersToAlias() {
FieldMapper field = new MockFieldMapper("field");
FieldAliasMapper alias = new FieldAliasMapper("alias", "alias", "field");
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid-alias", "invalid-alias", "alias");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateNewMappers(
emptyList(),
singletonList(field),
Arrays.asList(alias, invalidAlias),
new FieldTypeLookup()));
assertEquals("Invalid [path] value [alias] for field alias [invalid-alias]: an alias" +
" cannot refer to another alias.", e.getMessage());
}
public void testAliasThatRefersToItself() {
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid-alias", "invalid-alias", "invalid-alias");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateNewMappers(
emptyList(),
emptyList(),
singletonList(invalidAlias),
new FieldTypeLookup()));
assertEquals("Invalid [path] value [invalid-alias] for field alias [invalid-alias]: an alias" +
" cannot refer to itself.", e.getMessage());
}
public void testAliasWithNonExistentPath() {
FieldAliasMapper invalidAlias = new FieldAliasMapper("invalid-alias", "invalid-alias", "non-existent");
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
MapperMergeValidator.validateNewMappers(
emptyList(),
emptyList(),
singletonList(invalidAlias),
new FieldTypeLookup()));
assertEquals("Invalid [path] value [non-existent] for field alias [invalid-alias]: an alias" +
" must refer to an existing field in the mappings.", e.getMessage());
}
public void testFieldAliasWithNestedScope() {
ObjectMapper objectMapper = createNestedObjectMapper("nested");
FieldAliasMapper aliasMapper = new FieldAliasMapper("alias", "nested.alias", "nested.field");