Add unit test for AnalysisRegistry#reloadIndexAnalyzers

Christoph Büscher · Christoph Büscher · commit 9aa1f28dbd32 · 2019-04-23T17:14:38.000+02:00
diff --git a/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java b/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java
@@ -34,6 +34,7 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -46,7 +47,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import static java.util.Collections.unmodifiableMap;
 
@@ -535,24 +535,27 @@ private void processNormalizerFactory(
     }
 
     /**
-     * Create an new IndexAnalyzer instance based on the existing one. Analyzers that are in {@link AnalysisMode#SEARCH_TIME} are tried to
-     * be reloaded. All other analyzers are reused from the old {@link IndexAnalyzers} instance.
+     * Creates an new IndexAnalyzer instance based on the existing one passed in. If there are no analysis components that need reloading,
+     * 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 rebuildIndexAnalyzers(IndexAnalyzers indexAnalyzers, IndexSettings indexSettings) throws IOException {
-
+    public IndexAnalyzers reloadIndexAnalyzers(IndexAnalyzers indexAnalyzers, IndexSettings indexSettings) throws IOException {
         // scan analyzers to collect token filters that we need to reload
-        List<NamedAnalyzer> analyzers = Stream.concat(
-                Stream.of(indexAnalyzers.getDefaultSearchAnalyzer(), indexAnalyzers.getDefaultIndexAnalyzer()),
-                indexAnalyzers.getAnalyzers().values().stream()).collect(Collectors.toList());
-
+        Map<String, NamedAnalyzer> oldAnalyzers = indexAnalyzers.getAnalyzers();
+        List<NamedAnalyzer> analyzers = new ArrayList<>(oldAnalyzers.values());
+        analyzers.add(indexAnalyzers.getDefaultIndexAnalyzer());
+        analyzers.add(indexAnalyzers.getDefaultSearchAnalyzer());
         Set<String> filtersThatNeedReloading = filtersThatNeedReloading(analyzers);
-        final Map<String, Settings> tokenFiltersToReloading = indexSettings.getSettings().getGroups(INDEX_ANALYSIS_FILTER)
+        if (filtersThatNeedReloading.size() == 0) {
+            return indexAnalyzers;
+        }
+        final Map<String, Settings> tokenFiltersToReload = indexSettings.getSettings().getGroups(INDEX_ANALYSIS_FILTER)
                 .entrySet().stream()
                 .filter(entry -> filtersThatNeedReloading.contains(entry.getKey()))
                 .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
 
         final Map<String, TokenFilterFactory> newTokenFilterFactories = buildMapping(Component.FILTER, indexSettings,
-                tokenFiltersToReloading, this.tokenFilters, prebuiltAnalysis.preConfiguredTokenFilters);
+                tokenFiltersToReload, this.tokenFilters, prebuiltAnalysis.preConfiguredTokenFilters);
 
         // fill the rest of the token filter factory map with the entries that are missing (were not reloaded)
         for (Entry<String, TokenFilterFactory> entry : indexAnalyzers.getTokenFilterFactoryFactories().entrySet()) {
@@ -567,9 +570,10 @@ public IndexAnalyzers rebuildIndexAnalyzers(IndexAnalyzers indexAnalyzers, Index
         NamedAnalyzer newDefaultSearchQuoteAnalyzer = rebuildIfNecessary(indexAnalyzers.getDefaultSearchQuoteAnalyzer(), indexSettings,
                 currentCharFilterFactories, currentTokenizerFactories, newTokenFilterFactories);
         Map<String, NamedAnalyzer> newAnalyzers = new HashMap<>();
-        for (NamedAnalyzer analyzer : indexAnalyzers.getAnalyzers().values()) {
-            newAnalyzers.put(analyzer.name(), rebuildIfNecessary(analyzer, indexSettings, currentCharFilterFactories,
-                    currentTokenizerFactories, newTokenFilterFactories));
+        for (String analyzerName : oldAnalyzers.keySet()) {
+            NamedAnalyzer analyzer = rebuildIfNecessary(oldAnalyzers.get(analyzerName), indexSettings, currentCharFilterFactories,
+                    currentTokenizerFactories, newTokenFilterFactories);
+            newAnalyzers.put(analyzerName, analyzer);
         }
 
         IndexAnalysisProviders analysisProviders = new IndexAnalysisProviders(currentTokenizerFactories, currentCharFilterFactories,
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java
@@ -852,7 +852,7 @@ public synchronized     void reloadSearchAnalyzers(AnalysisRegistry registry) th
         logger.info("reloading search analyzers");
 
         // refresh indexAnalyzers and search analyzers
-        this.indexAnalyzers = registry.rebuildIndexAnalyzers(this.indexAnalyzers, indexSettings);
+        this.indexAnalyzers = registry.reloadIndexAnalyzers(this.indexAnalyzers, indexSettings);
         this.searchAnalyzer = new MapperAnalyzerWrapper(this.indexAnalyzers.getDefaultSearchAnalyzer(), p -> p.searchAnalyzer());
         this.searchQuoteAnalyzer = new MapperAnalyzerWrapper(this.indexAnalyzers.getDefaultSearchQuoteAnalyzer(),
                 p -> p.searchQuoteAnalyzer());
diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java
@@ -325,12 +325,6 @@ public void testRebuildIfNecessary() throws IOException {
         assertSame(noReloadingEither, AnalysisRegistry.rebuildIfNecessary(noReloadingEither, null, null, null, null));
 
 
-        Settings indexSettings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
-                .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();
-
         final AtomicInteger factoryCounter = new AtomicInteger(0);
         TestAnalysis testAnalysis = createTestAnalysis(new Index("test", "_na_"), Settings.EMPTY, new AnalysisPlugin() {
 
@@ -351,9 +345,14 @@ public TokenFilterFactory get(IndexSettings indexSettings, Environment environme
         tokenFilters[0] = testAnalysis.tokenFilter.get("myReloadableFilter");
         NamedAnalyzer reloadableAnalyzer = new NamedAnalyzer("reloadableAnalyzer", AnalyzerScope.INDEX,
                 new CustomAnalyzer("tokenizer", null, null, tokenFilters));
-        IndexSettings indexSetings = new IndexSettings(IndexMetaData.builder("testIndex").settings(indexSettings).build(), indexSettings);
+        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                .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();
+        IndexSettings indexSettings = new IndexSettings(IndexMetaData.builder("testIndex").settings(settings).build(), settings);
 
-        NamedAnalyzer rebuilt = AnalysisRegistry.rebuildIfNecessary(reloadableAnalyzer, indexSetings, testAnalysis.charFilter,
+        NamedAnalyzer rebuilt = AnalysisRegistry.rebuildIfNecessary(reloadableAnalyzer, indexSettings, testAnalysis.charFilter,
                 testAnalysis.tokenizer, testAnalysis.tokenFilter);
         assertEquals(reloadableAnalyzer.name(), rebuilt.name());
         assertNotSame(reloadableAnalyzer, rebuilt);
@@ -363,12 +362,62 @@ public TokenFilterFactory get(IndexSettings indexSettings, Environment environme
         assertEquals(2, MyReloadableFilter.constructorCounter);
     }
 
+    public void testRebuildIndexAnalyzers() throws IOException {
+
+        final AtomicInteger factoryCounter = new AtomicInteger(0);
+        AnalysisPlugin testPlugin = new 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 {
+                        factoryCounter.getAndIncrement();
+                        return new MyReloadableFilter();
+                    }
+                });
+            }
+        };
+
+        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
+                .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
+                .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();
+        AnalysisModule analysisModule = new AnalysisModule(TestEnvironment.newEnvironment(settings), singletonList(testPlugin));
+        AnalysisRegistry registry = analysisModule.getAnalysisRegistry();
+        IndexSettings indexSettings = new IndexSettings(IndexMetaData.builder("testIndex").settings(settings).build(), settings);
+        IndexAnalyzers oldIndexAnalyzers = registry.build(indexSettings);
+        assertEquals(1, factoryCounter.get());
+
+        IndexAnalyzers rebuildAnalyzers = registry.reloadIndexAnalyzers(oldIndexAnalyzers, indexSettings);
+        assertNotSame(oldIndexAnalyzers, rebuildAnalyzers);
+        assertEquals(2, factoryCounter.get());
+        assertSame(oldIndexAnalyzers.getDefaultIndexAnalyzer(), rebuildAnalyzers.getDefaultIndexAnalyzer());
+        assertSame(oldIndexAnalyzers.getDefaultSearchAnalyzer(), rebuildAnalyzers.getDefaultSearchAnalyzer());
+        assertSame(oldIndexAnalyzers.getDefaultSearchQuoteAnalyzer(), rebuildAnalyzers.getDefaultSearchQuoteAnalyzer());
+        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);
+        assertThat(((CustomAnalyzer) newVersion.analyzer()).tokenFilters()[0], instanceOf(MyReloadableFilter.class));
+        assertEquals(2, ((MyReloadableFilter) ((CustomAnalyzer) newVersion.analyzer()).tokenFilters()[0]).generation);
+    }
+
     static class MyReloadableFilter implements TokenFilterFactory {
 
         static int constructorCounter = 0;
+        private final int generation;
 
         MyReloadableFilter() {
             constructorCounter++;
+            generation = constructorCounter;
         }
 
         @Override
@@ -378,11 +427,12 @@ public String name() {
 
         @Override
         public TokenStream create(TokenStream tokenStream) {
-            return null;
+            return tokenStream;
         }
         @Override
         public AnalysisMode getAnalysisMode() {
             return AnalysisMode.SEARCH_TIME;
         }
-    };
+    }
+
 }
diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java
@@ -29,6 +29,7 @@
 import com.carrotsearch.randomizedtesting.generators.RandomPicks;
 import com.carrotsearch.randomizedtesting.generators.RandomStrings;
 import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter;
+
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
