Mappings: Remove traverse functions from Mapper

The mapper listener abstractions for object and field mappers are used
to notify the mapper service of new fields, as well as collect
all object and field mappers through a set of traversal functions.

This change removes the traversal functions in favor of simple
iteration over subfields of a mapper.
This commit is contained in:
Ryan Ernst 2015-05-06 23:07:07 -07:00
parent 82c21ff5b3
commit 0b31efb328
11 changed files with 106 additions and 151 deletions

View File

@ -19,10 +19,10 @@
package org.elasticsearch.index.mapper;
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import org.apache.lucene.document.Field;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.DocIdSet;
@ -207,27 +207,24 @@ public class DocumentMapper implements ToXContent {
rootMapper(RoutingFieldMapper.class).markAsRequired();
}
FieldMapperListener.Aggregator fieldMappersAgg = new FieldMapperListener.Aggregator();
for (RootMapper rootMapper : this.mapping.rootMappers) {
// collect all the mappers for this type
List<ObjectMapper> newObjectMappers = new ArrayList<>();
List<FieldMapper<?>> newFieldMappers = new ArrayList<>();
for (RootMapper rootMapper : this.mapping.rootMappersNotIncludedInObject) {
if (rootMapper instanceof FieldMapper) {
fieldMappersAgg.mappers.add((FieldMapper) rootMapper);
newFieldMappers.add((FieldMapper) rootMapper);
}
}
MapperUtils.collect(this.mapping.root, newObjectMappers, newFieldMappers);
// now traverse and get all the statically defined ones
rootObjectMapper.traverse(fieldMappersAgg);
this.fieldMappers = new DocumentFieldMappers(docMapperParser.analysisService).copyAndAllAll(fieldMappersAgg.mappers);
final Map<String, ObjectMapper> objectMappers = Maps.newHashMap();
rootObjectMapper.traverse(new ObjectMapperListener() {
this.fieldMappers = new DocumentFieldMappers(docMapperParser.analysisService).copyAndAllAll(newFieldMappers);
this.objectMappers = Maps.uniqueIndex(newObjectMappers, new Function<ObjectMapper, String>() {
@Override
public void objectMapper(ObjectMapper objectMapper) {
objectMappers.put(objectMapper.fullPath(), objectMapper);
public String apply(ObjectMapper mapper) {
return mapper.fullPath();
}
});
this.objectMappers = ImmutableMap.copyOf(objectMappers);
for (ObjectMapper objectMapper : objectMappers.values()) {
for (ObjectMapper objectMapper : newObjectMappers) {
if (objectMapper.nested().isNested()) {
hasNestedObjects = true;
}
@ -426,20 +423,7 @@ public class DocumentMapper implements ToXContent {
fieldMapperListeners.add(fieldMapperListener);
}
public void traverse(FieldMapperListener listener) {
for (RootMapper rootMapper : mapping.rootMappers) {
if (!rootMapper.includeInObject() && rootMapper instanceof FieldMapper) {
listener.fieldMapper((FieldMapper) rootMapper);
}
}
mapping.root.traverse(listener);
}
public void addObjectMappers(Collection<ObjectMapper> objectMappers) {
addObjectMappers(objectMappers.toArray(new ObjectMapper[objectMappers.size()]));
}
private void addObjectMappers(ObjectMapper... objectMappers) {
private void addObjectMappers(Collection<ObjectMapper> objectMappers) {
synchronized (mappersMutex) {
MapBuilder<String, ObjectMapper> builder = MapBuilder.newMapBuilder(this.objectMappers);
for (ObjectMapper objectMapper : objectMappers) {
@ -459,10 +443,6 @@ public class DocumentMapper implements ToXContent {
objectMapperListeners.add(objectMapperListener);
}
public void traverse(ObjectMapperListener listener) {
mapping.root.traverse(listener);
}
private MergeResult newMergeContext(boolean simulate) {
return new MergeResult(simulate) {

View File

@ -20,7 +20,6 @@
package org.elasticsearch.index.mapper;
import com.google.common.collect.ImmutableMap;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
@ -29,17 +28,13 @@ import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.index.analysis.AnalysisService;
import org.elasticsearch.index.similarity.SimilarityLookupService;
import java.io.IOException;
import java.util.Map;
/**
*
*/
public interface Mapper extends ToXContent {
public interface Mapper extends ToXContent, Iterable<Mapper> {
public static final Mapper[] EMPTY_ARRAY = new Mapper[0];
Mapper[] EMPTY_ARRAY = new Mapper[0];
public static class BuilderContext {
class BuilderContext {
private final Settings indexSettings;
private final ContentPath contentPath;
@ -66,7 +61,7 @@ public interface Mapper extends ToXContent {
}
}
public static abstract class Builder<T extends Builder, Y extends Mapper> {
abstract class Builder<T extends Builder, Y extends Mapper> {
public String name;
@ -83,9 +78,9 @@ public interface Mapper extends ToXContent {
public abstract Y build(BuilderContext context);
}
public interface TypeParser {
interface TypeParser {
public static class ParserContext {
class ParserContext {
private final AnalysisService analysisService;
@ -127,9 +122,5 @@ public interface Mapper extends ToXContent {
void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException;
void traverse(FieldMapperListener fieldMapperListener);
void traverse(ObjectMapperListener objectMapperListener);
void close();
}

View File

@ -61,6 +61,7 @@ import org.elasticsearch.percolator.PercolatorService;
import org.elasticsearch.script.ScriptService;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@ -265,14 +266,17 @@ public class MapperService extends AbstractIndexComponent {
fieldDataService.onMappingUpdate();
return oldMapper;
} else {
FieldMapperListener.Aggregator fieldMappersAgg = new FieldMapperListener.Aggregator();
mapper.traverse(fieldMappersAgg);
addFieldMappers(fieldMappersAgg.mappers);
List<ObjectMapper> newObjectMappers = new ArrayList<>();
List<FieldMapper<?>> newFieldMappers = new ArrayList<>();
for (RootMapper rootMapper : mapper.mapping().rootMappers) {
if (!rootMapper.includeInObject() && rootMapper instanceof FieldMapper) {
newFieldMappers.add((FieldMapper<?>) rootMapper);
}
}
MapperUtils.collect(mapper.mapping().root, newObjectMappers, newFieldMappers);
addFieldMappers(newFieldMappers);
mapper.addFieldMapperListener(fieldMapperListener);
ObjectMapperListener.Aggregator objectMappersAgg = new ObjectMapperListener.Aggregator();
mapper.traverse(objectMappersAgg);
addObjectMappers(objectMappersAgg.mappers.toArray(new ObjectMapper[objectMappersAgg.mappers.size()]));
addObjectMappers(newObjectMappers);
mapper.addObjectMapperListener(objectMapperListener);
for (DocumentTypeListener typeListener : typeListeners) {
@ -284,7 +288,7 @@ public class MapperService extends AbstractIndexComponent {
}
}
private void addObjectMappers(ObjectMapper[] objectMappers) {
private void addObjectMappers(Collection<ObjectMapper> objectMappers) {
synchronized (mappersMutex) {
ImmutableOpenMap.Builder<String, ObjectMappers> fullPathObjectMappers = ImmutableOpenMap.builder(this.fullPathObjectMappers);
for (ObjectMapper objectMapper : objectMappers) {
@ -860,11 +864,11 @@ public class MapperService extends AbstractIndexComponent {
class InternalObjectMapperListener extends ObjectMapperListener {
@Override
public void objectMapper(ObjectMapper objectMapper) {
addObjectMappers(new ObjectMapper[]{objectMapper});
addObjectMappers(Collections.singletonList(objectMapper));
}
@Override
public void objectMappers(ObjectMapper... objectMappers) {
public void objectMappers(Collection<ObjectMapper> objectMappers) {
addObjectMappers(objectMappers);
}
}

View File

@ -21,6 +21,7 @@ package org.elasticsearch.index.mapper;
import org.elasticsearch.common.Strings;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.mapper.object.RootObjectMapper;
import java.io.IOException;
import java.util.Collection;
@ -28,8 +29,6 @@ import java.util.Collection;
public enum MapperUtils {
;
private static MergeResult newStrictMergeContext() {
return new MergeResult(false) {
@ -76,4 +75,17 @@ public enum MapperUtils {
mergeInto.merge(mergeWith, newStrictMergeContext());
}
/** Split mapper and its descendants into object and field mappers. */
public static void collect(Mapper mapper, Collection<ObjectMapper> objectMappers, Collection<FieldMapper<?>> fieldMappers) {
if (mapper instanceof RootObjectMapper) {
// root mapper isn't really an object mapper
} else if (mapper instanceof ObjectMapper) {
objectMappers.add((ObjectMapper)mapper);
} else if (mapper instanceof FieldMapper<?>) {
fieldMappers.add((FieldMapper<?>)mapper);
}
for (Mapper child : mapper) {
collect(child, objectMappers, fieldMappers);
}
}
}

View File

@ -22,11 +22,9 @@ package org.elasticsearch.index.mapper;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
/**
*
*/
public abstract class ObjectMapperListener {
public static class Aggregator extends ObjectMapperListener {
@ -40,7 +38,7 @@ public abstract class ObjectMapperListener {
public abstract void objectMapper(ObjectMapper objectMapper);
public void objectMappers(ObjectMapper... objectMappers) {
public void objectMappers(Collection<ObjectMapper> objectMappers) {
for (ObjectMapper objectMapper : objectMappers) {
objectMapper(objectMapper);
}

View File

@ -22,14 +22,16 @@ package org.elasticsearch.index.mapper.core;
import com.carrotsearch.hppc.ObjectOpenHashSet;
import com.carrotsearch.hppc.cursors.ObjectCursor;
import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
import com.google.common.base.Function;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterators;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.index.Terms;
import org.apache.lucene.queries.TermsQuery;
import org.apache.lucene.search.Filter;
import org.apache.lucene.search.FuzzyQuery;
@ -40,7 +42,6 @@ import org.apache.lucene.search.QueryWrapperFilter;
import org.apache.lucene.search.RegexpQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.TermRangeQuery;
import org.apache.lucene.index.Terms;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.Version;
import org.elasticsearch.action.fieldstats.FieldStats;
@ -55,11 +56,14 @@ import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.fielddata.FieldDataType;
import org.elasticsearch.index.mapper.*;
import org.elasticsearch.index.mapper.ParseContext.Document;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.MergeMappingException;
import org.elasticsearch.index.mapper.MergeResult;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.internal.AllFieldMapper;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.mapper.object.RootObjectMapper;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.index.search.FieldDataTermsFilter;
import org.elasticsearch.index.similarity.SimilarityLookupService;
@ -68,7 +72,9 @@ import org.elasticsearch.index.similarity.SimilarityProvider;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.TreeMap;
@ -444,15 +450,11 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
return false;
}
@Override
public void traverse(FieldMapperListener fieldMapperListener) {
fieldMapperListener.fieldMapper(this);
multiFields.traverse(fieldMapperListener);
public Iterator<Mapper> iterator() {
if (multiFields == null) {
return Collections.emptyIterator();
}
@Override
public void traverse(ObjectMapperListener objectMapperListener) {
// nothing to do here...
return multiFields.iterator();
}
@Override
@ -955,10 +957,13 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
}
}
public void traverse(FieldMapperListener fieldMapperListener) {
for (ObjectCursor<FieldMapper> cursor : mappers.values()) {
cursor.value.traverse(fieldMapperListener);
public Iterator<Mapper> iterator() {
return Iterators.transform(mappers.values().iterator(), new Function<ObjectCursor<FieldMapper>, Mapper>() {
@Override
public Mapper apply(@Nullable ObjectCursor<FieldMapper> cursor) {
return cursor.value;
}
});
}
public void close() {

View File

@ -22,7 +22,7 @@ package org.elasticsearch.index.mapper.geo;
import com.carrotsearch.hppc.ObjectOpenHashSet;
import com.carrotsearch.hppc.cursors.ObjectCursor;
import com.google.common.base.Objects;
import com.google.common.collect.Iterators;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.DocValuesType;
@ -45,12 +45,10 @@ import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.fielddata.FieldDataType;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.FieldMapperListener;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.MergeResult;
import org.elasticsearch.index.mapper.MergeMappingException;
import org.elasticsearch.index.mapper.ObjectMapperListener;
import org.elasticsearch.index.mapper.MergeResult;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.core.AbstractFieldMapper;
import org.elasticsearch.index.mapper.core.DoubleFieldMapper;
@ -61,6 +59,7 @@ import org.elasticsearch.index.mapper.object.ArrayValueMapperParser;
import org.elasticsearch.index.similarity.SimilarityProvider;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
@ -678,19 +677,16 @@ public class GeoPointFieldMapper extends AbstractFieldMapper<GeoPoint> implement
}
@Override
public void traverse(FieldMapperListener fieldMapperListener) {
super.traverse(fieldMapperListener);
public Iterator<Mapper> iterator() {
List<Mapper> extras = new ArrayList<>();
if (enableGeoHash) {
geohashMapper.traverse(fieldMapperListener);
extras.add(geohashMapper);
}
if (enableLatLon) {
latMapper.traverse(fieldMapperListener);
lonMapper.traverse(fieldMapperListener);
extras.add(latMapper);
extras.add(lonMapper);
}
}
@Override
public void traverse(ObjectMapperListener objectMapperListener) {
return Iterators.concat(super.iterator(), extras.iterator());
}
@Override

View File

@ -36,10 +36,12 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentMapperParser;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.FieldMapperListener;
import org.elasticsearch.index.mapper.InternalMapper;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.MapperUtils;
import org.elasticsearch.index.mapper.MergeMappingException;
import org.elasticsearch.index.mapper.MergeResult;
import org.elasticsearch.index.mapper.ObjectMapperListener;
@ -468,18 +470,8 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll, Clonea
}
@Override
public void traverse(FieldMapperListener fieldMapperListener) {
for (Mapper mapper : mappers.values()) {
mapper.traverse(fieldMapperListener);
}
}
@Override
public void traverse(ObjectMapperListener objectMapperListener) {
objectMapperListener.objectMapper(this);
for (Mapper mapper : mappers.values()) {
mapper.traverse(objectMapperListener);
}
public Iterator<Mapper> iterator() {
return mappers.values().iterator();
}
public String fullPath() {
@ -523,27 +515,26 @@ public class ObjectMapper implements Mapper, AllFieldMapper.IncludeInAll, Clonea
doMerge(mergeWithObject, mergeResult);
List<Mapper> mappersToPut = new ArrayList<>();
FieldMapperListener.Aggregator newFieldMappers = new FieldMapperListener.Aggregator();
ObjectMapperListener.Aggregator newObjectMappers = new ObjectMapperListener.Aggregator();
for (Mapper mapper : mergeWithObject.mappers.values()) {
List<ObjectMapper> newObjectMappers = new ArrayList<>();
List<FieldMapper<?>> newFieldMappers = new ArrayList<>();
for (Mapper mapper : mergeWithObject) {
Mapper mergeWithMapper = mapper;
Mapper mergeIntoMapper = mappers.get(mergeWithMapper.name());
if (mergeIntoMapper == null) {
// no mapping, simply add it if not simulating
if (!mergeResult.simulate()) {
mappersToPut.add(mergeWithMapper);
mergeWithMapper.traverse(newFieldMappers);
mergeWithMapper.traverse(newObjectMappers);
MapperUtils.collect(mergeWithMapper, newObjectMappers, newFieldMappers);
}
} else {
mergeIntoMapper.merge(mergeWithMapper, mergeResult);
}
}
if (!newFieldMappers.mappers.isEmpty()) {
mergeResult.addFieldMappers(newFieldMappers.mappers);
if (!newFieldMappers.isEmpty()) {
mergeResult.addFieldMappers(newFieldMappers);
}
if (!newObjectMappers.mappers.isEmpty()) {
mergeResult.addObjectMappers(newObjectMappers.mappers);
if (!newObjectMappers.isEmpty()) {
mergeResult.addObjectMappers(newObjectMappers);
}
// add the mappers only after the administration have been done, so it will not be visible to parser (which first try to read with no lock)
for (Mapper mapper : mappersToPut) {

View File

@ -89,17 +89,9 @@ public class BooleanFieldMapperTests extends ElasticsearchSingleNodeTest {
.endObject().endObject().string();
DocumentMapper defaultMapper = parser.parse(mapping);
final FieldMapper<?>[] mapper = new BooleanFieldMapper[1];
defaultMapper.root().traverse(new FieldMapperListener() {
@Override
public void fieldMapper(FieldMapper<?> fieldMapper) {
if (fieldMapper.name().equals("field")) {
mapper[0] = fieldMapper;
}
}
});
FieldMapper<?> mapper = defaultMapper.mappers().getMapper("field");
XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
mapper[0].toXContent(builder, ToXContent.EMPTY_PARAMS);
mapper.toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
assertEquals("{\"field\":{\"type\":\"boolean\"}}", builder.string());
@ -113,16 +105,9 @@ public class BooleanFieldMapperTests extends ElasticsearchSingleNodeTest {
.endObject().endObject().string();
defaultMapper = parser.parse(mapping);
defaultMapper.root().traverse(new FieldMapperListener() {
@Override
public void fieldMapper(FieldMapper<?> fieldMapper) {
if (fieldMapper.name().equals("field")) {
mapper[0] = fieldMapper;
}
}
});
mapper = defaultMapper.mappers().getMapper("field");
builder = XContentFactory.jsonBuilder().startObject();
mapper[0].toXContent(builder, ToXContent.EMPTY_PARAMS);
mapper.toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
assertEquals("{\"field\":{\"type\":\"boolean\",\"doc_values\":false,\"null_value\":true}}", builder.string());
}

View File

@ -19,6 +19,8 @@
package org.elasticsearch.index.mapper.externalvalues;
import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
import com.spatial4j.core.shape.Point;
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
@ -221,16 +223,8 @@ public class ExternalMapper extends AbstractFieldMapper<Object> {
}
@Override
public void traverse(FieldMapperListener fieldMapperListener) {
binMapper.traverse(fieldMapperListener);
boolMapper.traverse(fieldMapperListener);
pointMapper.traverse(fieldMapperListener);
shapeMapper.traverse(fieldMapperListener);
stringMapper.traverse(fieldMapperListener);
}
@Override
public void traverse(ObjectMapperListener objectMapperListener) {
public Iterator<Mapper> iterator() {
return Iterators.concat(super.iterator(), Lists.newArrayList(binMapper, boolMapper, pointMapper, shapeMapper, stringMapper).iterator());
}
@Override

View File

@ -25,6 +25,8 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.*;
import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.Map;
public class ExternalRootMapper implements RootMapper {
@ -46,11 +48,8 @@ public class ExternalRootMapper implements RootMapper {
}
@Override
public void traverse(FieldMapperListener fieldMapperListener) {
}
@Override
public void traverse(ObjectMapperListener objectMapperListener) {
public Iterator<Mapper> iterator() {
return Collections.emptyIterator();
}
@Override