From 0c2dfe47fc816e0fb851f3493f16484f70dedf2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Wed, 8 Nov 2017 19:25:21 +0100 Subject: [PATCH 1/2] Deprecate use of `htmlStrip` as name for HtmlStripCharFilter The camel case name `htmlStip` should be removed in favour of `html_strip`, but we need to deprecate it first. This change adds deprecation warnings for lucene indices with lucene version larger than 7.0.0 and logs deprecation warnings for those cases. --- .../analysis/common/CommonAnalysisPlugin.java | 15 +++- .../HtmlStripCharFilterFactoryTests.java | 73 +++++++++++++++++++ .../test/indices.analyze/10_analyze.yml | 54 ++++++++++++++ .../analysis/PreConfiguredCharFilter.java | 11 +++ 4 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java index e0193e50313f3..541f23b7e3cc5 100644 --- a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java +++ b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java @@ -67,6 +67,8 @@ import org.apache.lucene.analysis.standard.ClassicFilter; import org.apache.lucene.analysis.tr.ApostropheFilter; import org.apache.lucene.analysis.util.ElisionFilter; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.index.analysis.CharFilterFactory; import org.elasticsearch.index.analysis.PreConfiguredCharFilter; import org.elasticsearch.index.analysis.PreConfiguredTokenFilter; @@ -88,6 +90,9 @@ import static org.elasticsearch.plugins.AnalysisPlugin.requriesAnalysisSettings; public class CommonAnalysisPlugin extends Plugin implements AnalysisPlugin { + + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(CommonAnalysisPlugin.class)); + @Override public Map> getTokenFilters() { Map> filters = new TreeMap<>(); @@ -171,8 +176,14 @@ public Map> getTokenizers() { public List getPreConfiguredCharFilters() { List filters = new ArrayList<>(); filters.add(PreConfiguredCharFilter.singleton("html_strip", false, HTMLStripCharFilter::new)); - // TODO deprecate htmlStrip - filters.add(PreConfiguredCharFilter.singleton("htmlStrip", false, HTMLStripCharFilter::new)); + filters.add(PreConfiguredCharFilter.singleton("htmlStrip", false, (reader, version) -> { + if (version.onOrAfter(org.elasticsearch.Version.V_7_0_0_alpha1)) { + DEPRECATION_LOGGER.deprecatedAndMaybeLog("htmlStrip_deprecation", + "The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + + "Please change the filter name to [html_strip] instead."); + } + return new HTMLStripCharFilter(reader); + })); return filters; } diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java new file mode 100644 index 0000000000000..ffb0c3d94c002 --- /dev/null +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java @@ -0,0 +1,73 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.analysis.common; + +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.IndexSettings; +import org.elasticsearch.index.analysis.CharFilterFactory; +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 HtmlStripCharFilterFactoryTests extends ESTestCase { + + /** + * Check that the deprecated name "htmlStrip" issues a deprecation warning for indices created since 6.0.0 + */ + public void testDeprecationWarning() 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.CURRENT)) + .build(); + + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); + try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { + Map charFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).charFilter; + CharFilterFactory charFilterFactory = charFilters.get("htmlStrip"); + assertNotNull(charFilterFactory.create(new StringReader("input"))); + assertWarnings( + "The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + + "Please change the filter name to [html_strip] instead."); + } + } + + /** + * Check that the deprecated name "htmlStrip" does NOT issues a deprecation warning for indices created before 6.0.0 + */ + public void testNoDeprecationWarningPre6() throws IOException { + Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, Version.V_5_6_4)) + .build(); + + IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); + try (CommonAnalysisPlugin commonAnalysisPlugin = new CommonAnalysisPlugin()) { + Map charFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).charFilter; + CharFilterFactory charFilterFactory = charFilters.get("htmlStrip"); + assertNotNull(charFilterFactory.create(new StringReader(""))); + } + } +} diff --git a/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml b/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml index cbb8f053cfbba..85833c5ae0f28 100644 --- a/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml +++ b/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml @@ -17,3 +17,57 @@ - match: { error.type: illegal_argument_exception } - match: { error.reason: "Custom normalizer may not use filter [word_delimiter]" } +--- +"htmlStrip_deprecated": + - skip: + version: " - 6.0.99" + reason: deprecated in 6.1 + features: "warnings" + + - do: + indices.create: + index: test_deprecated_htmlstrip + body: + settings: + index: + analysis: + analyzer: + my_htmlStripWithCharfilter: + tokenizer: keyword + char_filter: ["htmlStrip"] + mappings: + type: + properties: + name: + type: text + analyzer: my_htmlStripWithCharfilter + + - do: + warnings: + - 'The [htmpStrip] char filter name is deprecated and will be removed in a future version. Please change the filter name to [html_strip] instead.' + index: + index: test_deprecated_htmlstrip + type: type + id: 1 + body: { "name": "foo bar" } + + - do: + warnings: + - 'The [htmpStrip] char filter name is deprecated and will be removed in a future version. Please change the filter name to [html_strip] instead.' + index: + index: test_deprecated_htmlstrip + type: type + id: 1 + body: { "name": "foo baz" } + + - do: + warnings: + - 'The [htmpStrip] char filter name is deprecated and will be removed in a future version. Please change the filter name to [html_strip] instead.' + indices.analyze: + index: test_deprecated_htmlstrip + body: + analyzer: "my_htmlStripWithCharfilter" + text: "foo" + - length: { tokens: 1 } + - match: { tokens.0.token: "\nfoo\n" } + diff --git a/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java b/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java index a979e9e34fe4e..039b60b756c7a 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java @@ -40,6 +40,15 @@ public static PreConfiguredCharFilter singleton(String name, boolean useFilterFo (reader, version) -> create.apply(reader)); } + /** + * Create a pre-configured char filter that may not vary at all, provide access to the elasticsearch verison + */ + public static PreConfiguredCharFilter singleton(String name, boolean useFilterForMultitermQueries, + BiFunction create) { + return new PreConfiguredCharFilter(name, CachingStrategy.ONE, useFilterForMultitermQueries, + (reader, version) -> create.apply(reader, version)); + } + /** * Create a pre-configured token filter that may vary based on the Lucene version. */ @@ -57,6 +66,8 @@ public static PreConfiguredCharFilter elasticsearchVersion(String name, boolean return new PreConfiguredCharFilter(name, CachingStrategy.ELASTICSEARCH, useFilterForMultitermQueries, create); } + + private final boolean useFilterForMultitermQueries; private final BiFunction create; From 66b694c91ebccb1bab7780cfa28af052dcaf29d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 13 Apr 2018 16:17:58 +0200 Subject: [PATCH 2/2] iter --- .../analysis/common/CommonAnalysisPlugin.java | 4 ++-- .../common/HtmlStripCharFilterFactoryTests.java | 14 +++++++------- .../test/indices.analyze/10_analyze.yml | 7 +++---- .../index/analysis/PreConfiguredCharFilter.java | 4 +--- 4 files changed, 13 insertions(+), 16 deletions(-) diff --git a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java index 541f23b7e3cc5..a01eb52fdd498 100644 --- a/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java +++ b/modules/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java @@ -176,8 +176,8 @@ public Map> getTokenizers() { public List getPreConfiguredCharFilters() { List filters = new ArrayList<>(); filters.add(PreConfiguredCharFilter.singleton("html_strip", false, HTMLStripCharFilter::new)); - filters.add(PreConfiguredCharFilter.singleton("htmlStrip", false, (reader, version) -> { - if (version.onOrAfter(org.elasticsearch.Version.V_7_0_0_alpha1)) { + filters.add(PreConfiguredCharFilter.singletonWithVersion("htmlStrip", false, (reader, version) -> { + if (version.onOrAfter(org.elasticsearch.Version.V_6_3_0)) { DEPRECATION_LOGGER.deprecatedAndMaybeLog("htmlStrip_deprecation", "The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + "Please change the filter name to [html_strip] instead."); diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java index ffb0c3d94c002..0d5389a6d6594 100644 --- a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/HtmlStripCharFilterFactoryTests.java @@ -37,11 +37,11 @@ public class HtmlStripCharFilterFactoryTests extends ESTestCase { /** - * Check that the deprecated name "htmlStrip" issues a deprecation warning for indices created since 6.0.0 + * Check that the deprecated name "htmlStrip" issues a deprecation warning for indices created since 6.3.0 */ public void testDeprecationWarning() 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.CURRENT)) + .put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(), Version.V_6_3_0, Version.CURRENT)) .build(); IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); @@ -49,18 +49,18 @@ public void testDeprecationWarning() throws IOException { Map charFilters = createTestAnalysis(idxSettings, settings, commonAnalysisPlugin).charFilter; CharFilterFactory charFilterFactory = charFilters.get("htmlStrip"); assertNotNull(charFilterFactory.create(new StringReader("input"))); - assertWarnings( - "The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + assertWarnings("The [htmpStrip] char filter name is deprecated and will be removed in a future version. " + "Please change the filter name to [html_strip] instead."); } } /** - * Check that the deprecated name "htmlStrip" does NOT issues a deprecation warning for indices created before 6.0.0 + * Check that the deprecated name "htmlStrip" does NOT issues a deprecation warning for indices created before 6.3.0 */ - public void testNoDeprecationWarningPre6() throws IOException { + public void testNoDeprecationWarningPre6_3() throws IOException { Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) - .put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, Version.V_5_6_4)) + .put(IndexMetaData.SETTING_VERSION_CREATED, + VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, Version.V_6_2_4)) .build(); IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", settings); diff --git a/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml b/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml index 85833c5ae0f28..f8fc3acc02c4c 100644 --- a/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml +++ b/modules/analysis-common/src/test/resources/rest-api-spec/test/indices.analyze/10_analyze.yml @@ -20,8 +20,8 @@ --- "htmlStrip_deprecated": - skip: - version: " - 6.0.99" - reason: deprecated in 6.1 + version: " - 6.2.99" + reason: deprecated in 6.3 features: "warnings" - do: @@ -57,7 +57,7 @@ index: index: test_deprecated_htmlstrip type: type - id: 1 + id: 2 body: { "name": "foo baz" } - do: @@ -70,4 +70,3 @@ text: "foo" - length: { tokens: 1 } - match: { tokens.0.token: "\nfoo\n" } - diff --git a/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java b/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java index 039b60b756c7a..84eb0c4c3498c 100644 --- a/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java +++ b/server/src/main/java/org/elasticsearch/index/analysis/PreConfiguredCharFilter.java @@ -43,7 +43,7 @@ public static PreConfiguredCharFilter singleton(String name, boolean useFilterFo /** * Create a pre-configured char filter that may not vary at all, provide access to the elasticsearch verison */ - public static PreConfiguredCharFilter singleton(String name, boolean useFilterForMultitermQueries, + public static PreConfiguredCharFilter singletonWithVersion(String name, boolean useFilterForMultitermQueries, BiFunction create) { return new PreConfiguredCharFilter(name, CachingStrategy.ONE, useFilterForMultitermQueries, (reader, version) -> create.apply(reader, version)); @@ -66,8 +66,6 @@ public static PreConfiguredCharFilter elasticsearchVersion(String name, boolean return new PreConfiguredCharFilter(name, CachingStrategy.ELASTICSEARCH, useFilterForMultitermQueries, create); } - - private final boolean useFilterForMultitermQueries; private final BiFunction create;