Explorar o código

Merge pull request #11027 from rjernst/pr/mapper-subfields

Mappings: Remove traverse functions from Mapper
Ryan Ernst %!s(int64=10) %!d(string=hai) anos
pai
achega
f1e0fb6b85

+ 13 - 33
src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -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) {
 

+ 6 - 15
src/main/java/org/elasticsearch/index/mapper/Mapper.java

@@ -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();
 }

+ 14 - 10
src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -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);
         }
     }

+ 14 - 2
src/main/java/org/elasticsearch/index/mapper/MapperUtils.java

@@ -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);
+        }
+    }
 }

+ 2 - 4
src/main/java/org/elasticsearch/index/mapper/ObjectMapperListener.java

@@ -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);
         }

+ 24 - 19
src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java

@@ -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);
-    }
-
-    @Override
-    public void traverse(ObjectMapperListener objectMapperListener) {
-        // nothing to do here...
+    public Iterator<Mapper> iterator() {
+        if (multiFields == null) {
+            return Collections.emptyIterator();
+        }
+        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() {

+ 9 - 13
src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java

@@ -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

+ 12 - 21
src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java

@@ -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) {

+ 4 - 19
src/test/java/org/elasticsearch/index/mapper/core/BooleanFieldMapperTests.java

@@ -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());
     }

+ 4 - 10
src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalMapper.java

@@ -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

+ 4 - 5
src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalRootMapper.java

@@ -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