From 757ccf00b296228e4b2e63485f7aa0d37a131d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 19 May 2016 14:53:55 +0200 Subject: [PATCH] Enforce MatchQueryBuilder#maxExpansions() to be strictly positive --- .../index/query/MatchQueryBuilder.java | 6 +- .../index/query/MatchQueryBuilderTests.java | 67 +++++++------------ 2 files changed, 27 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java index 58207f7ca54..6c867590653 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java @@ -267,7 +267,7 @@ public class MatchQueryBuilder extends AbstractQueryBuilder { */ public MatchQueryBuilder prefixLength(int prefixLength) { if (prefixLength < 0 ) { - throw new IllegalArgumentException("No negative prefix length allowed."); + throw new IllegalArgumentException("[" + NAME + "] requires prefix length to be non-negative."); } this.prefixLength = prefixLength; return this; @@ -284,8 +284,8 @@ public class MatchQueryBuilder extends AbstractQueryBuilder { * When using fuzzy or prefix type query, the number of term expansions to use. */ public MatchQueryBuilder maxExpansions(int maxExpansions) { - if (maxExpansions < 0 ) { - throw new IllegalArgumentException("No negative maxExpansions allowed."); + if (maxExpansions <= 0 ) { + throw new IllegalArgumentException("[" + NAME + "] requires maxExpansions to be positive."); } this.maxExpansions = maxExpansions; return this; diff --git a/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index 87f8f3981d3..c5e01c1d0ff 100644 --- a/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -23,7 +23,6 @@ import org.apache.lucene.queries.ExtendedCommonTermsQuery; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.FuzzyQuery; -import org.apache.lucene.search.LegacyNumericRangeQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.PhraseQuery; import org.apache.lucene.search.PointRangeQuery; @@ -202,64 +201,46 @@ public class MatchQueryBuilderTests extends AbstractQueryTestCase new MatchQueryBuilder(null, "value")); + assertEquals("[match] requires fieldName", e.getMessage()); } - try { - new MatchQueryBuilder("fieldName", null); - fail("value must not be non-null"); - } catch (IllegalArgumentException ex) { - // expected + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new MatchQueryBuilder("fieldName", null)); + assertEquals("[match] requires query value", e.getMessage()); } MatchQueryBuilder matchQuery = new MatchQueryBuilder("fieldName", "text"); - try { - matchQuery.prefixLength(-1); - fail("must not be positive"); - } catch (IllegalArgumentException ex) { - // expected + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> matchQuery.prefixLength(-1)); + assertEquals("[match] requires prefix length to be non-negative.", e.getMessage()); } - try { - matchQuery.maxExpansions(-1); - fail("must not be positive"); - } catch (IllegalArgumentException ex) { - // expected + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> matchQuery.maxExpansions(randomIntBetween(-10, 0))); + assertEquals("[match] requires maxExpansions to be positive.", e.getMessage()); } - try { - matchQuery.operator(null); - fail("must not be non-null"); - } catch (IllegalArgumentException ex) { - // expected + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> matchQuery.operator(null)); + assertEquals("[match] requires operator to be non-null", e.getMessage()); } - try { - matchQuery.type(null); - fail("must not be non-null"); - } catch (IllegalArgumentException ex) { - // expected + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> matchQuery.type(null)); + assertEquals("[match] requires type to be non-null", e.getMessage()); } - try { - matchQuery.zeroTermsQuery(null); - fail("must not be non-null"); - } catch (IllegalArgumentException ex) { - // expected + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> matchQuery.zeroTermsQuery(null)); + assertEquals("[match] requires zeroTermsQuery to be non-null", e.getMessage()); } - } - public void testBadAnalyzer() throws IOException { - MatchQueryBuilder matchQuery = new MatchQueryBuilder("fieldName", "text"); matchQuery.analyzer("bogusAnalyzer"); - try { - matchQuery.toQuery(createShardContext()); - fail("Expected QueryShardException"); - } catch (QueryShardException e) { + { + QueryShardException e = expectThrows(QueryShardException.class, () -> matchQuery.toQuery(createShardContext())); assertThat(e.getMessage(), containsString("analyzer [bogusAnalyzer] not found")); } }