Skip to content

Deprecate word_delimiter in favour of word_delimiter_graph #29216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Deprecate word_delimiter in favour of word_delimiter_graph #29216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
40c147a
Deprecate word_delimiter in 6.3 (#29061)
liketic Mar 23, 2018
aa751c0
Update deprecated logger
liketic Oct 27, 2018
92ba6e2
Fix comment
liketic Oct 27, 2018
06889d7
Add tests
liketic Dec 9, 2018
7773a1c
remove empty line
liketic Dec 9, 2018
9a6e3d6
Merge master
liketic Dec 9, 2018
d851ae4
Fix removed version
liketic Dec 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions ...analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
import org.apache.lucene.analysis.tr.TurkishAnalyzer;
import org.apache.lucene.analysis.util.ElisionFilter;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.Version;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
Expand Down Expand Up @@ -471,13 +472,22 @@ public List<PreConfiguredTokenFilter> getPreConfiguredTokenFilters() {
filters.add(PreConfiguredTokenFilter.singleton("type_as_payload", false, TypeAsPayloadTokenFilter::new));
filters.add(PreConfiguredTokenFilter.singleton("unique", false, UniqueTokenFilter::new));
filters.add(PreConfiguredTokenFilter.singleton("uppercase", true, UpperCaseFilter::new));
filters.add(PreConfiguredTokenFilter.singleton("word_delimiter", false, input ->
new WordDelimiterFilter(input,
WordDelimiterFilter.GENERATE_WORD_PARTS
| WordDelimiterFilter.GENERATE_NUMBER_PARTS
| WordDelimiterFilter.SPLIT_ON_CASE_CHANGE
| WordDelimiterFilter.SPLIT_ON_NUMERICS
| WordDelimiterFilter.STEM_ENGLISH_POSSESSIVE, null)));
filters.add(PreConfiguredTokenFilter.singletonWithVersion("word_delimiter", false, (reader, version) -> {
if (version.onOrAfter(Version.V_7_0_0)) {
throw new IllegalArgumentException(
"The [word_delimiter] token filter has been removed. Please change the filter name to [word_delimiter_graph] instead.");
} else {
deprecationLogger.deprecatedAndMaybeLog("word_delimiter_deprecation",
"The [word_delimiter] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [word_delimiter_graph] instead.");
}
return new WordDelimiterFilter(reader,
WordDelimiterFilter.GENERATE_WORD_PARTS
| WordDelimiterFilter.GENERATE_NUMBER_PARTS
| WordDelimiterFilter.SPLIT_ON_CASE_CHANGE
| WordDelimiterFilter.SPLIT_ON_NUMERICS
| WordDelimiterFilter.STEM_ENGLISH_POSSESSIVE, null);
}));
filters.add(PreConfiguredTokenFilter.singleton("word_delimiter_graph", false, input ->
new WordDelimiterGraphFilter(input,
WordDelimiterGraphFilter.GENERATE_WORD_PARTS
Expand Down
8 changes: 8 additions & 0 deletions ...mmon/src/main/java/org/elasticsearch/analysis/common/WordDelimiterTokenFilterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ public WordDelimiterTokenFilterFactory(IndexSettings indexSettings, Environment
Set<?> protectedWords = Analysis.getWordSet(env, settings, "protected_words");
this.protoWords = protectedWords == null ? null : CharArraySet.copy(protectedWords);
this.flags = flags;
if (indexSettings.getIndexVersionCreated().onOrAfter(Version.V_7_0_0)) {
throw new IllegalArgumentException(
"The [word_delimiter] token filter has been removed. Please change the filter name to [word_delimiter_graph] instead.");
} else {
deprecationLogger.deprecatedAndMaybeLog("word_delimiter_deprecation",
"The [word_delimiter] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [word_delimiter_graph] instead.");
}
}

@Override
Expand Down
42 changes: 42 additions & 0 deletions ...sis-common/src/test/java/org/elasticsearch/analysis/common/CommonAnalysisPluginTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,46 @@ public void testEdgeNGramNoDeprecationWarningPre6_4() throws IOException {
assertNotNull(tokenFilterFactory.create(tokenizer));
}
}

/**
* Check that the deprecated name "word_delimiter" issues a deprecation warning for indices created before 7.0.0
*/
public void testWordDelimiterDeprecationWarning() throws IOException {
Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir())
.put(IndexMetaData.SETTING_VERSION_CREATED,
VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.V_6_6_0))
.build();

IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings);
try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) {
Map<String, TokenFilterFactory> tokenFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).tokenFilter;
TokenFilterFactory tokenFilterFactory = tokenFilters.get("word_delimiter");
Tokenizer tokenizer = new MockTokenizer();
tokenizer.setReader(new StringReader("foo bar"));
assertNotNull(tokenFilterFactory.create(tokenizer));
assertWarnings("The [word_delimiter] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [word_delimiter_graph] instead.");
}
}

/**
* Check that the deprecated name "word_delimiter" issues a exception for indices created since 7.0.0
*/
public void testWordDelimiterRemoval() throws IOException {
Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir())
.put(IndexMetaData.SETTING_VERSION_CREATED,
VersionUtils.randomVersionBetween(random(), Version.V_7_0_0, Version.CURRENT))
.build();

IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings);
try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) {
Map<String, TokenFilterFactory> tokenFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).tokenFilter;
TokenFilterFactory tokenFilterFactory = tokenFilters.get("word_delimiter");
Copy link
Contributor

@romseygeek romseygeek Dec 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect the exception to be thrown here, rather than when create() is called below? Can you double-check this test?

Copy link
Author

@liketic liketic Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @romansanchez . The test is passed in my local environment. The tokenFilters is a map and if we do nothing with the tokenFilterFactory, no exception will be thrown.

Copy link
Contributor

@romseygeek romseygeek Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but there should be an Exception thrown in the WordDelimiterTokenFilterFactory constructor, so createTestAnalysis ought to fail, I think? I can have a look tomorrow if you don't get to it.

Tokenizer tokenizer = new MockTokenizer();
tokenizer.setReader(new StringReader("foo bar"));
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, ()-> tokenFilterFactory.create(tokenizer));
assertEquals("The [word_delimiter] token filter has been removed. Please change the filter name to " +
"[word_delimiter_graph] instead.", ex.getMessage());
}
}
}
9 changes: 9 additions & 0 deletions ...nalysis-common/src/test/resources/rest-api-spec/test/analysis-common/40_token_filters.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@

---
"word_delimiter":
- skip:
version: " - 6.9.99"
reason: word_delimiter was deprecated in 7.0.0
features: "warnings"

- do:
warnings:
- "The [word_delimiter] token filter name is deprecated and will be removed in a future version. Please change the filter name to [word_delimiter_graph] instead."
indices.analyze:
body:
text: the qu1ck brown fox
Expand All @@ -114,6 +121,8 @@
- match: { tokens.5.token: fox }

- do:
warnings:
- "The [word_delimiter] token filter name is deprecated and will be removed in a future version. Please change the filter name to [word_delimiter_graph] instead."
indices.analyze:
body:
text: the qu1ck brown fox
Expand Down