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

[Code] Control whether MapperService docMapper iterator should contain DEFAULT_MAPPING 

At the moment one can iterate the MapperService to go through all document mappers. This includes the document mapper of DEFAULT_MAPPING, which may be surprising and lead to unintended results. This commit removes the Iterable implementation and add a docMappers method that asks the caller to make an explicit choice

Closes #6793
This commit is contained in:
Boaz Leskes 2014-07-09 08:25:53 +02:00
parent b6baa4be4a
commit 684e698627
10 changed files with 47 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
View File

@ -372,7 +372,7 @@ public class MetaDataCreateIndexService extends AbstractComponent {
// now, update the mappings with the actual source
Map<String, MappingMetaData> mappingsMetaData = Maps.newHashMap();
for (DocumentMapper mapper : mapperService) {
for (DocumentMapper mapper : mapperService.docMappers(true)) {
MappingMetaData mappingMd = new MappingMetaData(mapper);
mappingsMetaData.put(mapper.type(), mappingMd);
}

2
src/main/java/org/elasticsearch/index/fielddata/plain/ParentChildIndexFieldData.java
View File

@ -73,7 +73,7 @@ public class ParentChildIndexFieldData extends AbstractIndexFieldData<ParentChil
parentTypes = new TreeSet<>(BytesRef.getUTF8SortedAsUnicodeComparator());
this.breakerService = breakerService;
this.globalOrdinalsBuilder = globalOrdinalsBuilder;
for (DocumentMapper documentMapper : mapperService) {
for (DocumentMapper documentMapper : mapperService.docMappers(false)) {
beforeCreate(documentMapper);
}
mapperService.addTypeListener(this);

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

@ -21,9 +21,9 @@ package org.elasticsearch.index.mapper;
import com.carrotsearch.hppc.ObjectOpenHashSet;
import com.google.common.base.Charsets;
import com.google.common.base.Predicate;
import com.google.common.collect.*;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.AnalyzerWrapper;
import org.apache.lucene.analysis.SimpleAnalyzerWrapper;
import org.apache.lucene.index.Term;
import org.apache.lucene.queries.FilterClause;
@ -75,7 +75,7 @@ import static org.elasticsearch.index.mapper.DocumentMapper.MergeFlags.mergeFlag
/**
*
*/
public class MapperService extends AbstractIndexComponent implements Iterable<DocumentMapper> {
public class MapperService extends AbstractIndexComponent {
public static final String DEFAULT_MAPPING = "_default_";
private static ObjectOpenHashSet<String> META_FIELDS = ObjectOpenHashSet.from(
@ -226,11 +226,34 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
return this.hasNested;
}
@Override
public UnmodifiableIterator<DocumentMapper> iterator() {
return Iterators.unmodifiableIterator(mappers.values().iterator());
/**
* returns an immutable iterator over current document mappers.
*
* @param includingDefaultMapping indicates whether the iterator should contain the {@link #DEFAULT_MAPPING} document mapper.
* As is this not really an active type, you would typically set this to false
*/
public Iterable<DocumentMapper> docMappers(final boolean includingDefaultMapping) {
return new Iterable<DocumentMapper>() {
@Override
public Iterator<DocumentMapper> iterator() {
final Iterator<DocumentMapper> iterator;
if (includingDefaultMapping) {
iterator = mappers.values().iterator();
} else {
iterator = Iterators.filter(mappers.values().iterator(), NOT_A_DEFAULT_DOC_MAPPER);
}
return Iterators.unmodifiableIterator(iterator);
}
};
}
private static final Predicate<DocumentMapper> NOT_A_DEFAULT_DOC_MAPPER = new Predicate<DocumentMapper>() {
@Override
public boolean apply(DocumentMapper input) {
return !DEFAULT_MAPPING.equals(input.type());
}
};
public AnalysisService analysisService() {
return this.analysisService;
}

4
src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java
View File

@ -263,7 +263,7 @@ public class ParentFieldMapper extends AbstractFieldMapper<Uid> implements Inter
}
List<String> types = new ArrayList<>(context.mapperService().types().size());
for (DocumentMapper documentMapper : context.mapperService()) {
for (DocumentMapper documentMapper : context.mapperService().docMappers(false)) {
if (!documentMapper.parentFieldMapper().active()) {
types.add(documentMapper.type());
}
@ -294,7 +294,7 @@ public class ParentFieldMapper extends AbstractFieldMapper<Uid> implements Inter
}
List<String> types = new ArrayList<>(context.mapperService().types().size());
for (DocumentMapper documentMapper : context.mapperService()) {
for (DocumentMapper documentMapper : context.mapperService().docMappers(false)) {
if (!documentMapper.parentFieldMapper().active()) {
types.add(documentMapper.type());
}

2
src/main/java/org/elasticsearch/index/query/HasParentFilterParser.java
View File

@ -132,7 +132,7 @@ public class HasParentFilterParser implements FilterParser {
Set<String> parentTypes = new HashSet<>(5);
parentTypes.add(parentType);
ParentChildIndexFieldData parentChildIndexFieldData = null;
for (DocumentMapper documentMapper : parseContext.mapperService()) {
for (DocumentMapper documentMapper : parseContext.mapperService().docMappers(false)) {
ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper();
if (parentFieldMapper.active()) {
DocumentMapper parentTypeDocumentMapper = parseContext.mapperService().documentMapper(parentFieldMapper.type());

2
src/main/java/org/elasticsearch/index/query/HasParentQueryParser.java
View File

@ -135,7 +135,7 @@ public class HasParentQueryParser implements QueryParser {
ParentChildIndexFieldData parentChildIndexFieldData = null;
Set<String> parentTypes = new HashSet<>(5);
parentTypes.add(parentType);
for (DocumentMapper documentMapper : parseContext.mapperService()) {
for (DocumentMapper documentMapper : parseContext.mapperService().docMappers(false)) {
ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper();
if (parentFieldMapper.active()) {
parentChildIndexFieldData = parseContext.getForField(parentFieldMapper);

2
src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java
View File

@ -374,7 +374,7 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent<Indic
}
}
// go over and remove mappings
for (DocumentMapper documentMapper : mapperService) {
for (DocumentMapper documentMapper : mapperService.docMappers(true)) {
if (seenMappings.containsKey(new Tuple<>(index, documentMapper.type())) && !indexMetaData.mappings().containsKey(documentMapper.type())) {
// we have it in our mappings, but not in the metadata, and we have seen it in the cluster state, remove it
mapperService.remove(documentMapper.type());

10
src/main/java/org/elasticsearch/indices/recovery/RecoverySource.java
View File

@ -43,7 +43,6 @@ import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.deletionpolicy.SnapshotIndexCommit;
import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.service.IndexService;
import org.elasticsearch.index.shard.IllegalIndexShardStateException;
import org.elasticsearch.index.shard.IndexShardClosedException;
@ -289,12 +288,9 @@ public class RecoverySource extends AbstractComponent {
metaDataMappings = indexMetaData.getMappings();
}
List<DocumentMapper> documentMappersToUpdate = Lists.newArrayList();
for (DocumentMapper documentMapper : indexService.mapperService()) {
// default mapping should not be sent back, it can only be updated by put mapping API, and its
// a full in place replace, we don't want to override a potential update coming it
if (documentMapper.type().equals(MapperService.DEFAULT_MAPPING)) {
continue;
}
// default mapping should not be sent back, it can only be updated by put mapping API, and its
// a full in place replace, we don't want to override a potential update coming it
for (DocumentMapper documentMapper : indexService.mapperService().docMappers(false)) {
MappingMetaData mappingMetaData = metaDataMappings == null ? null : metaDataMappings.get(documentMapper.type());
if (mappingMetaData == null || !documentMapper.refreshSource().equals(mappingMetaData.source())) {

11
src/main/java/org/elasticsearch/search/SearchService.java
View File

@ -69,8 +69,11 @@ import org.elasticsearch.search.dfs.CachedDfSource;
import org.elasticsearch.search.dfs.DfsPhase;
import org.elasticsearch.search.dfs.DfsSearchResult;
import org.elasticsearch.search.fetch.*;
import org.elasticsearch.search.internal.*;
import org.elasticsearch.search.internal.DefaultSearchContext;
import org.elasticsearch.search.internal.InternalScrollSearchRequest;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.internal.SearchContext.Lifetime;
import org.elasticsearch.search.internal.ShardSearchRequest;
import org.elasticsearch.search.query.*;
import org.elasticsearch.search.warmer.IndexWarmersMetaData;
import org.elasticsearch.threadpool.ThreadPool;
@ -721,7 +724,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
final Loading defaultLoading = Loading.parse(indexMetaData.settings().get(NORMS_LOADING_KEY), Loading.LAZY);
final MapperService mapperService = indexShard.mapperService();
final ObjectSet<String> warmUp = new ObjectOpenHashSet<>();
for (DocumentMapper docMapper : mapperService) {
for (DocumentMapper docMapper : mapperService.docMappers(false)) {
for (FieldMapper<?> fieldMapper : docMapper.mappers().mappers()) {
final String indexName = fieldMapper.names().indexName();
if (fieldMapper.fieldType().indexed() && !fieldMapper.fieldType().omitNorms() && fieldMapper.normsLoading(defaultLoading) == Loading.EAGER) {
@ -772,7 +775,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
public TerminationHandle warm(final IndexShard indexShard, IndexMetaData indexMetaData, final WarmerContext context, ThreadPool threadPool) {
final MapperService mapperService = indexShard.mapperService();
final Map<String, FieldMapper<?>> warmUp = new HashMap<>();
for (DocumentMapper docMapper : mapperService) {
for (DocumentMapper docMapper : mapperService.docMappers(false)) {
for (FieldMapper<?> fieldMapper : docMapper.mappers().mappers()) {
final FieldDataType fieldDataType = fieldMapper.fieldDataType();
if (fieldDataType == null) {
@ -826,7 +829,7 @@ public class SearchService extends AbstractLifecycleComponent<SearchService> {
public TerminationHandle warmTop(final IndexShard indexShard, IndexMetaData indexMetaData, final WarmerContext context, ThreadPool threadPool) {
final MapperService mapperService = indexShard.mapperService();
final Map<String, FieldMapper<?>> warmUpGlobalOrdinals = new HashMap<>();
for (DocumentMapper docMapper : mapperService) {
for (DocumentMapper docMapper : mapperService.docMappers(false)) {
for (FieldMapper<?> fieldMapper : docMapper.mappers().mappers()) {
final FieldDataType fieldDataType = fieldMapper.fieldDataType();
if (fieldDataType == null) {

4
src/test/java/org/elasticsearch/indices/leaks/IndicesLeaksTests.java
View File

@ -34,7 +34,7 @@ import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.List;
import static org.elasticsearch.test.ElasticsearchIntegrationTest.*;
import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
import static org.hamcrest.Matchers.nullValue;
/**
@ -72,7 +72,7 @@ public class IndicesLeaksTests extends ElasticsearchIntegrationTest {
indexReferences.add(new WeakReference(indexService));
indexReferences.add(new WeakReference(indexInjector));
indexReferences.add(new WeakReference(indexService.mapperService()));
for (DocumentMapper documentMapper : indexService.mapperService()) {
for (DocumentMapper documentMapper : indexService.mapperService().docMappers(true)) {
indexReferences.add(new WeakReference(documentMapper));
}
indexReferences.add(new WeakReference(indexService.aliasesService()));