From fd564b10db86ad52c0934e2b49e42b8ec81c7a47 Mon Sep 17 00:00:00 2001 From: olcbean Date: Thu, 23 Nov 2017 13:53:47 +0100 Subject: [PATCH] Deprecate `levenstein` in favor of `levenshtein` (#27409) Support both spellings thoughout 6.x, reporting the incorrect one as deprecated. --- .../DirectCandidateGeneratorBuilder.java | 14 +++++++-- .../suggest/term/TermSuggestionBuilder.java | 18 ++++++++--- .../phrase/DirectCandidateGeneratorTests.java | 27 ++++++++++++++-- .../suggest/term/StringDistanceImplTests.java | 31 +++++++++---------- .../term/TermSuggestionBuilderTests.java | 2 +- .../search/suggesters/term-suggest.asciidoc | 2 +- 6 files changed, 66 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorBuilder.java index 52d1126e434..b5d4ff716e5 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorBuilder.java @@ -31,6 +31,8 @@ import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -45,6 +47,9 @@ import java.util.function.Consumer; public final class DirectCandidateGeneratorBuilder implements CandidateGenerator { + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger( + Loggers.getLogger(DirectCandidateGeneratorBuilder.class)); + private static final String TYPE = "direct_generator"; public static final ParseField DIRECT_GENERATOR_FIELD = new ParseField(TYPE); @@ -211,8 +216,8 @@ public final class DirectCandidateGeneratorBuilder implements CandidateGenerator * string distance for terms inside the index. *
  • damerau_levenshtein - String distance algorithm * based on Damerau-Levenshtein algorithm. - *
  • levenstein - String distance algorithm based on - * Levenstein edit distance algorithm. + *
  • levenshtein - String distance algorithm based on + * Levenshtein edit distance algorithm. *
  • jarowinkler - String distance algorithm based on * Jaro-Winkler algorithm. *
  • ngram - String distance algorithm based on character @@ -458,13 +463,16 @@ public final class DirectCandidateGeneratorBuilder implements CandidateGenerator } } - private static StringDistance resolveDistance(String distanceVal) { + static StringDistance resolveDistance(String distanceVal) { distanceVal = distanceVal.toLowerCase(Locale.US); if ("internal".equals(distanceVal)) { return DirectSpellChecker.INTERNAL_LEVENSHTEIN; } else if ("damerau_levenshtein".equals(distanceVal) || "damerauLevenshtein".equals(distanceVal)) { return new LuceneLevenshteinDistance(); } else if ("levenstein".equals(distanceVal)) { + DEPRECATION_LOGGER.deprecated("Deprecated distance [levenstein] used, replaced by [levenshtein]"); + return new LevensteinDistance(); + } else if ("levenshtein".equals(distanceVal)) { return new LevensteinDistance(); // TODO Jaro and Winkler are 2 people - so apply same naming logic // as damerau_levenshtein diff --git a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java index f701ff36426..e8c8d6c3ae1 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilder.java @@ -30,6 +30,8 @@ import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.query.QueryShardContext; @@ -66,6 +68,9 @@ import static org.elasticsearch.search.suggest.phrase.DirectCandidateGeneratorBu * global options, but are only applicable for this suggestion. */ public class TermSuggestionBuilder extends SuggestionBuilder { + + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(TermSuggestionBuilder.class)); + private static final String SUGGESTION_NAME = "term"; private SuggestMode suggestMode = SuggestMode.MISSING; @@ -214,8 +219,8 @@ public class TermSuggestionBuilder extends SuggestionBuilderdamerau_levenshtein - String distance algorithm based on * Damerau-Levenshtein algorithm. - *
  • levenstein - String distance algorithm based on - * Levenstein edit distance algorithm. + *
  • levenshtein - String distance algorithm based on + * Levenshtein edit distance algorithm. *
  • jarowinkler - String distance algorithm based on * Jaro-Winkler algorithm. *
  • ngram - String distance algorithm based on character @@ -543,8 +548,8 @@ public class TermSuggestionBuilder extends SuggestionBuilder DirectCandidateGeneratorBuilder.resolveDistance("doesnt_exist")); + expectThrows(NullPointerException.class, () -> DirectCandidateGeneratorBuilder.resolveDistance(null)); + } + + public void testLevensteinDeprecation() { + assertThat(DirectCandidateGeneratorBuilder.resolveDistance("levenstein"), instanceOf(LevensteinDistance.class)); + assertWarnings("Deprecated distance [levenstein] used, replaced by [levenshtein]"); + } + private static DirectCandidateGeneratorBuilder mutate(DirectCandidateGeneratorBuilder original) throws IOException { DirectCandidateGeneratorBuilder mutation = copy(original); List> mutators = new ArrayList<>(); @@ -89,7 +112,7 @@ public class DirectCandidateGeneratorTests extends ESTestCase { mutators.add(() -> mutation.preFilter(original.preFilter() == null ? "preFilter" : original.preFilter() + "_other")); mutators.add(() -> mutation.sort(original.sort() == null ? "score" : original.sort() + "_other")); mutators.add( - () -> mutation.stringDistance(original.stringDistance() == null ? "levenstein" : original.stringDistance() + "_other")); + () -> mutation.stringDistance(original.stringDistance() == null ? "levenshtein" : original.stringDistance() + "_other")); mutators.add(() -> mutation.suggestMode(original.suggestMode() == null ? "missing" : original.suggestMode() + "_other")); return randomFrom(mutators).get(); } @@ -189,7 +212,7 @@ public class DirectCandidateGeneratorTests extends ESTestCase { maybeSet(generator::postFilter, randomAlphaOfLengthBetween(1, 20)); maybeSet(generator::size, randomIntBetween(1, 20)); maybeSet(generator::sort, randomFrom("score", "frequency")); - maybeSet(generator::stringDistance, randomFrom("internal", "damerau_levenshtein", "levenstein", "jarowinkler", "ngram")); + maybeSet(generator::stringDistance, randomFrom("internal", "damerau_levenshtein", "levenshtein", "jarowinkler", "ngram")); maybeSet(generator::suggestMode, randomFrom("missing", "popular", "always")); return generator; } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/term/StringDistanceImplTests.java b/core/src/test/java/org/elasticsearch/search/suggest/term/StringDistanceImplTests.java index b94b42741b0..42f1928d1dc 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/term/StringDistanceImplTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/term/StringDistanceImplTests.java @@ -20,10 +20,10 @@ package org.elasticsearch.search.suggest.term; import org.elasticsearch.common.io.stream.AbstractWriteableEnumTestCase; +import org.elasticsearch.search.suggest.term.TermSuggestionBuilder.StringDistanceImpl; import java.io.IOException; -import static org.elasticsearch.search.suggest.term.TermSuggestionBuilder.StringDistanceImpl; import static org.hamcrest.Matchers.equalTo; /** @@ -38,7 +38,7 @@ public class StringDistanceImplTests extends AbstractWriteableEnumTestCase { public void testValidOrdinals() { assertThat(StringDistanceImpl.INTERNAL.ordinal(), equalTo(0)); assertThat(StringDistanceImpl.DAMERAU_LEVENSHTEIN.ordinal(), equalTo(1)); - assertThat(StringDistanceImpl.LEVENSTEIN.ordinal(), equalTo(2)); + assertThat(StringDistanceImpl.LEVENSHTEIN.ordinal(), equalTo(2)); assertThat(StringDistanceImpl.JAROWINKLER.ordinal(), equalTo(3)); assertThat(StringDistanceImpl.NGRAM.ordinal(), equalTo(4)); } @@ -47,28 +47,27 @@ public class StringDistanceImplTests extends AbstractWriteableEnumTestCase { public void testFromString() { assertThat(StringDistanceImpl.resolve("internal"), equalTo(StringDistanceImpl.INTERNAL)); assertThat(StringDistanceImpl.resolve("damerau_levenshtein"), equalTo(StringDistanceImpl.DAMERAU_LEVENSHTEIN)); - assertThat(StringDistanceImpl.resolve("levenstein"), equalTo(StringDistanceImpl.LEVENSTEIN)); + assertThat(StringDistanceImpl.resolve("levenshtein"), equalTo(StringDistanceImpl.LEVENSHTEIN)); assertThat(StringDistanceImpl.resolve("jarowinkler"), equalTo(StringDistanceImpl.JAROWINKLER)); assertThat(StringDistanceImpl.resolve("ngram"), equalTo(StringDistanceImpl.NGRAM)); + final String doesntExist = "doesnt_exist"; - try { - StringDistanceImpl.resolve(doesntExist); - fail("StringDistanceImpl should not have an element " + doesntExist); - } catch (IllegalArgumentException e) { - } - try { - StringDistanceImpl.resolve(null); - fail("StringDistanceImpl.resolve on a null value should throw an exception."); - } catch (NullPointerException e) { - assertThat(e.getMessage(), equalTo("Input string is null")); - } + expectThrows(IllegalArgumentException.class, () -> StringDistanceImpl.resolve(doesntExist)); + + NullPointerException e = expectThrows(NullPointerException.class, () -> StringDistanceImpl.resolve(null)); + assertThat(e.getMessage(), equalTo("Input string is null")); + } + + public void testLevensteinDeprecation() { + assertThat(StringDistanceImpl.resolve("levenstein"), equalTo(StringDistanceImpl.LEVENSHTEIN)); + assertWarnings("Deprecated distance [levenstein] used, replaced by [levenshtein]"); } @Override public void testWriteTo() throws IOException { assertWriteToStream(StringDistanceImpl.INTERNAL, 0); assertWriteToStream(StringDistanceImpl.DAMERAU_LEVENSHTEIN, 1); - assertWriteToStream(StringDistanceImpl.LEVENSTEIN, 2); + assertWriteToStream(StringDistanceImpl.LEVENSHTEIN, 2); assertWriteToStream(StringDistanceImpl.JAROWINKLER, 3); assertWriteToStream(StringDistanceImpl.NGRAM, 4); } @@ -77,7 +76,7 @@ public class StringDistanceImplTests extends AbstractWriteableEnumTestCase { public void testReadFrom() throws IOException { assertReadFromStream(0, StringDistanceImpl.INTERNAL); assertReadFromStream(1, StringDistanceImpl.DAMERAU_LEVENSHTEIN); - assertReadFromStream(2, StringDistanceImpl.LEVENSTEIN); + assertReadFromStream(2, StringDistanceImpl.LEVENSHTEIN); assertReadFromStream(3, StringDistanceImpl.JAROWINKLER); assertReadFromStream(4, StringDistanceImpl.NGRAM); } diff --git a/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java b/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java index e366a5c4508..1e18fc1253b 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/term/TermSuggestionBuilderTests.java @@ -99,7 +99,7 @@ public class TermSuggestionBuilderTests extends AbstractSuggestionBuilderTestCas switch (randomVal) { case 0: return StringDistanceImpl.INTERNAL; case 1: return StringDistanceImpl.DAMERAU_LEVENSHTEIN; - case 2: return StringDistanceImpl.LEVENSTEIN; + case 2: return StringDistanceImpl.LEVENSHTEIN; case 3: return StringDistanceImpl.JAROWINKLER; case 4: return StringDistanceImpl.NGRAM; default: throw new IllegalArgumentException("No string distance algorithm with an ordinal of " + randomVal); diff --git a/docs/reference/search/suggesters/term-suggest.asciidoc b/docs/reference/search/suggesters/term-suggest.asciidoc index f76b17e0ed2..d8feeaf1760 100644 --- a/docs/reference/search/suggesters/term-suggest.asciidoc +++ b/docs/reference/search/suggesters/term-suggest.asciidoc @@ -116,7 +116,7 @@ doesn't take the query into account that is part of request. for comparing string distance for terms inside the index. `damerau_levenshtein` - String distance algorithm based on Damerau-Levenshtein algorithm. - `levenstein` - String distance algorithm based on Levenstein edit distance + `levenshtein` - String distance algorithm based on Levenshtein edit distance algorithm. `jarowinkler` - String distance algorithm based on Jaro-Winkler algorithm. `ngram` - String distance algorithm based on character n-grams.