Mappings: Remove mapper listeners

The mapper listener concept is only now used as a callback to the
MapperService when new fields are added. This change removes the
listeners, instead storing a link to the mapper service in
each doc mapper.

src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -158,12 +158,14 @@ public class DocumentMapper implements ToXContent {
             return this;
         }
 
-        public DocumentMapper build(DocumentMapperParser docMapperParser) {
+        public DocumentMapper build(MapperService mapperService, DocumentMapperParser docMapperParser) {
             Preconditions.checkNotNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
-            return new DocumentMapper(index, indexSettings, docMapperParser, rootObjectMapper, meta, rootMappers, sourceTransforms);
+            return new DocumentMapper(mapperService, index, indexSettings, docMapperParser, rootObjectMapper, meta, rootMappers, sourceTransforms);
         }
     }
 
+    private final MapperService mapperService;
+
     private final String type;
     private final StringAndBytesText typeText;
 
@@ -177,20 +179,17 @@ public class DocumentMapper implements ToXContent {
 
     private volatile ImmutableMap<String, ObjectMapper> objectMappers = ImmutableMap.of();
 
-    private final List<FieldMapperListener> fieldMapperListeners = new CopyOnWriteArrayList<>();
-
-    private final List<ObjectMapperListener> objectMapperListeners = new CopyOnWriteArrayList<>();
-
     private boolean hasNestedObjects = false;
 
     private final Filter typeFilter;
 
     private final Object mappersMutex = new Object();
 
-    public DocumentMapper(String index, @Nullable Settings indexSettings, DocumentMapperParser docMapperParser,
+    public DocumentMapper(MapperService mapperService, String index, @Nullable Settings indexSettings, DocumentMapperParser docMapperParser,
                           RootObjectMapper rootObjectMapper,
                           ImmutableMap<String, Object> meta,
                           Map<Class<? extends RootMapper>, RootMapper> rootMappers, List<SourceTransform> sourceTransforms) {
+        this.mapperService = mapperService;
         this.type = rootObjectMapper.name();
         this.typeText = new StringAndBytesText(this.type);
         this.mapping = new Mapping(
@@ -414,13 +413,7 @@ public class DocumentMapper implements ToXContent {
         synchronized (mappersMutex) {
             this.fieldMappers = this.fieldMappers.copyAndAllAll(fieldMappers);
         }
-        for (FieldMapperListener listener : fieldMapperListeners) {
+        mapperService.addFieldMappers(fieldMappers);
-            listener.fieldMappers(fieldMappers);
-        }
-    }
-
-    public void addFieldMapperListener(FieldMapperListener fieldMapperListener) {
-        fieldMapperListeners.add(fieldMapperListener);
     }
 
     private void addObjectMappers(Collection<ObjectMapper> objectMappers) {
@@ -434,30 +427,36 @@ public class DocumentMapper implements ToXContent {
             }
             this.objectMappers = builder.immutableMap();
         }
-        for (ObjectMapperListener objectMapperListener : objectMapperListeners) {
+        mapperService.addObjectMappers(objectMappers);
-            objectMapperListener.objectMappers(objectMappers);
-        }
-    }
-
-    public void addObjectMapperListener(ObjectMapperListener objectMapperListener) {
-        objectMapperListeners.add(objectMapperListener);
     }
 
     private MergeResult newMergeContext(boolean simulate) {
         return new MergeResult(simulate) {
 
-            List<String> conflicts = new ArrayList<>();
+            final List<String> conflicts = new ArrayList<>();
+            final List<FieldMapper<?>> newFieldMappers = new ArrayList<>();
+            final List<ObjectMapper> newObjectMappers = new ArrayList<>();
 
             @Override
             public void addFieldMappers(Collection<FieldMapper<?>> fieldMappers) {
                 assert simulate() == false;
-                DocumentMapper.this.addFieldMappers(fieldMappers);
+                newFieldMappers.addAll(fieldMappers);
             }
 
             @Override
             public void addObjectMappers(Collection<ObjectMapper> objectMappers) {
                 assert simulate() == false;
-                DocumentMapper.this.addObjectMappers(objectMappers);
+                newObjectMappers.addAll(objectMappers);
+            }
+
+            @Override
+            public Collection<FieldMapper<?>> getNewFieldMappers() {
+                return newFieldMappers;
+            }
+
+            @Override
+            public Collection<ObjectMapper> getNewObjectMappers() {
+                return newObjectMappers;
             }
 
             @Override
@@ -482,6 +481,8 @@ public class DocumentMapper implements ToXContent {
         final MergeResult mergeResult = newMergeContext(simulate);
         this.mapping.merge(mapping, mergeResult);
         if (simulate == false) {
+            addFieldMappers(mergeResult.getNewFieldMappers());
+            addObjectMappers(mergeResult.getNewObjectMappers());
             refreshSource();
         }
         return mergeResult;

src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java

@@ -87,6 +87,7 @@ import static org.elasticsearch.index.mapper.MapperBuilders.doc;
  */
 public class DocumentMapperParser extends AbstractIndexComponent {
 
+    final MapperService mapperService;
     final AnalysisService analysisService;
     private static final ESLogger logger = Loggers.getLogger(DocumentMapperParser.class);
     private final SimilarityLookupService similarityLookupService;
@@ -100,9 +101,10 @@ public class DocumentMapperParser extends AbstractIndexComponent {
     private volatile ImmutableMap<String, Mapper.TypeParser> typeParsers;
     private volatile ImmutableMap<String, Mapper.TypeParser> rootTypeParsers;
 
-    public DocumentMapperParser(Index index, @IndexSettings Settings indexSettings, AnalysisService analysisService,
+    public DocumentMapperParser(Index index, @IndexSettings Settings indexSettings, MapperService mapperService, AnalysisService analysisService,
                                 SimilarityLookupService similarityLookupService, ScriptService scriptService) {
         super(index, indexSettings);
+        this.mapperService = mapperService;
         this.analysisService = analysisService;
         this.similarityLookupService = similarityLookupService;
         this.scriptService = scriptService;
@@ -269,7 +271,7 @@ public class DocumentMapperParser extends AbstractIndexComponent {
 
         checkNoRemainingFields(mapping, parserContext.indexVersionCreated(), "Root mapping definition has unsupported parameters: ");
 
-        DocumentMapper documentMapper = docBuilder.build(this);
+        DocumentMapper documentMapper = docBuilder.build(mapperService, this);
         // update the source with the generated one
         documentMapper.refreshSource();
         return documentMapper;

src/main/java/org/elasticsearch/index/mapper/FieldMapperListener.java

@@ -1,47 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.index.mapper;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-
-/**
- *
- */
-public abstract class FieldMapperListener {
-
-    public static class Aggregator extends FieldMapperListener {
-        public final List<FieldMapper<?>> mappers = new ArrayList<>();
-
-        @Override
-        public void fieldMapper(FieldMapper<?> fieldMapper) {
-            mappers.add(fieldMapper);
-        }
-    }
-
-    public abstract void fieldMapper(FieldMapper<?> fieldMapper);
-
-    public void fieldMappers(Collection<FieldMapper<?>> fieldMappers) {
-        for (FieldMapper<?> mapper : fieldMappers) {
-            fieldMapper(mapper);
-        }
-    }
-}

src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -104,9 +104,6 @@ public class MapperService extends AbstractIndexComponent  {
 
     private final DocumentMapperParser documentParser;
 
-    private final InternalFieldMapperListener fieldMapperListener = new InternalFieldMapperListener();
-    private final InternalObjectMapperListener objectMapperListener = new InternalObjectMapperListener();
-
     private final SmartIndexNameSearchAnalyzer searchAnalyzer;
     private final SmartIndexNameSearchQuoteAnalyzer searchQuoteAnalyzer;
 
@@ -122,7 +119,7 @@ public class MapperService extends AbstractIndexComponent  {
         this.analysisService = analysisService;
         this.fieldDataService = fieldDataService;
         this.fieldMappers = new FieldMappersLookup();
-        this.documentParser = new DocumentMapperParser(index, indexSettings, analysisService, similarityLookupService, scriptService);
+        this.documentParser = new DocumentMapperParser(index, indexSettings, this, analysisService, similarityLookupService, scriptService);
         this.searchAnalyzer = new SmartIndexNameSearchAnalyzer(analysisService.defaultSearchAnalyzer());
         this.searchQuoteAnalyzer = new SmartIndexNameSearchQuoteAnalyzer(analysisService.defaultSearchQuoteAnalyzer());
 
@@ -275,9 +272,7 @@ public class MapperService extends AbstractIndexComponent  {
                 }
                 MapperUtils.collect(mapper.mapping().root, newObjectMappers, newFieldMappers);
                 addFieldMappers(newFieldMappers);
-                mapper.addFieldMapperListener(fieldMapperListener);
                 addObjectMappers(newObjectMappers);
-                mapper.addObjectMapperListener(objectMapperListener);
 
                 for (DocumentTypeListener typeListener : typeListeners) {
                     typeListener.beforeCreate(mapper);
@@ -288,7 +283,7 @@ public class MapperService extends AbstractIndexComponent  {
         }
     }
 
-    private void addObjectMappers(Collection<ObjectMapper> objectMappers) {
+    protected void addObjectMappers(Collection<ObjectMapper> objectMappers) {
         synchronized (mappersMutex) {
             ImmutableOpenMap.Builder<String, ObjectMappers> fullPathObjectMappers = ImmutableOpenMap.builder(this.fullPathObjectMappers);
             for (ObjectMapper objectMapper : objectMappers) {
@@ -308,7 +303,7 @@ public class MapperService extends AbstractIndexComponent  {
         }
     }
 
-    private void addFieldMappers(Collection<FieldMapper<?>> fieldMappers) {
+    protected void addFieldMappers(Collection<FieldMapper<?>> fieldMappers) {
         synchronized (mappersMutex) {
             this.fieldMappers = this.fieldMappers.copyAndAddAll(fieldMappers);
         }
@@ -848,28 +843,4 @@ public class MapperService extends AbstractIndexComponent  {
             return defaultAnalyzer;
         }
     }
-
-    class InternalFieldMapperListener extends FieldMapperListener {
-        @Override
-        public void fieldMapper(FieldMapper<?> fieldMapper) {
-            addFieldMappers(Collections.<FieldMapper<?>>singletonList(fieldMapper));
-        }
-
-        @Override
-        public void fieldMappers(Collection<FieldMapper<?>> fieldMappers) {
-            addFieldMappers(fieldMappers);
-        }
-    }
-
-    class InternalObjectMapperListener extends ObjectMapperListener {
-        @Override
-        public void objectMapper(ObjectMapper objectMapper) {
-            addObjectMappers(Collections.singletonList(objectMapper));
-        }
-
-        @Override
-        public void objectMappers(Collection<ObjectMapper> objectMappers) {
-            addObjectMappers(objectMappers);
-        }
-    }
 }

src/main/java/org/elasticsearch/index/mapper/MapperUtils.java

@@ -29,7 +29,7 @@ import java.util.Collection;
 public enum MapperUtils {
     ;
 
-    private static MergeResult newStrictMergeContext() {
+    private static MergeResult newStrictMergeResult() {
         return new MergeResult(false) {
 
             @Override
@@ -42,14 +42,24 @@ public enum MapperUtils {
                 return Strings.EMPTY_ARRAY;
             }
 
+            @Override
+            public void addFieldMappers(Collection<FieldMapper<?>> fieldMappers) {
+                // no-op
+            }
+
             @Override
             public void addObjectMappers(Collection<ObjectMapper> objectMappers) {
                 // no-op
             }
 
             @Override
-            public void addFieldMappers(Collection<FieldMapper<?>> fieldMappers) {
+            public Collection<FieldMapper<?>> getNewFieldMappers() {
-                // no-op
+                throw new UnsupportedOperationException("Strict merge result does not support new field mappers");
+            }
+
+            @Override
+            public Collection<ObjectMapper> getNewObjectMappers() {
+                throw new UnsupportedOperationException("Strict merge result does not support new object mappers");
             }
 
             @Override
@@ -64,7 +74,7 @@ public enum MapperUtils {
      * merges mappings, not lookup structures. Conflicts are returned as exceptions.
      */
     public static void merge(Mapper mergeInto, Mapper mergeWith) {
-        mergeInto.merge(mergeWith, newStrictMergeContext());
+        mergeInto.merge(mergeWith, newStrictMergeResult());
     }
 
     /**
@@ -72,7 +82,7 @@ public enum MapperUtils {
      * merges mappings, not lookup structures. Conflicts are returned as exceptions.
      */
     public static void merge(Mapping mergeInto, Mapping mergeWith) {
-        mergeInto.merge(mergeWith, newStrictMergeContext());
+        mergeInto.merge(mergeWith, newStrictMergeResult());
     }
 
     /** Split mapper and its descendants into object and field mappers. */

src/main/java/org/elasticsearch/index/mapper/MergeResult.java

@@ -38,6 +38,10 @@ public abstract class MergeResult {
 
     public abstract void addObjectMappers(Collection<ObjectMapper> objectMappers);
 
+    public abstract Collection<FieldMapper<?>> getNewFieldMappers();
+
+    public abstract Collection<ObjectMapper> getNewObjectMappers();
+
     public boolean simulate() {
         return simulate;
     }

src/main/java/org/elasticsearch/index/mapper/ObjectMapperListener.java

@@ -1,46 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-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 {
-        public final List<ObjectMapper> mappers = new ArrayList<>();
-
-        @Override
-        public void objectMapper(ObjectMapper objectMapper) {
-            mappers.add(objectMapper);
-        }
-    }
-
-    public abstract void objectMapper(ObjectMapper objectMapper);
-
-    public void objectMappers(Collection<ObjectMapper> objectMappers) {
-        for (ObjectMapper objectMapper : objectMappers) {
-            objectMapper(objectMapper);
-        }
-    }
-}

src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java

@@ -37,14 +37,12 @@ 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;
 import org.elasticsearch.index.mapper.internal.AllFieldMapper;
 import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
 import org.elasticsearch.index.settings.IndexSettings;

src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java

@@ -1753,15 +1753,17 @@ public class InternalEngineTests extends ElasticsearchTestCase {
 
         public final AtomicInteger recoveredOps = new AtomicInteger(0);
 
-        public TranslogHandler(String index) {
+        public TranslogHandler(String indexName) {
             super(null, new MapperAnalyzer(null), null, null, null);
             Settings settings = ImmutableSettings.settingsBuilder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build();
             RootObjectMapper.Builder rootBuilder = new RootObjectMapper.Builder("test");
-            DocumentMapper.Builder b = new DocumentMapper.Builder(index, settings, rootBuilder);
+            Index index = new Index(indexName);
-            DocumentMapperParser parser = new DocumentMapperParser(new Index(index), settings,
+            AnalysisService analysisService = new AnalysisService(index, settings);
-                    new AnalysisService(new Index(index), settings),
+            SimilarityLookupService similarityLookupService = new SimilarityLookupService(index, settings);
-                    new SimilarityLookupService(new Index(index), settings), null);
+            MapperService mapperService = new MapperService(index, settings, analysisService, null, similarityLookupService, null);
-            this.docMapper = b.build(parser);
+            DocumentMapper.Builder b = new DocumentMapper.Builder(indexName, settings, rootBuilder);
+            DocumentMapperParser parser = new DocumentMapperParser(index, settings, mapperService, analysisService, similarityLookupService, null);
+            this.docMapper = b.build(null, parser);
 
         }
 

src/test/java/org/elasticsearch/index/mapper/core/BooleanFieldMapperTests.java

@@ -35,9 +35,7 @@ import org.elasticsearch.index.IndexService;
 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.ParsedDocument;
-import org.elasticsearch.index.mapper.core.BooleanFieldMapper;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 import org.junit.Before;
 

src/test/java/org/elasticsearch/index/mapper/externalvalues/ExternalMapper.java

@@ -32,12 +32,10 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 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.BinaryFieldMapper;

src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java

@@ -147,7 +147,7 @@ public class MultiFieldTests extends ElasticsearchSingleNodeTest {
                 stringField("name").store(true)
                         .addMultiField(stringField("indexed").index(true).tokenized(true))
                         .addMultiField(stringField("not_indexed").index(false).store(true))
-        )).build(mapperParser);
+        )).build(indexService.mapperService(), mapperParser);
         builderDocMapper.refreshSource();
 
         String builtMapping = builderDocMapper.mappingSource().string();

src/test/java/org/elasticsearch/index/mapper/simple/SimpleMapperTests.java

@@ -48,7 +48,7 @@ public class SimpleMapperTests extends ElasticsearchSingleNodeTest {
         DocumentMapper docMapper = doc("test", settings,
                 rootObject("person")
                         .add(object("name").add(stringField("first").store(true).index(false)))
-        ).build(mapperParser);
+        ).build(indexService.mapperService(), mapperParser);
 
         BytesReference json = new BytesArray(copyToBytesFromClasspath("/org/elasticsearch/index/mapper/simple/test1.json"));
         Document doc = docMapper.parse("person", "1", json).rootDoc();
@@ -126,7 +126,7 @@ public class SimpleMapperTests extends ElasticsearchSingleNodeTest {
         DocumentMapper docMapper = doc("test", settings,
                 rootObject("person")
                         .add(object("name").add(stringField("first").store(true).index(false)))
-        ).build(mapperParser);
+        ).build(indexService.mapperService(), mapperParser);
 
         BytesReference json = new BytesArray("".getBytes(Charsets.UTF_8));
         try {