From 91dacc3e9e7fcac30985d05b76be4446664f119c Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Thu, 4 Aug 2022 16:54:48 +0800 Subject: [PATCH 1/4] make param check when create custom analyzer --- .../test/indices.analyze/10_analyze.yml | 19 +++++++++++++++ .../test/search.query/10_match.yml | 1 - .../index/analysis/AnalyzerComponents.java | 6 +++++ .../index/analysis/AnalysisTests.java | 24 +++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml index 971f530cebeb5..e8f9c6477edef 100644 --- a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml +++ b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml @@ -59,3 +59,22 @@ - match: { detail.tokenizer.tokens.0.token: ABc } - match: { detail.tokenfilters.0.name: lowercase } - match: { detail.tokenfilters.0.tokens.0.token: abc } + +--- +"Custom analyzer with not supported param in request": + - do: + catch: bad_request + indices.create: + index: test + body: + settings: + analysis: + analyzer: + my_analyzer: + type: custom + tokenizer: standard + foo: bar + + - match: { status: 400 } + - match: { error.type: illegal_argument_exception } + - match: { error.reason: "Custom Analyzer not support [foo] now" } diff --git a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml index 543a806b92153..e83bbd9aeb756 100644 --- a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml +++ b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml @@ -16,7 +16,6 @@ tokenizer: standard filter: [lowercase] search: - rest_total_hits_as_int: true tokenizer: standard filter: [lowercase, keyword_repeat, porter_stem, unique_stem] filter: diff --git a/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java b/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java index 71e2175dacc69..4fec594e9226f 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java @@ -44,6 +44,12 @@ static AnalyzerComponents createComponents( final Map charFilters, final Map tokenFilters ) { + for (String key : analyzerSettings.keySet()) { + switch (key) { + case "tokenizer", "char_filter", "filter", "type", "position_increment_gap" -> {} + default -> throw new IllegalArgumentException("Custom Analyzer not support [" + key + "] now"); + } + } String tokenizerName = analyzerSettings.get("tokenizer"); if (tokenizerName == null) { throw new IllegalArgumentException("Custom Analyzer [" + name + "] must be configured with a tokenizer"); diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java index ac1a455a3cdff..34ca46b04d90d 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java @@ -9,10 +9,14 @@ package org.elasticsearch.index.analysis; import org.apache.lucene.analysis.CharArraySet; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; +import org.elasticsearch.index.Index; import org.elasticsearch.test.ESTestCase; +import org.junit.BeforeClass; import java.io.BufferedWriter; import java.io.FileNotFoundException; @@ -27,9 +31,18 @@ import java.util.Arrays; import java.util.List; +import static org.elasticsearch.index.analysis.AnalyzerComponents.createComponents; import static org.hamcrest.Matchers.is; public class AnalysisTests extends ESTestCase { + private static TestAnalysis testAnalysis; + private static Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build(); + + @BeforeClass + public static void setup() throws IOException { + testAnalysis = createTestAnalysis(new Index("test", "_na_"), settings); + } + public void testParseStemExclusion() { /* Comma separated list */ Settings settings = Settings.builder().put("stem_exclusion", "foo,bar").build(); @@ -103,4 +116,15 @@ public void testParseWordList() throws IOException { List wordList = Analysis.getWordList(env, nodeSettings, "foo.bar"); assertEquals(Arrays.asList("hello", "world"), wordList); } + + public void testCustomAnalyzerWithNotSupportKey() { + Settings analyzerSettings = Settings.builder().put("tokenizer", "standard").put("foo", "bar").put("type", "custom").build(); + + try { + createComponents("my_analyzer", analyzerSettings, testAnalysis.tokenizer, testAnalysis.charFilter, testAnalysis.tokenFilter); + fail("expected failure"); + } catch (IllegalArgumentException e) { + assertEquals(e.getMessage(), "Custom Analyzer not support [foo] now"); + } + } } From 250959dcec0c085e97c59d511584e8f31b83e5f5 Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Thu, 4 Aug 2022 19:40:56 +0800 Subject: [PATCH 2/4] add changelog --- docs/changelog/89112.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/89112.yaml diff --git a/docs/changelog/89112.yaml b/docs/changelog/89112.yaml new file mode 100644 index 0000000000000..d4346fb181f59 --- /dev/null +++ b/docs/changelog/89112.yaml @@ -0,0 +1,6 @@ +pr: 89112 +summary: Make param check when create custom analyzer +area: Search/Analysis +type: enhancement +issues: + - 89112 From ea7106ce7b78faba271007e8d0c38fb887b7f1f8 Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Thu, 4 Aug 2022 19:41:25 +0800 Subject: [PATCH 3/4] update issue in changelog --- docs/changelog/89112.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/89112.yaml b/docs/changelog/89112.yaml index d4346fb181f59..58762559cea4c 100644 --- a/docs/changelog/89112.yaml +++ b/docs/changelog/89112.yaml @@ -3,4 +3,4 @@ summary: Make param check when create custom analyzer area: Search/Analysis type: enhancement issues: - - 89112 + - 85710 From 90ba7bf1d736e7de460c224167e4911c9191a5b1 Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Sun, 18 Sep 2022 22:27:03 +0800 Subject: [PATCH 4/4] resolve comment --- CONTRIBUTING.md | 2 +- docs/changelog/89112.yaml | 6 --- .../test/indices.analyze/10_analyze.yml | 4 +- .../test/search.query/10_match.yml | 1 + .../index/analysis/AnalysisRegistry.java | 9 ++++ .../index/analysis/AnalyzerComponents.java | 6 --- .../index/analysis/AnalysisTests.java | 51 ++++++++++++------- 7 files changed, 45 insertions(+), 34 deletions(-) delete mode 100644 docs/changelog/89112.yaml diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4a21a653fb80e..c61f5c6ce1e80 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,7 +79,7 @@ Once your changes and tests are ready to submit for review: 1. Test your changes Run the test suite to make sure that nothing is broken. See the -[TESTING](TESTING.asciidoc) file for help running tests. +[TESTING](TESTING.asciidoc) file for helping run tests. 2. Sign the Contributor License Agreement diff --git a/docs/changelog/89112.yaml b/docs/changelog/89112.yaml deleted file mode 100644 index 58762559cea4c..0000000000000 --- a/docs/changelog/89112.yaml +++ /dev/null @@ -1,6 +0,0 @@ -pr: 89112 -summary: Make param check when create custom analyzer -area: Search/Analysis -type: enhancement -issues: - - 85710 diff --git a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml index e8f9c6477edef..78fa5c0393d65 100644 --- a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml +++ b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/indices.analyze/10_analyze.yml @@ -61,7 +61,7 @@ - match: { detail.tokenfilters.0.tokens.0.token: abc } --- -"Custom analyzer with not supported param in request": +"Custom analyzer with unsupported param in request": - do: catch: bad_request indices.create: @@ -77,4 +77,4 @@ - match: { status: 400 } - match: { error.type: illegal_argument_exception } - - match: { error.reason: "Custom Analyzer not support [foo] now" } + - match: { error.reason: "Custom analyzer [my_analyzer] does not support [foo]" } diff --git a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml index e83bbd9aeb756..543a806b92153 100644 --- a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml +++ b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/search.query/10_match.yml @@ -16,6 +16,7 @@ tokenizer: standard filter: [lowercase] search: + rest_total_hits_as_int: true tokenizer: standard filter: [lowercase, keyword_repeat, porter_stem, unique_stem] filter: 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 b18bfc3464bac..d8ee7e3c73f80 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java @@ -60,6 +60,7 @@ public final class AnalysisRegistry implements Closeable { private final Map> tokenizers; private final Map>> analyzers; private final Map>> normalizers; + private static final Version CUSTOM_ANALYZER_KEY_CHECK_VERSION = Version.V_8_5_0; public AnalysisRegistry( Environment environment, @@ -457,6 +458,14 @@ private Map buildMapping( Settings currentSettings = entry.getValue(); String typeName = currentSettings.get("type"); if (component == Component.ANALYZER) { + if (settings.getIndexVersionCreated().onOrAfter(CUSTOM_ANALYZER_KEY_CHECK_VERSION)) { + for (String key : currentSettings.keySet()) { + switch (key) { + case "tokenizer", "char_filter", "filter", "type", "position_increment_gap" -> {} + default -> throw new IllegalArgumentException("Custom analyzer [" + name + "] does not support [" + key + "]"); + } + } + } T factory = null; if (typeName == null) { if (currentSettings.get("tokenizer") != null) { diff --git a/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java b/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java index 4fec594e9226f..71e2175dacc69 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/AnalyzerComponents.java @@ -44,12 +44,6 @@ static AnalyzerComponents createComponents( final Map charFilters, final Map tokenFilters ) { - for (String key : analyzerSettings.keySet()) { - switch (key) { - case "tokenizer", "char_filter", "filter", "type", "position_increment_gap" -> {} - default -> throw new IllegalArgumentException("Custom Analyzer not support [" + key + "] now"); - } - } String tokenizerName = analyzerSettings.get("tokenizer"); if (tokenizerName == null) { throw new IllegalArgumentException("Custom Analyzer [" + name + "] must be configured with a tokenizer"); diff --git a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java index 34ca46b04d90d..b272cdfa1942d 100644 --- a/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java +++ b/server/src/test/java/org/elasticsearch/index/analysis/AnalysisTests.java @@ -14,9 +14,9 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; -import org.elasticsearch.index.Index; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.test.ESTestCase; -import org.junit.BeforeClass; +import org.elasticsearch.test.IndexSettingsModule; import java.io.BufferedWriter; import java.io.FileNotFoundException; @@ -31,18 +31,10 @@ import java.util.Arrays; import java.util.List; -import static org.elasticsearch.index.analysis.AnalyzerComponents.createComponents; +import static java.util.Collections.emptyMap; import static org.hamcrest.Matchers.is; public class AnalysisTests extends ESTestCase { - private static TestAnalysis testAnalysis; - private static Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build(); - - @BeforeClass - public static void setup() throws IOException { - testAnalysis = createTestAnalysis(new Index("test", "_na_"), settings); - } - public void testParseStemExclusion() { /* Comma separated list */ Settings settings = Settings.builder().put("stem_exclusion", "foo,bar").build(); @@ -117,14 +109,35 @@ public void testParseWordList() throws IOException { assertEquals(Arrays.asList("hello", "world"), wordList); } - public void testCustomAnalyzerWithNotSupportKey() { - Settings analyzerSettings = Settings.builder().put("tokenizer", "standard").put("foo", "bar").put("type", "custom").build(); + public void testCustomAnalyzerWithUnsupportedKey() { + Settings analyzerSettings = Settings.builder(). + put("index.analysis.analyzer.my_analyzer.tokenizer", "standard"). + put("index.analysis.analyzer.my_analyzer.type", "custom"). + put("index.analysis.analyzer.my_analyzer.foo", "bar"). + build(); + Settings settings = Settings.builder(). + put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_8_5_0). + put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()). + put(analyzerSettings). + build(); - try { - createComponents("my_analyzer", analyzerSettings, testAnalysis.tokenizer, testAnalysis.charFilter, testAnalysis.tokenFilter); - fail("expected failure"); - } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), "Custom Analyzer not support [foo] now"); - } + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); + AnalysisRegistry emptyAnalysisRegistry = new AnalysisRegistry( + TestEnvironment.newEnvironment(settings), + emptyMap(), + emptyMap(), + emptyMap(), + emptyMap(), + emptyMap(), + emptyMap(), + emptyMap(), + emptyMap(), + emptyMap() + ); + IllegalArgumentException ex = expectThrows( + IllegalArgumentException.class, + () -> emptyAnalysisRegistry.build(idxSettings) + ); + assertEquals(ex.getMessage(), "Custom analyzer [my_analyzer] does not support [foo]"); } }