More tests

Author: Christoph Büscher

server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java

@@ -539,7 +539,8 @@ private void processNormalizerFactory(
      * the same instance of {@link IndexAnalyzers} is returned. Otherwise, analyzers that are in {@link AnalysisMode#SEARCH_TIME} are tried
      * to be reloaded. All other analyzers are reused from the old {@link IndexAnalyzers} instance.
      */
-    public IndexAnalyzers reloadIndexAnalyzers(IndexAnalyzers indexAnalyzers, IndexSettings indexSettings) throws IOException {
+    public IndexAnalyzers reloadIndexAnalyzers(IndexAnalyzers indexAnalyzers) throws IOException {
+        IndexSettings indexSettings = indexAnalyzers.getIndexSettings();
         // scan analyzers to collect token filters that we need to reload
         Map<String, NamedAnalyzer> oldAnalyzers = indexAnalyzers.getAnalyzers();
         List<NamedAnalyzer> analyzers = new ArrayList<>(oldAnalyzers.values());
@@ -578,9 +579,7 @@ public IndexAnalyzers reloadIndexAnalyzers(IndexAnalyzers indexAnalyzers, IndexS
 
         IndexAnalysisProviders analysisProviders = new IndexAnalysisProviders(currentTokenizerFactories, currentCharFilterFactories,
                 newTokenFilterFactories);
-        return new IndexAnalyzers(indexSettings, indexAnalyzers.getDefaultIndexAnalyzer(), newDefaultSearchAnalyzer,
-                newDefaultSearchQuoteAnalyzer, newAnalyzers, indexAnalyzers.getNormalizers(), indexAnalyzers.getWhitespaceNormalizers(),
-                analysisProviders);
+        return new IndexAnalyzers(indexAnalyzers, newDefaultSearchAnalyzer, newDefaultSearchQuoteAnalyzer, newAnalyzers, analysisProviders);
     }
 
     static Set<String> filtersThatNeedReloading(List<NamedAnalyzer> analyzers) {

server/src/main/java/org/elasticsearch/index/analysis/IndexAnalyzers.java

@@ -71,6 +71,22 @@ public IndexAnalyzers(IndexSettings indexSettings, NamedAnalyzer defaultIndexAna
         this.analysisProviders = analysisProviders;
     }
 
+    /**
+     * Partial copy-constructor that keeps references to settings, default index analyzer and normalizers from original
+     * {@link IndexAnalyzers} passed in but takes other search time analyzers as inputs.
+     */
+    IndexAnalyzers(IndexAnalyzers original, NamedAnalyzer defaultSearchAnalyzer, NamedAnalyzer defaultSearchQuoteAnalyzer,
+            Map<String, NamedAnalyzer> analyzers, IndexAnalysisProviders analysisProviders) {
+        super(original.getIndexSettings());
+        this.defaultIndexAnalyzer = original.defaultIndexAnalyzer;
+        this.defaultSearchAnalyzer = defaultSearchAnalyzer;
+        this.defaultSearchQuoteAnalyzer = defaultSearchQuoteAnalyzer;
+        this.analyzers =  unmodifiableMap(new HashMap<>(analyzers));
+        this.normalizers = original.normalizers;
+        this.whitespaceNormalizers = original.whitespaceNormalizers;
+        this.analysisProviders = analysisProviders;
+    }
+
     /**
      * Returns an analyzer mapped to the given name or <code>null</code> if not present
      */
@@ -134,15 +150,15 @@ public NamedAnalyzer getDefaultSearchQuoteAnalyzer() {
         return defaultSearchQuoteAnalyzer;
     }
 
-    public Map<String, TokenizerFactory> getTokenizerFactoryFactories() {
+    Map<String, TokenizerFactory> getTokenizerFactoryFactories() {
         return analysisProviders.tokenizerFactoryFactories;
     }
 
-    public Map<String, CharFilterFactory> getCharFilterFactoryFactories() {
+    Map<String, CharFilterFactory> getCharFilterFactoryFactories() {
         return analysisProviders.charFilterFactoryFactories;
     }
 
-    public Map<String, TokenFilterFactory> getTokenFilterFactoryFactories() {
+    Map<String, TokenFilterFactory> getTokenFilterFactoryFactories() {
         return analysisProviders.tokenFilterFactoryFactories;
     }
 

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

@@ -850,14 +850,17 @@ protected Analyzer getWrappedAnalyzer(String fieldName) {
 
     public synchronized     void reloadSearchAnalyzers(AnalysisRegistry registry) throws IOException {
         logger.info("reloading search analyzers");
-
         // refresh indexAnalyzers and search analyzers
-        this.indexAnalyzers = registry.reloadIndexAnalyzers(this.indexAnalyzers, indexSettings);
+        IndexAnalyzers reloadedIndexAnalyzers = registry.reloadIndexAnalyzers(this.indexAnalyzers);
+        if (indexAnalyzers == reloadedIndexAnalyzers) {
+            // nothing changed, so we can simply return;
+            return;
+        }
+        this.indexAnalyzers = reloadedIndexAnalyzers;
         this.searchAnalyzer = new MapperAnalyzerWrapper(this.indexAnalyzers.getDefaultSearchAnalyzer(), p -> p.searchAnalyzer());
         this.searchQuoteAnalyzer = new MapperAnalyzerWrapper(this.indexAnalyzers.getDefaultSearchQuoteAnalyzer(),
                 p -> p.searchQuoteAnalyzer());
 
-        // also reload search time analyzers in MappedFieldTypes
         // refresh search time analyzers in MappedFieldTypes
         List<MappedFieldType> mftsToRefresh = StreamSupport.stream(fieldTypes.spliterator(), false)
                 .filter(mft -> (mft.searchAnalyzer() != null && mft.searchAnalyzer().getAnalysisMode() == AnalysisMode.SEARCH_TIME)

server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java

@@ -352,14 +352,16 @@ public TokenFilterFactory get(IndexSettings indexSettings, Environment environme
                 .putList("index.analysis.analyzer.reloadableAnalyzer.filter", "myReloadableFilter").build();
         IndexSettings indexSettings = new IndexSettings(IndexMetaData.builder("testIndex").settings(settings).build(), settings);
 
+        int initialFilterCreationCount = MyReloadableFilter.constructorCounter.get();
         NamedAnalyzer rebuilt = AnalysisRegistry.rebuildIfNecessary(reloadableAnalyzer, indexSettings, testAnalysis.charFilter,
                 testAnalysis.tokenizer, testAnalysis.tokenFilter);
         assertEquals(reloadableAnalyzer.name(), rebuilt.name());
         assertNotSame(reloadableAnalyzer, rebuilt);
         assertEquals(2, factoryCounter.get()); // once on intialization, once again for reloading
         TokenFilterFactory reloadedFactory = ((CustomAnalyzer) rebuilt.analyzer()).tokenFilters()[0];
         assertThat(reloadedFactory, instanceOf(MyReloadableFilter.class));
-        assertEquals(2, MyReloadableFilter.constructorCounter);
+        // the filter factories should not be used at this poing since the function only re-creates the analyzer
+        assertEquals(initialFilterCreationCount, MyReloadableFilter.constructorCounter.get());
     }
 
     public void testRebuildIndexAnalyzers() throws IOException {
@@ -393,31 +395,33 @@ public TokenFilterFactory get(IndexSettings indexSettings, Environment environme
         IndexAnalyzers oldIndexAnalyzers = registry.build(indexSettings);
         assertEquals(1, factoryCounter.get());
 
-        IndexAnalyzers rebuildAnalyzers = registry.reloadIndexAnalyzers(oldIndexAnalyzers, indexSettings);
+        IndexAnalyzers rebuildAnalyzers = registry.reloadIndexAnalyzers(oldIndexAnalyzers);
         assertNotSame(oldIndexAnalyzers, rebuildAnalyzers);
         assertEquals(2, factoryCounter.get());
         assertSame(oldIndexAnalyzers.getDefaultIndexAnalyzer(), rebuildAnalyzers.getDefaultIndexAnalyzer());
         assertSame(oldIndexAnalyzers.getDefaultSearchAnalyzer(), rebuildAnalyzers.getDefaultSearchAnalyzer());
         assertSame(oldIndexAnalyzers.getDefaultSearchQuoteAnalyzer(), rebuildAnalyzers.getDefaultSearchQuoteAnalyzer());
+        assertSame(oldIndexAnalyzers.getNormalizers(), rebuildAnalyzers.getNormalizers());
+        assertSame(oldIndexAnalyzers.getWhitespaceNormalizers(), rebuildAnalyzers.getWhitespaceNormalizers());
         assertNotSame(oldIndexAnalyzers.getAnalyzers(), rebuildAnalyzers.getAnalyzers());
         assertEquals(oldIndexAnalyzers.getAnalyzers().size(), rebuildAnalyzers.getAnalyzers().size());
         NamedAnalyzer oldVersion = oldIndexAnalyzers.get("reloadableAnalyzer");
         NamedAnalyzer newVersion = rebuildAnalyzers.get("reloadableAnalyzer");
         assertNotSame(oldVersion, newVersion);
         assertThat(((CustomAnalyzer) oldVersion.analyzer()).tokenFilters()[0], instanceOf(MyReloadableFilter.class));
-        assertEquals(1, ((MyReloadableFilter) ((CustomAnalyzer) oldVersion.analyzer()).tokenFilters()[0]).generation);
+        int oldGeneration = ((MyReloadableFilter) ((CustomAnalyzer) oldVersion.analyzer()).tokenFilters()[0]).generation.get();
         assertThat(((CustomAnalyzer) newVersion.analyzer()).tokenFilters()[0], instanceOf(MyReloadableFilter.class));
-        assertEquals(2, ((MyReloadableFilter) ((CustomAnalyzer) newVersion.analyzer()).tokenFilters()[0]).generation);
+        assertEquals(oldGeneration + 1, ((MyReloadableFilter) ((CustomAnalyzer) newVersion.analyzer()).tokenFilters()[0]).generation.get());
     }
 
     static class MyReloadableFilter implements TokenFilterFactory {
 
-        static int constructorCounter = 0;
-        private final int generation;
+        private static AtomicInteger constructorCounter = new AtomicInteger();
+        private final AtomicInteger generation;
 
         MyReloadableFilter() {
-            constructorCounter++;
-            generation = constructorCounter;
+            constructorCounter.getAndIncrement();
+            generation = new AtomicInteger(constructorCounter.get());
         }
 
         @Override

server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java

@@ -19,26 +19,39 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.lucene.analysis.TokenStream;
 import org.elasticsearch.ExceptionsHelper;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.env.Environment;
 import org.elasticsearch.index.IndexService;
+import org.elasticsearch.index.IndexSettings;
+import org.elasticsearch.index.analysis.AnalysisMode;
+import org.elasticsearch.index.analysis.AnalysisRegistry;
+import org.elasticsearch.index.analysis.IndexAnalyzers;
+import org.elasticsearch.index.analysis.NamedAnalyzer;
+import org.elasticsearch.index.analysis.TokenFilterFactory;
 import org.elasticsearch.index.mapper.KeywordFieldMapper.KeywordFieldType;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType;
 import org.elasticsearch.indices.InvalidTypeNameException;
+import org.elasticsearch.indices.analysis.AnalysisModule.AnalysisProvider;
+import org.elasticsearch.plugins.AnalysisPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.elasticsearch.test.InternalSettingsPlugin;
 
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 
 import static org.hamcrest.CoreMatchers.containsString;
@@ -49,7 +62,7 @@ public class MapperServiceTests extends ESSingleNodeTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> getPlugins() {
-        return Collections.singleton(InternalSettingsPlugin.class);
+        return List.of(InternalSettingsPlugin.class, ReloadableFilterPlugin.class);
     }
 
     public void testTypeNameStartsWithIllegalDot() {
@@ -434,4 +447,93 @@ public void testMappingRecoverySkipFieldNameLengthLimit() throws Throwable {
         assertEquals(testString, documentMapper.mappers().getMapper(testString).simpleName());
     }
 
+    public void testReloadSearchAnalyzersNoReload() throws IOException {
+        MapperService mapperService = createIndex("no_reload_index", Settings.EMPTY).mapperService();
+        IndexAnalyzers current = mapperService.getIndexAnalyzers();
+        mapperService.reloadSearchAnalyzers(getInstanceFromNode(AnalysisRegistry.class));
+        assertSame(current, mapperService.getIndexAnalyzers());
+    }
+
+    public void testReloadSearchAnalyzers() throws IOException {
+        Settings settings = Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
+                .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)
+                .put("index.analysis.analyzer.reloadableAnalyzer.type", "custom")
+                .put("index.analysis.analyzer.reloadableAnalyzer.tokenizer", "standard")
+                .putList("index.analysis.analyzer.reloadableAnalyzer.filter", "myReloadableFilter").build();
+
+        MapperService mapperService = createIndex("test_index", settings).mapperService();
+        CompressedXContent mapping = new CompressedXContent(BytesReference.bytes(
+                XContentFactory.jsonBuilder().startObject().startObject("_doc")
+                    .startObject("properties")
+                        .startObject("field")
+                            .field("type", "text")
+                            .field("analyzer", "simple")
+                            .field("search_analyzer", "reloadableAnalyzer")
+                            .field("search_quote_analyzer", "stop")
+                        .endObject()
+                        .startObject("otherField")
+                            .field("type", "text")
+                            .field("analyzer", "standard")
+                            .field("search_analyzer", "simple")
+                            .field("search_quote_analyzer", "reloadableAnalyzer")
+                        .endObject()
+                    .endObject()
+                .endObject().endObject()));
+
+        mapperService.merge("_doc", mapping, MergeReason.MAPPING_UPDATE);
+        IndexAnalyzers current = mapperService.getIndexAnalyzers();
+        MappedFieldType fieldType = mapperService.fullName("field");
+        NamedAnalyzer originalFieldsearchAnalyzer = fieldType.searchAnalyzer();
+        NamedAnalyzer originalFieldsearchQuoteAnalyzer = fieldType.searchQuoteAnalyzer();
+
+        MappedFieldType otherFieldType = mapperService.fullName("otherField");
+        NamedAnalyzer originalOtherFieldsearchAnalyzer = otherFieldType.searchAnalyzer();
+        NamedAnalyzer originalOtherFieldsearchQuoteAnalyzer = otherFieldType.searchQuoteAnalyzer();
+
+        mapperService.reloadSearchAnalyzers(getInstanceFromNode(AnalysisRegistry.class));
+        IndexAnalyzers updatedAnalyzers = mapperService.getIndexAnalyzers();
+        assertNotSame(current, updatedAnalyzers);
+        assertSame(current.getDefaultIndexAnalyzer(), updatedAnalyzers.getDefaultIndexAnalyzer());
+        assertSame(current.getDefaultSearchAnalyzer(), updatedAnalyzers.getDefaultSearchAnalyzer());
+        assertSame(current.getDefaultSearchQuoteAnalyzer(), updatedAnalyzers.getDefaultSearchQuoteAnalyzer());
+
+        assertNotSame(originalFieldsearchAnalyzer, mapperService.fullName("field").searchAnalyzer());
+        assertSame(originalOtherFieldsearchAnalyzer, mapperService.fullName("otherField").searchAnalyzer());
+
+        assertSame(originalFieldsearchQuoteAnalyzer, mapperService.fullName("field").searchQuoteAnalyzer());
+        assertNotSame(originalOtherFieldsearchQuoteAnalyzer, mapperService.fullName("otherField").searchQuoteAnalyzer());
+
+    }
+
+    public static final class ReloadableFilterPlugin extends Plugin implements AnalysisPlugin {
+
+        @Override
+        public Map<String, AnalysisProvider<TokenFilterFactory>> getTokenFilters() {
+            return Collections.singletonMap("myReloadableFilter", new AnalysisProvider<TokenFilterFactory>() {
+
+                @Override
+                public TokenFilterFactory get(IndexSettings indexSettings, Environment environment, String name, Settings settings)
+                        throws IOException {
+                    return new TokenFilterFactory() {
+
+                        @Override
+                        public String name() {
+                            return "myReloadableFilter";
+                        }
+
+                        @Override
+                        public TokenStream create(TokenStream tokenStream) {
+                            return tokenStream;
+                        }
+
+                        @Override
+                        public AnalysisMode getAnalysisMode() {
+                            return AnalysisMode.SEARCH_TIME;
+                        }
+                    };
+                }
+            });
+        }
+    }
+
 }