From 54c659cfe923d0a86e547bd0d963a0cad4e88a98 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 13 Jan 2020 11:24:48 +0000 Subject: [PATCH 1/5] Check for deprecations when analyzers are built --- .../index/analysis/AnalysisRegistry.java | 19 ++++ .../indices/TransportAnalyzeActionTests.java | 46 ++++++++- .../elasticsearch/index/IndexModuleTests.java | 3 +- .../index/analysis/AnalysisRegistryTests.java | 96 ++++++++++++++++++- .../indices/analysis/AnalysisModuleTests.java | 5 +- 5 files changed, 159 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java b/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java index 882dcf6bb6cd9..aa2e827b9e742 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.analysis; import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.analysis.TokenStream; import org.apache.lucene.analysis.core.KeywordTokenizer; import org.apache.lucene.analysis.core.WhitespaceTokenizer; import org.elasticsearch.ElasticsearchException; @@ -536,6 +537,10 @@ public IndexAnalyzers build(IndexSettings indexSettings, tokenFilterFactoryFactories, charFilterFactoryFactories); } + for (Analyzer analyzer : normalizers.values()) { + analyzer.normalize("", ""); // check for deprecations + } + if (!analyzers.containsKey(DEFAULT_ANALYZER_NAME)) { analyzers.put(DEFAULT_ANALYZER_NAME, produceAnalyzer(DEFAULT_ANALYZER_NAME, @@ -599,6 +604,7 @@ private static NamedAnalyzer produceAnalyzer(String name, } else { analyzer = new NamedAnalyzer(name, analyzerFactory.scope(), analyzerF, overridePositionIncrementGap); } + checkDeprecations(analyzer); return analyzer; } @@ -626,4 +632,17 @@ private void processNormalizerFactory( NamedAnalyzer normalizer = new NamedAnalyzer(name, normalizerFactory.scope(), normalizerF); normalizers.put(name, normalizer); } + + // Deprecation warnings are emitted when actual TokenStreams are built; this is usually + // too late to be useful, so we build an empty tokenstream at construction time and + // use it, to ensure that warnings are emitted immediately. + private static void checkDeprecations(Analyzer analyzer) { + try (TokenStream ts = analyzer.tokenStream("", "")) { + ts.reset(); + while (ts.incrementToken()) {} + ts.end(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java index 23129ae546fe6..39e4562c75267 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java @@ -38,6 +38,7 @@ import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.index.analysis.CharFilterFactory; import org.elasticsearch.index.analysis.IndexAnalyzers; +import org.elasticsearch.index.analysis.NormalizingTokenFilterFactory; import org.elasticsearch.index.analysis.PreConfiguredCharFilter; import org.elasticsearch.index.analysis.TokenFilterFactory; import org.elasticsearch.index.analysis.TokenizerFactory; @@ -108,6 +109,25 @@ public TokenStream create(TokenStream tokenStream) { } } + class DeprecatedTokenFilterFactory extends AbstractTokenFilterFactory implements NormalizingTokenFilterFactory { + + public DeprecatedTokenFilterFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + super(indexSettings, name, settings); + } + + @Override + public TokenStream create(TokenStream tokenStream) { + deprecationLogger.deprecated("Using deprecated token filter [deprecated]"); + return tokenStream; + } + + @Override + public TokenStream normalize(TokenStream tokenStream) { + deprecationLogger.deprecated("Using deprecated token filter [deprecated]"); + return tokenStream; + } + } + class AppendCharFilterFactory extends AbstractCharFilterFactory { final String suffix; @@ -136,7 +156,7 @@ public Map> getTokenizers() { @Override public Map> getTokenFilters() { - return singletonMap("mock", MockFactory::new); + return Map.of("mock", MockFactory::new, "deprecated", DeprecatedTokenFilterFactory::new); } @Override @@ -492,4 +512,28 @@ public void testExceedSetMaxTokenLimit() { assertEquals(e.getMessage(), "The number of tokens produced by calling _analyze has exceeded the allowed maximum of [" + idxMaxTokenCount + "]." + " This limit can be set by changing the [index.analyze.max_token_count] index level setting."); } + + public void testDeprecationWarnings() throws IOException { + AnalyzeAction.Request req = new AnalyzeAction.Request(); + req.tokenizer("standard"); + req.addTokenFilter("lowercase"); + req.addTokenFilter("deprecated"); + req.text("test text"); + + AnalyzeAction.Response analyze = + TransportAnalyzeAction.analyze(req, registry, mockIndexService(), maxTokenCount); + assertEquals(2, analyze.getTokens().size()); + assertWarnings("Using deprecated token filter [deprecated]"); + + // normalizer + req = new AnalyzeAction.Request(); + req.addTokenFilter("lowercase"); + req.addTokenFilter("deprecated"); + req.text("text"); + + analyze = + TransportAnalyzeAction.analyze(req, registry, mockIndexService(), maxTokenCount); + assertEquals(1, analyze.getTokens().size()); + assertWarnings("Using deprecated token filter [deprecated]"); + } } diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index adeb49faa8941..2c11b26580f3d 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index; import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.analysis.standard.StandardTokenizer; import org.apache.lucene.index.AssertingDirectoryReader; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.FieldInvertState; @@ -442,7 +443,7 @@ public Analyzer get() { final Analyzer analyzer = new Analyzer() { @Override protected TokenStreamComponents createComponents(String fieldName) { - throw new AssertionError("should not be here"); + return new TokenStreamComponents(new StandardTokenizer()); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java index f7df2ee97f932..3590ccdb4b63d 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java @@ -20,12 +20,13 @@ package org.elasticsearch.index.analysis; import com.carrotsearch.randomizedtesting.generators.RandomPicks; - import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.MockTokenFilter; import org.apache.lucene.analysis.TokenStream; +import org.apache.lucene.analysis.Tokenizer; import org.apache.lucene.analysis.en.EnglishAnalyzer; import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.analysis.standard.StandardTokenizer; import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -108,8 +109,8 @@ public void testOverrideDefaultAnalyzer() throws IOException { public void testOverrideDefaultAnalyzerWithoutAnalysisModeAll() throws IOException { Version version = VersionUtils.randomVersion(random()); Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build(); - TokenFilterFactory tokenFilter = new AbstractTokenFilterFactory(IndexSettingsModule.newIndexSettings("index", settings), - "my_filter", Settings.EMPTY) { + IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("index", settings); + TokenFilterFactory tokenFilter = new AbstractTokenFilterFactory(indexSettings, "my_filter", Settings.EMPTY) { @Override public AnalysisMode getAnalysisMode() { return randomFrom(AnalysisMode.SEARCH_TIME, AnalysisMode.INDEX_TIME); @@ -117,10 +118,16 @@ public AnalysisMode getAnalysisMode() { @Override public TokenStream create(TokenStream tokenStream) { - return null; + return tokenStream; + } + }; + TokenizerFactory tokenizer = new AbstractTokenizerFactory(indexSettings, Settings.EMPTY, "my_tokenizer") { + @Override + public Tokenizer create() { + return new StandardTokenizer(); } }; - Analyzer analyzer = new CustomAnalyzer(null, new CharFilterFactory[0], new TokenFilterFactory[] { tokenFilter }); + Analyzer analyzer = new CustomAnalyzer(tokenizer, new CharFilterFactory[0], new TokenFilterFactory[] { tokenFilter }); MapperException ex = expectThrows(MapperException.class, () -> emptyRegistry.build(IndexSettingsModule.newIndexSettings("index", settings), singletonMap("default", new PreBuiltAnalyzerProvider("default", AnalyzerScope.INDEX, analyzer)), emptyMap(), @@ -264,4 +271,83 @@ public void testEnsureCloseInvocationProperlyDelegated() throws IOException { registry.close(); verify(mock).close(); } + + public void testDeprecations() throws IOException { + + AnalysisPlugin plugin = new AnalysisPlugin() { + + class MockFactory extends AbstractTokenFilterFactory { + MockFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + super(indexSettings, name, settings); + } + + @Override + public TokenStream create(TokenStream tokenStream) { + deprecationLogger.deprecated("Using deprecated token filter [deprecated]"); + return tokenStream; + } + } + + class UnusedMockFactory extends AbstractTokenFilterFactory { + UnusedMockFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + super(indexSettings, name, settings); + } + + @Override + public TokenStream create(TokenStream tokenStream) { + deprecationLogger.deprecated("Using deprecated token filter [unused]"); + return tokenStream; + } + } + + class NormalizerFactory extends AbstractTokenFilterFactory implements NormalizingTokenFilterFactory { + + public NormalizerFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + super(indexSettings, name, settings); + } + + @Override + public TokenStream create(TokenStream tokenStream) { + deprecationLogger.deprecated("Using deprecated token filter [deprecated_normalizer]"); + return tokenStream; + } + + } + + @Override + public Map> getTokenFilters() { + return Map.of("deprecated", MockFactory::new, "unused", UnusedMockFactory::new, + "deprecated_normalizer", NormalizerFactory::new); + } + }; + + Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(); + Settings indexSettings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put("index.analysis.filter.deprecated.type", "deprecated") + .put("index.analysis.analyzer.custom.tokenizer", "standard") + .putList("index.analysis.analyzer.custom.filter", "lowercase", "deprecated") + .build(); + + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", indexSettings); + + new AnalysisModule(TestEnvironment.newEnvironment(settings), + singletonList(plugin)).getAnalysisRegistry().build(idxSettings); + + // We should only get a warning from the token filter that is referenced in settings + assertWarnings("Using deprecated token filter [deprecated]"); + + indexSettings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put("index.analysis.filter.deprecated.type", "deprecated_normalizer") + .putList("index.analysis.normalizer.custom.filter", "lowercase", "deprecated_normalizer") + .build(); + idxSettings = IndexSettingsModule.newIndexSettings("index", indexSettings); + + new AnalysisModule(TestEnvironment.newEnvironment(settings), + singletonList(plugin)).getAnalysisRegistry().build(idxSettings); + + assertWarnings("Using deprecated token filter [deprecated_normalizer]"); + + } } diff --git a/server/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java b/server/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java index 675305a17cce6..3c8455224e646 100644 --- a/server/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java +++ b/server/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java @@ -228,14 +228,13 @@ public void testUnderscoreInAnalyzerName() throws IOException { } } - public void testStandardFilterBWC() throws IOException { + public void testStandardFilterBWC() { Version version = VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.CURRENT); final Settings settings = Settings.builder().put("index.analysis.analyzer.my_standard.tokenizer", "standard") .put("index.analysis.analyzer.my_standard.filter", "standard") .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).put(IndexMetaData.SETTING_VERSION_CREATED, version) .build(); - IndexAnalyzers analyzers = getIndexAnalyzers(settings); - IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> analyzers.get("my_standard").tokenStream("", "")); + IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> getIndexAnalyzers(settings)); assertThat(exc.getMessage(), equalTo("The [standard] token filter has been removed.")); } From 1ca8b8bc73c1722622ce87e8f20d000c9eca98cc Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 13 Jan 2020 12:09:57 +0000 Subject: [PATCH 2/5] checkstyle; tests --- .../common/CommonAnalysisPluginTests.java | 38 ++++--------------- .../indices/TransportAnalyzeActionTests.java | 2 +- .../index/analysis/AnalysisRegistryTests.java | 2 +- 3 files changed, 9 insertions(+), 33 deletions(-) diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/CommonAnalysisPluginTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/CommonAnalysisPluginTests.java index 90190e42f2f85..607450b9322d6 100644 --- a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/CommonAnalysisPluginTests.java +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/CommonAnalysisPluginTests.java @@ -19,20 +19,15 @@ package org.elasticsearch.analysis.common; -import org.apache.lucene.analysis.MockTokenizer; -import org.apache.lucene.analysis.Tokenizer; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; -import org.elasticsearch.index.analysis.TokenFilterFactory; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; import org.elasticsearch.test.VersionUtils; import java.io.IOException; -import java.io.StringReader; -import java.util.Map; public class CommonAnalysisPluginTests extends ESTestCase { @@ -51,13 +46,8 @@ public void testNGramFilterInCustomAnalyzerDeprecationError() throws IOException .build(); try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { - Map tokenFilters = createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settings), - settings, commonAnalysisPlugin).tokenFilter; - TokenFilterFactory tokenFilterFactory = tokenFilters.get("nGram"); - Tokenizer tokenizer = new MockTokenizer(); - tokenizer.setReader(new StringReader("foo bar")); - - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> tokenFilterFactory.create(tokenizer)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settings), settings, commonAnalysisPlugin)); assertEquals("The [nGram] token filter name was deprecated in 6.4 and cannot be used in new indices. " + "Please change the filter name to [ngram] instead.", ex.getMessage()); } @@ -69,12 +59,7 @@ public void testNGramFilterInCustomAnalyzerDeprecationError() throws IOException .putList("index.analysis.analyzer.custom_analyzer.filter", "my_ngram").put("index.analysis.filter.my_ngram.type", "nGram") .build(); try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { - Map tokenFilters = createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settingsPre7), - settingsPre7, commonAnalysisPlugin).tokenFilter; - TokenFilterFactory tokenFilterFactory = tokenFilters.get("nGram"); - Tokenizer tokenizer = new MockTokenizer(); - tokenizer.setReader(new StringReader("foo bar")); - assertNotNull(tokenFilterFactory.create(tokenizer)); + createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settingsPre7), settingsPre7, commonAnalysisPlugin); assertWarnings("The [nGram] token filter name is deprecated and will be removed in a future version. " + "Please change the filter name to [ngram] instead."); } @@ -95,13 +80,8 @@ public void testEdgeNGramFilterInCustomAnalyzerDeprecationError() throws IOExcep .build(); try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { - Map tokenFilters = createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settings), - settings, commonAnalysisPlugin).tokenFilter; - TokenFilterFactory tokenFilterFactory = tokenFilters.get("edgeNGram"); - Tokenizer tokenizer = new MockTokenizer(); - tokenizer.setReader(new StringReader("foo bar")); - - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> tokenFilterFactory.create(tokenizer)); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, + () -> createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settings), settings, commonAnalysisPlugin)); assertEquals("The [edgeNGram] token filter name was deprecated in 6.4 and cannot be used in new indices. " + "Please change the filter name to [edge_ngram] instead.", ex.getMessage()); } @@ -116,12 +96,8 @@ public void testEdgeNGramFilterInCustomAnalyzerDeprecationError() throws IOExcep .build(); try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { - Map tokenFilters = createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settingsPre7), - settingsPre7, commonAnalysisPlugin).tokenFilter; - TokenFilterFactory tokenFilterFactory = tokenFilters.get("edgeNGram"); - Tokenizer tokenizer = new MockTokenizer(); - tokenizer.setReader(new StringReader("foo bar")); - assertNotNull(tokenFilterFactory.create(tokenizer)); + createTestAnalysis(IndexSettingsModule.newIndexSettings("index", settingsPre7), + settingsPre7, commonAnalysisPlugin); assertWarnings("The [edgeNGram] token filter name is deprecated and will be removed in a future version. " + "Please change the filter name to [edge_ngram] instead."); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java index 39e4562c75267..7b344bf335c88 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java @@ -111,7 +111,7 @@ public TokenStream create(TokenStream tokenStream) { class DeprecatedTokenFilterFactory extends AbstractTokenFilterFactory implements NormalizingTokenFilterFactory { - public DeprecatedTokenFilterFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + DeprecatedTokenFilterFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { super(indexSettings, name, settings); } diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java index 3590ccdb4b63d..0a3b59b162d8e 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java @@ -302,7 +302,7 @@ public TokenStream create(TokenStream tokenStream) { class NormalizerFactory extends AbstractTokenFilterFactory implements NormalizingTokenFilterFactory { - public NormalizerFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + NormalizerFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { super(indexSettings, name, settings); } From bcdd2b0c2bc3bd833606d12d61c7a67b40910cd5 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 13 Jan 2020 14:43:07 +0000 Subject: [PATCH 3/5] Add version check --- .../index/analysis/AnalysisRegistryTests.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java index 0a3b59b162d8e..c3ebabcb25193 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java @@ -283,7 +283,9 @@ class MockFactory extends AbstractTokenFilterFactory { @Override public TokenStream create(TokenStream tokenStream) { - deprecationLogger.deprecated("Using deprecated token filter [deprecated]"); + if (indexSettings.getIndexVersionCreated().equals(Version.CURRENT)) { + deprecationLogger.deprecated("Using deprecated token filter [deprecated]"); + } return tokenStream; } } @@ -338,15 +340,20 @@ public Map> getTokenFilters() { assertWarnings("Using deprecated token filter [deprecated]"); indexSettings = Settings.builder() - .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.getPreviousVersion()) .put("index.analysis.filter.deprecated.type", "deprecated_normalizer") .putList("index.analysis.normalizer.custom.filter", "lowercase", "deprecated_normalizer") + .put("index.analysis.filter.deprecated.type", "deprecated") + .put("index.analysis.analyzer.custom.tokenizer", "standard") + .putList("index.analysis.analyzer.custom.filter", "lowercase", "deprecated") .build(); idxSettings = IndexSettingsModule.newIndexSettings("index", indexSettings); new AnalysisModule(TestEnvironment.newEnvironment(settings), singletonList(plugin)).getAnalysisRegistry().build(idxSettings); + // We should only get a warning from the normalizer, because we're on a version where 'deprecated' + // works fine assertWarnings("Using deprecated token filter [deprecated_normalizer]"); } From 494147055697679e63b1eb80493fc448970bb92e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 14 Jan 2020 09:36:51 +0000 Subject: [PATCH 4/5] Add tests for exceptions as well --- .../index/analysis/AnalysisRegistry.java | 13 ++++--- .../index/analysis/AnalysisRegistryTests.java | 38 +++++++++++++++++-- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java b/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java index aa2e827b9e742..31b343f300880 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java @@ -604,7 +604,7 @@ private static NamedAnalyzer produceAnalyzer(String name, } else { analyzer = new NamedAnalyzer(name, analyzerFactory.scope(), analyzerF, overridePositionIncrementGap); } - checkDeprecations(analyzer); + checkVersions(analyzer); return analyzer; } @@ -633,10 +633,13 @@ private void processNormalizerFactory( normalizers.put(name, normalizer); } - // Deprecation warnings are emitted when actual TokenStreams are built; this is usually - // too late to be useful, so we build an empty tokenstream at construction time and - // use it, to ensure that warnings are emitted immediately. - private static void checkDeprecations(Analyzer analyzer) { + // Some analysis components emit deprecation warnings or throw exceptions when used + // with the wrong version of elasticsearch. These exceptions and warnings are + // normally thrown when tokenstreams are constructed, which unless we build a + // tokenstream up-front does not happen until a document is indexed. In order to + // surface these warnings or exceptions as early as possible, we build an empty + // tokenstream and pull it through an Analyzer at construction time. + private static void checkVersions(Analyzer analyzer) { try (TokenStream ts = analyzer.tokenStream("", "")) { ts.reset(); while (ts.incrementToken()) {} diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java index c3ebabcb25193..e8f00907d1022 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java @@ -272,7 +272,7 @@ public void testEnsureCloseInvocationProperlyDelegated() throws IOException { verify(mock).close(); } - public void testDeprecations() throws IOException { + public void testDeprecationsAndExceptions() throws IOException { AnalysisPlugin plugin = new AnalysisPlugin() { @@ -290,6 +290,21 @@ public TokenStream create(TokenStream tokenStream) { } } + class ExceptionFactory extends AbstractTokenFilterFactory { + + public ExceptionFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + super(indexSettings, name, settings); + } + + @Override + public TokenStream create(TokenStream tokenStream) { + if (indexSettings.getIndexVersionCreated().equals(Version.CURRENT)) { + throw new IllegalArgumentException("Cannot use token filter [exception]"); + } + return tokenStream; + } + } + class UnusedMockFactory extends AbstractTokenFilterFactory { UnusedMockFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { super(indexSettings, name, settings); @@ -319,7 +334,7 @@ public TokenStream create(TokenStream tokenStream) { @Override public Map> getTokenFilters() { return Map.of("deprecated", MockFactory::new, "unused", UnusedMockFactory::new, - "deprecated_normalizer", NormalizerFactory::new); + "deprecated_normalizer", NormalizerFactory::new, "exception", ExceptionFactory::new); } }; @@ -344,8 +359,10 @@ public Map> getTokenFilters() { .put("index.analysis.filter.deprecated.type", "deprecated_normalizer") .putList("index.analysis.normalizer.custom.filter", "lowercase", "deprecated_normalizer") .put("index.analysis.filter.deprecated.type", "deprecated") + .put("index.analysis.filter.exception.type", "exception") .put("index.analysis.analyzer.custom.tokenizer", "standard") - .putList("index.analysis.analyzer.custom.filter", "lowercase", "deprecated") + // exception will not throw because we're not on Version.CURRENT + .putList("index.analysis.analyzer.custom.filter", "lowercase", "deprecated", "exception") .build(); idxSettings = IndexSettingsModule.newIndexSettings("index", indexSettings); @@ -356,5 +373,20 @@ public Map> getTokenFilters() { // works fine assertWarnings("Using deprecated token filter [deprecated_normalizer]"); + indexSettings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put("index.analysis.filter.exception.type", "exception") + .put("index.analysis.analyzer.custom.tokenizer", "standard") + // exception will not throw because we're not on Version.LATEST + .putList("index.analysis.analyzer.custom.filter", "lowercase", "exception") + .build(); + IndexSettings exceptionSettings = IndexSettingsModule.newIndexSettings("index", indexSettings); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + new AnalysisModule(TestEnvironment.newEnvironment(settings), + singletonList(plugin)).getAnalysisRegistry().build(exceptionSettings); + }); + assertEquals("Cannot use token filter [exception]", e.getMessage()); + } } From d938eaa2ddfa66beb28535a6c6226bc1fce50322 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 14 Jan 2020 10:21:54 +0000 Subject: [PATCH 5/5] checkstyle --- .../org/elasticsearch/index/analysis/AnalysisRegistryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java index e8f00907d1022..4a156db4f2101 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java @@ -292,7 +292,7 @@ public TokenStream create(TokenStream tokenStream) { class ExceptionFactory extends AbstractTokenFilterFactory { - public ExceptionFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { + ExceptionFactory(IndexSettings indexSettings, Environment env, String name, Settings settings) { super(indexSettings, name, settings); }