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

Don't treat _default_ as a regular type. 

This adds safety that you can't index into the `_default_` type (it was possible
before), and can't add default mappers to the field type lookups (was not
happening in tests but I think this is still a good check).

Also MapperService.types() now excludes `_default` so that eg. the `ids` query
does not try to search on this type anymore.
This commit is contained in:
Adrien Grand 2015-12-01 18:03:11 +01:00
parent 77095a3e97
commit 304695e7ee
6 changed files with 111 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View File

@ -351,7 +351,7 @@ public class DocumentMapper implements ToXContent {
this.fieldMappers = this.fieldMappers.copyAndAllAll(fieldMappers);
// finally update for the entire index
mapperService.addMappers(objectMappers, fieldMappers);
mapperService.addMappers(type, objectMappers, fieldMappers);
}
public MergeResult merge(Mapping mapping, boolean simulate, boolean updateAllTypes) {

4
core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View File

@ -79,6 +79,10 @@ class DocumentParser implements Closeable {
}
private ParsedDocument innerParseDocument(SourceToParse source) throws MapperParsingException {
if (docMapper.type().equals(MapperService.DEFAULT_MAPPING)) {
throw new IllegalArgumentException("It is forbidden to index into the default mapping [" + MapperService.DEFAULT_MAPPING + "]");
}
ParseContext.InternalParseContext context = cache.get();
final Mapping mapping = docMapper.mapping();

7
core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java
View File

@ -27,6 +27,7 @@ import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import java.util.Set;
/**
@ -56,7 +57,11 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
* from the provided fields. If a field already exists, the field type will be updated
* to use the new mappers field type.
*/
public FieldTypeLookup copyAndAddAll(Collection<FieldMapper> newFieldMappers) {
public FieldTypeLookup copyAndAddAll(String type, Collection<FieldMapper> newFieldMappers) {
Objects.requireNonNull(type, "type must not be null");
if (MapperService.DEFAULT_MAPPING.equals(type)) {
throw new IllegalArgumentException("Default mappings should not be added to the lookup");
}
CopyOnWriteHashMap<String, MappedFieldTypeReference> fullName = this.fullNameToFieldType;
CopyOnWriteHashMap<String, MappedFieldTypeReference> indexName = this.indexNameToFieldType;

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

@ -267,7 +267,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
MapperUtils.collect(mapper.mapping().root, newObjectMappers, newFieldMappers);
checkNewMappersCompatibility(newObjectMappers, newFieldMappers, updateAllTypes);
addMappers(newObjectMappers, newFieldMappers);
addMappers(mapper.type(), newObjectMappers, newFieldMappers);
for (DocumentTypeListener typeListener : typeListeners) {
typeListener.beforeCreate(mapper);
@ -318,7 +318,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
fieldTypes.checkCompatibility(newFieldMappers, updateAllTypes);
}
protected void addMappers(Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
protected void addMappers(String type, Collection<ObjectMapper> objectMappers, Collection<FieldMapper> fieldMappers) {
assert mappingLock.isWriteLockedByCurrentThread();
ImmutableOpenMap.Builder<String, ObjectMapper> fullPathObjectMappers = ImmutableOpenMap.builder(this.fullPathObjectMappers);
for (ObjectMapper objectMapper : objectMappers) {
@ -328,7 +328,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
}
this.fullPathObjectMappers = fullPathObjectMappers.build();
this.fieldTypes = this.fieldTypes.copyAndAddAll(fieldMappers);
this.fieldTypes = this.fieldTypes.copyAndAddAll(type, fieldMappers);
}
public DocumentMapper parse(String mappingType, CompressedXContent mappingSource, boolean applyDefault) throws MapperParsingException {
@ -345,10 +345,21 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
return mappers.containsKey(mappingType);
}
/**
* Return the set of concrete types that have a mapping.
* NOTE: this does not return the default mapping.
*/
public Collection<String> types() {
return mappers.keySet();
final Set<String> types = new HashSet<>(mappers.keySet());
types.remove(DEFAULT_MAPPING);
return Collections.unmodifiableSet(types);
}
/**
* Return the {@link DocumentMapper} for the given type. By using the special
* {@value #DEFAULT_MAPPING} type, you can get a {@link DocumentMapper} for
* the default mapping.
*/
public DocumentMapper documentMapper(String type) {
return mappers.get(type);
}

41
core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java
View File

@ -27,6 +27,7 @@ import org.elasticsearch.test.ESTestCase;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
@ -47,10 +48,20 @@ public class FieldTypeLookupTests extends ESTestCase {
assertFalse(itr.hasNext());
}
public void testDefaultMapping() {
FieldTypeLookup lookup = new FieldTypeLookup();
try {
lookup.copyAndAddAll(MapperService.DEFAULT_MAPPING, Collections.emptyList());
fail();
} catch (IllegalArgumentException expected) {
assertEquals("Default mappings should not be added to the lookup", expected.getMessage());
}
}
public void testAddNewField() {
FieldTypeLookup lookup = new FieldTypeLookup();
FakeFieldMapper f = new FakeFieldMapper("foo", "bar");
FieldTypeLookup lookup2 = lookup.copyAndAddAll(newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll("type", newList(f));
assertNull(lookup.get("foo"));
assertNull(lookup.get("bar"));
assertNull(lookup.getByIndexName("foo"));
@ -67,8 +78,8 @@ public class FieldTypeLookupTests extends ESTestCase {
MappedFieldType originalFieldType = f.fieldType();
FakeFieldMapper f2 = new FakeFieldMapper("foo", "foo");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll(newList(f2));
lookup = lookup.copyAndAddAll("type1", newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll("type2", newList(f2));
assertNotSame(originalFieldType, f.fieldType());
assertSame(f.fieldType(), f2.fieldType());
@ -82,8 +93,8 @@ public class FieldTypeLookupTests extends ESTestCase {
FakeFieldMapper f2 = new FakeFieldMapper("bar", "foo");
MappedFieldType originalFieldType = f.fieldType();
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll(newList(f2));
lookup = lookup.copyAndAddAll("type1", newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll("type2", newList(f2));
assertNotSame(originalFieldType, f.fieldType());
assertSame(f.fieldType(), f2.fieldType());
@ -98,8 +109,8 @@ public class FieldTypeLookupTests extends ESTestCase {
FakeFieldMapper f2 = new FakeFieldMapper("foo", "bar");
MappedFieldType originalFieldType = f.fieldType();
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll(newList(f2));
lookup = lookup.copyAndAddAll("type1", newList(f));
FieldTypeLookup lookup2 = lookup.copyAndAddAll("type2", newList(f2));
assertNotSame(originalFieldType, f.fieldType());
assertSame(f.fieldType(), f2.fieldType());
@ -113,18 +124,18 @@ public class FieldTypeLookupTests extends ESTestCase {
FakeFieldMapper f = new FakeFieldMapper("foo", "foo");
FakeFieldMapper f2 = new FakeFieldMapper("bar", "bar");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f, f2));
lookup = lookup.copyAndAddAll("type1", newList(f, f2));
try {
FakeFieldMapper f3 = new FakeFieldMapper("foo", "bar");
lookup.copyAndAddAll(newList(f3));
lookup.copyAndAddAll("type2", newList(f3));
} catch (IllegalStateException e) {
assertTrue(e.getMessage().contains("insane mappings"));
}
try {
FakeFieldMapper f3 = new FakeFieldMapper("bar", "foo");
lookup.copyAndAddAll(newList(f3));
lookup.copyAndAddAll("type2", newList(f3));
} catch (IllegalStateException e) {
assertTrue(e.getMessage().contains("insane mappings"));
}
@ -139,7 +150,7 @@ public class FieldTypeLookupTests extends ESTestCase {
public void testCheckCompatibilityMismatchedTypes() {
FieldMapper f1 = new FakeFieldMapper("foo", "bar");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f1));
lookup = lookup.copyAndAddAll("type", newList(f1));
MappedFieldType ft2 = FakeFieldMapper.makeOtherFieldType("foo", "foo");
FieldMapper f2 = new FakeFieldMapper("foo", ft2);
@ -161,7 +172,7 @@ public class FieldTypeLookupTests extends ESTestCase {
public void testCheckCompatibilityConflict() {
FieldMapper f1 = new FakeFieldMapper("foo", "bar");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f1));
lookup = lookup.copyAndAddAll("type", newList(f1));
MappedFieldType ft2 = FakeFieldMapper.makeFieldType("foo", "bar");
ft2.setBoost(2.0f);
@ -196,7 +207,7 @@ public class FieldTypeLookupTests extends ESTestCase {
FakeFieldMapper f1 = new FakeFieldMapper("foo", "baz");
FakeFieldMapper f2 = new FakeFieldMapper("bar", "boo");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f1, f2));
lookup = lookup.copyAndAddAll("type", newList(f1, f2));
Collection<String> names = lookup.simpleMatchToIndexNames("b*");
assertTrue(names.contains("baz"));
assertTrue(names.contains("boo"));
@ -206,7 +217,7 @@ public class FieldTypeLookupTests extends ESTestCase {
FakeFieldMapper f1 = new FakeFieldMapper("foo", "baz");
FakeFieldMapper f2 = new FakeFieldMapper("bar", "boo");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f1, f2));
lookup = lookup.copyAndAddAll("type", newList(f1, f2));
Collection<String> names = lookup.simpleMatchToFullName("b*");
assertTrue(names.contains("foo"));
assertTrue(names.contains("bar"));
@ -215,7 +226,7 @@ public class FieldTypeLookupTests extends ESTestCase {
public void testIteratorImmutable() {
FakeFieldMapper f1 = new FakeFieldMapper("foo", "bar");
FieldTypeLookup lookup = new FieldTypeLookup();
lookup = lookup.copyAndAddAll(newList(f1));
lookup = lookup.copyAndAddAll("type", newList(f1));
try {
Iterator<MappedFieldType> itr = lookup.iterator();

59
core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java
View File

@ -21,6 +21,8 @@ package org.elasticsearch.index.mapper;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.junit.Rule;
import org.junit.rules.ExpectedException;
@ -31,6 +33,11 @@ import static org.elasticsearch.test.VersionUtils.randomVersionBetween;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.hasToString;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.concurrent.ExecutionException;
public class MapperServiceTests extends ESSingleNodeTestCase {
@Rule
public ExpectedException expectedException = ExpectedException.none();
@ -82,4 +89,56 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
.execute()
.actionGet();
}
public void testTypes() throws Exception {
IndexService indexService1 = createIndex("index1");
MapperService mapperService = indexService1.mapperService();
assertEquals(Collections.emptySet(), mapperService.types());
mapperService.merge("type1", new CompressedXContent("{\"type1\":{}}"), true, false);
assertNull(mapperService.documentMapper(MapperService.DEFAULT_MAPPING));
assertEquals(Collections.singleton("type1"), mapperService.types());
mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedXContent("{\"_default_\":{}}"), true, false);
assertNotNull(mapperService.documentMapper(MapperService.DEFAULT_MAPPING));
assertEquals(Collections.singleton("type1"), mapperService.types());
mapperService.merge("type2", new CompressedXContent("{\"type2\":{}}"), true, false);
assertNotNull(mapperService.documentMapper(MapperService.DEFAULT_MAPPING));
assertEquals(new HashSet<>(Arrays.asList("type1", "type2")), mapperService.types());
}
public void testIndexIntoDefaultMapping() throws Throwable {
// 1. test implicit index creation
try {
client().prepareIndex("index1", MapperService.DEFAULT_MAPPING, "1").setSource("{").execute().get();
fail();
} catch (Throwable t) {
if (t instanceof ExecutionException) {
t = ((ExecutionException) t).getCause();
}
if (t instanceof IllegalArgumentException) {
assertEquals("It is forbidden to index into the default mapping [_default_]", t.getMessage());
} else {
throw t;
}
}
// 2. already existing index
IndexService indexService = createIndex("index2");
try {
client().prepareIndex("index2", MapperService.DEFAULT_MAPPING, "2").setSource().execute().get();
fail();
} catch (Throwable t) {
if (t instanceof ExecutionException) {
t = ((ExecutionException) t).getCause();
}
if (t instanceof IllegalArgumentException) {
assertEquals("It is forbidden to index into the default mapping [_default_]", t.getMessage());
} else {
throw t;
}
}
assertFalse(indexService.mapperService().hasMapping(MapperService.DEFAULT_MAPPING));
}
}